Selected Bitcoin v25 and onwards patches #1451
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…enerated 7cb9367 rpc: keep .cookie if it was not generated (Roman Zeyde) Pull request description: Otherwise, starting bitcoind twice may cause the `.cookie` file generated by the first instance to be deleted by the second instance shutdown (after failing to obtain a lock). ACKs for top commit: willcl-ark: re-ACK 7cb9367 achow101: ACK 7cb9367 kristapsk: re-ACK 7cb9367 stickies-v: ACK 7cb9367 Tree-SHA512: 0960dbc457975b0e0535f3d814824a879d7f85c9f1191537415b3fc253429a316a8e4badde56c8bc139778f132392983cec5fbe03891fb15ff61d3bc3f6e681b
…dedNodeInfo() d0b0474 test: add GetAddedNodeInfo() CJDNS regression unit test (Jon Atack) 684da97 p2p, bugfix: detect addnode cjdns peers in GetAddedNodeInfo() (Jon Atack) Pull request description: Addnode peers connected to us via the cjdns network are currently not detected by `CConnman::GetAddedNodeInfo()`, i.e. `fConnected` is always false. This causes the following issues: - RPC `getaddednodeinfo` incorrectly shows them as not connected - `CConnman::ThreadOpenAddedConnections()` continually retries to connect them Fix the issue and add a unit regression test. Extracted from #28248. Suggest running the test with: `./src/test/test_bitcoin -t net_peer_connection_tests -l test_suite` ACKs for top commit: mzumsande: utACK d0b0474 brunoerg: crACK d0b0474 pinheadmz: ACK d0b0474 Tree-SHA512: a4d81425f79558f5792585611f3fe8ab999b82144daeed5c3ec619861c69add934c2b2afdad24c8488a0ade94f5ce8112f5555d60a1ce913d4f5a1cf5dbba55a
b77bad3 rpc: move UniValue in blockToJSON (willcl-ark) Pull request description: Fixes: #24542 Fixes: #30052 Without explicitly declaring the move, these `UniValues` get copied, causing increased memory usage. Fix this by explicitly moving the `UniValue` objects. Used by `rest_block` and `getblock` RPC. ACKs for top commit: maflcko: review ACK b77bad3 ismaelsadeeq: ACK b77bad3 TheCharlatan: ACK b77bad3 theuni: utACK b77bad3 hebasto: ACK b77bad3, I have reviewed the code and it looks OK. BrandonOdiwuor: ACK b77bad3 Tree-SHA512: 767608331040f9cfe5c3568ed0e3c338920633472a1a50d4bbb47d1dc69d2bb11466d611f050ac8ad1a894b47fe1ea4d968cf34cbd44d4bb8d479fc5c7475f6d
bbe82c1 Fix #29767, set m_synced = true after Commit() (nanlour) Pull request description: I think this problem bitcoin/bitcoin#29767 (comment) is because of in BaseIndex::Sync https://github.com/bitcoin/bitcoin/blob/61de64df6790077857faba84796bb874b59c5d15/src/index/base.cpp#L163-L168 Setup m_synced = true; before Commit(); So this may cause a race condition window to BaseIndex::BlockConnected https://github.com/bitcoin/bitcoin/blob/61de64df6790077857faba84796bb874b59c5d15/src/index/base.cpp#L271-L274 So i try to fix it with move m_synced = true after Commit(). Also see comment of Sync(): https://github.com/bitcoin/bitcoin/blob/61de64df6790077857faba84796bb874b59c5d15/src/index/base.h#L151-L156 I am a newcomer interested in Bitcoin, trying to become a member of the Bitcoin Core development team. Please give me some feedback if you could, as I may be doing something wrong. Thank you! ACKs for top commit: fjahr: Code review ACK bbe82c1 ryanofsky: Code review ACK bbe82c1 Tree-SHA512: 89a09498a232c87ef1e083d4cc4ed9bb15f045ad0624d5d150a87187b2b8a48a41137974dbc7ea5c37f73da90742c43259f5aa7f84b4179eb8d62033e44fa479
…k thread and scheduler 5fd4836 init: change shutdown order of load block thread and scheduler (Martin Zumsande) Pull request description: This avoids situations during a reindex, in which the shutdown doesn't finish since `LimitValidationInterfaceQueue()` is called by the load block thread when the scheduler is already stopped, in which case it would block indefinitely. This can lead to intermittent failures in `feature_reindex.py` (#30424), which I could locally reproduce with ```diff diff --git a/src/validation.cpp b/src/validation.cpp index 74f0e49..be1706fdaf 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3446,6 +3446,7 @@ static void LimitValidationInterfaceQueue(ValidationSignals& signals) LOCKS_EXCL AssertLockNotHeld(cs_main); if (signals.CallbacksPending() > 10) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); signals.SyncWithValidationInterfaceQueue(); } } ``` It has also been reported by users running `reindex-chainstate` (#23234). I thought for a bit about potential downsides of changing this order, but couldn't find any. Fixes #30424 Fixes #23234 ACKs for top commit: maflcko: review ACK 5fd4836 hebasto: re-ACK 5fd4836. tdb3: ACK 5fd4836 BrandonOdiwuor: Code Review ACK 5fd4836 Tree-SHA512: 3b8894e99551c5d4392b55eaa718eee05841a7287aeef2978699e1d633d5234399fa2f5a3e71eac1508d97845906bd33e0e63e5351855139e7be04c421359b36
|
Looks like feature_filelock.py is failing |
tomt1664
force-pushed
the
v25+_cherry_pick
branch
5 times, most recently
from
b55c8ce to
e49eaa9
Compare
…detection 16bd283 Reapply "test: p2p: check that connecting to ourself leads to disconnect" (Sebastian Falbesoner) 0dbcd4c net: prevent sending messages in `NetEventsInterface::InitializeNode` (Sebastian Falbesoner) 66673f1 net: fix race condition in self-connect detection (Sebastian Falbesoner) Pull request description: This PR fixes a recently discovered race condition in the self-connect detection (see #30362 and #30368). Initiating an outbound network connection currently involves the following steps after the socket connection is established (see [`CConnman::OpenNetworkConnection`](https://github.com/bitcoin/bitcoin/blob/bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8/src/net.cpp#L2923-L2930) method): 1. set up node state 2. queue VERSION message (both steps 1 and 2 happen in [`InitializeNode`](https://github.com/bitcoin/bitcoin/blob/bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8/src/net_processing.cpp#L1662-L1683)) 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`. The temporarily reverted test introduced in #30362 is readded. Fixes #30368. Thanks go to vasild, mzumsande and dergoegge for suggestions on how to fix this (see bitcoin/bitcoin#30368 (comment) ff. and bitcoin/bitcoin#30394 (comment)). ACKs for top commit: naiyoma: tested ACK [https://github.com/bitcoin/bitcoin/pull/30394/commits/16bd283b3ad05daa41259a062aee0fc05b463fa6](https://github.com/bitcoin/bitcoin/pull/30394/commits/16bd283b3ad05daa41259a062aee0fc05b463fa6), built and tested locally, test passes successfully. mzumsande: ACK 16bd283 tdb3: ACK 16bd283 glozow: ACK 16bd283 dergoegge: ACK 16bd283 Tree-SHA512: 5b8aced6cda8deb38d4cd3fe4980b8af505d37ffa0925afaa734c5d81efe9d490dc48a42e1d0d45dd2961c0e1172a3d5b6582ae9a2d642f2592a17fbdc184445
tomt1664
force-pushed
the
v25+_cherry_pick
branch
from
e49eaa9 to
b9bc403
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-picked commits included in bitcoin 25, 26 and 27 releases, covering bug fixes, relevant to Elements