[24.x] Backports for rc2

fanquake commented at 10:03 am on September 20, 2022: member

Will collect backports for rc2 as they become available. Currently:

fanquake added the label Backport on Sep 20, 2022

fanquake added this to the milestone 24.0 on Sep 20, 2022

fanquake force-pushed on Sep 21, 2022

fanquake force-pushed on Sep 22, 2022

fanquake force-pushed on Sep 26, 2022

fanquake force-pushed on Sep 29, 2022

instagibbs commented at 7:17 pm on October 6, 2022: member

anymore backports expected for rc2?

maflcko commented at 10:38 am on October 7, 2022: member

Yes, see https://github.com/bitcoin/bitcoin/milestone/54

docs: Add 371 to bips.md

GitHub-Pull: #26124
Rebased-From: d3d6a18f711fc4c3841679e7f75e9837848dde13

da6fba6fe7

Correct sanity-checking script_size calculation

GitHub-Pull: #26149
Rebased-From: 648f6950cd8d9ac767d76a1e302f37c611936a7a

c97d924880

p2p: ProcessHeadersMessage(): fix received_new_header

Follow-up to #25717. The commit "Utilize anti-DoS headers download
strategy" changed how this bool variable is computed, so that its value
is now the opposite of what it should be.

GitHub-Pull: #26172
Rebased-From: bdcafb913398f0cdaff9c880618f9ebfc85c7693

7e0bcfbfef

contrib: Fix capture_output in getcoins.py

Our required Python version 3.6.12 does not support `capture_output` as
a subprocess.run argument; this was added in python 3.7.

We can emulate it by setting stdout and stderr to subprocess.PIPE

Github-Pull: #26212
Rebased-From: be59bd17ec753af7cc763474f2432d12bfc88c2f

997faf6b6c

index: Improve BaseIndex::BlockUntilSyncedToCurrentChain reliability

Since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb from PR
https://github.com/bitcoin/bitcoin/pull/21726, index
`BlockUntilSyncedToCurrentChain` behavior has been less reliable, and there has
also been a race condition in the `coinstatsindex_initial_sync` unit test.

It seems better for `BlockUntilSyncedToCurrentChain` to actually wait for the
last connected block to be fully processed, than to be able to return before
prune locks are set, so this switches the order of `m_best_block_index =
block;` and `UpdatePruneLock` statements in `SetBestBlockIndex` to make it more
reliable.

Also since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb, there has been a
race condition in the `coinstatsindex_initial_sync` test. Before that commit,
the atomic index best block pointer `m_best_block_index` was updated as the
last step of `BaseIndex::BlockConnected`, so `BlockUntilSyncedToCurrentChain`
could safely be used in tests to wait for the last `BlockConnected`
notification to be finished before stopping and destroying the index. But
after that commit, calling `BlockUntilSyncedToCurrentChain` is no longer
sufficient, and there is a race between the test shutdown code which destroys
the index object and the new code introduced in that commit calling
`AllowPrune()` and `GetName()` on the index object. Reproducibility
instructions for this are in
https://github.com/bitcoin/bitcoin/issues/25365#issuecomment-1259744133

This commit fixes the `coinstatsindex_initial_sync` race condition, even though
it will require an additional change to silence TSAN false positives,
https://github.com/bitcoin/bitcoin/pull/26188, after it is fixed. So this
partially addresses but does not resolve the bug reporting TSAN errors
https://github.com/bitcoin/bitcoin/issues/25365.

There is no known race condition outside of test code currently, because the
bitcoind `Shutdown` function calls `FlushBackgroundCallbacks` not
`BlockUntilSyncedToCurrentChain` to safely shut down.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Co-authored-by: MacroFake <falke.marco@gmail.com>

Github-Pull: #26215
Rebased-From: 8891949bdcb25093d3a6703ae8228c3c3687d3a4

5ad82a09b4

fanquake force-pushed on Oct 11, 2022

dergoegge commented at 2:12 pm on October 13, 2022: member

ACK 5ad82a09b409d416236092062a4201e238dfd68b

refactor: remove unused locks for ResubmitWalletTransactions

ReacceptWalletTransactions is replaced by ResubmitWalletTransactions
which already handles acquiring the necessary locks internally.

Github-Pull: #26205
Rebased-From: 01f3534632d18c772901fb6ce22f6394eae96799

a6fb674f96

refactor: carve out tx resend timer logic into ShouldResend

Moves the logic of whether or not transactions should actually be
resent out of the function that's resending them. This reduces
responsibilities of ResubmitWalletTransactions and allows
carving out the updating of m_next_resend in a future commit.

Github-Pull: #26205
Rebased-From: 7fbde8af5c06694eecd4ce601109bd826a54bd6f

fc8f2bfa3a

wallet: only update m_next_resend when actually resending

We only want to relay our resubmitted transactions once every 12-36h.
By separating the timer update logic out of ResubmitWalletTransactions
and into MaybeResendWalletTxs we avoid non-relay calls (previously in
the separate ReacceptWalletTransactions function) from resetting that
timer.

Github-Pull: #26205
Rebased-From: 9245f456705b285e2d9afcc01a6155e1b3f92fad

43ced0b436

refactor: revert m_next_resend to not be std::atomic

Since m_next_resend is now only called from MaybeResendWalletTxs()
we don't have any potential race conditions anymore, so the usage
of std::atomic can be reverted.

Github-Pull: #26205
Rebased-From: b01682a812f0841170657708ef0e896b904fcd77

9b438f06ec

[BugFix]: Do not allow deserializing PSBT with empty PSBT_OUT_TAP_TREE

Github-Pull: #25858
Rebased-From: 0652dc53b291bd295caff4093ec2854fd4b34645

1390c96c8e

psbt: Fix merging of m_tap_tree

Merging should be checking that the current PSBTOutput doesn't have a
taptree and the other one's is copied over. The original merging had
this inverted and would remove m_tap_tree if the other did not have it.

Github-Pull: #25858
Rebased-From: 7df6e1bb77a96eac4fbcba424bbe780636b86650

4abd2ab18e

tests: Test that PSBT_OUT_TAP_TREE is combined correctly

Github-Pull: #25858
Rebased-From: 22c051ca70bae73e0430b05fb9d879591df27699

a9419eff0c

psbt: Change m_tap_tree to store just the tuples

Instead of having an entire TaprootBuilder which may or may not be
complete, and could potentially have future changes that interact oddly
with taproot tree tuples, have m_tap_tree be just the tuples.

When needed in other a TaprootBuilder for actual use, the tuples will be
added to a a TaprootBuilder that, in the future, can take in whatever
other data is needed as well.

Github-Pull: #25858
Rebased-From: 0577d423adda8e719d7611d03355680c8fbacab8

d810fde8ea

psbt: Only include m_tap_tree if it has scripts

Github-Pull: #25858
Rebased-From: 30ff25cf37eec4b09ab40424eb5d6a4a80410955

4d42c3a240

tests: Test that PSBT_OUT_TAP_TREE is included correctly

Github-Pull: #25858
Rebased-From: 9e386afb67bf8fa71b72f730da1695eeb11828cd

e2e4c2969b

dergoegge commented at 5:07 pm on October 13, 2022: member

ACK e2e4c2969ba753a94587985582123a596f57067b

achow101 commented at 5:18 pm on October 13, 2022: member

ACK e2e4c2969ba753a94587985582123a596f57067b

instagibbs commented at 5:29 pm on October 13, 2022: member

ACK https://github.com/bitcoin/bitcoin/pull/26133/commits/e2e4c2969ba753a94587985582123a596f57067b

achow101 merged this on Oct 13, 2022

achow101 closed this on Oct 13, 2022

fanquake deleted the branch on Oct 14, 2022

LarryRuane commented at 3:22 am on October 14, 2022: contributor

post-merge ACK e2e4c2969ba753a94587985582123a596f57067b

bitcoin locked this on Oct 14, 2023

[24.x] Backports for rc2 #26133