[22.x] rc3 backports

hebasto commented at 6:59 am on August 5, 2021: member

Backported:

hebasto added the label Backport on Aug 5, 2021

hebasto added the label GUI on Aug 5, 2021

hebasto commented at 7:03 am on August 5, 2021: member

cc @achow101 @jarolrod @meshcollider @katesalazar

MarcoFalke added this to the milestone 22.0 on Aug 5, 2021

MarcoFalke commented at 3:03 pm on August 5, 2021: member

Maybe turn this into a backport pull and also add #22597 ?

hebasto force-pushed on Aug 5, 2021

hebasto renamed this:
~~[22.x] qt: Backport a fix of the regression in "Encrypt Wallet" menu item~~
[22.x] rc3 backports
on Aug 5, 2021

hebasto commented at 3:23 pm on August 5, 2021: member

@MarcoFalke

Maybe turn this into a backport pull and also add #22597 ?

Done :tiger2:

hebasto removed the label GUI on Aug 5, 2021

hebasto commented at 7:07 am on August 6, 2021: member

Should we backport #22584 as well to make CI tests green?

MarcoFalke commented at 7:11 am on August 6, 2021: member

Sounds good

hebasto force-pushed on Aug 6, 2021

hebasto commented at 7:49 am on August 6, 2021: member

Added #22584.

jonatack commented at 8:18 am on August 6, 2021: member

Should we backport #22584 as well to make CI tests green?

Note that #22584 is fixed by #22586.

hebasto commented at 1:49 pm on August 6, 2021: member

CI failure looks related to #22322.

hebasto commented at 2:39 pm on August 6, 2021: member

@MarcoFalke

The current fuzz issue could be fixed by backporting #22517 and #22557.

Is it worth?

MarcoFalke commented at 3:15 pm on August 6, 2021: member

backporting #22517 and #22557.

Seems ok to backport them. Though, apart from the CI I don’t think that anyone is running the fuzz targets on release branches.

hebasto force-pushed on Aug 6, 2021

hebasto commented at 3:55 pm on August 6, 2021: member

Added #22517 and #22557.

hebasto force-pushed on Aug 6, 2021

hebasto commented at 4:20 pm on August 6, 2021: member

Added https://github.com/bitcoin-core/gui/pull/396.

MarcoFalke commented at 6:37 am on August 7, 2021: member

The current failure is #22537

hebasto force-pushed on Aug 7, 2021

hebasto commented at 10:16 am on August 7, 2021: member

The current failure is #22537

Added #22232, and CI is green now.

Also, there were some concerns about backporting #22232 from @fanquake:

I can’t imagine we are going to backport #22232.

So, if it seems controversial, ready to roll the last change back.

fanquake commented at 10:20 am on August 7, 2021: member

Added #22232, and CI is green now.

NACK. I don’t know why you’ve added it. We don’t backport refactors.

hebasto force-pushed on Aug 7, 2021

hebasto commented at 10:26 am on August 7, 2021: member

@fanquake

Added #22232, and CI is green now.

NACK. I don’t know why you’ve added it. We don’t backport refactors.

Reverted back.

But I cannot consider #22232 as a pure refactor, because it improves code quality.

hebasto commented at 10:30 am on August 7, 2021: member

@fanquake

How about #22597, #22584, #22517, #22557?

hebasto force-pushed on Aug 9, 2021

hebasto commented at 8:13 am on August 9, 2021: member

Added #22643.

hebasto force-pushed on Aug 9, 2021

hebasto commented at 10:18 am on August 9, 2021: member

Added #22642.

fanquake commented at 2:37 am on August 11, 2021: member

How about #22597, #22584, #22517, #22557?

I think we need to decide whether running the fuzzer CI in any branch other than master is something we want to be doing / maintaining. This seems pretty unsustainable unless we at least make changes in regards to the fuzz inputs being used by the different branches. I’m pretty sure Marco has mentioned this before.

Having to backport something like #22517, which is just meant to be a temporary measure to disable a fuzzer in master, and as a result also backporting #22557, which only silences a warning, is messy. Same with #22584. Ideally, in either case, we’d just be backporting an actual bug fix, if anything.

hebasto commented at 2:05 pm on August 17, 2021: member

I think we need to decide whether running the fuzzer CI in any branch other than master is something we want to be doing / maintaining. This seems pretty unsustainable unless we at least make changes in regards to the fuzz inputs being used by the different branches. I’m pretty sure Marco has mentioned this before.

Right. Using the same qa-assets for any branch other than master one seems pointless. Considering this, my vote is to skip the fuzzer CI in such cases.

hebasto commented at 6:13 pm on August 17, 2021: member

I think we need to decide whether running the fuzzer CI in any branch other than master is something we want to be doing / maintaining. This seems pretty unsustainable unless we at least make changes in regards to the fuzz inputs being used by the different branches. I’m pretty sure Marco has mentioned this before.

Addressed in #22730.

achow101 commented at 8:18 pm on August 18, 2021: member

Can #22685 and #22686 be added to this?

hebasto force-pushed on Aug 19, 2021

hebasto commented at 7:50 am on August 19, 2021: member

Can #22685 and #22686 be added to this?

Done.

jonatack commented at 10:01 am on August 19, 2021: member

I would submit #22651 and #22648 for consideration. 22.0 with the introduction of a second hidden network via I2P support is when they will help the most.

Rspigler commented at 10:47 pm on August 19, 2021: contributor

I agree with @jonatack for 22651 and 22648, if they can get enough ACK’s

hebasto force-pushed on Aug 20, 2021

hebasto commented at 7:14 am on August 20, 2021: member

Added #22654.

hebasto force-pushed on Aug 20, 2021

fanquake referenced this in commit ff1e633897 on Aug 20, 2021

hebasto commented at 7:17 am on August 20, 2021: member

Added #22742.

fanquake commented at 7:19 am on August 20, 2021: member

Lets add #22730, and remove #22584, #22517 and #22557.

ci: Run fuzzer task for the master branch only

Github-Pull: bitcoin/bitcoin#22730
Rebased-From: 5a9e255e5a324e7aa0b63a9634aa3cfda9a300bd

e9d30fbb3a

consensus/params: simplify ValidDeployment check to avoid gcc warning

Github-Pull: bitcoin/bitcoin#22597
Rebased-From: 059171009b0138555f311cedc2553015ff618323

57fce067a3

qt: Fix regression in "Encrypt Wallet" menu item

Adding a new item to the m_wallet_selector must follow the establishment
of signal-slot connections.

Github-Pull: bitcoin-core/gui#393
Rebased-From: d54d94959869b0c363939163b99ba0475751dcb6

e9b4487684

gui: ensure external signer option remains disabled without signers

When no external signers are available, the option to enable external
signers should always be disabled. However the encrypt wallet checkbox
can erroneously re-enable the external signer checkbox. To avoid this,
CreateWalletDialog now stores whether signers were available during
setSigners so that future calls to external_signer_checkbox->setEnabled
can account for whether signers are available.

Github-Pull: bitcoin-core/gui#396
Rebased-From: a9b9ca82daefc77ee3c884d3f250460d7cf734a5

6a611d2e3c

guix-verify: Non-zero exit code when anything fails

Previously, if verification fails, the correct message will be printed,
but the exit code would still be 0.

Github-Pull: bitcoin/bitcoin#22643
Rebased-From: d451b60d22576dff7a2c8d6a8b5880d9d69e397c

cb491bd5a7

release: Release with separate SHA256SUMS and sig files

This allows us to remove the rfc4880 EOL hacks and release with a
SHA256SUMS.asc file that's a combination of all signer signatures.

Github-Pull: bitcoin/bitcoin#22642
Rebased-From: 90b3e482e911fde73133a157c3b354471682275a

ce77b45a1f

wallet: Use GetSelectionAmount for target value calculations

For target value calculations, GetSelectionAmount should be used, not
m_effective_value or m_value.

Specifically, ApproximateBestSubset mistakenly uses m_value when
calculating whether the target value has been met. This has been changed
to use GetSelectionAmount.

Github-Pull: bitcoin/bitcoin#22686
Rebased-From: 2de222c40198d3d528668d78cc52e2ce3fa96765

ffc81e2048

wallet: Assert that enough was selected to cover the fees

When the fee is not subtracted from the outputs, the amount that has
been reserved for the fee (change_and_fee - change_amount) must be
enough to cover the fee that is needed. It would be a bug to not do so,
so use an assert to make this obvious if such a situation were to occur.

Github-Pull: bitcoin/bitcoin#22686
Rebased-From: d9262324e80da32725e21d4e0fbfee56f25490b1

e86b023606

test: Test for ApproximateBestSubset edge case with too little fees

ApproximateBestSubset had an edge case (due to not using
GetSelectionAmount) where it was possible for it to return success but
fail to select enough to cover transaction fees. A test is added that
could trigger this failure prior to the fix being implemented.

Github-Pull: bitcoin/bitcoin#22686
Rebased-From: 92885c4f69a5e6fc4989677d6e5be8a666fbff0d

dfaffbeb63

clientversion: No suffix #if CLIENT_VERSION_IS_RELEASE

Previously, building from a release source tarball would result in a
version string like v22.0.0-<commithash>, but we expect just v22.0.0.
This commit solves this problem.

Also use PACKAGE_VERSION instead of reconstructing it.

Github-Pull: bitcoin/bitcoin#22685
Rebased-From: 5100deee5822795d385570a380d3c117d05d851d

63fec7e295

test: fix bug in 22686

Github-Pull: bitcoin/bitcoin#22742
Rebased-From: 8dcbbbea6486e9ab7d5e7397b82585141f9910bf

88fb7e37ad

guix: Don't include directory name in SHA256SUMS

The SHA256SUMS file can be used in a sha256sum -c command to verify
downloaded binaries. However users are likely to download just a single
file and not place this file in the correct directory relative to the
SHA256SUMS file for the simple verification command to work. By not
including the directory name in the SHA256SUMS file, it will be easier
for users to verify downloaded binaries.

Co-authored-by: Carl Dong <contact@carldong.me>

Github-Pull: bitcoin/bitcoin#22654
Rebased-From: fb17c99e35e72f3b21ec3b5473e84c21dc964776

27d43e5bd4

doc: Mention the flat directory structure for uploads

The uploaded binaries need to match the same flat directory structure of
the SHA256SUMS file in order for torrent downloaders to be able to
verify the download without moving files. Mention this in the release
process doc.

Github-Pull: bitcoin/bitcoin#22654
Rebased-From: 132cae44f2d031bdaa1e459b92ec89ad585dfc9f

068985c02e

hebasto force-pushed on Aug 20, 2021

hebasto commented at 7:37 am on August 20, 2021: member

Lets add #22730, and remove #22584, #22517 and #22557.

Done.

fanquake commented at 7:51 am on August 20, 2021: member

Done.

PR description needs updating.

hebasto commented at 7:56 am on August 20, 2021: member

Done.

PR description needs updating.

Updated.

sidhujag referenced this in commit a34da59b05 on Aug 20, 2021

qt: Handle new added plurals in bitcoin_en.ts

This step was missed. See translation_process.md

Github-Pull: bitcoin-core/gui#406
Rebased-From: 2b3d8f3dde383a53f29b7e7ee53ea364d4ef8938

cb34a0aafe

hebasto commented at 7:44 pm on August 23, 2021: member

Added https://github.com/bitcoin-core/gui/pull/406.

fanquake commented at 8:22 am on August 26, 2021: member

Lets add #22713, then this is just about ready to merge.

jonatack commented at 8:26 am on August 26, 2021: member

I agree with @jonatack for 22651 and 22648, if they can get enough ACK’s

They’re RFM IMO.

jonatack commented at 8:26 am on August 26, 2021: member

I would submit #22651 and #22648 for consideration. 22.0 with the introduction of a second hidden network via I2P support is when they will help the most.

👆

Fix build with Boost 1.77.0

BOOST_FILESYSTEM_C_STR changed to accept the path as an argument

Github-Pull: bitcoin/bitcoin#22713
Rebased-From: acb7aad27ec8a184808aa7905887e3b2c5d54e9c

32e1424f84

hebasto commented at 11:50 am on August 26, 2021: member

Lets add #22713, then this is just about ready to merge.

Done.

laanwj commented at 12:05 pm on August 26, 2021: member

Code list-of-backported-PRs review ACK 32e1424f84a30194dc5baa7108cf7d958ea0afcd

laanwj merged this on Aug 26, 2021

laanwj closed this on Aug 26, 2021

hebasto deleted the branch on Aug 26, 2021

DrahtBot locked this on Aug 26, 2022

[22.x] rc3 backports #22629