From a5f52cfcc400ad0adb41a78c65b8abb971e0d622 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 21 Apr 2025 15:12:48 -0400 Subject: [PATCH 1/6] qa: timelock coinbase transactions created in functional tests --- test/functional/feature_block.py | 2 ++ test/functional/mining_mainnet.py | 5 +++++ test/functional/test_framework/blocktools.py | 4 +++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 2dfa568c5b6..f4f15d293ec 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -831,6 +831,7 @@ class FullBlockTest(BitcoinTestFramework): 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 @@ class FullBlockTest(BitcoinTestFramework): 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/mining_mainnet.py b/test/functional/mining_mainnet.py index 1a537cd416d..4f95696f78c 100755 --- a/test/functional/mining_mainnet.py +++ b/test/functional/mining_mainnet.py @@ -30,6 +30,7 @@ from test_framework.blocktools import ( from test_framework.messages import ( CBlock, + SEQUENCE_FINAL, ) import json @@ -61,6 +62,10 @@ class MiningMainnetTest(BitcoinTestFramework): block.nBits = DIFF_1_N_BITS block.nNonce = blocks['nonces'][height - 1] block.vtx = [create_coinbase(height=height, script_pubkey=bytes.fromhex(COINBASE_SCRIPT_PUBKEY), retarget_period=2016)] + # The alternate mainnet chain was mined with non-timelocked coinbase txs. + block.vtx[0].nLockTime = 0 + block.vtx[0].vin[0].nSequence = SEQUENCE_FINAL + block.vtx[0].rehash() block.hashMerkleRoot = block.calc_merkle_root() block.rehash() block_hex = block.serialize(with_witness=False).hex() diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 8e616285572..c36fdb5c76e 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -29,6 +29,7 @@ from .messages import ( tx_from_hex, uint256_from_compact, WITNESS_SCALE_FACTOR, + MAX_SEQUENCE_NONFINAL, ) from .script import ( CScript, @@ -151,7 +152,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 9c94069d8b6cf67a24eb03c51230a4f2b2bf2d64 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 21 Apr 2025 15:53:09 -0400 Subject: [PATCH 2/6] 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 e020c4589c1..155e198ae68 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 c76dbe9b8b6f03b761a0ef97e1b8cd133b934714 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 21 Apr 2025 14:47:39 -0400 Subject: [PATCH 3/6] 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 4a747e7317c..6b64c79d05c 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -589,9 +589,9 @@ public: { // 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 e574c5b85c5..9be85df99fa 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 8baac7bba3e..1cd8f8ee718 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 788aeebf343526760fa8f3ed969ac3713212a5b6 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 21 Apr 2025 15:16:28 -0400 Subject: [PATCH 4/6] 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 4b576bb084f..81119f52b19 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 5c1f195868b..a0b23f5d3b7 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 8f2078af6a55448c003b3f7f3021955fbb351caa Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 19 Feb 2025 10:26:50 -0500 Subject: [PATCH 5/6] 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 | 8 ++--- 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 | 20 ++++++------ 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, 54 insertions(+), 51 deletions(-) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 6b64c79d05c..ad712ba6c62 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -582,9 +582,9 @@ public: 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 @@ -596,9 +596,9 @@ public: { // 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"}), }, }; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index c24debd1e69..408b7ec7e73 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -158,10 +158,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); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 5b87d4443d9..773392b7eaa 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -70,27 +70,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 c6d06c937c5..671dab56f71 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 82bbc4adcba..76a42d19ea2 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -370,7 +370,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 2e378ed30b5..7e86b3a35c1 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 2680fc4e410..094e8549c31 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -140,12 +140,12 @@ class AssumeutxoTest(BitcoinTestFramework): 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 - [(2).to_bytes(1, "little"), 32, None, "Bad snapshot data after deserializing 1 coins."], # wrong txid coins count + [b"\xff" * 32, 0, "77874d48d932a5cb7a7f770696f5224ff05746fdcf732a58270b45da0f665934", None], # wrong outpoint hash + [(2).to_bytes(1, "little"), 32, None, "Bad snapshot format or truncated snapshot 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 - [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 @@ -164,12 +164,12 @@ class AssumeutxoTest(BitcoinTestFramework): 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 (3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0) 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 (7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2) 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): @@ -228,7 +228,7 @@ class AssumeutxoTest(BitcoinTestFramework): 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 (3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0) is part of an invalid chain." + msg = "Unable to load UTXO snapshot: The base block header (7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2) is part of an invalid chain." assert_raises_rpc_error(-32603, msg, node.loadtxoutset, dump_output_path) node.reconsiderblock(block_hash) @@ -446,7 +446,7 @@ class AssumeutxoTest(BitcoinTestFramework): 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) @@ -476,7 +476,7 @@ class AssumeutxoTest(BitcoinTestFramework): dump_output4 = n0.dumptxoutset(path='utxos4.dat', rollback=prev_snap_height) assert_equal( dump_output4['txoutset_hash'], - "8a1db0d6e958ce0d7c963bc6fc91ead596c027129bacec68acc40351037b09d7") + "45ac2777b6ca96588210e2a4f14b602b41ec37b8b9370673048cc0af434a1ec8") assert_not_equal(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 3ab779b87dc..21a3c13f7e4 100755 --- a/test/functional/feature_utxo_set_hash.py +++ b/test/functional/feature_utxo_set_hash.py @@ -67,8 +67,8 @@ class UTXOSetHashTest(BitcoinTestFramework): 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 4dc6f8fe364..b638c5f512a 100755 --- a/test/functional/mempool_package_rbf.py +++ b/test/functional/mempool_package_rbf.py @@ -384,7 +384,7 @@ class PackageRBFTest(BitcoinTestFramework): # 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) @@ -438,7 +438,7 @@ class PackageRBFTest(BitcoinTestFramework): # 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 292744116b2..ba1cca49996 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -49,15 +49,15 @@ class DumptxoutsetTest(BitcoinTestFramework): # 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 62b3d664e0f..05fe7699e0a 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -143,7 +143,7 @@ class GetBlockFromPeerTest(BitcoinTestFramework): 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"], "36c56c5b5ebbaf90d76b0d1a074dcb32d42abab75b7ec6fa0ffd9b4fbce8f0f7") + assert_equal(pruned_node.getblock(pruned_block)["hash"], "196ee3a1a6db2353965081c48ef8e6b031cb2115d084bec6fec937e91a2c6277") 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 2d437e1c93f..22b3c51f183 100755 --- a/test/functional/wallet_assumeutxo.py +++ b/test/functional/wallet_assumeutxo.py @@ -118,7 +118,7 @@ class AssumeutxoTest(BitcoinTestFramework): assert_equal( dump_output['txoutset_hash'], - "a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27") + "d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2") assert_equal(dump_output["nchaintx"], 334) assert_equal(n0.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT) From a58cb3b1c12c8cb75a87375c50f94c4605bb805d Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 27 Mar 2025 14:41:49 -0400 Subject: [PATCH 6/6] 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 63fb7002c17..9a77ad7a719 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -28,9 +28,10 @@ from test_framework.messages import ( 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 @@ -362,6 +363,12 @@ class MiningTest(BitcoinTestFramework): 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] @@ -592,6 +599,7 @@ class MiningTest(BitcoinTestFramework): self.test_block_max_weight() self.test_timewarp() self.test_pruning() + self.test_height_in_locktime() if __name__ == '__main__':