From 7c4aa8f6920dc835e13201c933b6a414746e0a08 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 4 Mar 2025 09:55:57 -0500 Subject: [PATCH 01/14] test: introduce VARINT (de)serialization routines --- test/functional/test_framework/messages.py | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index dfb59488fe03..bc600d7f96a7 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -122,6 +122,26 @@ def deser_compact_size(f): return nit +def ser_varint(l): + r = b"" + while True: + r = bytes([(l & 0x7f) | (0x80 if len(r) > 0 else 0x00)]) + r + if l <= 0x7f: + return r + l = (l >> 7) - 1 + + +def deser_varint(f): + n = 0 + while True: + dat = f.read(1)[0] + n = (n << 7) | (dat & 0x7f) + if (dat & 0x80) > 0: + n += 1 + else: + return n + + def deser_string(f): nit = deser_compact_size(f) return f.read(nit) @@ -1980,3 +2000,20 @@ def check_addrv2(ip, net): check_addrv2("2bqghnldu6mcug4pikzprwhtjjnsyederctvci6klcwzepnjd46ikjyd.onion", CAddress.NET_TORV3) check_addrv2("255fhcp6ajvftnyo7bwz3an3t4a4brhopm3bamyh2iu5r3gnr2rq.b32.i2p", CAddress.NET_I2P) check_addrv2("fc32:17ea:e415:c3bf:9808:149d:b5a2:c9aa", CAddress.NET_CJDNS) + + def test_varint_encode_decode(self): + def check_varint(num, expected_encoding_hex): + expected_encoding = bytes.fromhex(expected_encoding_hex) + self.assertEqual(ser_varint(num), expected_encoding) + self.assertEqual(deser_varint(BytesIO(expected_encoding)), num) + + # test cases from serialize_tests.cpp:varint_bitpatterns + check_varint(0, "00") + check_varint(0x7f, "7f") + check_varint(0x80, "8000") + check_varint(0x1234, "a334") + check_varint(0xffff, "82fe7f") + check_varint(0x123456, "c7e756") + check_varint(0x80123456, "86ffc7e756") + check_varint(0xffffffff, "8efefefe7f") + check_varint(0xffffffffffffffff, "80fefefefefefefefe7f") From 8e0b37bcfc24e5f22863ffe77b5ddeeba8e75bdf Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 4 Mar 2025 09:57:49 -0500 Subject: [PATCH 02/14] test: introduce output amount (de)compression routines --- .../feature_framework_unit_tests.py | 1 + test/functional/test_framework/compressor.py | 58 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 test/functional/test_framework/compressor.py diff --git a/test/functional/feature_framework_unit_tests.py b/test/functional/feature_framework_unit_tests.py index 14d83f8a7071..4ee2143ad6ca 100755 --- a/test/functional/feature_framework_unit_tests.py +++ b/test/functional/feature_framework_unit_tests.py @@ -18,6 +18,7 @@ "address", "crypto.bip324_cipher", "blocktools", + "compressor", "crypto.chacha20", "crypto.ellswift", "key", diff --git a/test/functional/test_framework/compressor.py b/test/functional/test_framework/compressor.py new file mode 100644 index 000000000000..1c30d749df55 --- /dev/null +++ b/test/functional/test_framework/compressor.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 +# Copyright (c) 2025-present The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Routines for compressing transaction output amounts and scripts.""" +import unittest + +from .messages import COIN + + +def compress_amount(n): + if n == 0: + return 0 + e = 0 + while ((n % 10) == 0) and (e < 9): + n //= 10 + e += 1 + if e < 9: + d = n % 10 + assert (d >= 1 and d <= 9) + n //= 10 + return 1 + (n*9 + d - 1)*10 + e + else: + return 1 + (n - 1)*10 + 9 + + +def decompress_amount(x): + if x == 0: + return 0 + x -= 1 + e = x % 10 + x //= 10 + n = 0 + if e < 9: + d = (x % 9) + 1 + x //= 9 + n = x * 10 + d + else: + n = x + 1 + while e > 0: + n *= 10 + e -= 1 + return n + + +class TestFrameworkCompressor(unittest.TestCase): + def test_amount_compress_decompress(self): + def check_amount(amount, expected_compressed): + self.assertEqual(compress_amount(amount), expected_compressed) + self.assertEqual(decompress_amount(expected_compressed), amount) + + # test cases from compress_tests.cpp:compress_amounts + check_amount(0, 0x0) + check_amount(1, 0x1) + check_amount(1000000, 0x7) + check_amount(COIN, 0x9) + check_amount(50*COIN, 0x32) + check_amount(21000000*COIN, 0x1406f40) From 5cc0a3131b15cbe846d0fa3ad13d79beaf209e6d Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 19 Feb 2025 09:46:06 -0500 Subject: [PATCH 03/14] qa: use a clearer and documented amount error in malleated snapshot In the assumeutxo functional tests, the final test case with alternated UTxO data tests the error raised when deserializing a snapshot that contains a coin with an amount not in range (<0 or >MAX_MONEY). The current malleation uses an undocumented byte string and offset which makes it hard to maintain. In addition, the undocumented offset is set surprisingly high (39 bytes is well into the serialization of the amount which starts at offset 36). Similarly the value is surprisingly small, presumably one was adjusted for the other. But there is no comment explaining how they were chosen, why not in a clearer manner and what they are supposed to represent. Instead replace this seemingly magic value with a clear one, MAX_MONEY + 1, serialize the whole value for the amount field at the correct offset, and document the whole thing for the next person around. --- test/functional/feature_assumeutxo.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 81be04e1c057..3a45d206aecb 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -16,11 +16,16 @@ create_block, create_coinbase ) +from test_framework.compressor import ( + compress_amount, +) from test_framework.messages import ( CBlockHeader, from_hex, msg_headers, - tx_from_hex + tx_from_hex, + ser_varint, + MAX_MONEY, ) from test_framework.p2p import ( P2PInterface, @@ -139,7 +144,14 @@ def expected_error(msg): [b"\x81", 34, "3da966ba9826fb6d2604260e01607b55ba44e1a5de298606b08704bc62570ea8", None], # wrong coin code VARINT [b"\x80", 34, "091e893b3ccb4334378709578025356c8bcb0a623f37c7c4e493133c988648e5", None], # another wrong coin code [b"\x84\x58", 34, None, "Bad snapshot data after deserializing 0 coins"], # wrong coin case with height 364 and coinbase 0 - [b"\xCA\xD2\x8F\x5A", 39, None, "Bad snapshot data after deserializing 0 coins - bad tx out value"], # Amount exceeds MAX_MONEY + [ + # compressed txout value + scriptpubkey + ser_varint(compress_amount(MAX_MONEY + 1)) + ser_varint(0), + # txid + coins per txid + vout + coin height + 32 + 1 + 1 + 2, + None, + "Bad snapshot data after deserializing 0 coins - bad tx out value" + ], # Amount exceeds MAX_MONEY ] for content, offset, wrong_hash, custom_message in cases: From 4d4fabcd1131793f6e893c9d25efe64a8ba6abf2 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 19 Feb 2025 15:56:45 -0500 Subject: [PATCH 04/14] qa: correct off-by-one in utxo snapshot fuzz target The chain starts at block 1, not genesis. --- src/kernel/chainparams.cpp | 2 +- src/test/fuzz/utxo_snapshot.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index fd61d255f77d..e43757ff75f1 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -635,7 +635,7 @@ class CRegTestParams : public CChainParams { // For use by fuzz target src/test/fuzz/utxo_snapshot.cpp .height = 200, - .hash_serialized = AssumeutxoHash{uint256{"4f34d431c3e482f6b0d67b64609ece3964dc8d7976d02ac68dd7c9c1421738f2"}}, + .hash_serialized = AssumeutxoHash{uint256{"7e3b7780fbd2fa479a01f66950dc8f728dc1b11f03d06d5bf223168520df3a48"}}, .m_chain_tx_count = 201, .blockhash = consteval_ctor(uint256{"5e93653318f294fb5aa339d00bbf8cf1c3515488ad99412c37608b139ea63b27"}), }, diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index 5eb6a3232024..5a9df1e48a4a 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -101,7 +101,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer) std::vector file_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)}; outfile << Span{file_data}; } else { - int height{0}; + int height{1}; for (const auto& block : *g_chain) { auto coinbase{block->vtx.at(0)}; outfile << coinbase->GetHash(); From ea2e06f08e4d9636bd549b3438e8faabbf502e29 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 20 Feb 2025 10:18:21 -0500 Subject: [PATCH 05/14] test util: split up ConnectBlock from MineBlock --- src/test/util/mining.cpp | 5 +++++ src/test/util/mining.h | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp index 04925792dce5..8baac7bba3e3 100644 --- a/src/test/util/mining.cpp +++ b/src/test/util/mining.cpp @@ -92,6 +92,11 @@ COutPoint MineBlock(const NodeContext& node, std::shared_ptr& block) assert(block->nNonce); } + return ProcessBlock(node, block); +} + +COutPoint ProcessBlock(const NodeContext& node, const std::shared_ptr& block) +{ auto& chainman{*Assert(node.chainman)}; const auto old_height = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveHeight()); bool new_block; diff --git a/src/test/util/mining.h b/src/test/util/mining.h index 9c6e29b4d34d..9ad8692276cc 100644 --- a/src/test/util/mining.h +++ b/src/test/util/mining.h @@ -32,6 +32,11 @@ COutPoint MineBlock(const node::NodeContext&, **/ COutPoint MineBlock(const node::NodeContext&, std::shared_ptr& block); +/** + * Returns the generated coin (or Null if the block was invalid). + */ +COutPoint ProcessBlock(const node::NodeContext&, const std::shared_ptr& block); + /** Prepare a block to be mined */ std::shared_ptr PrepareBlock(const node::NodeContext&); std::shared_ptr PrepareBlock(const node::NodeContext& node, From a2f46626728249420ca5651aab347beac122ef77 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 20 Feb 2025 10:19:18 -0500 Subject: [PATCH 06/14] fuzz: sanity check hardcoded snapshot in utxo_snapshot target The assumeutxo data for the fuzz target could change and invalidate the hash silently, preventing the fuzz target from reaching some code paths. Fix this by sanity checking the snapshot values during initialization. --- src/test/fuzz/utxo_snapshot.cpp | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index 5a9df1e48a4a..825fada9dda7 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -39,7 +40,31 @@ using node::SnapshotMetadata; namespace { const std::vector>* g_chain; -TestingSetup* g_setup; +TestingSetup* g_setup{nullptr}; + +/** Sanity check the assumeutxo values hardcoded in chainparams for the fuzz target. */ +void sanity_check_snapshot() +{ + Assert(g_chain && g_setup == nullptr); + + // Create a temporary chainstate manager to connect the chain to. + const auto tmp_setup{MakeNoLogFileContext(ChainType::REGTEST, TestOpts{.setup_net = false})}; + const auto& node{tmp_setup->m_node}; + for (auto& block: *g_chain) { + ProcessBlock(node, block); + } + + // Connect the chain to the tmp chainman and sanity check the chainparams snapshot values. + LOCK(cs_main); + auto& cs{node.chainman->ActiveChainstate()}; + cs.ForceFlushStateToDisk(); + const auto stats{*Assert(kernel::ComputeUTXOStats(kernel::CoinStatsHashType::HASH_SERIALIZED, &cs.CoinsDB(), node.chainman->m_blockman))}; + const auto cp_au_data{*Assert(node.chainman->GetParams().AssumeutxoForHeight(2 * COINBASE_MATURITY))}; + Assert(stats.nHeight == cp_au_data.height); + Assert(stats.nTransactions + 1 == cp_au_data.m_chain_tx_count); // +1 for the genesis tx. + Assert(stats.hashBlock == cp_au_data.blockhash); + Assert(AssumeutxoHash{stats.hashSerialized} == cp_au_data.hash_serialized); +} template void initialize_chain() @@ -47,6 +72,10 @@ void initialize_chain() const auto params{CreateChainParams(ArgsManager{}, ChainType::REGTEST)}; static const auto chain{CreateBlockChain(2 * COINBASE_MATURITY, *params)}; g_chain = &chain; + + // Make sure we can generate a valid snapshot. + sanity_check_snapshot(); + static const auto setup{ MakeNoLogFileContext(ChainType::REGTEST, TestOpts{ From 2597572ad5e01e12716461cf5f824906046ee809 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 3 Oct 2025 17:30:09 -0400 Subject: [PATCH 07/14] qa: delete one "bad snapshot data" case in feature_assumeutxo.py This test case is brittle as it asserts a specific error string, when the error string depends on data in the snapshot not controlled by the test (i.e. not injected by the test before asserting the error string). This can be fixed in a more involved way as per Bitcoin Core PR 32117, but since this PR is now closed in Core, in the meantime just disable the brittle test in inquisition (see discussion in Bitcoin Inquisition PR 90). --- test/functional/feature_assumeutxo.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 3a45d206aecb..9320294b5808 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -138,7 +138,6 @@ def expected_error(msg): cases = [ # (content, offset, wrong_hash, custom_message) [b"\xff" * 32, 0, "7d52155c9a9fdc4525b637ef6170568e5dad6fabd0b1fdbb9432010b8453095b", None], # wrong outpoint hash - [(2).to_bytes(1, "little"), 32, None, "Bad snapshot data after deserializing 1 coins."], # wrong txid coins count [b"\xfd\xff\xff", 32, None, "Mismatch in coins count in snapshot metadata and actual snapshot data"], # txid coins count exceeds coins left [b"\x01", 33, "9f4d897031ab8547665b4153317ae2fdbf0130c7840b66427ebc48b881cb80ad", None], # wrong outpoint index [b"\x81", 34, "3da966ba9826fb6d2604260e01607b55ba44e1a5de298606b08704bc62570ea8", None], # wrong coin code VARINT From 24d6cb5a0b787d5796f5b035df0f0203ca42128e Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 18 Dec 2025 07:39:59 -0500 Subject: [PATCH 08/14] qa: introduce a MAX_SEQUENCE_NONFINAL constant to the functional test framework This constant was introduced in Bitcoin Core PR 31953 (commit fa86190e6ed2aeda7bcceaf96f52403816bcd751), and PR 32155, which we are about to backport, depends on it. Instead of backporting unrelated PR 31953, just introduce the constant here. --- test/functional/test_framework/messages.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index bc600d7f96a7..55a5c17985cd 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -42,6 +42,7 @@ MAX_MONEY = 21000000 * COIN MAX_BIP125_RBF_SEQUENCE = 0xfffffffd # Sequence number that is rbf-opt-in (BIP 125) and csv-opt-out (BIP 68) +MAX_SEQUENCE_NONFINAL = 0xfffffffe # Sequence number that is csv-opt-out (BIP 68) SEQUENCE_FINAL = 0xffffffff # Sequence number that disables nLockTime if set for every input of a tx MAX_PROTOCOL_MESSAGE_LENGTH = 4000000 # Maximum length of incoming protocol messages From 3dd7bb23cf7da04d4e27537d367f36741fac4034 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 21 Apr 2025 15:12:48 -0400 Subject: [PATCH 09/14] qa: timelock coinbase transactions created in functional tests --- test/functional/feature_block.py | 2 ++ test/functional/test_framework/blocktools.py | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 2dfa568c5b6c..f4f15d293ecd 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -831,6 +831,7 @@ def run_test(self): self.log.info("Reject a block with a transaction with a duplicate hash of a previous transaction (BIP30)") self.move_tip(60) b61 = self.next_block(61) + b61.vtx[0].nLockTime = 0 b61.vtx[0].vin[0].scriptSig = DUPLICATE_COINBASE_SCRIPT_SIG b61.vtx[0].rehash() b61 = self.update_block(61, []) @@ -853,6 +854,7 @@ def run_test(self): b_spend_dup_cb = self.update_block('spend_dup_cb', [tx]) b_dup_2 = self.next_block('dup_2') + b_dup_2.vtx[0].nLockTime = 0 b_dup_2.vtx[0].vin[0].scriptSig = DUPLICATE_COINBASE_SCRIPT_SIG b_dup_2.vtx[0].rehash() b_dup_2 = self.update_block('dup_2', []) diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 38600bc005a1..528df015c58d 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -30,6 +30,7 @@ uint256_from_compact, uint256_from_str, WITNESS_SCALE_FACTOR, + MAX_SEQUENCE_NONFINAL, ) from .script import ( CScript, @@ -152,7 +153,8 @@ def create_coinbase(height, pubkey=None, *, script_pubkey=None, extra_output_scr If extra_output_script is given, make a 0-value output to that script. This is useful to pad block weight/sigops as needed. """ coinbase = CTransaction() - coinbase.vin.append(CTxIn(COutPoint(0, 0xffffffff), script_BIP34_coinbase_height(height), SEQUENCE_FINAL)) + coinbase.nLockTime = height - 1 + coinbase.vin.append(CTxIn(COutPoint(0, 0xffffffff), script_BIP34_coinbase_height(height), MAX_SEQUENCE_NONFINAL)) coinbaseoutput = CTxOut() coinbaseoutput.nValue = nValue * COIN if nValue == 50: From d735ff69a0f5ba0168eda6d653ebb6af106106d5 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 21 Apr 2025 15:53:09 -0400 Subject: [PATCH 10/14] contrib: timelock coinbase transactions in signet miner --- contrib/signet/miner | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contrib/signet/miner b/contrib/signet/miner index a709c8c8a777..a92529ec8680 100755 --- a/contrib/signet/miner +++ b/contrib/signet/miner @@ -19,7 +19,7 @@ PATH_BASE_TEST_FUNCTIONAL = os.path.abspath(os.path.join(PATH_BASE_CONTRIB_SIGNE sys.path.insert(0, PATH_BASE_TEST_FUNCTIONAL) from test_framework.blocktools import get_witness_script, script_BIP34_coinbase_height # noqa: E402 -from test_framework.messages import CBlock, CBlockHeader, COutPoint, CTransaction, CTxIn, CTxInWitness, CTxOut, from_binary, from_hex, ser_string, ser_uint256, tx_from_hex # noqa: E402 +from test_framework.messages import CBlock, CBlockHeader, COutPoint, CTransaction, CTxIn, CTxInWitness, CTxOut, from_binary, from_hex, ser_string, ser_uint256, tx_from_hex, MAX_SEQUENCE_NONFINAL # noqa: E402 from test_framework.psbt import PSBT, PSBTMap, PSBT_GLOBAL_UNSIGNED_TX, PSBT_IN_FINAL_SCRIPTSIG, PSBT_IN_FINAL_SCRIPTWITNESS, PSBT_IN_NON_WITNESS_UTXO, PSBT_IN_SIGHASH_TYPE # noqa: E402 from test_framework.script import CScript, CScriptOp # noqa: E402 @@ -102,7 +102,8 @@ def generate_psbt(tmpl, reward_spk, *, blocktime=None, poolid=None): scriptSig = CScript(b"" + scriptSig + CScriptOp.encode_op_pushdata(poolid)) cbtx = CTransaction() - cbtx.vin = [CTxIn(COutPoint(0, 0xffffffff), scriptSig, 0xffffffff)] + cbtx.nLockTime = tmpl["height"] - 1 + cbtx.vin = [CTxIn(COutPoint(0, 0xffffffff), scriptSig, MAX_SEQUENCE_NONFINAL)] cbtx.vout = [CTxOut(tmpl["coinbasevalue"], reward_spk)] cbtx.vin[0].nSequence = 2**32-2 cbtx.rehash() From 11e62263fa216dfa9efbd3cc79e45de79d9b130f Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 21 Apr 2025 14:47:39 -0400 Subject: [PATCH 11/14] qa: timelock coinbase transactions created in fuzz targets --- src/kernel/chainparams.cpp | 4 ++-- src/test/fuzz/utxo_total_supply.cpp | 1 + src/test/util/mining.cpp | 5 +++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index e43757ff75f1..9f1e2409700c 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -635,9 +635,9 @@ class CRegTestParams : public CChainParams { // For use by fuzz target src/test/fuzz/utxo_snapshot.cpp .height = 200, - .hash_serialized = AssumeutxoHash{uint256{"7e3b7780fbd2fa479a01f66950dc8f728dc1b11f03d06d5bf223168520df3a48"}}, + .hash_serialized = AssumeutxoHash{uint256{"17dcc016d188d16068907cdeb38b75691a118d43053b8cd6a25969419381d13a"}}, .m_chain_tx_count = 201, - .blockhash = consteval_ctor(uint256{"5e93653318f294fb5aa339d00bbf8cf1c3515488ad99412c37608b139ea63b27"}), + .blockhash = consteval_ctor(uint256{"385901ccbd69dff6bbd00065d01fb8a9e464dede7cfe0372443884f9b1dcf6b9"}), }, { // For use by test/functional/feature_assumeutxo.py diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index e574c5b85c5a..9be85df99fa3 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -52,6 +52,7 @@ FUZZ_TARGET(utxo_total_supply) // Replace OP_FALSE with OP_TRUE { CMutableTransaction tx{*block->vtx.back()}; + tx.nLockTime = 0; // Use the same nLockTime for all as we want to duplicate one of them. tx.vout.at(0).scriptPubKey = CScript{} << OP_TRUE; block->vtx.back() = MakeTransactionRef(tx); } diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp index 8baac7bba3e3..1cd8f8ee7189 100644 --- a/src/test/util/mining.cpp +++ b/src/test/util/mining.cpp @@ -17,6 +17,8 @@ #include #include +#include + using node::BlockAssembler; using node::NodeContext; @@ -34,12 +36,15 @@ std::vector> CreateBlockChain(size_t total_height, const { std::vector> ret{total_height}; auto time{params.GenesisBlock().nTime}; + // NOTE: here `height` does not correspond to the block height but the block height - 1. for (size_t height{0}; height < total_height; ++height) { CBlock& block{*(ret.at(height) = std::make_shared())}; CMutableTransaction coinbase_tx; + coinbase_tx.nLockTime = static_cast(height); coinbase_tx.vin.resize(1); coinbase_tx.vin[0].prevout.SetNull(); + coinbase_tx.vin[0].nSequence = CTxIn::MAX_SEQUENCE_NONFINAL; // Make sure timelock is enforced. coinbase_tx.vout.resize(1); coinbase_tx.vout[0].scriptPubKey = P2WSH_OP_TRUE; coinbase_tx.vout[0].nValue = GetBlockSubsidy(height + 1, params.GetConsensus()); From 55851a669b668caf201baee87a4458015b6d9a00 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 21 Apr 2025 15:16:28 -0400 Subject: [PATCH 12/14] qa: use prev height as nLockTime for coinbase txs created in unit tests We don't set the nSequence as it will be set directly in the block template generator in a following commit. --- src/test/blockfilter_index_tests.cpp | 1 + src/test/validation_block_tests.cpp | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 4b576bb084f3..81119f52b192 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -81,6 +81,7 @@ CBlock BuildChainTestingSetup::CreateBlock(const CBlockIndex* prev, } { CMutableTransaction tx_coinbase{*block.vtx.at(0)}; + tx_coinbase.nLockTime = static_cast(prev->nHeight); tx_coinbase.vin.at(0).scriptSig = CScript{} << prev->nHeight + 1; block.vtx.at(0) = MakeTransactionRef(std::move(tx_coinbase)); block.hashMerkleRoot = BlockMerkleRoot(block); diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 5c1f195868b2..a0b23f5d3b7c 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -82,7 +82,9 @@ std::shared_ptr MinerTestingSetup::Block(const uint256& prev_hash) txCoinbase.vout[0].nValue = 0; txCoinbase.vin[0].scriptWitness.SetNull(); // Always pad with OP_0 at the end to avoid bad-cb-length error - txCoinbase.vin[0].scriptSig = CScript{} << WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(prev_hash)->nHeight + 1) << OP_0; + const int prev_height{WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(prev_hash)->nHeight)}; + txCoinbase.vin[0].scriptSig = CScript{} << prev_height + 1 << OP_0; + txCoinbase.nLockTime = static_cast(prev_height); pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); return pblock; From 452eb34c1234c05698bfc60ec55fcc811ffc2877 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 19 Feb 2025 10:26:50 -0500 Subject: [PATCH 13/14] miner: timelock coinbase transactions The Consensus Cleanup soft fork proposal includes enforcing that coinbase transactions set their locktime field to the block height, minus 1 (as well as their nSequence such as to not disable the timelock). If such a fork were to be activated by Bitcoin users, miners need to be ready to produce compliant blocks at the risk of losing substantial amounts mining would-be invalid blocks. As miners are unfamously slow to upgrade, it's good to make this change as early as possible. Although Bitcoin Core's GBT implementation does not provide the "coinbasetxn" field, and mining pool software crafts the coinbase on its own, updating the Bitcoin Core mining code is a first step toward convincing pools to update their (often closed source) code. A possible followup is also to introduce new fields to GBT. In addition, this first step also makes it possible to test future Consensus Cleanup changes. The changes to the seemingly-unrelated RBF tests is because these tests assert an error message which may vary depending on the txid of the transactions used in the test. This commit changes the coinbase transaction structure and therefore impact the txid of transactions in all tests. The change to the "Bad snapshot" error message in the assumeutxo functional test is because this specific test case reads into the txid of the next transaction in the snapshot and asserts the error message based it gets on deserializing this txid as a coin for the previous transaction. As this commit changes this txid it impacts the deserialization error raised. --- src/kernel/chainparams.cpp | 12 +++---- src/node/miner.cpp | 3 ++ src/test/miner_tests.cpp | 40 ++++++++++++------------ src/test/rbf_tests.cpp | 8 ++--- src/test/util/setup_common.cpp | 2 +- src/test/validation_tests.cpp | 6 ++-- test/functional/feature_assumeutxo.py | 18 +++++------ test/functional/feature_utxo_set_hash.py | 4 +-- test/functional/mempool_package_rbf.py | 4 +-- test/functional/rpc_dumptxoutset.py | 6 ++-- test/functional/rpc_getblockfrompeer.py | 2 +- test/functional/wallet_assumeutxo.py | 2 +- 12 files changed, 55 insertions(+), 52 deletions(-) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 9f1e2409700c..090a49ef8480 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -628,9 +628,9 @@ class CRegTestParams : public CChainParams m_assumeutxo_data = { { // For use by unit tests .height = 110, - .hash_serialized = AssumeutxoHash{uint256{"6657b736d4fe4db0cbc796789e812d5dba7f5c143764b1b6905612f1830609d1"}}, + .hash_serialized = AssumeutxoHash{uint256{"b952555c8ab81fec46f3d4253b7af256d766ceb39fb7752b9d18cdf4a0141327"}}, .m_chain_tx_count = 111, - .blockhash = consteval_ctor(uint256{"696e92821f65549c7ee134edceeeeaaa4105647a3c4fd9f298c0aec0ab50425c"}), + .blockhash = consteval_ctor(uint256{"6affe030b7965ab538f820a56ef56c8149b7dc1d1c144af57113be080db7c397"}), }, { // For use by fuzz target src/test/fuzz/utxo_snapshot.cpp @@ -642,18 +642,18 @@ class CRegTestParams : public CChainParams { // For use by test/functional/feature_assumeutxo.py .height = 299, - .hash_serialized = AssumeutxoHash{uint256{"a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27"}}, + .hash_serialized = AssumeutxoHash{uint256{"d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2"}}, .m_chain_tx_count = 334, - .blockhash = consteval_ctor(uint256{"3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0"}), + .blockhash = consteval_ctor(uint256{"7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2"}), }, { // For use by test/functional/feature_assumeutxo.py // heretical deployments signals differently, generating // incompatible block hashes in the functional test cache .height = 299, - .hash_serialized = AssumeutxoHash{uint256{"a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27"}}, + .hash_serialized = AssumeutxoHash{uint256{"d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2"}}, .m_chain_tx_count = 334, - .blockhash = consteval_ctor(uint256{"0300fa441b815903744564ef7eb87199b056e7ddb56283c51b6c40b1d6257b21"}), + .blockhash = consteval_ctor(uint256{"5d72ceddcba989e3b0165eab863aafb8c0119b77126e423f4136cd65ef5ba9e9"}), }, }; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 29e11a935a03..ac9555633dcc 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -167,10 +167,13 @@ std::unique_ptr BlockAssembler::CreateNewBlock() CMutableTransaction coinbaseTx; coinbaseTx.vin.resize(1); coinbaseTx.vin[0].prevout.SetNull(); + coinbaseTx.vin[0].nSequence = CTxIn::MAX_SEQUENCE_NONFINAL; // Make sure timelock is enforced. coinbaseTx.vout.resize(1); coinbaseTx.vout[0].scriptPubKey = m_options.coinbase_output_script; coinbaseTx.vout[0].nValue = nFees + GetBlockSubsidy(nHeight, chainparams.GetConsensus()); coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0; + Assert(nHeight > 0); + coinbaseTx.nLockTime = static_cast(nHeight - 1); pblock->vtx[0] = MakeTransactionRef(std::move(coinbaseTx)); pblocktemplate->vchCoinbaseCommitment = m_chainstate.m_chainman.GenerateCoinbaseCommitment(*pblock, pindexPrev); pblocktemplate->vTxFees[0] = -nFees; diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 10f8be4b324d..954c30f72398 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -71,27 +71,27 @@ BOOST_FIXTURE_TEST_SUITE(miner_tests, MinerTestingSetup) static CFeeRate blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE); constexpr static struct { - unsigned char extranonce; + unsigned int extranonce; unsigned int nonce; -} BLOCKINFO[]{{8, 582909131}, {0, 971462344}, {2, 1169481553}, {6, 66147495}, {7, 427785981}, {8, 80538907}, - {8, 207348013}, {2, 1951240923}, {4, 215054351}, {1, 491520534}, {8, 1282281282}, {4, 639565734}, - {3, 248274685}, {8, 1160085976}, {6, 396349768}, {5, 393780549}, {5, 1096899528}, {4, 965381630}, - {0, 728758712}, {5, 318638310}, {3, 164591898}, {2, 274234550}, {2, 254411237}, {7, 561761812}, - {2, 268342573}, {0, 402816691}, {1, 221006382}, {6, 538872455}, {7, 393315655}, {4, 814555937}, - {7, 504879194}, {6, 467769648}, {3, 925972193}, {2, 200581872}, {3, 168915404}, {8, 430446262}, - {5, 773507406}, {3, 1195366164}, {0, 433361157}, {3, 297051771}, {0, 558856551}, {2, 501614039}, - {3, 528488272}, {2, 473587734}, {8, 230125274}, {2, 494084400}, {4, 357314010}, {8, 60361686}, - {7, 640624687}, {3, 480441695}, {8, 1424447925}, {4, 752745419}, {1, 288532283}, {6, 669170574}, - {5, 1900907591}, {3, 555326037}, {3, 1121014051}, {0, 545835650}, {8, 189196651}, {5, 252371575}, - {0, 199163095}, {6, 558895874}, {6, 1656839784}, {6, 815175452}, {6, 718677851}, {5, 544000334}, - {0, 340113484}, {6, 850744437}, {4, 496721063}, {8, 524715182}, {6, 574361898}, {6, 1642305743}, - {6, 355110149}, {5, 1647379658}, {8, 1103005356}, {7, 556460625}, {3, 1139533992}, {5, 304736030}, - {2, 361539446}, {2, 143720360}, {6, 201939025}, {7, 423141476}, {4, 574633709}, {3, 1412254823}, - {4, 873254135}, {0, 341817335}, {6, 53501687}, {3, 179755410}, {5, 172209688}, {8, 516810279}, - {4, 1228391489}, {8, 325372589}, {6, 550367589}, {0, 876291812}, {7, 412454120}, {7, 717202854}, - {2, 222677843}, {6, 251778867}, {7, 842004420}, {7, 194762829}, {4, 96668841}, {1, 925485796}, - {0, 792342903}, {6, 678455063}, {6, 773251385}, {5, 186617471}, {6, 883189502}, {7, 396077336}, - {8, 254702874}, {0, 455592851}}; +} BLOCKINFO[]{{0, 3552706918}, {500, 37506755}, {1000, 948987788}, {400, 524762339}, {800, 258510074}, {300, 102309278}, + {1300, 54365202}, {600, 1107740426}, {1000, 203094491}, {900, 391178848}, {800, 381177271}, {600, 87188412}, + {0, 66522866}, {800, 874942736}, {1000, 89200838}, {400, 312638088}, {400, 66263693}, {500, 924648304}, + {400, 369913599}, {500, 47630099}, {500, 115045364}, {100, 277026602}, {1100, 809621409}, {700, 155345322}, + {800, 943579953}, {400, 28200730}, {900, 77200495}, {0, 105935488}, {400, 698721821}, {500, 111098863}, + {1300, 445389594}, {500, 621849894}, {1400, 56010046}, {1100, 370669776}, {1200, 380301940}, {1200, 110654905}, + {400, 213771024}, {1500, 120014726}, {1200, 835019014}, {1500, 624817237}, {900, 1404297}, {400, 189414558}, + {400, 293178348}, {1100, 15393789}, {600, 396764180}, {800, 1387046371}, {800, 199368303}, {700, 111496662}, + {100, 129759616}, {200, 536577982}, {500, 125881300}, {500, 101053391}, {1200, 471590548}, {900, 86957729}, + {1200, 179604104}, {600, 68658642}, {1000, 203295701}, {500, 139615361}, {900, 233693412}, {300, 153225163}, + {0, 27616254}, {1200, 9856191}, {100, 220392722}, {200, 66257599}, {1100, 145489641}, {1300, 37859442}, + {400, 5816075}, {1200, 215752117}, {1400, 32361482}, {1400, 6529223}, {500, 143332977}, {800, 878392}, + {700, 159290408}, {400, 123197595}, {700, 43988693}, {300, 304224916}, {700, 214771621}, {1100, 274148273}, + {400, 285632418}, {1100, 923451065}, {600, 12818092}, {1200, 736282054}, {1000, 246683167}, {600, 92950402}, + {1400, 29223405}, {1000, 841327192}, {700, 174301283}, {1400, 214009854}, {1000, 6989517}, {1200, 278226956}, + {700, 540219613}, {400, 93663104}, {1100, 152345635}, {1500, 464194499}, {1300, 333850111}, {600, 258311263}, + {600, 90173162}, {1000, 33590797}, {1500, 332866027}, {100, 204704427}, {1000, 463153545}, {800, 303244785}, + {600, 88096214}, {0, 137477892}, {1200, 195514506}, {300, 704114595}, {900, 292087369}, {1400, 758684870}, + {1300, 163493028}, {1200, 53151293}}; static std::unique_ptr CreateBlockIndex(int nHeight, CBlockIndex* active_chain_tip) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index db328f87412e..937a538a53e1 100644 --- a/src/test/rbf_tests.cpp +++ b/src/test/rbf_tests.cpp @@ -311,7 +311,7 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup) // Tests for CheckConflictTopology // Tx4 has 23 descendants - BOOST_CHECK_EQUAL(pool.CheckConflictTopology(set_34_cpfp).value(), strprintf("%s has 23 descendants, max 1 allowed", entry4_high->GetSharedTx()->GetHash().ToString())); + BOOST_CHECK_EQUAL(pool.CheckConflictTopology(set_34_cpfp).value(), strprintf("%s has 24 descendants, max 1 allowed", entry3_low->GetSharedTx()->GetHash().ToString())); // No descendants yet BOOST_CHECK(pool.CheckConflictTopology({entry9_unchained}) == std::nullopt); @@ -441,7 +441,7 @@ BOOST_FIXTURE_TEST_CASE(improves_feerate, TestChain100Setup) const auto res3 = ImprovesFeerateDiagram(*changeset); BOOST_CHECK(res3.has_value()); BOOST_CHECK(res3.value().first == DiagramCheckError::UNCALCULABLE); - BOOST_CHECK(res3.value().second == strprintf("%s has 2 ancestors, max 1 allowed", tx5->GetHash().GetHex())); + BOOST_CHECK_MESSAGE(res3.value().second == strprintf("%s has 2 descendants, max 1 allowed", tx1->GetHash().GetHex()), res3.value().second); } BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) @@ -536,7 +536,7 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) changeset->StageAddition(replacement_tx, high_fee, 0, 1, 0, false, 4, LockPoints()); const auto replace_too_large{changeset->CalculateChunksForRBF()}; BOOST_CHECK(!replace_too_large.has_value()); - BOOST_CHECK_EQUAL(util::ErrorString(replace_too_large).original, strprintf("%s has 2 ancestors, max 1 allowed", normal_tx->GetHash().GetHex())); + BOOST_CHECK_EQUAL(util::ErrorString(replace_too_large).original, strprintf("%s has both ancestor and descendant, exceeding cluster limit of 2", high_tx->GetHash().GetHex())); } // Make a size 2 cluster that is itself two chunks; evict both txns @@ -623,7 +623,7 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto replace_cluster_size_3{changeset->CalculateChunksForRBF()}; BOOST_CHECK(!replace_cluster_size_3.has_value()); - BOOST_CHECK_EQUAL(util::ErrorString(replace_cluster_size_3).original, strprintf("%s has both ancestor and descendant, exceeding cluster limit of 2", conflict_1_child->GetHash().GetHex())); + BOOST_CHECK_EQUAL(util::ErrorString(replace_cluster_size_3).original, strprintf("%s has 2 descendants, max 1 allowed", conflict_1->GetHash().GetHex())); } } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 72acad5a5f98..06c1d50447a9 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -369,7 +369,7 @@ TestChain100Setup::TestChain100Setup( LOCK(::cs_main); assert( m_node.chainman->ActiveChain().Tip()->GetBlockHash().ToString() == - "571d80a9967ae599cec0448b0b0ba1cfb606f584d8069bd7166b86854ba7a191"); + "0c8c5f79505775a0f6aed6aca2350718ceb9c6f2c878667864d5c7a6d8ffa2a6"); } } diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp index 2e378ed30b52..7e86b3a35c1f 100644 --- a/src/test/validation_tests.cpp +++ b/src/test/validation_tests.cpp @@ -142,11 +142,11 @@ BOOST_AUTO_TEST_CASE(test_assumeutxo) } const auto out110 = *params->AssumeutxoForHeight(110); - BOOST_CHECK_EQUAL(out110.hash_serialized.ToString(), "6657b736d4fe4db0cbc796789e812d5dba7f5c143764b1b6905612f1830609d1"); + BOOST_CHECK_EQUAL(out110.hash_serialized.ToString(), "b952555c8ab81fec46f3d4253b7af256d766ceb39fb7752b9d18cdf4a0141327"); BOOST_CHECK_EQUAL(out110.m_chain_tx_count, 111U); - const auto out110_2 = *params->AssumeutxoForBlockhash(uint256{"696e92821f65549c7ee134edceeeeaaa4105647a3c4fd9f298c0aec0ab50425c"}); - BOOST_CHECK_EQUAL(out110_2.hash_serialized.ToString(), "6657b736d4fe4db0cbc796789e812d5dba7f5c143764b1b6905612f1830609d1"); + const auto out110_2 = *params->AssumeutxoForBlockhash(uint256{"6affe030b7965ab538f820a56ef56c8149b7dc1d1c144af57113be080db7c397"}); + BOOST_CHECK_EQUAL(out110_2.hash_serialized.ToString(), "b952555c8ab81fec46f3d4253b7af256d766ceb39fb7752b9d18cdf4a0141327"); BOOST_CHECK_EQUAL(out110_2.m_chain_tx_count, 111U); } diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 9320294b5808..5d14426fbf48 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -137,11 +137,11 @@ def expected_error(msg): self.log.info(" - snapshot file with alternated but parsable UTXO data results in different hash") cases = [ # (content, offset, wrong_hash, custom_message) - [b"\xff" * 32, 0, "7d52155c9a9fdc4525b637ef6170568e5dad6fabd0b1fdbb9432010b8453095b", None], # wrong outpoint hash + [b"\xff" * 32, 0, "77874d48d932a5cb7a7f770696f5224ff05746fdcf732a58270b45da0f665934", None], # wrong outpoint hash [b"\xfd\xff\xff", 32, None, "Mismatch in coins count in snapshot metadata and actual snapshot data"], # txid coins count exceeds coins left - [b"\x01", 33, "9f4d897031ab8547665b4153317ae2fdbf0130c7840b66427ebc48b881cb80ad", None], # wrong outpoint index - [b"\x81", 34, "3da966ba9826fb6d2604260e01607b55ba44e1a5de298606b08704bc62570ea8", None], # wrong coin code VARINT - [b"\x80", 34, "091e893b3ccb4334378709578025356c8bcb0a623f37c7c4e493133c988648e5", None], # another wrong coin code + [b"\x01", 33, "9f562925721e4f97e6fde5b590dbfede51e2204a68639525062ad064545dd0ea", None], # wrong outpoint index + [b"\x82", 34, "161393f07f8ad71760b3910a914f677f2cb166e5bcf5354e50d46b78c0422d15", None], # wrong coin code VARINT + [b"\x80", 34, "e6fae191ef851554467b68acff01ca09ad0a2e48c9b3dfea46cf7d35a7fd0ad0", None], # another wrong coin code [b"\x84\x58", 34, None, "Bad snapshot data after deserializing 0 coins"], # wrong coin case with height 364 and coinbase 0 [ # compressed txout value + scriptpubkey @@ -160,12 +160,12 @@ def expected_error(msg): f.write(content) f.write(valid_snapshot_contents[(5 + 2 + 4 + 32 + 8 + offset + len(content)):]) - msg = custom_message if custom_message is not None else f"Bad snapshot content hash: expected a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27, got {wrong_hash}." + msg = custom_message if custom_message is not None else f"Bad snapshot content hash: expected d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2, got {wrong_hash}." expected_error(msg) def test_headers_not_synced(self, valid_snapshot_path): for node in self.nodes[1:]: - msg = "Unable to load UTXO snapshot: The base block header (0300fa441b815903744564ef7eb87199b056e7ddb56283c51b6c40b1d6257b21) must appear in the headers chain. Make sure all headers are syncing, and call loadtxoutset again." + msg = "Unable to load UTXO snapshot: The base block header (5d72ceddcba989e3b0165eab863aafb8c0119b77126e423f4136cd65ef5ba9e9) must appear in the headers chain. Make sure all headers are syncing, and call loadtxoutset again." assert_raises_rpc_error(-32603, msg, node.loadtxoutset, valid_snapshot_path) def test_invalid_chainstate_scenarios(self): @@ -224,7 +224,7 @@ def test_snapshot_block_invalidated(self, dump_output_path): block_hash = node.getblockhash(height) node.invalidateblock(block_hash) assert_equal(node.getblockcount(), height - 1) - msg = "Unable to load UTXO snapshot: The base block header (0300fa441b815903744564ef7eb87199b056e7ddb56283c51b6c40b1d6257b21) is part of an invalid chain." + msg = "Unable to load UTXO snapshot: The base block header (5d72ceddcba989e3b0165eab863aafb8c0119b77126e423f4136cd65ef5ba9e9) is part of an invalid chain." assert_raises_rpc_error(-32603, msg, node.loadtxoutset, dump_output_path) node.reconsiderblock(block_hash) @@ -426,7 +426,7 @@ def run_test(self): def check_dump_output(output): assert_equal( output['txoutset_hash'], - "a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27") + "d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2") assert_equal(output["nchaintx"], blocks[SNAPSHOT_BASE_HEIGHT].chain_tx) check_dump_output(dump_output) @@ -456,7 +456,7 @@ def check_dump_output(output): dump_output4 = n0.dumptxoutset(path='utxos4.dat', rollback=prev_snap_height) assert_equal( dump_output4['txoutset_hash'], - "8a1db0d6e958ce0d7c963bc6fc91ead596c027129bacec68acc40351037b09d7") + "45ac2777b6ca96588210e2a4f14b602b41ec37b8b9370673048cc0af434a1ec8") assert sha256sum_file(dump_output['path']) != sha256sum_file(dump_output4['path']) # Use a hash instead of a height diff --git a/test/functional/feature_utxo_set_hash.py b/test/functional/feature_utxo_set_hash.py index 3ab779b87dc8..21a3c13f7e4e 100755 --- a/test/functional/feature_utxo_set_hash.py +++ b/test/functional/feature_utxo_set_hash.py @@ -67,8 +67,8 @@ def test_muhash_implementation(self): assert_equal(finalized[::-1].hex(), node_muhash) self.log.info("Test deterministic UTXO set hash results") - assert_equal(node.gettxoutsetinfo()['hash_serialized_3'], "d1c7fec1c0623f6793839878cbe2a531eb968b50b27edd6e2a57077a5aed6094") - assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "d1725b2fe3ef43e55aa4907480aea98d406fc9e0bf8f60169e2305f1fbf5961b") + assert_equal(node.gettxoutsetinfo()['hash_serialized_3'], "e0b4c80f2880985fdf1adc331ed0735ac207588f986c91c7c05e8cf5fe6780f0") + assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "8739b878f23030ef39a5547edc7b57f88d50fdaaf47314ff0524608deb13067e") def run_test(self): self.test_muhash_implementation() diff --git a/test/functional/mempool_package_rbf.py b/test/functional/mempool_package_rbf.py index 8558d9054688..f1075bda6002 100755 --- a/test/functional/mempool_package_rbf.py +++ b/test/functional/mempool_package_rbf.py @@ -383,7 +383,7 @@ def test_wrong_conflict_cluster_size_parents_child(self): # Now make conflicting packages for each coin package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex1) - assert_equal(f"package RBF failed: {child_result['tx'].rehash()} has 2 ancestors, max 1 allowed", package_result["package_msg"]) + assert_equal(f"package RBF failed: {parent1_result['tx'].rehash()} is not the only parent of child {child_result['tx'].rehash()}", package_result["package_msg"]) package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex2) @@ -437,7 +437,7 @@ def test_wrong_conflict_cluster_size_parent_children(self): # Now make conflicting packages for each coin package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex1) - assert_equal(f"package RBF failed: {child2_result['tx'].rehash()} is not the only child of parent {parent_result['tx'].rehash()}", package_result["package_msg"]) + assert_equal(f"package RBF failed: {child1_result['tx'].rehash()} is not the only child of parent {parent_result['tx'].rehash()}", package_result["package_msg"]) package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE) package_result = node.submitpackage(package_hex2) diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index 292744116b26..ba1cca49996e 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -49,15 +49,15 @@ def run_test(self): # Blockhash should be deterministic based on mocked time. assert_equal( out['base_hash'], - '09abf0e7b510f61ca6cf33bab104e9ee99b3528b371d27a2d4b39abb800fba7e') + '6885775faa46290bedfa071f22d0598c93f1d7e01f24607c4dedd69b9baa4a8f') # UTXO snapshot hash should be deterministic based on mocked time. assert_equal( sha256sum_file(str(expected_path)).hex(), - '31fcdd0cf542a4b1dfc13c3c05106620ce48951ef62907dd8e5e8c15a0aa993b') + 'd9506d541437f5e2892d6b6ea173f55233de11601650c157a27d8f2b9d08cb6f') assert_equal( - out['txoutset_hash'], 'a0b7baa3bf5ccbd3279728f230d7ca0c44a76e9923fca8f32dbfd08d65ea496a') + out['txoutset_hash'], 'd4453995f4f20db7bb3a604afd10d7128e8ee11159cde56d5b2fd7f55be7c74c') assert_equal(out['nchaintx'], 101) # Specifying a path to an existing or invalid file will fail. diff --git a/test/functional/rpc_getblockfrompeer.py b/test/functional/rpc_getblockfrompeer.py index c039d6640e9e..69d44d0f8404 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -143,7 +143,7 @@ def run_test(self): self.sync_blocks([self.nodes[0], pruned_node]) pruneheight += 251 assert_equal(pruned_node.pruneblockchain(700), pruneheight) - assert_equal(pruned_node.getblock(pruned_block)["hash"], "16e7919507fc0aec34ee0817c88c0b869fca0410a035b411026bc3bbf639498f") + assert_equal(pruned_node.getblock(pruned_block)["hash"], "49f6a6ba208e23f49d9f638d4970636381c71c328a9deb7a6319eb0b6b46cee5") self.log.info("Fetched block can be pruned again when prune height exceeds the height of the tip at the time when the block was fetched") self.generate(self.nodes[0], 250, sync_fun=self.no_op) diff --git a/test/functional/wallet_assumeutxo.py b/test/functional/wallet_assumeutxo.py index e362cd39ec90..fdd8c72330cb 100755 --- a/test/functional/wallet_assumeutxo.py +++ b/test/functional/wallet_assumeutxo.py @@ -121,7 +121,7 @@ def run_test(self): assert_equal( dump_output['txoutset_hash'], - "a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27") + "d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2") assert_equal(dump_output["nchaintx"], 334) assert_equal(n0.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT) From 9ac878a612987f58e43723274d2a59f21fbc02c4 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 27 Mar 2025 14:41:49 -0400 Subject: [PATCH 14/14] qa: sanity check mined block have their coinbase timelocked to height --- test/functional/mining_basic.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index 300433c846e5..dce6a28775da 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -28,9 +28,10 @@ COIN, DEFAULT_BLOCK_RESERVED_WEIGHT, MAX_BLOCK_WEIGHT, + MAX_SEQUENCE_NONFINAL, MINIMUM_BLOCK_RESERVED_WEIGHT, ser_uint256, - WITNESS_SCALE_FACTOR + WITNESS_SCALE_FACTOR, ) from test_framework.p2p import P2PDataStore from test_framework.test_framework import BitcoinTestFramework @@ -318,6 +319,12 @@ def test_block_max_weight(self): expected_msg=f"Error: Specified -blockmaxweight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})", ) + def test_height_in_locktime(self): + self.log.info("Sanity check generated blocks have their coinbase timelocked to their height.") + self.generate(self.nodes[0], 1, sync_fun=self.no_op) + block = self.nodes[0].getblock(self.nodes[0].getbestblockhash(), 2) + assert_equal(block["tx"][0]["locktime"], block["height"] - 1) + assert_equal(block["tx"][0]["vin"][0]["sequence"], MAX_SEQUENCE_NONFINAL) def run_test(self): node = self.nodes[0] @@ -547,6 +554,7 @@ def chain_tip(b_hash, *, status='headers-only', branchlen=1): self.test_block_max_weight() self.test_timewarp() self.test_pruning() + self.test_height_in_locktime() if __name__ == '__main__':