diff --git a/CMakeLists.txt b/CMakeLists.txt index dc613a7655..05a86a1d97 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") diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh index 25a03d5f50..1b624f3894 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" \ diff --git a/cmake/module/FindUSDT.cmake b/cmake/module/FindUSDT.cmake index 0be7c28ff5..234a099f3f 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/doc/man/bitcoin-cli.1 b/doc/man/bitcoin-cli.1 index 428ddd3e2a..a8dc092a6c 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 3665a6a48a..7821b8fb44 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 16058f1bf9..a14a6be602 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 a103bf40a5..e0cc27e2d7 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 b63494dc47..58bbf2715b 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 b8ee6bab52..0846f3e061 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 diff --git a/doc/release-notes.md b/doc/release-notes.md index b73e52dc57..0325d3a3e2 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/). diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 5f4061a71d..5975a99faa 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 c92aa05e80..fce59bc561 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 { @@ -141,15 +139,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/crypto/sha256.cpp b/src/crypto/sha256.cpp index 09c5d3123e..c5f495708d 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 diff --git a/src/net.cpp b/src/net.cpp index 735985a841..7684877ec3 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 e64d9a67f4..e025b20bcd 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); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1da3ec9d21..d9c2163c1d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3314,7 +3314,21 @@ 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); + + 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, + /*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"); @@ -3322,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); @@ -3331,23 +3348,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 @@ -4462,7 +4463,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 ed95a8831e..d40a0a94ae 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 82d781cd53..c9635cae8c 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,22 @@ 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; diff --git a/src/util/trace.h b/src/util/trace.h index 3deefeade3..ab005dd8bc 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 diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index ca36b2fbc0..da8a0aed9a 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()