[27.x] More backports

fanquake commented at 10:31 am on July 17, 2024: member

Backports:

#29855
#30357
#30394 (modified test commit)
https://github.com/bitcoin/bitcoin/pull/30435

psbt: Check non witness utxo outpoint early

A common issue that our fuzzers keep finding is that outpoints don't
exist in the non witness utxos. Instead of trying to track this down and
checking in various individual places, do the check early during
deserialization.

Github-Pull: #29855
Rebased-From: 9e13ccc50eec9d2efe0f472e6d50dc822df70d84

fa90989503

net: fix race condition in self-connect detection

Initiating an outbound network connection currently involves the
following steps after the socket connection is established (see
 `CConnman::OpenNetworkConnection` method):
    1. set up node state
    2. queue VERSION message
    3. add new node to vector `m_nodes`

If we connect to ourself, it can happen that the sent VERSION message
(step 2) is received and processed locally *before* the node object
is added to the connection manager's `m_nodes` vector (step 3). In this
case, the self-connect remains undiscovered, as the detection doesn't
find the outbound peer in `m_nodes` yet (see `CConnman::CheckIncomingNonce`).

Fix this by swapping the order of 2. and 3., by taking the `PushNodeVersion`
call out of `InitializeNode` and doing that in the `SendMessages` method
instead, which is only called for `CNode` instances in `m_nodes`.

Thanks go to vasild, mzumsande, dergoegge and sipa for suggestions on
how to fix this.

Github-Pull: #30394
Rebased-From: 66673f1c1302c986e344c7f44bb0b352213d5dc8

0933cf53b4

net: prevent sending messages in `NetEventsInterface::InitializeNode`

Now that the queueing of the VERSION messages has been moved out of
`InitializeNode`, there is no need to pass a mutable `CNode` reference any
more. With a const reference, trying to send messages in this method would
lead to a compile-time error, e.g.:

----------------------------------------------------------------------------------------------------------------------------------
...
net_processing.cpp: In member function ‘virtual void {anonymous}::PeerManagerImpl::InitializeNode(const CNode&, ServiceFlags)’:
net_processing.cpp:1683:21: error: binding reference of type ‘CNode&’ to ‘const CNode’ discards qualifiers
 1683 |     PushNodeVersion(node, *peer);
...
----------------------------------------------------------------------------------------------------------------------------------

Github-Pull: #30394
Rebased-From: 0dbcd4c14855fe2cba15a32245572b693dc18c4e

064f214673

Reapply "test: p2p: check that connecting to ourself leads to disconnect"

This reverts commit 9ec2c53701a391629b55aeb2804e8060d2c453a4 with
a tiny change included (identation of the wait_until call).

Github-Pull: #30394
Rebased-From: 16bd283b3ad05daa41259a062aee0fc05b463fa6

ab42206652

init: change shutdown order of load block thread and scheduler

This avoids situations during a reindex in which shutdown
doesn't finish since SyncWithValidationInterfaceQueue is
called by the load block thread when the scheduler is already stopped.

Github-Pull: #30435
Rebased-From: 5fd48360198d2ac49e43b24cc1469557b03567b8

05192ba84c

wallet: fix FillPSBT errantly showing as complete

Fix cases of calls to `FillPSBT` returning `complete=true` when it's not
the case.

This can happen when some inputs have been signed but the transaction is
subsequently modified, e.g. in the context of PayJoins.

Also fixes a related bug where a finalized hex string is attempted to be
added during `walletprocesspsbt` but a CHECK_NONFATAL causes an abort.

Reported in #30077.

Github-Pull: #30357
Rebased-From: 39cea21ec51b9838669c38fefa14f25c36ae7096

f22b9ca70c

test: add test for modififed walletprocesspsbt calls

This test checks that we can successfully process PSBTs and opt out of
finalization.

Previously trying to call `walletprocesspsbt` would attempt to
auto-finalize (as a convenience), and would not permit opt-out of
finalization, instead aborting via `CHECK_NONFATAL`.

Github-Pull: #30357
Rebased-From: 7e36dca657c66bc70b04d5b850e5a335aecfb902

54bb9b0541

DrahtBot commented at 10:32 am on July 17, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	stickies-v, willcl-ark

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Backport on Jul 17, 2024

fanquake added this to the milestone 27.2 on Jul 23, 2024

fanquake marked this as ready for review on Jul 23, 2024

fanquake requested review from stickies-v on Jul 23, 2024

fanquake requested review from willcl-ark on Jul 23, 2024

stickies-v approved

stickies-v commented at 2:45 pm on July 23, 2024: contributor

ACK 885c4b7bc65c294b4b2fac91392d23a37a1f6485 but #29855 is missing from release-notes.md

I verified that all backport commits are clean, except:

ab422066527992d53c92bb482c6b993f089b2999 backported from 16bd283b3ad05daa41259a062aee0fc05b463fa6: the relevant part of the test has been minimally carved out and added to 27.x which I think is a good approach

[WIP] doc: update release notes for 27.x 4f23c86364

fanquake force-pushed on Jul 23, 2024

fanquake commented at 2:47 pm on July 23, 2024: member

but #29855 is missing from release-notes.md

Thanks, added.

stickies-v commented at 2:53 pm on July 23, 2024: contributor

ACK 4f23c8636498f7e5adbe0264a0dc66a566c4b1b9

willcl-ark approved

willcl-ark commented at 3:22 pm on July 23, 2024: member

ACK 4f23c8636498f7e5adbe0264a0dc66a566c4b1b9

Checked all commits were backported cleanly and had a release note for their original PR.

I agree with stickies that carving out the relevant portion of the test for https://github.com/bitcoin/bitcoin/commit/ab422066527992d53c92bb482c6b993f089b2999 makes best sense for a backport.

fanquake merged this on Jul 24, 2024

fanquake closed this on Jul 24, 2024

fanquake deleted the branch on Jul 24, 2024

[27.x] More backports #30467

Code Coverage

Reviews