From 2717331981ec94fd616a08f31e643391a2118639 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 8 Sep 2025 12:11:47 +0100 Subject: [PATCH 01/11] Fix benchmark CSV output The `SHA256AutoDetect` return output is used, among other use cases, to name benchmarks. Using a comma breaks the CSV output. This change replaces the comma with a semicolon, which fixes the issue. Github-Pull: #33340 Rebased-From: 790b440197bde322432a5bab161f1869b667e681 --- src/crypto/sha256.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/crypto/sha256.cpp b/src/crypto/sha256.cpp index 09c5d3123e8..c5f495708d6 100644 --- a/src/crypto/sha256.cpp +++ b/src/crypto/sha256.cpp @@ -627,7 +627,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem Transform = sha256_x86_shani::Transform; TransformD64 = TransformD64Wrapper; TransformD64_2way = sha256d64_x86_shani::Transform_2way; - ret = "x86_shani(1way,2way)"; + ret = "x86_shani(1way;2way)"; have_sse4 = false; // Disable SSE4/AVX2; have_avx2 = false; } @@ -641,14 +641,14 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem #endif #if defined(ENABLE_SSE41) TransformD64_4way = sha256d64_sse41::Transform_4way; - ret += ",sse41(4way)"; + ret += ";sse41(4way)"; #endif } #if defined(ENABLE_AVX2) if (have_avx2 && have_avx && enabled_avx) { TransformD64_8way = sha256d64_avx2::Transform_8way; - ret += ",avx2(8way)"; + ret += ";avx2(8way)"; } #endif #endif // defined(HAVE_GETCPUID) @@ -682,7 +682,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem Transform = sha256_arm_shani::Transform; TransformD64 = TransformD64Wrapper; TransformD64_2way = sha256d64_arm_shani::Transform_2way; - ret = "arm_shani(1way,2way)"; + ret = "arm_shani(1way;2way)"; } #endif #endif // DISABLE_OPTIMIZED_SHA256 From 324caa84977cc74ac19df605503483e59739773e Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 10 Sep 2025 09:12:40 +0100 Subject: [PATCH 02/11] ci: always use tag for LLVM checkout Rather than trying to match the apt installed clang version, which is prone to intermittent issues. i.e #33345. Github-Pull: #33364 Rebased-From: b736052e39f1f466f63f261ace3dd2deba171e8a --- ci/test/01_base_install.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh index 25a03d5f503..1b624f38942 100755 --- a/ci/test/01_base_install.sh +++ b/ci/test/01_base_install.sh @@ -55,10 +55,9 @@ if [ -n "$PIP_PACKAGES" ]; then fi if [[ -n "${USE_INSTRUMENTED_LIBCPP}" ]]; then + ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-21.1.1" /llvm-project + if [ -n "${APT_LLVM_V}" ]; then - ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-$( clang --version | sed --silent 's@.*clang version \([0-9.]*\).*@\1@p' )" /llvm-project - else - ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-21.1.0" /llvm-project cmake -G Ninja -B /clang_build/ \ -DLLVM_ENABLE_PROJECTS="clang" \ From e97588fc3d1e1a02382312ade7d529c5b4b60016 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 4 Sep 2025 19:25:33 +0000 Subject: [PATCH 03/11] trace: Workaround GCC bug compiling with old systemtap Github-Pull: #33310 Rebased-From: 93a29ff2830162c8129d35c7b9beb43fab984503 --- cmake/module/FindUSDT.cmake | 4 ++++ src/util/trace.h | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/cmake/module/FindUSDT.cmake b/cmake/module/FindUSDT.cmake index 0be7c28ff58..234a099f3fd 100644 --- a/cmake/module/FindUSDT.cmake +++ b/cmake/module/FindUSDT.cmake @@ -36,6 +36,10 @@ if(USDT_INCLUDE_DIR) include(CheckCXXSourceCompiles) set(CMAKE_REQUIRED_INCLUDES ${USDT_INCLUDE_DIR}) check_cxx_source_compiles(" + #if defined(__arm__) + # define STAP_SDT_ARG_CONSTRAINT g + #endif + // Setting SDT_USE_VARIADIC lets systemtap (sys/sdt.h) know that we want to use // the optional variadic macros to define tracepoints. #define SDT_USE_VARIADIC 1 diff --git a/src/util/trace.h b/src/util/trace.h index 3deefeade37..ab005dd8bce 100644 --- a/src/util/trace.h +++ b/src/util/trace.h @@ -9,6 +9,13 @@ #ifdef ENABLE_TRACING +// Workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103395 +// systemtap 4.6 on 32-bit ARM triggers internal compiler error +// (this workaround is included in systemtap 4.7+) +#if defined(__arm__) +# define STAP_SDT_ARG_CONSTRAINT g +#endif + // Setting SDT_USE_VARIADIC lets systemtap (sys/sdt.h) know that we want to use // the optional variadic macros to define tracepoints. #define SDT_USE_VARIADIC 1 From 9b95ab5e9db1691be5f26fc5bc1c186777d2dc5b Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Wed, 21 May 2025 13:36:43 -0400 Subject: [PATCH 04/11] p2p: Add witness mutation check inside FillBlock Since #29412, we have not allowed mutated blocks to continue being processed immediately the block is received, but this is only done for the legacy BLOCK message. Extend these checks as belt-and-suspenders to not allow similar mutation strategies to affect relay by honest peers by applying the check inside PartiallyDownloadedBlock::FillBlock, immediately before returning READ_STATUS_OK. This also removes the extraneous CheckBlock call. Github-Pull: #32646 Rebased-From: bac9ee4830664c86c1cb3d38a5b19c722aae2f54 --- src/blockencodings.cpp | 17 +++------ src/blockencodings.h | 7 ++-- src/net_processing.cpp | 10 ++++- src/test/blockencodings_tests.cpp | 16 ++++---- src/test/fuzz/partially_downloaded_block.cpp | 39 ++++++-------------- 5 files changed, 37 insertions(+), 52 deletions(-) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 5f4061a71dc..5975a99faab 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -180,7 +180,7 @@ bool PartiallyDownloadedBlock::IsTxAvailable(size_t index) const return txn_available[index] != nullptr; } -ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector& vtx_missing) +ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector& vtx_missing, bool segwit_active) { if (header.IsNull()) return READ_STATUS_INVALID; @@ -205,16 +205,11 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< if (vtx_missing.size() != tx_missing_offset) return READ_STATUS_INVALID; - BlockValidationState state; - CheckBlockFn check_block = m_check_block_mock ? m_check_block_mock : CheckBlock; - if (!check_block(block, state, Params().GetConsensus(), /*fCheckPoW=*/true, /*fCheckMerkleRoot=*/true)) { - // TODO: We really want to just check merkle tree manually here, - // but that is expensive, and CheckBlock caches a block's - // "checked-status" (in the CBlock?). CBlock should be able to - // check its own merkle root and cache that check. - if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) - return READ_STATUS_FAILED; // Possible Short ID collision - return READ_STATUS_CHECKBLOCK_FAILED; + // Check for possible mutations early now that we have a seemingly good block + IsBlockMutatedFn check_mutated{m_check_block_mutated_mock ? m_check_block_mutated_mock : IsBlockMutated}; + if (check_mutated(/*block=*/block, + /*check_witness_root=*/segwit_active)) { + return READ_STATUS_FAILED; // Possible Short ID collision } LogDebug(BCLog::CMPCTBLOCK, "Successfully reconstructed block %s with %lu txn prefilled, %lu txn from mempool (incl at least %lu from extra pool) and %lu txn requested\n", hash.ToString(), prefilled_count, mempool_count, extra_count, vtx_missing.size()); diff --git a/src/blockencodings.h b/src/blockencodings.h index c92aa05e805..b1f82d18c5d 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -141,15 +141,16 @@ public: CBlockHeader header; // Can be overridden for testing - using CheckBlockFn = std::function; - CheckBlockFn m_check_block_mock{nullptr}; + using IsBlockMutatedFn = std::function; + IsBlockMutatedFn m_check_block_mutated_mock{nullptr}; explicit PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {} // extra_txn is a list of extra orphan/conflicted/etc transactions to look at ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, const std::vector& extra_txn); bool IsTxAvailable(size_t index) const; - ReadStatus FillBlock(CBlock& block, const std::vector& vtx_missing); + // segwit_active enforces witness mutation checks just before reporting a healthy status + ReadStatus FillBlock(CBlock& block, const std::vector& vtx_missing, bool segwit_active); }; #endif // BITCOIN_BLOCKENCODINGS_H diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1da3ec9d211..0f1d6d98aa4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3314,7 +3314,11 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl } PartiallyDownloadedBlock& partialBlock = *range_flight.first->second.second->partialBlock; - ReadStatus status = partialBlock.FillBlock(*pblock, block_transactions.txn); + + // We should not have gotten this far in compact block processing unless it's attached to a known header + const CBlockIndex* prev_block{Assume(m_chainman.m_blockman.LookupBlockIndex(partialBlock.header.hashPrevBlock))}; + ReadStatus status = partialBlock.FillBlock(*pblock, block_transactions.txn, + /*segwit_active=*/DeploymentActiveAfter(prev_block, m_chainman, Consensus::DEPLOYMENT_SEGWIT)); if (status == READ_STATUS_INVALID) { RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect Misbehaving(peer, "invalid compact block/non-matching block transactions"); @@ -4462,7 +4466,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } std::vector dummy; - status = tempBlock.FillBlock(*pblock, dummy); + const CBlockIndex* prev_block{Assume(m_chainman.m_blockman.LookupBlockIndex(cmpctblock.header.hashPrevBlock))}; + status = tempBlock.FillBlock(*pblock, dummy, + /*segwit_active=*/DeploymentActiveAfter(prev_block, m_chainman, Consensus::DEPLOYMENT_SEGWIT)); if (status == READ_STATUS_OK) { fBlockReconstructed = true; } diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index ed95a8831e3..d40a0a94aef 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -95,21 +95,21 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) CBlock block2; { PartiallyDownloadedBlock tmp = partialBlock; - BOOST_CHECK(partialBlock.FillBlock(block2, {}) == READ_STATUS_INVALID); // No transactions + BOOST_CHECK(partialBlock.FillBlock(block2, {}, /*segwit_active=*/true) == READ_STATUS_INVALID); // No transactions partialBlock = tmp; } // Wrong transaction { PartiallyDownloadedBlock tmp = partialBlock; - partialBlock.FillBlock(block2, {block.vtx[2]}); // Current implementation doesn't check txn here, but don't require that + partialBlock.FillBlock(block2, {block.vtx[2]}, /*segwit_active=*/true); // Current implementation doesn't check txn here, but don't require that partialBlock = tmp; } bool mutated; BOOST_CHECK(block.hashMerkleRoot != BlockMerkleRoot(block2, &mutated)); CBlock block3; - BOOST_CHECK(partialBlock.FillBlock(block3, {block.vtx[1]}) == READ_STATUS_OK); + BOOST_CHECK(partialBlock.FillBlock(block3, {block.vtx[1]}, /*segwit_active=*/true) == READ_STATUS_OK); BOOST_CHECK_EQUAL(block.GetHash().ToString(), block3.GetHash().ToString()); BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString()); BOOST_CHECK(!mutated); @@ -182,14 +182,14 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) CBlock block2; { PartiallyDownloadedBlock tmp = partialBlock; - BOOST_CHECK(partialBlock.FillBlock(block2, {}) == READ_STATUS_INVALID); // No transactions + BOOST_CHECK(partialBlock.FillBlock(block2, {}, /*segwit_active=*/true) == READ_STATUS_INVALID); // No transactions partialBlock = tmp; } // Wrong transaction { PartiallyDownloadedBlock tmp = partialBlock; - partialBlock.FillBlock(block2, {block.vtx[1]}); // Current implementation doesn't check txn here, but don't require that + partialBlock.FillBlock(block2, {block.vtx[1]}, /*segwit_active=*/true); // Current implementation doesn't check txn here, but don't require that partialBlock = tmp; } BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 2); // +2 because of partialBlock and block2 @@ -198,7 +198,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) CBlock block3; PartiallyDownloadedBlock partialBlockCopy = partialBlock; - BOOST_CHECK(partialBlock.FillBlock(block3, {block.vtx[0]}) == READ_STATUS_OK); + BOOST_CHECK(partialBlock.FillBlock(block3, {block.vtx[0]}, /*segwit_active=*/true) == READ_STATUS_OK); BOOST_CHECK_EQUAL(block.GetHash().ToString(), block3.GetHash().ToString()); BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString()); BOOST_CHECK(!mutated); @@ -252,7 +252,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) CBlock block2; PartiallyDownloadedBlock partialBlockCopy = partialBlock; - BOOST_CHECK(partialBlock.FillBlock(block2, {}) == READ_STATUS_OK); + BOOST_CHECK(partialBlock.FillBlock(block2, {}, /*segwit_active=*/true) == READ_STATUS_OK); BOOST_CHECK_EQUAL(block.GetHash().ToString(), block2.GetHash().ToString()); bool mutated; BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block2, &mutated).ToString()); @@ -300,7 +300,7 @@ BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) CBlock block2; std::vector vtx_missing; - BOOST_CHECK(partialBlock.FillBlock(block2, vtx_missing) == READ_STATUS_OK); + BOOST_CHECK(partialBlock.FillBlock(block2, vtx_missing, /*segwit_active=*/true) == READ_STATUS_OK); BOOST_CHECK_EQUAL(block.GetHash().ToString(), block2.GetHash().ToString()); BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block2, &mutated).ToString()); BOOST_CHECK(!mutated); diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp index 82d781cd53c..1a06ef8b0af 100644 --- a/src/test/fuzz/partially_downloaded_block.cpp +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -32,14 +32,10 @@ void initialize_pdb() g_setup = testing_setup.get(); } -PartiallyDownloadedBlock::CheckBlockFn FuzzedCheckBlock(std::optional result) +PartiallyDownloadedBlock::IsBlockMutatedFn FuzzedIsBlockMutated(bool result) { - return [result](const CBlock&, BlockValidationState& state, const Consensus::Params&, bool, bool) { - if (result) { - return state.Invalid(*result); - } - - return true; + return [result](const CBlock& block, bool) { + return result; }; } @@ -111,36 +107,23 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb) skipped_missing |= (!pdb.IsTxAvailable(i) && skip); } - // Mock CheckBlock - bool fail_check_block{fuzzed_data_provider.ConsumeBool()}; - auto validation_result = - fuzzed_data_provider.PickValueInArray( - {BlockValidationResult::BLOCK_RESULT_UNSET, - BlockValidationResult::BLOCK_CONSENSUS, - BlockValidationResult::BLOCK_CACHED_INVALID, - BlockValidationResult::BLOCK_INVALID_HEADER, - BlockValidationResult::BLOCK_MUTATED, - BlockValidationResult::BLOCK_MISSING_PREV, - BlockValidationResult::BLOCK_INVALID_PREV, - BlockValidationResult::BLOCK_TIME_FUTURE, - BlockValidationResult::BLOCK_CHECKPOINT, - BlockValidationResult::BLOCK_HEADER_LOW_WORK}); - pdb.m_check_block_mock = FuzzedCheckBlock( - fail_check_block ? - std::optional{validation_result} : - std::nullopt); + bool segwit_active{fuzzed_data_provider.ConsumeBool()}; + + // Mock IsBlockMutated + bool fail_block_mutated{fuzzed_data_provider.ConsumeBool()}; + pdb.m_check_block_mutated_mock = FuzzedIsBlockMutated(fail_block_mutated); CBlock reconstructed_block; - auto fill_status{pdb.FillBlock(reconstructed_block, missing)}; + auto fill_status{pdb.FillBlock(reconstructed_block, missing, segwit_active)}; switch (fill_status) { case READ_STATUS_OK: assert(!skipped_missing); - assert(!fail_check_block); + assert(!fail_block_mutated); assert(block->GetHash() == reconstructed_block.GetHash()); break; case READ_STATUS_CHECKBLOCK_FAILED: [[fallthrough]]; case READ_STATUS_FAILED: - assert(fail_check_block); + assert(fail_block_mutated); break; case READ_STATUS_INVALID: break; From 4c940d47897bc380d3387dd6663c37c46b4020ec Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 3 Jun 2025 10:29:00 -0400 Subject: [PATCH 05/11] p2p: remove vestigial READ_STATUS_CHECKBLOCK_FAILED Github-Pull: #32646 Rebased-From: 28299ce77636d7563ec545d043cf1b61bd2f01c1 --- src/blockencodings.h | 2 -- src/net_processing.cpp | 18 +----------------- src/test/fuzz/partially_downloaded_block.cpp | 1 - 3 files changed, 1 insertion(+), 20 deletions(-) diff --git a/src/blockencodings.h b/src/blockencodings.h index b1f82d18c5d..fce59bc5614 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -84,8 +84,6 @@ typedef enum ReadStatus_t READ_STATUS_OK, READ_STATUS_INVALID, // Invalid object, peer is sending bogus crap READ_STATUS_FAILED, // Failed to process object - READ_STATUS_CHECKBLOCK_FAILED, // Used only by FillBlock to indicate a - // failure in CheckBlock. } ReadStatus; class CBlockHeaderAndShortTxIDs { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0f1d6d98aa4..fa27ceb38ac 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3335,23 +3335,7 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl return; } } else { - // Block is either okay, or possibly we received - // READ_STATUS_CHECKBLOCK_FAILED. - // Note that CheckBlock can only fail for one of a few reasons: - // 1. bad-proof-of-work (impossible here, because we've already - // accepted the header) - // 2. merkleroot doesn't match the transactions given (already - // caught in FillBlock with READ_STATUS_FAILED, so - // impossible here) - // 3. the block is otherwise invalid (eg invalid coinbase, - // block is too big, too many legacy sigops, etc). - // So if CheckBlock failed, #3 is the only possibility. - // Under BIP 152, we don't discourage the peer unless proof of work is - // invalid (we don't require all the stateless checks to have - // been run). This is handled below, so just treat this as - // though the block was successfully read, and rely on the - // handling in ProcessNewBlock to ensure the block index is - // updated, etc. + // Block is okay for further processing RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // it is now an empty pointer fBlockRead = true; // mapBlockSource is used for potentially punishing peers and diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp index 1a06ef8b0af..c9635cae8cd 100644 --- a/src/test/fuzz/partially_downloaded_block.cpp +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -121,7 +121,6 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb) assert(!fail_block_mutated); assert(block->GetHash() == reconstructed_block.GetHash()); break; - case READ_STATUS_CHECKBLOCK_FAILED: [[fallthrough]]; case READ_STATUS_FAILED: assert(fail_block_mutated); break; From 569ceb0df46fc619eed33f56b5b36f617c37bae7 Mon Sep 17 00:00:00 2001 From: Eugene Siegel Date: Wed, 3 Sep 2025 12:44:23 -0400 Subject: [PATCH 06/11] net: check for empty header before calling FillBlock Previously in debug builds, this would cause an Assume crash if FillBlock had been called previously. This could happen when multiple blocktxn messages were received. Co-Authored-By: Greg Sanders Github-Pull: #33296 Rebased-From: 5e585a0fc4fd68dd7b4982054b34deae2e7aeb89 --- src/net_processing.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index fa27ceb38ac..d9c2163c1d2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3315,6 +3315,16 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl PartiallyDownloadedBlock& partialBlock = *range_flight.first->second.second->partialBlock; + if (partialBlock.header.IsNull()) { + // It is possible for the header to be empty if a previous call to FillBlock wiped the header, but left + // the PartiallyDownloadedBlock pointer around (i.e. did not call RemoveBlockRequest). In this case, we + // should not call LookupBlockIndex below. + RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); + Misbehaving(peer, "previous compact block reconstruction attempt failed"); + LogDebug(BCLog::NET, "Peer %d sent compact block transactions multiple times", pfrom.GetId()); + return; + } + // We should not have gotten this far in compact block processing unless it's attached to a known header const CBlockIndex* prev_block{Assume(m_chainman.m_blockman.LookupBlockIndex(partialBlock.header.hashPrevBlock))}; ReadStatus status = partialBlock.FillBlock(*pblock, block_transactions.txn, @@ -3326,6 +3336,9 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl } else if (status == READ_STATUS_FAILED) { if (first_in_flight) { // Might have collided, fall back to getdata now :( + // We keep the failed partialBlock to disallow processing another compact block announcement from the same + // peer for the same block. We let the full block download below continue under the same m_downloading_since + // timer. std::vector invs; invs.emplace_back(MSG_BLOCK | GetFetchFlags(peer), block_transactions.blockhash); MakeAndPushMessage(pfrom, NetMsgType::GETDATA, invs); From 1288d44804cd6ecd8601d0aef55e6fbf500d2f31 Mon Sep 17 00:00:00 2001 From: Eugene Siegel Date: Wed, 3 Sep 2025 12:44:52 -0400 Subject: [PATCH 07/11] test: send duplicate blocktxn message in p2p_compactblocks.py Add test_multiple_blocktxn_response that checks that the peer is disconnected. Github-Pull: #33296 Rebased-From: 8b6264768030db1840041abeeaeefd6c227a2644 --- test/functional/p2p_compactblocks.py | 42 ++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index ca36b2fbc06..da8a0aed9ac 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -566,6 +566,42 @@ class CompactBlocksTest(BitcoinTestFramework): test_node.send_and_ping(msg_block(block)) assert_equal(int(node.getbestblockhash(), 16), block.sha256) + # Multiple blocktxn responses will cause a node to get disconnected. + def test_multiple_blocktxn_response(self, test_node): + node = self.nodes[0] + utxo = self.utxos[0] + + block = self.build_block_with_transactions(node, utxo, 2) + + # Send compact block + comp_block = HeaderAndShortIDs() + comp_block.initialize_from_block(block, prefill_list=[0], use_witness=True) + test_node.send_and_ping(msg_cmpctblock(comp_block.to_p2p())) + absolute_indexes = [] + with p2p_lock: + assert "getblocktxn" in test_node.last_message + absolute_indexes = test_node.last_message["getblocktxn"].block_txn_request.to_absolute() + assert_equal(absolute_indexes, [1, 2]) + + # Send a blocktxn that does not succeed in reconstruction, triggering + # getdata fallback. + msg = msg_blocktxn() + msg.block_transactions = BlockTransactions(block.sha256, [block.vtx[2]] + [block.vtx[1]]) + test_node.send_and_ping(msg) + + # Tip should not have updated + assert_equal(int(node.getbestblockhash(), 16), block.hashPrevBlock) + + # We should receive a getdata request + test_node.wait_for_getdata([block.sha256], timeout=10) + assert test_node.last_message["getdata"].inv[0].type == MSG_BLOCK or \ + test_node.last_message["getdata"].inv[0].type == MSG_BLOCK | MSG_WITNESS_FLAG + + # Send the same blocktxn and assert the sender gets disconnected. + with node.assert_debug_log(['previous compact block reconstruction attempt failed']): + test_node.send_message(msg) + test_node.wait_for_disconnect() + def test_getblocktxn_handler(self, test_node): node = self.nodes[0] # bitcoind will not send blocktxn responses for blocks whose height is @@ -957,6 +993,12 @@ class CompactBlocksTest(BitcoinTestFramework): self.log.info("Testing handling of invalid compact blocks...") self.test_invalid_tx_in_compactblock(self.segwit_node) + self.log.info("Testing handling of multiple blocktxn responses...") + self.test_multiple_blocktxn_response(self.segwit_node) + + # The previous test will lead to a disconnection. Reconnect before continuing. + self.segwit_node = self.nodes[0].add_p2p_connection(TestP2PConn()) + self.log.info("Testing invalid index in cmpctblock message...") self.test_invalid_cmpctblock_message() From 61cdc04a832cc5dfe98c48f8592c4de513258304 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Fri, 12 Sep 2025 17:29:04 -0400 Subject: [PATCH 08/11] net: Do not apply whitelist permission to onion inbounds Tor inbound connections do not reveal the peer's actual network address. Therefore do not apply whitelist permissions to them. Co-authored-by: Vasil Dimov Github-Pull: #33395 Rebased-From: f563ce90818d486d2a199439d2f6ba39cd106352 --- src/net.cpp | 11 +++++++---- src/net.h | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 735985a8414..7684877ec35 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -575,9 +575,9 @@ void CNode::CloseSocketDisconnect() m_i2p_sam_session.reset(); } -void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr, const std::vector& ranges) const { +void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, std::optional addr, const std::vector& ranges) const { for (const auto& subnet : ranges) { - if (subnet.m_subnet.Match(addr)) { + if (addr.has_value() && subnet.m_subnet.Match(addr.value())) { NetPermissions::AddFlag(flags, subnet.m_flags); } } @@ -1767,7 +1767,11 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, { int nInbound = 0; - AddWhitelistPermissionFlags(permission_flags, addr, vWhitelistedRangeIncoming); + const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end(); + + // Tor inbound connections do not reveal the peer's actual network address. + // Therefore do not apply address-based whitelist permissions to them. + AddWhitelistPermissionFlags(permission_flags, inbound_onion ? std::optional{} : addr, vWhitelistedRangeIncoming); { LOCK(m_nodes_mutex); @@ -1822,7 +1826,6 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, NodeId id = GetNewNodeId(); uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); - const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end(); // The V2Transport transparently falls back to V1 behavior when an incoming V1 connection is // detected, so use it whenever we signal NODE_P2P_V2. ServiceFlags local_services = GetLocalServices(); diff --git a/src/net.h b/src/net.h index e64d9a67f46..e025b20bcde 100644 --- a/src/net.h +++ b/src/net.h @@ -1364,7 +1364,7 @@ private: bool AttemptToEvictConnection(); CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); - void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr, const std::vector& ranges) const; + void AddWhitelistPermissionFlags(NetPermissionFlags& flags, std::optional addr, const std::vector& ranges) const; void DeleteNode(CNode* pnode); From 9bc4afb62cf04a41b62fe279f0db3d87e700cb3d Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 9 Sep 2025 10:15:08 +0100 Subject: [PATCH 09/11] doc: update release notes for 29.x --- doc/release-notes.md | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index b73e52dc570..0325d3a3e28 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -1,6 +1,6 @@ -Bitcoin Core version 29.x is now available from: +Bitcoin Core version 29.2rc1 is now available from: - + This release includes various bug fixes and performance improvements, as well as updated translations. @@ -37,19 +37,37 @@ unsupported systems. Notable changes =============== +### P2P + +- #32646 p2p: Add witness mutation check inside FillBlock +- #33296 net: check for empty header before calling FillBlock +- #33395 net: do not apply whitelist permissions to onion inbounds + ### CI - #32999 ci: Use APT_LLVM_V in msan task - #33099 ci: allow for any libc++ intrumentation & use it for TSAN - #33258 ci: use LLVM 21 +- #33364 ci: always use tag for LLVM checkout + +### Misc + +- #33310 trace: Workaround GCC bug compiling with old systemtap +- #33340 Fix benchmark CSV output Credits ======= Thanks to everyone who directly contributed to this release: +- Eugene Siegel - fanquake +- Greg Sanders +- Hennadii Stepanov +- Luke Dashjr - MarcoFalke +- Martin Zumsande +- Vasil Dimov As well as to everyone that helped with translations on [Transifex](https://explore.transifex.com/bitcoin/bitcoin/). From 461dd13fafa6f8175e2be4d96e8728e667ba4d69 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Sep 2025 15:47:34 +0100 Subject: [PATCH 10/11] build: bump version to v29.2rc1 --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index dc613a7655f..05a86a1d97f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,9 +28,9 @@ get_directory_property(precious_variables CACHE_VARIABLES) #============================= set(CLIENT_NAME "Bitcoin Core") set(CLIENT_VERSION_MAJOR 29) -set(CLIENT_VERSION_MINOR 1) +set(CLIENT_VERSION_MINOR 2) set(CLIENT_VERSION_BUILD 0) -set(CLIENT_VERSION_RC 0) +set(CLIENT_VERSION_RC 1) set(CLIENT_VERSION_IS_RELEASE "true") set(COPYRIGHT_YEAR "2025") From f2bd79f80c74a2b77f14954ac65679417697a332 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Sep 2025 15:54:29 +0100 Subject: [PATCH 11/11] doc: update manual pages for v29.2rc1 --- doc/man/bitcoin-cli.1 | 6 +++--- doc/man/bitcoin-qt.1 | 6 +++--- doc/man/bitcoin-tx.1 | 6 +++--- doc/man/bitcoin-util.1 | 6 +++--- doc/man/bitcoin-wallet.1 | 6 +++--- doc/man/bitcoind.1 | 6 +++--- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/doc/man/bitcoin-cli.1 b/doc/man/bitcoin-cli.1 index 428ddd3e2a2..a8dc092a6cc 100644 --- a/doc/man/bitcoin-cli.1 +++ b/doc/man/bitcoin-cli.1 @@ -1,7 +1,7 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH BITCOIN-CLI "1" "September 2025" "bitcoin-cli v29.1.0" "User Commands" +.TH BITCOIN-CLI "1" "September 2025" "bitcoin-cli v29.2.0rc1" "User Commands" .SH NAME -bitcoin-cli \- manual page for bitcoin-cli v29.1.0 +bitcoin-cli \- manual page for bitcoin-cli v29.2.0rc1 .SH SYNOPSIS .B bitcoin-cli [\fI\,options\/\fR] \fI\, \/\fR[\fI\,params\/\fR] @@ -15,7 +15,7 @@ bitcoin-cli \- manual page for bitcoin-cli v29.1.0 .B bitcoin-cli [\fI\,options\/\fR] \fI\,help \/\fR .SH DESCRIPTION -Bitcoin Core RPC client version v29.1.0 +Bitcoin Core RPC client version v29.2.0rc1 .PP The bitcoin\-cli utility provides a command line interface to interact with a Bitcoin Core RPC server. .PP diff --git a/doc/man/bitcoin-qt.1 b/doc/man/bitcoin-qt.1 index 3665a6a48ae..7821b8fb440 100644 --- a/doc/man/bitcoin-qt.1 +++ b/doc/man/bitcoin-qt.1 @@ -1,12 +1,12 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH BITCOIN-QT "1" "September 2025" "bitcoin-qt v29.1.0" "User Commands" +.TH BITCOIN-QT "1" "September 2025" "bitcoin-qt v29.2.0rc1" "User Commands" .SH NAME -bitcoin-qt \- manual page for bitcoin-qt v29.1.0 +bitcoin-qt \- manual page for bitcoin-qt v29.2.0rc1 .SH SYNOPSIS .B bitcoin-qt [\fI\,options\/\fR] [\fI\,URI\/\fR] .SH DESCRIPTION -Bitcoin Core version v29.1.0 +Bitcoin Core version v29.2.0rc1 .PP The bitcoin\-qt application provides a graphical interface for interacting with Bitcoin Core. .PP diff --git a/doc/man/bitcoin-tx.1 b/doc/man/bitcoin-tx.1 index 16058f1bf94..a14a6be6024 100644 --- a/doc/man/bitcoin-tx.1 +++ b/doc/man/bitcoin-tx.1 @@ -1,7 +1,7 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH BITCOIN-TX "1" "September 2025" "bitcoin-tx v29.1.0" "User Commands" +.TH BITCOIN-TX "1" "September 2025" "bitcoin-tx v29.2.0rc1" "User Commands" .SH NAME -bitcoin-tx \- manual page for bitcoin-tx v29.1.0 +bitcoin-tx \- manual page for bitcoin-tx v29.2.0rc1 .SH SYNOPSIS .B bitcoin-tx [\fI\,options\/\fR] \fI\, \/\fR[\fI\,commands\/\fR] @@ -9,7 +9,7 @@ bitcoin-tx \- manual page for bitcoin-tx v29.1.0 .B bitcoin-tx [\fI\,options\/\fR] \fI\,-create \/\fR[\fI\,commands\/\fR] .SH DESCRIPTION -Bitcoin Core bitcoin\-tx utility version v29.1.0 +Bitcoin Core bitcoin\-tx utility version v29.2.0rc1 .PP The bitcoin\-tx tool is used for creating and modifying bitcoin transactions. .PP diff --git a/doc/man/bitcoin-util.1 b/doc/man/bitcoin-util.1 index a103bf40a5b..e0cc27e2d77 100644 --- a/doc/man/bitcoin-util.1 +++ b/doc/man/bitcoin-util.1 @@ -1,7 +1,7 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH BITCOIN-UTIL "1" "September 2025" "bitcoin-util v29.1.0" "User Commands" +.TH BITCOIN-UTIL "1" "September 2025" "bitcoin-util v29.2.0rc1" "User Commands" .SH NAME -bitcoin-util \- manual page for bitcoin-util v29.1.0 +bitcoin-util \- manual page for bitcoin-util v29.2.0rc1 .SH SYNOPSIS .B bitcoin-util [\fI\,options\/\fR] [\fI\,command\/\fR] @@ -9,7 +9,7 @@ bitcoin-util \- manual page for bitcoin-util v29.1.0 .B bitcoin-util [\fI\,options\/\fR] \fI\,grind \/\fR .SH DESCRIPTION -Bitcoin Core bitcoin\-util utility version v29.1.0 +Bitcoin Core bitcoin\-util utility version v29.2.0rc1 .PP The bitcoin\-util tool provides bitcoin related functionality that does not rely on the ability to access a running node. Available [commands] are listed below. .SH OPTIONS diff --git a/doc/man/bitcoin-wallet.1 b/doc/man/bitcoin-wallet.1 index b63494dc479..58bbf2715b3 100644 --- a/doc/man/bitcoin-wallet.1 +++ b/doc/man/bitcoin-wallet.1 @@ -1,12 +1,12 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH BITCOIN-WALLET "1" "September 2025" "bitcoin-wallet v29.1.0" "User Commands" +.TH BITCOIN-WALLET "1" "September 2025" "bitcoin-wallet v29.2.0rc1" "User Commands" .SH NAME -bitcoin-wallet \- manual page for bitcoin-wallet v29.1.0 +bitcoin-wallet \- manual page for bitcoin-wallet v29.2.0rc1 .SH SYNOPSIS .B bitcoin-wallet [\fI\,options\/\fR] \fI\,\/\fR .SH DESCRIPTION -Bitcoin Core bitcoin\-wallet utility version v29.1.0 +Bitcoin Core bitcoin\-wallet utility version v29.2.0rc1 .PP bitcoin\-wallet is an offline tool for creating and interacting with Bitcoin Core wallet files. .PP diff --git a/doc/man/bitcoind.1 b/doc/man/bitcoind.1 index b8ee6bab52a..0846f3e0619 100644 --- a/doc/man/bitcoind.1 +++ b/doc/man/bitcoind.1 @@ -1,12 +1,12 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.3. -.TH BITCOIND "1" "September 2025" "bitcoind v29.1.0" "User Commands" +.TH BITCOIND "1" "September 2025" "bitcoind v29.2.0rc1" "User Commands" .SH NAME -bitcoind \- manual page for bitcoind v29.1.0 +bitcoind \- manual page for bitcoind v29.2.0rc1 .SH SYNOPSIS .B bitcoind [\fI\,options\/\fR] .SH DESCRIPTION -Bitcoin Core daemon version v29.1.0 +Bitcoin Core daemon version v29.2.0rc1 .PP The Bitcoin Core daemon (bitcoind) is a headless program that connects to the Bitcoin network to validate and relay transactions and blocks, as well as relaying addresses. .PP