diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 702f2c09366..297465be80f 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -30,7 +30,6 @@ static void AssembleBlock(benchmark::Bench& bench) witness.stack.push_back(WITNESS_STACK_ELEM_OP_TRUE); BlockAssembler::Options options; options.coinbase_output_script = P2WSH_OP_TRUE; - options.include_dummy_extranonce = true; // Collect some loose transactions that spend the coinbases of our mined blocks constexpr size_t NUM_BLOCKS{200}; diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index f4c42e204a7..943d34e75d9 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -34,7 +34,8 @@ public: virtual ~BlockTemplate() = default; virtual CBlockHeader getBlockHeader() = 0; - // Block contains a dummy coinbase transaction that should not be used. + // Block contains a dummy coinbase transaction that should not be used and + // it may not match a transaction constructed from getCoinbaseTx(). virtual CBlock getBlock() = 0; // Fees per transaction, not including coinbase transaction. @@ -64,6 +65,10 @@ public: * @note unlike the submitblock RPC, this method does NOT add the * coinbase witness automatically. * + * @note for heights <= 16, the BIP34 height push in getCoinbaseTx().script_sig_prefix + * is only one byte long, so the coinbase scriptSig needs at least + * one additional byte of data to avoid bad-cb-length. + * * @returns if the block was processed, does not necessarily indicate validity. * * @note Returns true if the block is already known, which can happen if diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 02c7ae9d220..969a838cd22 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -607,9 +607,9 @@ public: m_assumeutxo_data = { { // For use by unit tests .height = 110, - .hash_serialized = AssumeutxoHash{uint256{"b952555c8ab81fec46f3d4253b7af256d766ceb39fb7752b9d18cdf4a0141327"}}, + .hash_serialized = AssumeutxoHash{uint256{"86e9a1205b418b16dde3a18a78c730e30137e28466bda5dbf6b33ab8fc05447c"}}, .m_chain_tx_count = 111, - .blockhash = uint256{"6affe030b7965ab538f820a56ef56c8149b7dc1d1c144af57113be080db7c397"}, + .blockhash = uint256{"135eec25a6fb277884e5824e7aa7d052c4868161c99a5122170b5266f86c273d"}, }, { // For use by fuzz target src/test/fuzz/utxo_snapshot.cpp @@ -621,9 +621,9 @@ public: { // For use by test/functional/feature_assumeutxo.py and test/functional/tool_bitcoin_chainstate.py .height = 299, - .hash_serialized = AssumeutxoHash{uint256{"d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2"}}, + .hash_serialized = AssumeutxoHash{uint256{"106b2c56233e378a824cf0d5ff2be42ed32c72f1605c9be288d00942908a40ac"}}, .m_chain_tx_count = 334, - .blockhash = uint256{"7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2"}, + .blockhash = uint256{"0c552ced4721c249a389eb9b08cb8da261cd46f0e7b5f9d064d48f3113406853"}, }, }; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 836b828b5c4..7fc19deeb79 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -184,14 +184,18 @@ std::unique_ptr BlockAssembler::CreateNewBlock() // increasing its length would reduce the space they can use and may break // existing clients. coinbaseTx.vin[0].scriptSig = CScript() << nHeight; - if (m_options.include_dummy_extranonce) { + // Set script_sig_prefix here, so IPC mining clients are not affected by + // the optional scriptSig padding below. They provide their own extraNonce, + // and in a typical setup a pool name or realistic extraNonce already makes + // the scriptSig long enough. + coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig; + if (nHeight <= 16) { // For blocks at heights <= 16, the BIP34-encoded height alone is only // one byte. Consensus requires coinbase scriptSigs to be at least two - // bytes long (bad-cb-length), so tests and regtest include a dummy - // extraNonce (OP_0) + // bytes long (bad-cb-length), so an OP_0 is always appended at those + // heights. coinbaseTx.vin[0].scriptSig << OP_0; } - coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig; Assert(nHeight > 0); coinbaseTx.nLockTime = static_cast(nHeight - 1); coinbase_tx.lock_time = coinbaseTx.nLockTime; @@ -221,7 +225,6 @@ std::unique_ptr BlockAssembler::CreateNewBlock() pblock->nNonce = 0; if (m_options.test_block_validity) { - // if nHeight <= 16, and include_dummy_extranonce=false this will fail due to bad-cb-length. if (BlockValidationState state{TestBlockValidity(m_chainstate, *pblock, /*check_pow=*/false, /*check_merkle_root=*/false)}; !state.IsValid()) { throw std::runtime_error(strprintf("TestBlockValidity failed: %s", state.ToString())); } diff --git a/src/node/types.h b/src/node/types.h index 01e74528e06..d228fa41db6 100644 --- a/src/node/types.h +++ b/src/node/types.h @@ -72,10 +72,6 @@ struct BlockCreateOptions { * coinbase_max_additional_weight and coinbase_output_max_additional_sigops. */ CScript coinbase_output_script{CScript() << OP_TRUE}; - /** - * Whether to include an OP_0 as a dummy extraNonce in the template's coinbase - */ - bool include_dummy_extranonce{false}; }; struct BlockWaitOptions { @@ -125,7 +121,9 @@ struct CoinbaseTx { * Prefix which needs to be placed at the beginning of the scriptSig. * Clients may append extra data to this as long as the overall scriptSig * size is 100 bytes or less, to avoid the block being rejected with - * "bad-cb-length" error. + * "bad-cb-length" error. At heights <= 16 the BIP 34 height push is only + * one byte long, so clients must append at least one additional byte to + * meet the consensus minimum scriptSig length of two bytes. * * Currently with BIP 34, the prefix is guaranteed to be less than 8 bytes, * but future soft forks could require longer prefixes. diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index dcf5ee26457..223569fdafe 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -165,7 +165,7 @@ static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const { UniValue blockHashes(UniValue::VARR); while (nGenerate > 0 && !chainman.m_interrupt) { - std::unique_ptr block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true }, /*cooldown=*/false)); + std::unique_ptr block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script }, /*cooldown=*/false)); CHECK_NONFATAL(block_template); std::shared_ptr block_out; @@ -377,7 +377,7 @@ static RPCMethod generateblock() { LOCK(chainman.GetMutex()); { - std::unique_ptr block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true}, /*cooldown=*/false)}; + std::unique_ptr block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script}, /*cooldown=*/false)}; CHECK_NONFATAL(block_template); block = block_template->getBlock(); @@ -875,7 +875,7 @@ static RPCMethod getblocktemplate() // a delay to each getblocktemplate call. This differs from typical // long-lived IPC usage, where the overhead is paid only when creating // the initial template. - block_template = miner.createNewBlock({.include_dummy_extranonce = true}, /*cooldown=*/false); + block_template = miner.createNewBlock({}, /*cooldown=*/false); CHECK_NONFATAL(block_template); diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 10e4ffa4b08..b77942f741d 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -71,7 +71,6 @@ CBlock BuildChainTestingSetup::CreateBlock(const CBlockIndex* prev, { BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; - options.include_dummy_extranonce = true; std::unique_ptr pblocktemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(); CBlock& block = pblocktemplate->block; block.hashPrevBlock = prev->GetBlockHash(); diff --git a/src/test/fuzz/cmpctblock.cpp b/src/test/fuzz/cmpctblock.cpp index 6af9883a2e9..85e3bedfe92 100644 --- a/src/test/fuzz/cmpctblock.cpp +++ b/src/test/fuzz/cmpctblock.cpp @@ -123,7 +123,6 @@ void ResetChainmanAndMempool(TestingSetup& setup) node::BlockAssembler::Options options; options.coinbase_output_script = P2WSH_OP_TRUE; - options.include_dummy_extranonce = true; g_mature_coinbase.clear(); diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index 93cd8ad6bbd..1cc2caa396e 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -46,7 +46,6 @@ void initialize_tx_pool() BlockAssembler::Options options; options.coinbase_output_script = P2WSH_EMPTY; - options.include_dummy_extranonce = true; for (int i = 0; i < 2 * COINBASE_MATURITY; ++i) { COutPoint prevout{MineBlock(g_setup->m_node, options)}; diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 5f54857ea5f..d8a91d7f90a 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -43,7 +43,6 @@ void ResetChainman(TestingSetup& setup) setup.LoadVerifyActivateChainstate(); for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { node::BlockAssembler::Options options; - options.include_dummy_extranonce = true; MineBlock(setup.m_node, options); } } diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 9f33b0df24b..314388fac07 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -37,7 +37,6 @@ void ResetChainman(TestingSetup& setup) setup.m_make_chainman(); setup.LoadVerifyActivateChainstate(); node::BlockAssembler::Options options; - options.include_dummy_extranonce = true; for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { MineBlock(setup.m_node, options); } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index bb15552723a..f70dd710b35 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -48,7 +48,6 @@ void initialize_tx_pool() BlockAssembler::Options options; options.coinbase_output_script = P2WSH_OP_TRUE; - options.include_dummy_extranonce = true; for (int i = 0; i < 2 * COINBASE_MATURITY; ++i) { COutPoint prevout{MineBlock(g_setup->m_node, options)}; @@ -98,7 +97,6 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, Cha BlockAssembler::Options options; options.nBlockMaxWeight = fuzzed_data_provider.ConsumeIntegralInRange(0U, MAX_BLOCK_WEIGHT); options.blockMinFeeRate = CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)}; - options.include_dummy_extranonce = true; auto assembler = BlockAssembler{chainstate, &tx_pool, options}; auto block_template = assembler.CreateNewBlock(); Assert(block_template->block.vtx.size() >= 1); diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index 1275fd45c43..b281f20028e 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -46,7 +46,6 @@ FUZZ_TARGET(utxo_total_supply) }; BlockAssembler::Options options; options.coinbase_output_script = CScript() << OP_FALSE; - options.include_dummy_extranonce = true; const auto PrepareNextBlock = [&]() { // Use OP_FALSE to avoid BIP30 check from hitting early auto block = PrepareBlock(node, options); @@ -107,8 +106,12 @@ FUZZ_TARGET(utxo_total_supply) // Assuming that the fuzzer will mine relatively short chains (less than 200 blocks), we want the duplicate coinbase to be not too high. // Up to 300 seems reasonable. int64_t duplicate_coinbase_height = fuzzed_data_provider.ConsumeIntegralInRange(0, 300); - // Always pad with OP_0 at the end to avoid bad-cb-length error - const CScript duplicate_coinbase_script = CScript() << duplicate_coinbase_height << OP_0; + // Avoid bad-cb-length error at heights <= 16. Pad the BIP34-encoded height + // with OP_0 to satisfy the minimum 2-byte coinbase scriptSig length. + CScript duplicate_coinbase_script = CScript() << duplicate_coinbase_height; + if (duplicate_coinbase_height <= 16) { + duplicate_coinbase_script << OP_0; + } // Mine the first block with this duplicate current_block = PrepareNextBlock(); StoreLastTxo(); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 6b4e85a59e4..124ef08500c 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -117,7 +117,6 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const auto mining{MakeMining()}; BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; - options.include_dummy_extranonce = true; LOCK(tx_mempool.cs); BOOST_CHECK(tx_mempool.size() == 0); @@ -336,7 +335,6 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; - options.include_dummy_extranonce = true; { CTxMemPool& tx_mempool{MakeMempool()}; @@ -663,7 +661,6 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; - options.include_dummy_extranonce = true; CTxMemPool& tx_mempool{MakeMempool()}; LOCK(tx_mempool.cs); @@ -753,7 +750,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) CScript scriptPubKey = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG; BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; - options.include_dummy_extranonce = true; // Create and check a simple template std::unique_ptr block_template = mining->createNewBlock(options, /*cooldown=*/false); diff --git a/src/test/peerman_tests.cpp b/src/test/peerman_tests.cpp index e391e8b9c3f..36a66b14eb5 100644 --- a/src/test/peerman_tests.cpp +++ b/src/test/peerman_tests.cpp @@ -20,7 +20,6 @@ static void mineBlock(const node::NodeContext& node, std::chrono::seconds block_ { auto curr_time = GetTime(); node::BlockAssembler::Options options; - options.include_dummy_extranonce = true; SetMockTime(block_time); // update time so the block is created with it CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr, options}.CreateNewBlock()->block; while (!CheckProofOfWork(block.GetHash(), block.nBits, node.chainman->GetConsensus())) ++block.nNonce; diff --git a/src/test/testnet4_miner_tests.cpp b/src/test/testnet4_miner_tests.cpp index 1bfc19db270..c501b4eb086 100644 --- a/src/test/testnet4_miner_tests.cpp +++ b/src/test/testnet4_miner_tests.cpp @@ -36,7 +36,6 @@ BOOST_AUTO_TEST_CASE(MiningInterface) BOOST_REQUIRE(mining); BlockAssembler::Options options; - options.include_dummy_extranonce = true; std::unique_ptr block_template; // Set node time a few minutes past the testnet4 genesis block diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp index c4823bce351..05f1be77ee2 100644 --- a/src/test/util/mining.cpp +++ b/src/test/util/mining.cpp @@ -29,7 +29,6 @@ COutPoint generatetoaddress(const NodeContext& node, const std::string& address) assert(IsValidDestination(dest)); BlockAssembler::Options assembler_options; assembler_options.coinbase_output_script = GetScriptForDestination(dest); - assembler_options.include_dummy_extranonce = true; return MineBlock(node, assembler_options); } @@ -140,7 +139,6 @@ std::shared_ptr PrepareBlock(const NodeContext& node, const CScript& coi { BlockAssembler::Options assembler_options; assembler_options.coinbase_output_script = coinbase_scriptPubKey; - assembler_options.include_dummy_extranonce = true; ApplyArgsManOptions(*node.args, assembler_options); return PrepareBlock(node, assembler_options); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 39c691c3364..f924a0e8c7d 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -390,7 +390,7 @@ TestChain100Setup::TestChain100Setup( LOCK(::cs_main); assert( m_node.chainman->ActiveChain().Tip()->GetBlockHash().ToString() == - "0c8c5f79505775a0f6aed6aca2350718ceb9c6f2c878667864d5c7a6d8ffa2a6"); + "0ee6e270d6594249e548110619f7bd690695beb219b915da4a2e84e2b61ed60f"); } } @@ -412,7 +412,6 @@ CBlock TestChain100Setup::CreateBlock( { BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; - options.include_dummy_extranonce = true; CBlock block = BlockAssembler{chainstate, nullptr, options}.CreateNewBlock()->block; Assert(block.vtx.size() == 1); diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 083a32da963..e1f90477fc4 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -69,7 +69,6 @@ std::shared_ptr MinerTestingSetup::Block(const uint256& prev_hash) BlockAssembler::Options options; options.coinbase_output_script = CScript{} << i++ << OP_TRUE; - options.include_dummy_extranonce = true; auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(); auto pblock = std::make_shared(ptemplate->block); pblock->hashPrevBlock = prev_hash; @@ -338,7 +337,6 @@ BOOST_AUTO_TEST_CASE(witness_commitment_index) pubKey << 1 << OP_TRUE; BlockAssembler::Options options; options.coinbase_output_script = pubKey; - options.include_dummy_extranonce = true; auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(); CBlock pblock = ptemplate->block; diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp index b1e204f1741..268529fd802 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(), "b952555c8ab81fec46f3d4253b7af256d766ceb39fb7752b9d18cdf4a0141327"); + BOOST_CHECK_EQUAL(out110.hash_serialized.ToString(), "86e9a1205b418b16dde3a18a78c730e30137e28466bda5dbf6b33ab8fc05447c"); BOOST_CHECK_EQUAL(out110.m_chain_tx_count, 111U); - const auto out110_2 = *params->AssumeutxoForBlockhash(uint256{"6affe030b7965ab538f820a56ef56c8149b7dc1d1c144af57113be080db7c397"}); - BOOST_CHECK_EQUAL(out110_2.hash_serialized.ToString(), "b952555c8ab81fec46f3d4253b7af256d766ceb39fb7752b9d18cdf4a0141327"); + const auto out110_2 = *params->AssumeutxoForBlockhash(uint256{"135eec25a6fb277884e5824e7aa7d052c4868161c99a5122170b5266f86c273d"}); + BOOST_CHECK_EQUAL(out110_2.hash_serialized.ToString(), "86e9a1205b418b16dde3a18a78c730e30137e28466bda5dbf6b33ab8fc05447c"); 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 ae130ee2035..8727a9925ec 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -139,12 +139,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, "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"\xff" * 32, 0, "43bb0fcc50f3789d50ce41891810bbc571d549a32e2bbaabfc036f73ae2a23f9", None], # wrong outpoint hash + [(2).to_bytes(1, "little"), 32, None, "Bad snapshot data after deserializing 2 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, "9f562925721e4f97e6fde5b590dbfede51e2204a68639525062ad064545dd0ea", None], # wrong outpoint index - [b"\x82", 34, "161393f07f8ad71760b3910a914f677f2cb166e5bcf5354e50d46b78c0422d15", None], # wrong coin code VARINT - [b"\x80", 34, "e6fae191ef851554467b68acff01ca09ad0a2e48c9b3dfea46cf7d35a7fd0ad0", None], # another wrong coin code + [b"\x01", 33, "0b0e96fd9a556157a68d9e6fe6e2c7562169c730347565ce0725032f824bea34", None], # wrong outpoint index + [b"\x83", 34, "52084c732f760fbcfb804d854815a9145111e93d7c7f3f7bfcb0ade1ea4f6b99", None], # wrong coin code VARINT + [b"\x80", 34, "c1a1308fcf04cbb14fc777467eb5e98e77a921b66eac4ad47877d0b6ed7b15b1", 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 @@ -163,12 +163,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 d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2, got {wrong_hash}." + msg = custom_message if custom_message is not None else f"Bad snapshot content hash: expected 106b2c56233e378a824cf0d5ff2be42ed32c72f1605c9be288d00942908a40ac, 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 (7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2) 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 (0c552ced4721c249a389eb9b08cb8da261cd46f0e7b5f9d064d48f3113406853) 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 (7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2) is part of an invalid chain." + msg = "Unable to load UTXO snapshot: The base block header (0c552ced4721c249a389eb9b08cb8da261cd46f0e7b5f9d064d48f3113406853) is part of an invalid chain." assert_raises_rpc_error(-32603, msg, node.loadtxoutset, dump_output_path) node.reconsiderblock(block_hash) @@ -469,7 +469,7 @@ class AssumeutxoTest(BitcoinTestFramework): def check_dump_output(output): assert_equal( output['txoutset_hash'], - "d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2") + "106b2c56233e378a824cf0d5ff2be42ed32c72f1605c9be288d00942908a40ac") assert_equal(output["nchaintx"], blocks[SNAPSHOT_BASE_HEIGHT].chain_tx) check_dump_output(dump_output) @@ -499,7 +499,7 @@ class AssumeutxoTest(BitcoinTestFramework): dump_output4 = n0.dumptxoutset(path='utxos4.dat', rollback=prev_snap_height) assert_equal( dump_output4['txoutset_hash'], - "45ac2777b6ca96588210e2a4f14b602b41ec37b8b9370673048cc0af434a1ec8") + "147d1789e3e29a62e3e15b80078b50603afb5e1e70ea07001c94666afbde38df") 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_index_prune.py b/test/functional/feature_index_prune.py index 0654e04f586..ba4907acdcf 100755 --- a/test/functional/feature_index_prune.py +++ b/test/functional/feature_index_prune.py @@ -100,7 +100,7 @@ class FeatureIndexPruneTest(BitcoinTestFramework): pruneheight_new = node.pruneblockchain(400) # the prune heights used here and below are magic numbers that are determined by the # thresholds at which block files wrap, so they depend on disk serialization and default block file size. - assert_equal(pruneheight_new, 248) + assert_equal(pruneheight_new, 249) self.log.info("check if we can access the tips blockfilter and coinstats when we have pruned some blocks") tip = self.nodes[0].getbestblockhash() @@ -117,8 +117,8 @@ class FeatureIndexPruneTest(BitcoinTestFramework): assert node.gettxoutsetinfo(hash_type="muhash", hash_or_height=height_hash)['muhash'] # mine and sync index up to a height that will later be the pruneheight - self.generate(self.nodes[0], 51) - self.sync_index(height=751) + self.generate(self.nodes[0], 54) + self.sync_index(height=754) self.restart_without_indices() @@ -130,12 +130,12 @@ class FeatureIndexPruneTest(BitcoinTestFramework): msg = "Querying specific block heights requires coinstatsindex" assert_raises_rpc_error(-8, msg, node.gettxoutsetinfo, "muhash", height_hash) - self.generate(self.nodes[0], 749) + self.generate(self.nodes[0], 746) self.log.info("prune exactly up to the indices best blocks while the indices are disabled") for i in range(3): pruneheight_2 = self.nodes[i].pruneblockchain(1000) - assert_equal(pruneheight_2, 750) + assert_equal(pruneheight_2, 753) # Restart the nodes again with the indices activated self.restart_node(i, extra_args=self.extra_args[i]) @@ -195,7 +195,7 @@ class FeatureIndexPruneTest(BitcoinTestFramework): for node in self.nodes[:2]: with node.assert_debug_log(['limited pruning to height 2489']): pruneheight_new = node.pruneblockchain(2500) - assert_equal(pruneheight_new, 2005) + assert_equal(pruneheight_new, 2013) self.log.info("ensure that prune locks don't prevent indices from failing in a reorg scenario") with self.nodes[0].assert_debug_log(['basic block filter index prune lock moved back to 2480']): diff --git a/test/functional/feature_utxo_set_hash.py b/test/functional/feature_utxo_set_hash.py index 733b1fea185..fb64439ecc5 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'], "e0b4c80f2880985fdf1adc331ed0735ac207588f986c91c7c05e8cf5fe6780f0") - assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "8739b878f23030ef39a5547edc7b57f88d50fdaaf47314ff0524608deb13067e") + assert_equal(node.gettxoutsetinfo()['hash_serialized_3'], "396058cfd7f3b4c05dcdfaf360be9986d859d2c8315082d3aadda6d2d16add25") + assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "97f341d4226cb4451dd9da2856759fb97659cfc238a0b8d5452e64af6032bd3d") def run_test(self): self.test_muhash_implementation() diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index 31ccec63fa8..2740662f1c0 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -7,7 +7,7 @@ import asyncio import time from contextlib import AsyncExitStack from io import BytesIO -from test_framework.blocktools import NULL_OUTPOINT +from test_framework.blocktools import NULL_OUTPOINT, script_BIP34_coinbase_height from test_framework.messages import ( MAX_BLOCK_WEIGHT, CBlockHeader, @@ -20,10 +20,6 @@ from test_framework.messages import ( from_hex, msg_headers, ) -from test_framework.script import ( - CScript, - CScriptNum, -) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -67,7 +63,7 @@ class IPCMiningTest(BitcoinTestFramework): # as it is being called before knowing whether capnp is available). self.capnp_modules = load_capnp_modules(self.config) - async def build_coinbase_test(self, template, ctx, miniwallet): + async def build_coinbase_test(self, template, ctx, miniwallet, extra_nonce=b""): self.log.debug("Build coinbase transaction using getCoinbaseTx()") assert template is not None coinbase_res = await mining_get_coinbase_tx(template, ctx) @@ -79,11 +75,11 @@ class IPCMiningTest(BitcoinTestFramework): # Verify there's no dummy extraNonce in the coinbase scriptSig current_block_height = self.nodes[0].getchaintips()[0]["height"] - expected_scriptsig = CScript([CScriptNum(current_block_height + 1)]) - assert_equal(coinbase_res.scriptSigPrefix.hex(), expected_scriptsig.hex()) + bip34_prefix = script_BIP34_coinbase_height(current_block_height + 1, padding=False) + assert_equal(coinbase_res.scriptSigPrefix, bip34_prefix) # Typically a mining pool appends its name and an extraNonce - coinbase_tx.vin[0].scriptSig = coinbase_res.scriptSigPrefix + coinbase_tx.vin[0].scriptSig = coinbase_res.scriptSigPrefix + extra_nonce # We currently always provide a coinbase witness, even for empty # blocks, but this may change, so always check: @@ -407,6 +403,50 @@ class IPCMiningTest(BitcoinTestFramework): asyncio.run(capnp.run(async_routine())) + def run_low_height_test(self): + """Test that IPC createNewBlock() works at low block heights on a + clean chain, in particular with regard to bad-cb-length. + + createNewBlock pads the scriptSig with a dummy extranonce to pass + its internal CheckBlock(). This dummy is omitted from the + getCoinbaseTx() script_sig_prefix field. The client provides its + own extraNonce via submitSolution().""" + self.log.info("Running low block height test") + + node = self.nodes[0] + self.stop_node(0) + # Clear chain data to start from genesis + self.cleanup_folder(node.chain_path) + node.start() + node.wait_for_rpc_connection() + assert_equal(node.getblockcount(), 0) + + miniwallet = MiniWallet(node) + + async def async_routine(): + ctx, mining = await make_mining_ctx(self) + opts = self.capnp_modules['mining'].BlockCreateOptions() + + # Mine blocks 1-17 to exercise the boundary at height 16, where the + # internal scriptSig padding is no longer needed. + for height in range(1, 18): + async with AsyncExitStack() as stack: + # Disable cooldown to avoid hanging in the IBD loop on a fresh chain + template = await mining_create_block_template(mining, stack, ctx, opts, cooldown=False) + assert template is not None + block = await mining_get_block(template, ctx) + # Heights <= 16 need extra nonce padding. + extra_nonce = b'\xaa\xbb\xcc\xdd' if height <= 16 else b"" + coinbase = await self.build_coinbase_test(template, ctx, miniwallet, extra_nonce=extra_nonce) + block.vtx[0] = coinbase + block.hashMerkleRoot = block.calc_merkle_root() + block.solve() + submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result + assert_equal(submitted, True) + assert_equal(node.getblockcount(), height) + + asyncio.run(capnp.run(async_routine())) + def run_test(self): self.miniwallet = MiniWallet(self.nodes[0]) self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions() @@ -416,6 +456,9 @@ class IPCMiningTest(BitcoinTestFramework): self.run_coinbase_and_submission_test() self.run_ipc_option_override_test() + # Needs to run last because it resets the chain. + self.run_low_height_test() + if __name__ == '__main__': IPCMiningTest(__file__).main() diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index 80c4bf6fe6a..47568c248e0 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -68,15 +68,15 @@ class DumptxoutsetTest(BitcoinTestFramework): # Blockhash should be deterministic based on mocked time. assert_equal( out['base_hash'], - '6885775faa46290bedfa071f22d0598c93f1d7e01f24607c4dedd69b9baa4a8f') + '220aee93f0f5409631f35488898258f0930952bd620063cb4d7d87f7c28a8f50') # UTXO snapshot hash should be deterministic based on mocked time. assert_equal( sha256sum_file(str(expected_path)).hex(), - 'd9506d541437f5e2892d6b6ea173f55233de11601650c157a27d8f2b9d08cb6f') + 'e8c59b1bc1f19061c67eb7a392f4ea17eea83af58646ea2909e270546699c36c') assert_equal( - out['txoutset_hash'], 'd4453995f4f20db7bb3a604afd10d7128e8ee11159cde56d5b2fd7f55be7c74c') + out['txoutset_hash'], '771d773b5c27b6f35f598ce764652a2cf28fbc268341eb1827844e416c629c7d') 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 841cde02197..9adcd88c9ee 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -128,7 +128,7 @@ class GetBlockFromPeerTest(BitcoinTestFramework): self.generate(self.nodes[0], 400, sync_fun=self.no_op) self.sync_blocks([self.nodes[0], pruned_node]) pruneheight = pruned_node.pruneblockchain(300) - assert_equal(pruneheight, 248) + assert_equal(pruneheight, 249) # Ensure the block is actually pruned pruned_block = self.nodes[0].getblockhash(2) assert_raises_rpc_error(-1, "Block not available (pruned data)", pruned_node.getblock, pruned_block) @@ -144,14 +144,14 @@ class GetBlockFromPeerTest(BitcoinTestFramework): self.log.info("Fetched block persists after next pruning event") self.generate(self.nodes[0], 250, sync_fun=self.no_op) self.sync_blocks([self.nodes[0], pruned_node]) - pruneheight += 251 + pruneheight += 252 assert_equal(pruned_node.pruneblockchain(700), pruneheight) 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) self.sync_blocks([self.nodes[0], pruned_node]) - pruneheight += 250 + pruneheight += 251 assert_equal(pruned_node.pruneblockchain(1000), pruneheight) assert_raises_rpc_error(-1, "Block not available (pruned data)", pruned_node.getblock, pruned_block) diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 0a393d89024..529f07cd3c7 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -161,11 +161,13 @@ def add_witness_commitment(block, nonce=0): block.hashMerkleRoot = block.calc_merkle_root() -def script_BIP34_coinbase_height(height): +def script_BIP34_coinbase_height(height, *, padding=True): if height <= 16: res = CScriptOp.encode_op_n(height) - # Append dummy extraNonce to increase scriptSig size to 2 (see bad-cb-length consensus rule) - return CScript([res, OP_0]) + if padding: + # Append dummy extraNonce to increase scriptSig size to 2 (see bad-cb-length consensus rule) + return CScript([res, OP_0]) + return CScript([res]) return CScript([CScriptNum(height)]) diff --git a/test/functional/test_framework/ipc_util.py b/test/functional/test_framework/ipc_util.py index 0e10b90e023..340cd15c618 100644 --- a/test/functional/test_framework/ipc_util.py +++ b/test/functional/test_framework/ipc_util.py @@ -109,9 +109,9 @@ async def make_capnp_init_ctx(self): return ctx, init -async def mining_create_block_template(mining, stack, ctx, opts): +async def mining_create_block_template(mining, stack, ctx, *args, **kwargs): """Call mining.createNewBlock() and return template, then call template.destroy() when stack exits.""" - response = await mining.createNewBlock(ctx, opts) + response = await mining.createNewBlock(ctx, *args, **kwargs) if not response._has("result"): return None return await stack.enter_async_context(destroying(response.result, ctx)) diff --git a/test/functional/tool_bitcoin_chainstate.py b/test/functional/tool_bitcoin_chainstate.py index c57da1fc376..6b7128898d2 100755 --- a/test/functional/tool_bitcoin_chainstate.py +++ b/test/functional/tool_bitcoin_chainstate.py @@ -20,7 +20,7 @@ from test_framework.wallet import MiniWallet START_HEIGHT = 199 # Hardcoded in regtest chainparams SNAPSHOT_BASE_BLOCK_HEIGHT = 299 -SNAPSHOT_BASE_BLOCK_HASH = "7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2" +SNAPSHOT_BASE_BLOCK_HASH = "0c552ced4721c249a389eb9b08cb8da261cd46f0e7b5f9d064d48f3113406853" class BitcoinChainstateTest(BitcoinTestFramework): diff --git a/test/functional/wallet_assumeutxo.py b/test/functional/wallet_assumeutxo.py index a8c97be778f..3f28e36c805 100755 --- a/test/functional/wallet_assumeutxo.py +++ b/test/functional/wallet_assumeutxo.py @@ -172,7 +172,7 @@ class AssumeutxoTest(BitcoinTestFramework): assert_equal( dump_output['txoutset_hash'], - "d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2") + "106b2c56233e378a824cf0d5ff2be42ed32c72f1605c9be288d00942908a40ac") assert_equal(dump_output["nchaintx"], 334) assert_equal(n0.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT)