From 68688dcb4b8b363836982ad98769b071107fb8b0 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 10 Oct 2022 16:14:39 +0800 Subject: [PATCH 1/9] Merge bitcoin/bitcoin#25073: test: Cleanup miner_tests faa15527d7e0c7923ff9c0fad7bab4648705ed0f test: Use dedicated mempool in TestBasicMining (MacroFake) fafab384a0a5f6d80195307b7bbeb00515da432b test: Use dedicated mempool in TestPackageSelection (MacroFake) fa4055d79c7ea1d4c3b694e39cafa98a1c7ba8bb test: Use dedicated mempool in TestPrioritisedMining (MacroFake) fa2921828511816d0420c567386e1da0391b3ad7 test: Pass mempool reference to AssemblerForTest (MacroFake) Pull request description: This cleans up the miner tests: * Removes duplicate/redundant and thus confusing chainparams object. * Uses a fresh mempool for each subtest instead of using the "global" one from the testing setup. This makes it easier to follow the tests in smaller scopes. Also it makes sure the mempool is truly cleared by reconstructing it. Finally, this removes calls to `clear`, see https://github.com/bitcoin/bitcoin/pull/19909 ACKs for top commit: glozow: utACK faa15527d7e0c7923ff9c0fad7bab4648705ed0f Tree-SHA512: ced1260f6ab70fba74b0fac7ff4fc7adfddcd2f3bee785249d2a4a9055ac253eff9090edbda7a17e72a71a81b56ff708d5ff64e1f57ebc7b7747d6c88fec51e3 Co-authored-by: fanquake --- src/test/miner_tests.cpp | 510 ++++++++++++++++++++++----------------- 1 file changed, 288 insertions(+), 222 deletions(-) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 8452c69ee69b..d0393b2f1410 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -2,7 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include #include #include #include @@ -39,17 +38,26 @@ using node::CBlockTemplate; namespace miner_tests { struct MinerTestingSetup : public TestingSetup { - void TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs); - void TestBasicMining(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector& txFirst, int baseheight) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs); - void TestPrioritisedMining(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs); - bool TestSequenceLocks(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs) + void TestPackageSelection(const CScript& scriptPubKey, const std::vector& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void TestBasicMining(const CScript& scriptPubKey, const std::vector& txFirst, int baseheight) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void TestPrioritisedMining(const CScript& scriptPubKey, const std::vector& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool TestSequenceLocks(const CTransaction& tx, CTxMemPool& tx_mempool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - CCoinsViewMemPool view_mempool(&m_node.chainman->ActiveChainstate().CoinsTip(), *m_node.mempool); + CCoinsViewMemPool view_mempool{&m_node.chainman->ActiveChainstate().CoinsTip(), tx_mempool}; CBlockIndex* tip{m_node.chainman->ActiveChain().Tip()}; const std::optional lock_points{CalculateLockPointsAtTip(tip, view_mempool, tx)}; return lock_points.has_value() && CheckSequenceLocksAtTip(tip, *lock_points); } - BlockAssembler AssemblerForTest(const CChainParams& params); + CTxMemPool& MakeMempool() + { + // Delete the previous mempool to ensure with valgrind that the old + // pointer is not accessed, when the new one should be accessed + // instead. + m_node.mempool.reset(); + m_node.mempool = std::make_unique(MemPoolOptionsForTest(m_node)); + return *m_node.mempool; + } + BlockAssembler AssemblerForTest(CTxMemPool& tx_mempool); }; static constexpr int WAIT_FOR_ISLOCK_TIMEOUT{601}; @@ -59,13 +67,13 @@ BOOST_FIXTURE_TEST_SUITE(miner_tests, MinerTestingSetup) static CFeeRate blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE); -BlockAssembler MinerTestingSetup::AssemblerForTest(const CChainParams& params) +BlockAssembler MinerTestingSetup::AssemblerForTest(CTxMemPool& tx_mempool) { BlockAssembler::Options options; options.nBlockMaxSize = DEFAULT_BLOCK_MAX_SIZE; options.blockMinFeeRate = blockMinFeeRate; - return BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, m_node.mempool.get(), options); + return BlockAssembler(m_node.chainman->ActiveChainstate(), m_node, &tx_mempool, options); } constexpr static struct { @@ -104,11 +112,13 @@ static std::unique_ptr CreateBlockIndex(int nHeight, CBlockIndex* a // Test suite for ancestor feerate transaction selection. // Implemented as an additional function, rather than a separate test case, // to allow reusing the blockchain created in CreateNewBlock_validity. -void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector& txFirst) +void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const std::vector& txFirst) { // Disable free transactions, otherwise TX selection is non-deterministic gArgs.SoftSetArg("-blockprioritysize", "0"); + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); // Test the ancestor feerate transaction selection. TestMemPoolEntryHelper entry; @@ -123,23 +133,23 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co tx.vout[0].nValue = 5000000000LL - 1000; // This tx has a low fee: 1000 satoshis uint256 hashParentTx = tx.GetHash(); // save this txid for later use - m_node.mempool->addUnchecked(entry.Fee(1000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(1000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); // This tx has a medium fee: 10000 satoshis tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vout[0].nValue = 5000000000LL - 10000; uint256 hashMediumFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(10000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(10000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); // This tx has a high fee, but depends on the first transaction tx.vin[0].prevout.hash = hashParentTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 50k satoshi fee uint256 hashHighFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(50000).Time(Now()).SpendsCoinbase(false).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(50000).Time(Now()).SpendsCoinbase(false).FromTx(tx)); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hashParentTx, hashMediumFeeTx, hashHighFeeTx}, std::chrono::duration_cast(Now().time_since_epoch()).count() - WAIT_FOR_ISLOCK_TIMEOUT); - std::unique_ptr pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + std::unique_ptr pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 4U); BOOST_CHECK(pblocktemplate->block.vtx[1]->GetHash() == hashParentTx); BOOST_CHECK(pblocktemplate->block.vtx[2]->GetHash() == hashHighFeeTx); @@ -151,7 +161,7 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co uint256 hashFreeTx = tx.GetHash(); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hashFreeTx}, std::chrono::duration_cast(Now().time_since_epoch()).count() - WAIT_FOR_ISLOCK_TIMEOUT); - m_node.mempool->addUnchecked(entry.Fee(0).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(0).FromTx(tx)); size_t freeTxSize = GetVirtualTransactionSize(CTransaction(tx)); // Calculate a fee on child transaction that will put the package just @@ -161,8 +171,8 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co tx.vin[0].prevout.hash = hashFreeTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000 - feeToUse; uint256 hashLowFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(feeToUse).FromTx(tx)); - pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + tx_mempool.addUnchecked(entry.Fee(feeToUse).FromTx(tx)); + pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); // Verify that the free tx and the low fee tx didn't get selected for (size_t i=0; iblock.vtx.size(); ++i) { BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashFreeTx); @@ -172,13 +182,13 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co // Test that packages above the min relay fee do get included, even if one // of the transactions is below the min relay fee // Remove the low fee transaction and replace with a higher fee transaction - m_node.mempool->removeRecursive(CTransaction(tx), MemPoolRemovalReason::MANUAL); + tx_mempool.removeRecursive(CTransaction(tx), MemPoolRemovalReason::MANUAL); tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee hashLowFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(feeToUse+2).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(feeToUse+2).FromTx(tx)); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hashLowFeeTx}, std::chrono::duration_cast(Now().time_since_epoch()).count() - WAIT_FOR_ISLOCK_TIMEOUT); - pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 6U); BOOST_CHECK(pblocktemplate->block.vtx[4]->GetHash() == hashFreeTx); BOOST_CHECK(pblocktemplate->block.vtx[5]->GetHash() == hashLowFeeTx); @@ -191,7 +201,7 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co tx.vout[0].nValue = 5000000000LL - 100000000; tx.vout[1].nValue = 100000000; // 1BTC output uint256 hashFreeTx2 = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({tx.GetHash()}, std::chrono::duration_cast(Now().time_since_epoch()).count() - WAIT_FOR_ISLOCK_TIMEOUT); @@ -201,10 +211,10 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co feeToUse = blockMinFeeRate.GetFee(freeTxSize); tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse; uint256 hashLowFeeTx2 = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({tx.GetHash()}, std::chrono::duration_cast(Now().time_since_epoch()).count() - WAIT_FOR_ISLOCK_TIMEOUT); - pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); // Verify that this tx isn't selected. for (size_t i=0; iblock.vtx.size(); ++i) { @@ -216,201 +226,257 @@ void MinerTestingSetup::TestPackageSelection(const CChainParams& chainparams, co // as well. tx.vin[0].prevout.n = 1; tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee - m_node.mempool->addUnchecked(entry.Fee(10000).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(10000).FromTx(tx)); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({tx.GetHash()}, std::chrono::duration_cast(Now().time_since_epoch()).count() - WAIT_FOR_ISLOCK_TIMEOUT); - pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 9U); BOOST_CHECK(pblocktemplate->block.vtx[8]->GetHash() == hashLowFeeTx2); } -void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector& txFirst, int baseheight) +void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::vector& txFirst, int baseheight) { uint256 hash; CMutableTransaction tx; TestMemPoolEntryHelper entry; entry.nFee = 11; entry.nHeight = 11; - // Just to make sure we can still make simple blocks - auto pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); - BOOST_CHECK(pblocktemplate); const CAmount BLOCKSUBSIDY = 500*COIN; const CAmount LOWFEE = CENT; const CAmount HIGHFEE = COIN; - const CAmount HIGHERFEE = 4*COIN; + const CAmount HIGHERFEE = 4 * COIN; - // block sigops > limit: 1000 CHECKMULTISIG + 1 - tx.vin.resize(1); - // NOTE: OP_NOP is used to force 20 SigOps for the CHECKMULTISIG - tx.vin[0].scriptSig = CScript() << OP_0 << OP_0 << OP_0 << OP_NOP << OP_CHECKMULTISIG << OP_1; - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vin[0].prevout.n = 0; - tx.vout.resize(1); - tx.vout[0].nValue = BLOCKSUBSIDY; - for (unsigned int i = 0; i < 1001; ++i) { - tx.vout[0].nValue -= LOWFEE; + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + // Just to make sure we can still make simple blocks + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); + BOOST_CHECK(pblocktemplate); + + // block sigops > limit: 1000 CHECKMULTISIG + 1 + tx.vin.resize(1); + // NOTE: OP_NOP is used to force 20 SigOps for the CHECKMULTISIG + tx.vin[0].scriptSig = CScript() << OP_0 << OP_0 << OP_0 << OP_NOP << OP_CHECKMULTISIG << OP_1; + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vin[0].prevout.n = 0; + tx.vout.resize(1); + tx.vout[0].nValue = BLOCKSUBSIDY; + for (unsigned int i = 0; i < 1001; ++i) { + tx.vout[0].nValue -= LOWFEE; + hash = tx.GetHash(); + // Age transaction for InstantSend + m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); + bool spendsCoinbase = i == 0; // only first tx spends coinbase + // If we don't set the # of sig ops in the CTxMemPoolEntry, template creation fails + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + } + + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-blk-sigops")); + } + + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); + BOOST_CHECK(pblocktemplate); + + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vout[0].nValue = BLOCKSUBSIDY; + for (unsigned int i = 0; i < 1001; ++i) { + tx.vout[0].nValue -= LOWFEE; + hash = tx.GetHash(); + bool spendsCoinbase = i == 0; // only first tx spends coinbase + // If we do set the # of sig ops in the CTxMemPoolEntry, template creation passes + // Age transaction for InstantSend + m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(spendsCoinbase).SigOps(20).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + } + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + } + + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + // block size > limit + tx.vin[0].scriptSig = CScript(); + // 18 * (520char + DROP) + OP_1 = 9433 bytes + std::vector vchData(520); + for (unsigned int i = 0; i < 18; ++i) { + tx.vin[0].scriptSig << vchData << OP_DROP; + } + tx.vin[0].scriptSig << OP_1; + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vout[0].nValue = BLOCKSUBSIDY; + for (unsigned int i = 0; i < 128; ++i) { + tx.vout[0].nValue -= LOWFEE; + hash = tx.GetHash(); + bool spendsCoinbase = i == 0; // only first tx spends coinbase + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + } + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + } + + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); + BOOST_CHECK(pblocktemplate); + + // orphan in tx_mempool, template creation fails hash = tx.GetHash(); + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(Now()).FromTx(tx)); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); - bool spendsCoinbase = i == 0; // only first tx spends coinbase - // If we don't set the # of sig ops in the CTxMemPoolEntry, template creation fails + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); + } + + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); + BOOST_CHECK(pblocktemplate); - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); + // child with higher feerate than parent + tx.vin[0].scriptSig = CScript() << OP_1; + tx.vin[0].prevout.hash = txFirst[1]->GetHash(); + tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE; + hash = tx.GetHash(); + tx_mempool.addUnchecked(entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); tx.vin[0].prevout.hash = hash; + tx.vin.resize(2); + tx.vin[1].scriptSig = CScript() << OP_1; + tx.vin[1].prevout.hash = txFirst[0]->GetHash(); + tx.vin[1].prevout.n = 0; + tx.vout[0].nValue = tx.vout[0].nValue + BLOCKSUBSIDY - HIGHERFEE; // First txn output + fresh coinbase - new txn fee + hash = tx.GetHash(); + tx_mempool.addUnchecked(entry.Fee(HIGHERFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); } - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-blk-sigops")); - m_node.mempool->clear(); - - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vout[0].nValue = BLOCKSUBSIDY; - for (unsigned int i = 0; i < 1001; ++i) { - tx.vout[0].nValue -= LOWFEE; + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); + BOOST_CHECK(pblocktemplate); + + // coinbase in tx_mempool, template creation fails + tx.vin.resize(1); + tx.vin[0].prevout.SetNull(); + tx.vin[0].scriptSig = CScript() << OP_0 << OP_1; + tx.vout[0].nValue = 0; hash = tx.GetHash(); - bool spendsCoinbase = i == 0; // only first tx spends coinbase - // If we do set the # of sig ops in the CTxMemPoolEntry, template creation passes // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(spendsCoinbase).SigOps(20).FromTx(tx)); - tx.vin[0].prevout.hash = hash; + // give it a fee so it'll get mined + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); + // Should throw bad-cb-multiple + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-cb-multiple")); } - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); - m_node.mempool->clear(); - - // block size > limit - tx.vin[0].scriptSig = CScript(); - // 18 * (520char + DROP) + OP_1 = 9433 bytes - std::vector vchData(520); - for (unsigned int i = 0; i < 18; ++i) - tx.vin[0].scriptSig << vchData << OP_DROP; - tx.vin[0].scriptSig << OP_1; - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vout[0].nValue = BLOCKSUBSIDY; - for (unsigned int i = 0; i < 128; ++i) + { - tx.vout[0].nValue -= LOWFEE; + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + int64_t mocked_time_for_is = m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() - WAIT_FOR_ISLOCK_TIMEOUT; + + // double spend txn pair in tx_mempool, template creation fails + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vin[0].scriptSig = CScript() << OP_1; + tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE; + tx.vout[0].scriptPubKey = CScript() << OP_1; hash = tx.GetHash(); - bool spendsCoinbase = i == 0; // only first tx spends coinbase - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); - tx.vin[0].prevout.hash = hash; + tx_mempool.addUnchecked(entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + tx.vout[0].scriptPubKey = CScript() << OP_2; + hash = tx.GetHash(); + // Age transaction for InstantSend + m_node.clhandler->UpdateTxFirstSeenMap({hash}, mocked_time_for_is); + tx_mempool.addUnchecked(entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); } - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); - m_node.mempool->clear(); - // orphan in mempool, template creation fails - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(Now()).FromTx(tx)); - // Age transaction for InstantSend - m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); - m_node.mempool->clear(); + { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + + int nHeight = m_node.chainman->ActiveChain().Height(); + constexpr bool is_subsidy_changing{false}; + if constexpr (is_subsidy_changing) { + // subsidy changing + // Create an actual 209999-long block chain (without valid blocks). + while (m_node.chainman->ActiveChain().Tip()->nHeight < 209999) { + CBlockIndex* prev = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* next = new CBlockIndex(); + next->phashBlock = new uint256(InsecureRand256()); + m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash()); + next->pprev = prev; + next->nHeight = prev->nHeight + 1; + next->BuildSkip(); + m_node.chainman->ActiveChain().SetTip(*next); + } + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + // Extend to a 210000-long block chain. + while (m_node.chainman->ActiveChain().Tip()->nHeight < 210000) { + CBlockIndex* prev = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* next = new CBlockIndex(); + next->phashBlock = new uint256(InsecureRand256()); + m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash()); + next->pprev = prev; + next->nHeight = prev->nHeight + 1; + next->BuildSkip(); + m_node.chainman->ActiveChain().SetTip(*next); + } + BOOST_CHECK(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + } // is_subsidy_changing + + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); + BOOST_CHECK(pblocktemplate); + + // invalid p2sh txn in tx_mempool, template creation fails + tx.vin[0].prevout.hash = txFirst[0]->GetHash(); + tx.vin[0].prevout.n = 0; + tx.vin[0].scriptSig = CScript() << OP_1; + tx.vout[0].nValue = BLOCKSUBSIDY - LOWFEE; + CScript script = CScript() << OP_0; + tx.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(script)); + hash = tx.GetHash(); + // Age transaction for InstantSend + m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + tx.vin[0].prevout.hash = hash; + tx.vin[0].scriptSig = CScript() << std::vector(script.begin(), script.end()); + tx.vout[0].nValue -= LOWFEE; + hash = tx.GetHash(); + // Age transaction for InstantSend + m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); + tx_mempool.addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); + // Should throw block-validation-failed + BOOST_CHECK_EXCEPTION(AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("block-validation-failed")); + + if constexpr (is_subsidy_changing) { + // Delete the dummy blocks again. + while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) { + CBlockIndex* del = m_node.chainman->ActiveChain().Tip(); + m_node.chainman->ActiveChain().SetTip(*Assert(del->pprev)); + m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(del->pprev->GetBlockHash()); + delete del->phashBlock; + delete del; + } + } // is_subsidy_changing + // } + } - // child with higher feerate than parent - tx.vin[0].scriptSig = CScript() << OP_1; - tx.vin[0].prevout.hash = txFirst[1]->GetHash(); - tx.vout[0].nValue = BLOCKSUBSIDY-HIGHFEE; - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); - tx.vin[0].prevout.hash = hash; - tx.vin.resize(2); - tx.vin[1].scriptSig = CScript() << OP_1; - tx.vin[1].prevout.hash = txFirst[0]->GetHash(); - tx.vin[1].prevout.n = 0; - tx.vout[0].nValue = tx.vout[0].nValue+BLOCKSUBSIDY-HIGHERFEE; //First txn output + fresh coinbase - new txn fee - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(HIGHERFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); - m_node.mempool->clear(); - - // coinbase in mempool, template creation fails - tx.vin.resize(1); - tx.vin[0].prevout.SetNull(); - tx.vin[0].scriptSig = CScript() << OP_0 << OP_1; - tx.vout[0].nValue = 0; - hash = tx.GetHash(); - // Age transaction for InstantSend - m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); - // give it a fee so it'll get mined - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); - // Should throw bad-cb-multiple - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-cb-multiple")); - m_node.mempool->clear(); - - // double spend txn pair in mempool, template creation fails - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vin[0].scriptSig = CScript() << OP_1; - tx.vout[0].nValue = BLOCKSUBSIDY-HIGHFEE; - tx.vout[0].scriptPubKey = CScript() << OP_1; - hash = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); - tx.vout[0].scriptPubKey = CScript() << OP_2; - hash = tx.GetHash(); - // Age transaction for InstantSend - m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); - m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); - m_node.mempool->clear(); - - // subsidy changing - // int nHeight = m_node.chainman->ActiveChain().Height(); - // // Create an actual 209999-long block chain (without valid blocks). - // while (m_node.chainman->ActiveChain().Tip()->nHeight < 209999) { - // CBlockIndex* prev = m_node.chainman->ActiveChain().Tip(); - // CBlockIndex* next = new CBlockIndex(); - // next->phashBlock = new uint256(InsecureRand256()); - // pcoinsTip->SetBestBlock(next->GetBlockHash()); - // next->pprev = prev; - // next->nHeight = prev->nHeight + 1; - // next->BuildSkip(); - // m_node.chainman->ActiveChain().SetTip(*next); - // } - //BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); - // // Extend to a 210000-long block chain. - // while (m_node.chainman->ActiveChain().Tip()->nHeight < 210000) { - // CBlockIndex* prev = m_node.chainman->ActiveChain().Tip(); - // CBlockIndex* next = new CBlockIndex(); - // next->phashBlock = new uint256(InsecureRand256()); - // pcoinsTip->SetBestBlock(next->GetBlockHash()); - // next->pprev = prev; - // next->nHeight = prev->nHeight + 1; - // next->BuildSkip(); - // m_node.chainman->ActiveChain().SetTip(*next); - // } - //BOOST_CHECK(pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey)); - - // invalid (pre-p2sh) txn in mempool, template creation fails - tx.vin[0].prevout.hash = txFirst[0]->GetHash(); - tx.vin[0].prevout.n = 0; - tx.vin[0].scriptSig = CScript() << OP_1; - tx.vout[0].nValue = BLOCKSUBSIDY-LOWFEE; - CScript script = CScript() << OP_0; - tx.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(script)); - hash = tx.GetHash(); - // Age transaction for InstantSend - m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); - tx.vin[0].prevout.hash = hash; - tx.vin[0].scriptSig = CScript() << std::vector(script.begin(), script.end()); - tx.vout[0].nValue -= LOWFEE; - hash = tx.GetHash(); - // Age transaction for InstantSend - m_node.clhandler->UpdateTxFirstSeenMap({hash}, pblocktemplate->block.nTime - WAIT_FOR_ISLOCK_TIMEOUT); - m_node.mempool->addUnchecked(entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); - // Should throw block-validation-failed - BOOST_CHECK_EXCEPTION(AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey), std::runtime_error, HasReason("block-validation-failed")); - m_node.mempool->clear(); - - // // Delete the dummy blocks again. - // while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) { - // CBlockIndex* del = m_node.chainman->ActiveChain().Tip(); - // m_node.chainman->ActiveChain().SetTip(*Assert(del->pprev)); - // m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(del->pprev->GetBlockHash()); - // delete del->phashBlock; - // delete del; - // } + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); // non-final txs in mempool SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1); @@ -435,9 +501,9 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C hash = tx.GetHash(); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hash}, mocked_time_for_is); - m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes - BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail + BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail { CBlockIndex* active_chain_tip = m_node.chainman->ActiveChain().Tip(); @@ -451,9 +517,9 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C hash = tx.GetHash(); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hash}, mocked_time_for_is); - m_node.mempool->addUnchecked(entry.Time(Now()).FromTx(tx)); + tx_mempool.addUnchecked(entry.Time(Now()).FromTx(tx)); BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes - BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail + BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail const int SEQUENCE_LOCK_TIME = 512; // Sequence locks pass 512 seconds later for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i) @@ -476,9 +542,9 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C hash = tx.GetHash(); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hash}, mocked_time_for_is); - m_node.mempool->addUnchecked(entry.Time(Now()).FromTx(tx)); + tx_mempool.addUnchecked(entry.Time(Now()).FromTx(tx)); BOOST_CHECK(!CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime fails - BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass + BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass BOOST_CHECK(IsFinalTx(CTransaction(tx), m_node.chainman->ActiveChain().Tip()->nHeight + 2, m_node.chainman->ActiveChain().Tip()->GetMedianTimePast())); // Locktime passes on 2nd block // absolute time locked @@ -489,9 +555,9 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C hash = tx.GetHash(); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hash}, mocked_time_for_is); - m_node.mempool->addUnchecked(entry.Time(Now()).FromTx(tx)); + tx_mempool.addUnchecked(entry.Time(Now()).FromTx(tx)); BOOST_CHECK(!CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime fails - BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass + BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass BOOST_CHECK(IsFinalTx(CTransaction(tx), m_node.chainman->ActiveChain().Tip()->nHeight + 2, m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1)); // Locktime passes 1 second later // mempool-dependent transactions (not added) @@ -500,15 +566,16 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C tx.nLockTime = 0; tx.vin[0].nSequence = 0; BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes - BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass + BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass tx.vin[0].nSequence = 1; - BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail + BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG; - BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass + BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | 1; - BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail + BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); + BOOST_CHECK(pblocktemplate); // None of the of the absolute height/time locked tx should have made // it into the template because we still check IsFinalTx in CreateNewBlock, @@ -522,8 +589,11 @@ void MinerTestingSetup::TestBasicMining(const CChainParams& chainparams, const C } } -void MinerTestingSetup::TestPrioritisedMining(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector& txFirst) +void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const std::vector& txFirst) { + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + TestMemPoolEntryHelper entry; // Test that a tx below min fee but prioritised is included @@ -535,29 +605,29 @@ void MinerTestingSetup::TestPrioritisedMining(const CChainParams& chainparams, c tx.vout.resize(1); tx.vout[0].nValue = 5000000000LL; // 0 fee uint256 hashFreePrioritisedTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).Time(Now()).SpendsCoinbase(true).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashFreePrioritisedTx, 5 * COIN); + tx_mempool.addUnchecked(entry.Fee(0).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashFreePrioritisedTx, 5 * COIN); tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vin[0].prevout.n = 0; tx.vout[0].nValue = 5000000000LL - 1000; // This tx has a low fee: 1000 satoshis uint256 hashParentTx = tx.GetHash(); // save this txid for later use - m_node.mempool->addUnchecked(entry.Fee(1000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(1000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); // This tx has a medium fee: 10000 satoshis tx.vin[0].prevout.hash = txFirst[2]->GetHash(); tx.vout[0].nValue = 5000000000LL - 10000; uint256 hashMediumFeeTx = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(10000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashMediumFeeTx, -5 * COIN); + tx_mempool.addUnchecked(entry.Fee(10000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashMediumFeeTx, -5 * COIN); // This tx also has a low fee, but is prioritised tx.vin[0].prevout.hash = hashParentTx; tx.vout[0].nValue = 5000000000LL - 1000 - 1000; // 1000 satoshi fee uint256 hashPrioritsedChild = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(1000).Time(Now()).SpendsCoinbase(false).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashPrioritsedChild, 2 * COIN); + tx_mempool.addUnchecked(entry.Fee(1000).Time(Now()).SpendsCoinbase(false).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashPrioritsedChild, 2 * COIN); // Test that transaction selection properly updates ancestor fee calculations as prioritised // parents get included in a block. Create a transaction with two prioritised ancestors, each @@ -568,23 +638,23 @@ void MinerTestingSetup::TestPrioritisedMining(const CChainParams& chainparams, c tx.vin[0].prevout.hash = txFirst[3]->GetHash(); tx.vout[0].nValue = 5000000000LL; // 0 fee uint256 hashFreeParent = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashFreeParent, 10 * COIN); + tx_mempool.addUnchecked(entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashFreeParent, 10 * COIN); tx.vin[0].prevout.hash = hashFreeParent; tx.vout[0].nValue = 5000000000LL; // 0 fee uint256 hashFreeChild = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); - m_node.mempool->PrioritiseTransaction(hashFreeChild, 1 * COIN); + tx_mempool.addUnchecked(entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); + tx_mempool.PrioritiseTransaction(hashFreeChild, 1 * COIN); tx.vin[0].prevout.hash = hashFreeChild; tx.vout[0].nValue = 5000000000LL; // 0 fee uint256 hashFreeGrandchild = tx.GetHash(); - m_node.mempool->addUnchecked(entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); + tx_mempool.addUnchecked(entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); // Age transaction for InstantSend m_node.clhandler->UpdateTxFirstSeenMap({hashMediumFeeTx, hashPrioritsedChild, hashParentTx, hashFreeParent, hashFreeChild, hashFreeGrandchild, hashFreePrioritisedTx}, std::chrono::duration_cast(Now().time_since_epoch()).count() - WAIT_FOR_ISLOCK_TIMEOUT); - auto pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey); + auto pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey); BOOST_REQUIRE_EQUAL(pblocktemplate->block.vtx.size(), 6U); BOOST_CHECK(pblocktemplate->block.vtx[1]->GetHash() == hashFreeParent); BOOST_CHECK(pblocktemplate->block.vtx[2]->GetHash() == hashFreePrioritisedTx); @@ -603,13 +673,12 @@ void MinerTestingSetup::TestPrioritisedMining(const CChainParams& chainparams, c BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) { // Note that by default, these tests run with size accounting enabled. - const auto chainParams = CreateChainParams(*m_node.args, CBaseChainParams::MAIN); - const CChainParams& chainparams = *chainParams; CScript scriptPubKey = CScript() << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f") << OP_CHECKSIG; std::unique_ptr pblocktemplate, pemptyblocktemplate; + CTxMemPool& tx_mempool{*m_node.mempool}; // Simple block creation, nothing special yet: - BOOST_CHECK(pemptyblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); + BOOST_CHECK(pemptyblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); // We can't make transactions until we have inputs // Therefore, load 100 blocks :) @@ -637,7 +706,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // This will usually succeed in the first round as we take the nonce from BLOCKINFO // It's however useful when adding new blocks with unknown nonces (you should add the found block to BLOCKINFO) - while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits, chainparams.GetConsensus())) { + while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits, Assert(m_node.chainman)->GetParams().GetConsensus())) { pblock->nNonce++; } } @@ -653,9 +722,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) { LOCK(cs_main); - LOCK(m_node.mempool->cs); - TestBasicMining(chainparams, scriptPubKey, txFirst, baseheight); + TestBasicMining(scriptPubKey, txFirst, baseheight); } // Mine an empty block @@ -666,7 +734,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1); - BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); + BOOST_CHECK(pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 5U); } // unlock cs_main while calling InvalidateBlock @@ -674,16 +742,14 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) m_node.chainman->ActiveChainstate().InvalidateBlock(state, WITH_LOCK(cs_main, return m_node.chainman->ActiveChain().Tip())); SetMockTime(0); - m_node.mempool->clear(); - LOCK2(cs_main, m_node.mempool->cs); - TestPackageSelection(chainparams, scriptPubKey, txFirst); + LOCK(cs_main); + TestPackageSelection(scriptPubKey, txFirst); m_node.chainman->ActiveChain().Tip()->nHeight--; SetMockTime(0); - m_node.mempool->clear(); - TestPrioritisedMining(chainparams, scriptPubKey, txFirst); + TestPrioritisedMining(scriptPubKey, txFirst); } BOOST_AUTO_TEST_SUITE_END() From ff828ed5d20ed22532dc057f6756f21ac7edbc0a Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 22 Aug 2026 01:56:07 +0700 Subject: [PATCH 2/9] fix: asan failure + unification of Dash's miner_tests diverged in dash#2600 (2019) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upstream fakes chain progress with Tip()->nHeight++, which crashed the then height-based GetBlockTxOuts(), so Dash mined a real block and undid it with InvalidateBlock. The bitcoin#25073 backport made each sub-test delete and recreate m_node.mempool, leaving the raw Chainstate::m_mempool pointer dangling — harmless upstream, which never processes blocks afterwards, but Dash's real-block tail then read the freed mempool in FlushStateToDisk -> GetCoinsCacheSizeState (the ASan error; release builds survived only because the freed address was immediately reused by the next mempool). GetBlockTxOuts() is now pindexPrev-based and the CbTx path is inactive below DIP0003Height, so the upstream trick works again and the divergence is simply removed. Original asan failure that has been discovered by CI on 7630 after bitcoin#25073 is backported ==54419==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x51900000b838 at pc 0x560fbea430ad bp 0x7ffc2c9c4340 sp 0x7ffc2c9c4338 READ of size 8 at 0x51900000b838 thread T0 (d-test) #0 0x560fbea430ac in Chainstate::GetCoinsCacheSizeState() src/validation.cpp:2596:32 #1 0x560fbea1c7bd in Chainstate::FlushStateToDisk(BlockValidationState&, FlushStateMode, int) src/validation.cpp:2641:43 #2 0x560fbea6eefe in ChainstateManager::AcceptBlock(std::shared_ptr const&, BlockValidationState&, CBlockIndex**, bool, FlatFilePos const*, bool*, uint256 const*) src/validation.cpp:4472:28 #3 0x560fbea720a0 in ChainstateManager::ProcessNewBlock(std::shared_ptr const&, bool, bool*) src/validation.cpp:4505:19 #4 0x560fbc85dbff in miner_tests::CreateNewBlock_validity::test_method()::$_0::operator()() const src/test/miner_tests.cpp:714:9 #5 0x560fbc85b5db in miner_tests::CreateNewBlock_validity::test_method() src/test/miner_tests.cpp:730:5 #6 0x560fbc859fad in miner_tests::CreateNewBlock_validity_invoker() src/test/miner_tests.cpp:673:1 #7 0x560fbba6bfcd in boost::function0::operator()() const /__w/dash/dash/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 #8 0x560fbbaedb37 in operator() /__w/dash/dash/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1388:32 #9 0x560fbbaedb37 in boost::detail::function::function_obj_invoker0::invoke(boost::detail::function::function_buffer&) /__w/dash/dash/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 #10 0x560fbbae74cd in boost::function0::operator()() const --- src/test/miner_tests.cpp | 45 +++++++++++----------------------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index d0393b2f1410..46005a2df037 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include @@ -99,7 +98,6 @@ constexpr static struct { {0, 642154}, {0, 5835730}, {0, 164952}, {0, 1584353}, {0, 666367}, {0, 854797}, {0, 2407599}, {0, 3328128}, {0, 245451}, {0, 2154593}, {0, 4043042}, {0, 2939387}, {0, 3509685}, {0, 635871}, {0, 2645814}, {0, 1788871}, {0, 2263667}}; -constexpr static size_t blockinfo_size = sizeof(BLOCKINFO) / sizeof(BLOCKINFO[0]); static std::unique_ptr CreateBlockIndex(int nHeight, CBlockIndex* active_chain_tip) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { @@ -587,6 +585,11 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: CBlockIndex* ancestor{Assert(m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i))}; ancestor->nTime += SEQUENCE_LOCK_TIME; // Trick the MedianTimePast } + m_node.chainman->ActiveChain().Tip()->nHeight++; + SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1); + + BOOST_CHECK(pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 5U); } void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const std::vector& txFirst) @@ -674,27 +677,25 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) { // Note that by default, these tests run with size accounting enabled. CScript scriptPubKey = CScript() << ParseHex("04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f") << OP_CHECKSIG; - std::unique_ptr pblocktemplate, pemptyblocktemplate; + std::unique_ptr pblocktemplate; CTxMemPool& tx_mempool{*m_node.mempool}; // Simple block creation, nothing special yet: - BOOST_CHECK(pemptyblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); + BOOST_CHECK(pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); // We can't make transactions until we have inputs // Therefore, load 100 blocks :) int baseheight = 0; std::vector txFirst; - - auto createAndProcessEmptyBlock = [&]() { - int i = WITH_LOCK(::cs_main, return m_node.chainman->ActiveChain().Height()) % blockinfo_size; - CBlock *pblock = &pemptyblocktemplate->block; // pointer for convenience + for (const auto& bi : BLOCKINFO) { + CBlock *pblock = &pblocktemplate->block; // pointer for convenience { LOCK(cs_main); pblock->nVersion = VERSIONBITS_TOP_BITS; pblock->nTime = m_node.chainman->ActiveChain().Tip()->GetMedianTimePast()+1; CMutableTransaction txCoinbase(*pblock->vtx[0]); txCoinbase.nVersion = 1; - txCoinbase.vin[0].scriptSig = CScript{} << (m_node.chainman->ActiveChain().Height() + 1) << BLOCKINFO[i].extranonce; + txCoinbase.vin[0].scriptSig = CScript{} << (m_node.chainman->ActiveChain().Height() + 1) << bi.extranonce; txCoinbase.vout[0].scriptPubKey = CScript(); pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); if (txFirst.size() == 0) @@ -702,7 +703,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) if (txFirst.size() < 4) txFirst.push_back(pblock->vtx[0]); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); - pblock->nNonce = BLOCKINFO[i].nonce; + pblock->nNonce = bi.nonce; // This will usually succeed in the first round as we take the nonce from BLOCKINFO // It's however useful when adding new blocks with unknown nonces (you should add the found block to BLOCKINFO) @@ -713,37 +714,15 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) std::shared_ptr shared_pblock = std::make_shared(*pblock); BOOST_CHECK(Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, true, nullptr)); pblock->hashPrevBlock = pblock->GetHash(); - }; - - for ([[maybe_unused]] const auto& _ : BLOCKINFO) { - createAndProcessEmptyBlock(); } - - { LOCK(cs_main); TestBasicMining(scriptPubKey, txFirst, baseheight); - } - - // Mine an empty block - createAndProcessEmptyBlock(); - - { - LOCK(cs_main); - - SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1); - - BOOST_CHECK(pblocktemplate = AssemblerForTest(tx_mempool).CreateNewBlock(scriptPubKey)); - BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 5U); - } // unlock cs_main while calling InvalidateBlock - - BlockValidationState state; - m_node.chainman->ActiveChainstate().InvalidateBlock(state, WITH_LOCK(cs_main, return m_node.chainman->ActiveChain().Tip())); + m_node.chainman->ActiveChain().Tip()->nHeight--; SetMockTime(0); - LOCK(cs_main); TestPackageSelection(scriptPubKey, txFirst); m_node.chainman->ActiveChain().Tip()->nHeight--; From 15687481cb7e99f2ffd1472e8921d7812129ebc9 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 17 Feb 2022 08:01:45 +0100 Subject: [PATCH 3/9] fix: add missing changes from bitcoin/bitcoin#24177 Co-authored-by: MarcoFalke --- src/validation.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/validation.cpp b/src/validation.cpp index e1cc33d909ac..dd53c5b4da6a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1455,6 +1455,7 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra int64_t accept_time, bool bypass_limits, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + AssertLockHeld(::cs_main); const CChainParams& chainparams{active_chainstate.m_params}; assert(active_chainstate.GetMempool() != nullptr); CTxMemPool& pool{*active_chainstate.GetMempool()}; @@ -4585,6 +4586,7 @@ bool ChainstateManager::ProcessNewBlock(const std::shared_ptr& blo MempoolAcceptResult ChainstateManager::ProcessTransaction(const CTransactionRef& tx, bool test_accept, bool bypass_limits) { + AssertLockHeld(cs_main); Chainstate& active_chainstate = ActiveChainstate(); if (!active_chainstate.GetMempool()) { TxValidationState state; From 134a6d99eba784ffe6a6cecb7fa3fde2134795a1 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 19 Oct 2022 10:04:27 +0200 Subject: [PATCH 4/9] Merge bitcoin/bitcoin#25830: refactor: Replace m_params with chainman.GetParams() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5d3f98d27879cd6d84b8590e947336e8d09613ed refactor: Replace m_params with chainman.GetParams() (Aurèle Oulès) Pull request description: Fixes a TODO introduced in #24595. Removes `m_params` from `CChainState` class and replaces it with `m_chainman.GetParams()`. ACKs for top commit: MarcoFalke: review ACK 5d3f98d27879cd6d84b8590e947336e8d09613ed 🌎 Tree-SHA512: de0fe31450d281cc7307c0d820495e86c93c7998e77a148db2c703da66cff1059e6560c041f1864913c42075aa24d259c2623d45e929ca0a8056ed330a9f9978 Co-authored-by: MacroFake --- src/validation.cpp | 74 +++++++++++++++++++++++++--------------------- src/validation.h | 4 --- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index dd53c5b4da6a..bd0599561fd4 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1456,7 +1456,7 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); - const CChainParams& chainparams{active_chainstate.m_params}; + const CChainParams& chainparams{active_chainstate.m_chainman.GetParams()}; assert(active_chainstate.GetMempool() != nullptr); CTxMemPool& pool{*active_chainstate.GetMempool()}; @@ -1490,7 +1490,7 @@ PackageMempoolAcceptResult ProcessNewPackage(Chainstate& active_chainstate, CTxM assert(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;})); std::vector coins_to_uncache; - const CChainParams& chainparams = active_chainstate.m_params; + const CChainParams& chainparams = active_chainstate.m_chainman.GetParams(); auto result = [&]() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); if (test_accept) { @@ -1645,7 +1645,6 @@ Chainstate::Chainstate(CTxMemPool* mempool, m_chain_helper(chain_helper), m_evoDb(evoDb), m_blockman(blockman), - m_params(chainman.GetParams()), m_chainman(chainman), m_from_snapshot_blockhash(from_snapshot_blockhash) {} @@ -2012,7 +2011,7 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn AssertLockHeld(cs_main); assert(m_chain_helper); - bool fDIP0003Active = DeploymentActiveAt(*pindex, m_params.GetConsensus(), Consensus::DEPLOYMENT_DIP0003); + bool fDIP0003Active = DeploymentActiveAt(*pindex, m_chainman.GetConsensus(), Consensus::DEPLOYMENT_DIP0003); if (fDIP0003Active && !m_evoDb.VerifyBestBlock(EvoDbIdentity(), pindex->GetBlockHash())) { // Nodes that upgraded after DIP3 activation will have to reindex to ensure evodb consistency AbortNode(EvoDbInconsistencyMessage()); @@ -2199,6 +2198,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, CCoinsViewCache& view, bool fJustCheck) { const auto time_start{SteadyClock::now()}; + const CChainParams& params{m_chainman.GetParams()}; AssertLockHeld(cs_main); assert(pindex); @@ -2215,7 +2215,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // chainstate has already moved the process-wide flag to basic. Rolled back // below unless we fully connect the block. ScopedBLSLegacyScheme bls_scheme_guard{ - !DeploymentActiveAt(*pindex, m_params.GetConsensus(), Consensus::DEPLOYMENT_V19)}; + !DeploymentActiveAt(*pindex, params.GetConsensus(), Consensus::DEPLOYMENT_V19)}; // Check it again in case a previous version let a bad block in // NOTE: We don't currently (re-)invoke ContextualCheckBlock() or @@ -2229,7 +2229,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // Also, currently the rule against blocks more than 2 hours in the future // is enforced in ContextualCheckBlockHeader(); we wouldn't want to // re-enforce that rule here. - if (!CheckBlock(block, state, m_params.GetConsensus(), !fJustCheck, !fJustCheck)) { + if (!CheckBlock(block, state, params.GetConsensus(), !fJustCheck, !fJustCheck)) { if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) { // We don't write down blocks to disk if they may have been // corrupted, so this should be impossible unless we're having hardware @@ -2249,7 +2249,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, assert(hashPrevBlock == view.GetBestBlock()); if (pindex->pprev) { - bool fDIP0003Active = DeploymentActiveAt(*pindex, m_params.GetConsensus(), Consensus::DEPLOYMENT_DIP0003); + bool fDIP0003Active = DeploymentActiveAt(*pindex, m_chainman.GetConsensus(), Consensus::DEPLOYMENT_DIP0003); if (fDIP0003Active && !m_evoDb.VerifyBestBlock(EvoDbIdentity(), pindex->pprev->GetBlockHash())) { // Nodes that upgraded after DIP3 activation will have to reindex to ensure evodb consistency return AbortNode(state, EvoDbInconsistencyMessage()); @@ -2259,7 +2259,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // Special case for the genesis block, skipping connection of its transactions // (its coinbase is unspendable) - if (block_hash == m_params.GetConsensus().hashGenesisBlock) { + if (block_hash == params.GetConsensus().hashGenesisBlock) { if (!fJustCheck) view.SetBestBlock(pindex->GetBlockHash()); return true; @@ -2291,7 +2291,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // artificially set the default assumed verified block further back. // The test against nMinimumChainWork prevents the skipping when denied access to any chain at // least as good as the expected chain. - fScriptChecks = (GetBlockProofEquivalentTime(*m_chainman.m_best_header, *pindex, *m_chainman.m_best_header, m_params.GetConsensus()) <= 60 * 60 * 24 * 7 * 2); + fScriptChecks = (GetBlockProofEquivalentTime(*m_chainman.m_best_header, *pindex, *m_chainman.m_best_header, params.GetConsensus()) <= 60 * 60 * 24 * 7 * 2); } } } @@ -2322,9 +2322,9 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // duplicate transactions descending from the known pairs either. // If we're on the known chain at height greater than where BIP34 activated, we can save the db accesses needed for the BIP30 check. assert(pindex->pprev); - CBlockIndex* pindexBIP34height = pindex->pprev->GetAncestor(m_params.GetConsensus().BIP34Height); + CBlockIndex* pindexBIP34height = pindex->pprev->GetAncestor(params.GetConsensus().BIP34Height); //Only continue to enforce if we're below BIP34 activation height or the block hash at that height doesn't correspond. - fEnforceBIP30 = fEnforceBIP30 && (!pindexBIP34height || !(pindexBIP34height->GetBlockHash() == m_params.GetConsensus().BIP34Hash)); + fEnforceBIP30 = fEnforceBIP30 && (!pindexBIP34height || !(pindexBIP34height->GetBlockHash() == params.GetConsensus().BIP34Hash)); if (fEnforceBIP30) { for (const auto& tx : block.vtx) { @@ -2340,9 +2340,9 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, /// DASH: Check superblock start // make sure old budget is the real one - if (pindex->nHeight == m_params.GetConsensus().nSuperblockStartBlock && - m_params.GetConsensus().nSuperblockStartHash != uint256() && - block_hash != m_params.GetConsensus().nSuperblockStartHash) { + if (pindex->nHeight == params.GetConsensus().nSuperblockStartBlock && + params.GetConsensus().nSuperblockStartHash != uint256() && + block_hash != params.GetConsensus().nSuperblockStartHash) { LogPrintf("ERROR: ConnectBlock(): invalid superblock start\n"); return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-sb-start"); } @@ -2350,7 +2350,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // Enforce BIP68 (sequence locks) int nLockTimeFlags = 0; - if (DeploymentActiveAt(*pindex, m_params.GetConsensus(), Consensus::DEPLOYMENT_CSV)) { + if (DeploymentActiveAt(*pindex, params.GetConsensus(), Consensus::DEPLOYMENT_CSV)) { nLockTimeFlags |= LOCKTIME_VERIFY_SEQUENCE; } @@ -2379,7 +2379,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, int nInputs = 0; unsigned int nSigOps = 0; blockundo.vtxundo.reserve(block.vtx.size() - 1); - bool fDIP0001Active_context = DeploymentActiveAt(*pindex, m_params.GetConsensus(), Consensus::DEPLOYMENT_DIP0001); + bool fDIP0001Active_context = DeploymentActiveAt(*pindex, params.GetConsensus(), Consensus::DEPLOYMENT_DIP0001); // MUST process special txes before updating UTXO to ensure consistency between mempool and block processing std::optional mnlist_updates_opt{std::nullopt}; @@ -2526,7 +2526,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // DASH : MODIFIED TO CHECK MASTERNODE PAYMENTS AND SUPERBLOCKS // TODO: resync data (both ways?) and try to reprocess this block later. - CAmount blockSubsidy = GetBlockSubsidy(pindex, m_params.GetConsensus()); + CAmount blockSubsidy = GetBlockSubsidy(pindex, params.GetConsensus()); CAmount feeReward = nFees; std::string strError; @@ -2713,7 +2713,7 @@ bool Chainstate::FlushStateToDisk( } else { LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune", BCLog::BENCHMARK); - m_blockman.FindFilesToPrune(setFilesToPrune, m_params.PruneAfterHeight(), m_chain.Height(), last_prune, IsInitialBlockDownload()); + m_blockman.FindFilesToPrune(setFilesToPrune, m_chainman.GetParams().PruneAfterHeight(), m_chain.Height(), last_prune, IsInitialBlockDownload()); m_blockman.m_check_for_pruning = false; } if (!setFilesToPrune.empty()) { @@ -2892,13 +2892,15 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew) AssertLockHeld(::cs_main); const auto& coins_tip = this->CoinsTip(); + const CChainParams& params{m_chainman.GetParams()}; + // The remainder of the function isn't relevant if we are not acting on // the active chainstate, so return if need be. if (this != &m_chainman.ActiveChainstate()) { // Only log every so often so that we don't bury log messages at the tip. constexpr int BACKGROUND_LOG_INTERVAL = 2000; if (pindexNew->nHeight % BACKGROUND_LOG_INTERVAL == 0) { - UpdateTipLog(coins_tip, pindexNew, m_params, m_evoDb, __func__, "[background validation] ", ""); + UpdateTipLog(coins_tip, pindexNew, params, m_evoDb, __func__, "[background validation] ", ""); } return; } @@ -2920,7 +2922,7 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew) const CBlockIndex* pindex = pindexNew; for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) { WarningBitsConditionChecker checker(m_chainman, bit); - ThresholdState state = checker.GetStateFor(pindex, m_params.GetConsensus(), m_chainman.m_warningcache.at(bit)); + ThresholdState state = checker.GetStateFor(pindex, params.GetConsensus(), m_chainman.m_warningcache.at(bit)); if (state == ThresholdState::ACTIVE || state == ThresholdState::LOCKED_IN) { const bilingual_str warning = strprintf(_("Unknown new rules activated (versionbit %i)"), bit); if (state == ThresholdState::ACTIVE) { @@ -2931,7 +2933,7 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew) } } } - UpdateTipLog(coins_tip, pindexNew, m_params, m_evoDb, __func__, "", warning_messages.original); + UpdateTipLog(coins_tip, pindexNew, params, m_evoDb, __func__, "", warning_messages.original); } /** Disconnect m_chain's tip. @@ -2957,7 +2959,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra // Read block from disk. std::shared_ptr pblock = std::make_shared(); CBlock& block = *pblock; - if (!ReadBlockFromDisk(block, pindexDelete, m_params.GetConsensus())) { + if (!ReadBlockFromDisk(block, pindexDelete, m_chainman.GetConsensus())) { return error("DisconnectTip(): Failed to read block"); } // UndoSpecialTxsInBlock does its undo work under the scheme that will be in @@ -2967,7 +2969,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra // restores it if disconnect post-processing fails before m_chain.SetTip() // moves the tip backward. ScopedBLSLegacyScheme bls_scheme_guard{ - !DeploymentActiveAt(*pindexDelete, m_params.GetConsensus(), Consensus::DEPLOYMENT_V19)}; + !DeploymentActiveAt(*pindexDelete, m_chainman.GetConsensus(), Consensus::DEPLOYMENT_V19)}; // Apply the block atomically to the chain state. const auto time_start{SteadyClock::now()}; { @@ -3108,7 +3110,7 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, std::shared_ptr pthisBlock; if (!pblock) { std::shared_ptr pblockNew = std::make_shared(); - if (!ReadBlockFromDisk(*pblockNew, pindexNew, m_params.GetConsensus())) { + if (!ReadBlockFromDisk(*pblockNew, pindexNew, m_chainman.GetConsensus())) { return AbortNode(state, "Failed to read block"); } pthisBlock = pblockNew; @@ -4498,7 +4500,9 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr& pblock, if (pindex->nChainWork < nMinimumChainWork) return true; } - if (!CheckBlock(block, state, GetConsensus(), true, true, &hash) || + const CChainParams& params{GetParams()}; + + if (!CheckBlock(block, state, params.GetConsensus(), true, true, &hash) || !ContextualCheckBlock(block, state, *this, pindex->pprev)) { if (state.IsInvalid() && state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; @@ -4682,7 +4686,7 @@ bool Chainstate::LoadChainTip() tip->GetBlockHash().ToString(), m_chain.Height(), FormatISO8601DateTime(tip->GetBlockTime()), - GuessVerificationProgress(m_params.TxData(), tip)); + GuessVerificationProgress(m_chainman.GetParams().TxData(), tip)); return true; } @@ -4836,7 +4840,7 @@ bool Chainstate::RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& in // TODO: merge with ConnectBlock CBlock block; - if (!ReadBlockFromDisk(block, pindex, m_params.GetConsensus())) { + if (!ReadBlockFromDisk(block, pindex,m_chainman.GetConsensus())) { return error("RollforwardBlock(): ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); } @@ -4893,7 +4897,7 @@ bool Chainstate::ReplayBlocks() pindexOld = &(m_blockman.m_block_index[hashHeads[1]]); pindexFork = LastCommonAncestor(pindexOld, pindexNew); assert(pindexFork != nullptr); - const bool fDIP0003Active = DeploymentActiveAt(*pindexOld, m_params.GetConsensus(), Consensus::DEPLOYMENT_DIP0003); + const bool fDIP0003Active = DeploymentActiveAt(*pindexOld, m_chainman.GetConsensus(), Consensus::DEPLOYMENT_DIP0003); if (fDIP0003Active && !m_evoDb.VerifyBestBlock(EvoDbIdentity(), pindexOld->GetBlockHash())) { return error("ReplayBlocks(DASH): %s", EvoDbInconsistencyMessage()); } @@ -4905,7 +4909,7 @@ bool Chainstate::ReplayBlocks() while (pindexOld != pindexFork) { if (pindexOld->nHeight > 0) { // Never disconnect the genesis block. CBlock block; - if (!ReadBlockFromDisk(block, pindexOld, m_params.GetConsensus())) { + if (!ReadBlockFromDisk(block, pindexOld, m_chainman.GetConsensus())) { return error("ReplayBlocks(): ReadBlockFromDisk() failed at %d, hash=%s", pindexOld->nHeight, pindexOld->GetBlockHash().ToString()); } LogPrintf("Rolling back %s (%i)\n", pindexOld->GetBlockHash().ToString(), pindexOld->nHeight); @@ -5025,24 +5029,26 @@ bool Chainstate::LoadGenesisBlock() { LOCK(cs_main); + const CChainParams& params{m_chainman.GetParams()}; + // Check whether we're already initialized by checking for genesis in // m_blockman.m_block_index. Note that we can't use m_chain here, since it is // set based on the coins db, not the block index db, which is the only // thing loaded at this point. - if (m_blockman.m_block_index.count(m_params.GenesisBlock().GetHash())) + if (m_blockman.m_block_index.count(params.GenesisBlock().GetHash())) return true; try { BlockValidationState state; - if (!AddGenesisBlock(m_params.GenesisBlock(), state)) + if (!AddGenesisBlock(params.GenesisBlock(), state)) return false; - if (m_params.NetworkIDString() == CBaseChainParams::DEVNET) { + if (params.NetworkIDString() == CBaseChainParams::DEVNET) { // We can't continue if devnet genesis block is invalid std::shared_ptr shared_pblock = std::make_shared( - m_params.DevNetGenesisBlock()); - bool fCheckBlock = CheckBlock(*shared_pblock, state, m_params.GetConsensus()); + params.DevNetGenesisBlock()); + bool fCheckBlock = CheckBlock(*shared_pblock, state, params.GetConsensus()); assert(fCheckBlock); if (!m_chainman.AcceptBlock(shared_pblock, state, nullptr, true, nullptr, nullptr)) return false; diff --git a/src/validation.h b/src/validation.h index c325d8b637f2..230c2ea2ff6f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -523,10 +523,6 @@ class Chainstate //! Chainstate instances. node::BlockManager& m_blockman; - /** Chain parameters for this chainstate */ - /* TODO: replace with m_chainman.GetParams() */ - const CChainParams& m_params; - //! The chainstate manager that owns this chainstate. The reference is //! necessary so that this instance can check whether it is the active //! chainstate within deeply nested method calls. From 354534e5524a4d9429343d2e38bfdfa75f804768 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 17 Aug 2026 02:44:01 +0700 Subject: [PATCH 5/9] fix: follow-up bitcoin#19905 - removed dangling unused fLargeWorkForkFound Also removed fLargeWorkInvalidChainFound which is not used externally of warnings.cpp --- src/validation.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/validation.h b/src/validation.h index 230c2ea2ff6f..a180a4aa2f37 100644 --- a/src/validation.h +++ b/src/validation.h @@ -126,9 +126,6 @@ extern bool fCheckpointsEnabled; /** If the tip is older than this (in seconds), the node is considered to be in initial block download. */ extern int64_t nMaxTipAge; -extern bool fLargeWorkForkFound; -extern bool fLargeWorkInvalidChainFound; - /** Block hash whose ancestors we will assume to have valid scripts without checking them. */ extern uint256 hashAssumeValid; From 08cc7da572e4e0f7cdf11cc1867e0328f36b51dc Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 26 Oct 2022 11:41:42 +0200 Subject: [PATCH 6/9] Merge bitcoin/bitcoin#25704: refactor: Remove almost all validation option globals aaaa7bd0ba60aa7114810d4794940882d987c0aa iwyu: Add missing includes (MacroFake) fa9ebec096ae185576a54aa80bd2a9e57f867ed4 Remove g_parallel_script_checks (MacroFake) fa7c834b9f988fa7f2ace2d67b1628211b7819df Move ::fCheckBlockIndex into ChainstateManager (MacroFake) fa43188d86288fa6666307a77c106c8f069ebdbe Move ::fCheckpointsEnabled into ChainstateManager (MacroFake) cccca83099453bf0882bce4f897f77eee5836e8b Move ::nMinimumChainWork into ChainstateManager (MacroFake) fa29d0b57cdeb91c8798d5c90ba9cc18085e99fb Move ::hashAssumeValid into ChainstateManager (MacroFake) faf44876db555f7488c8df96db9fa88b793f897c Move ::nMaxTipAge into ChainstateManager (MacroFake) Pull request description: It seems preferable to assign globals to a class (in this case `ChainstateManager`), than to leave them dangling. This should clarify scope for code-readers, as well as clarifying unit test behaviour. ACKs for top commit: dergoegge: Code review ACK aaaa7bd0ba60aa7114810d4794940882d987c0aa ryanofsky: Code review ACK aaaa7bd0ba60aa7114810d4794940882d987c0aa. No changes since last review, other than rebase aureleoules: reACK aaaa7bd0ba60aa7114810d4794940882d987c0aa Tree-SHA512: 83ec3ba0fb4f1dad95810d4bd4e578454e0718dc1bdd3a794cc4e48aa819b6f5dad4ac4edab3719bdfd5f89cbe23c2740a50fd56c1ff81c99e521c5f6d4e898d Co-authored-by: MacroFake --- ci/dash/lint-tidy.sh | 1 + src/Makefile.am | 2 ++ src/checkqueue.h | 3 +- src/init.cpp | 41 +++++++++++------------ src/kernel/chainstatemanager_opts.h | 16 +++++++++ src/net_processing.cpp | 18 +++++----- src/node/chainstate.cpp | 11 ++++--- src/node/chainstatemanager_args.cpp | 39 ++++++++++++++++++++++ src/node/chainstatemanager_args.h | 19 +++++++++++ src/test/util/setup_common.cpp | 5 +-- src/validation.cpp | 49 ++++++++++++++++++---------- src/validation.h | 27 +++------------ test/functional/feature_maxtipage.py | 4 +-- 13 files changed, 153 insertions(+), 82 deletions(-) create mode 100644 src/node/chainstatemanager_args.cpp create mode 100644 src/node/chainstatemanager_args.h diff --git a/ci/dash/lint-tidy.sh b/ci/dash/lint-tidy.sh index e1d736aed287..09300327eea6 100755 --- a/ci/dash/lint-tidy.sh +++ b/ci/dash/lint-tidy.sh @@ -102,6 +102,7 @@ iwyu_tool.py \ "src/init" \ "src/kernel" \ "src/node/chainstate.cpp" \ + "src/node/chainstatemanager_args.cpp" \ "src/node/mempool_args.cpp" \ "src/node/minisketchwrapper.cpp" \ "src/policy/feerate.cpp" \ diff --git a/src/Makefile.am b/src/Makefile.am index c09876eca3fc..706ba0778b9f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -342,6 +342,7 @@ BITCOIN_CORE_H = \ node/blockstorage.h \ node/caches.h \ node/chainstate.h \ + node/chainstatemanager_args.h \ node/coin.h \ node/connection_types.h \ node/context.h \ @@ -612,6 +613,7 @@ libbitcoin_node_a_SOURCES = \ node/blockstorage.cpp \ node/caches.cpp \ node/chainstate.cpp \ + node/chainstatemanager_args.cpp \ node/coin.cpp \ node/connection_types.cpp \ node/context.cpp \ diff --git a/src/checkqueue.h b/src/checkqueue.h index 5ab91175028d..f8e7436447fc 100644 --- a/src/checkqueue.h +++ b/src/checkqueue.h @@ -191,11 +191,12 @@ class CCheckQueue WITH_LOCK(m_mutex, m_request_stop = false); } + bool HasThreads() const { return !m_worker_threads.empty(); } + ~CCheckQueue() { assert(m_worker_threads.empty()); } - }; /** diff --git a/src/init.cpp b/src/init.cpp index 09dff4da3cf9..cae85f3d70e7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -806,7 +807,10 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-capturemessages", "Capture all P2P messages to disk", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-disablegovernance", strprintf("Disable governance validation (0-1, default: %u)", 0), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-maxsigcachesize=", strprintf("Limit sum of signature cache and script execution cache sizes to MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_SIZE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-maxtipage=", strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", DEFAULT_MAX_TIP_AGE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-maxtipage=", + strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", + Ticks(DEFAULT_MAX_TIP_AGE)), + ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-mocktime=", "Replace actual time with " + UNIX_EPOCH_TIME + " (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-printpriority", strprintf("Log transaction fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-pushversion", "Protocol version to report to other nodes", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); @@ -1300,21 +1304,6 @@ bool AppInitParameterInteraction(const ArgsManager& args) init::SetLoggingCategories(args); init::SetLoggingLevel(args); - fCheckBlockIndex = args.GetBoolArg("-checkblockindex", chainparams.DefaultConsistencyChecks()); - fCheckpointsEnabled = args.GetBoolArg("-checkpoints", DEFAULT_CHECKPOINTS_ENABLED); - - hashAssumeValid = uint256S(args.GetArg("-assumevalid", chainparams.GetConsensus().defaultAssumeValid.GetHex())); - - if (args.IsArgSet("-minimumchainwork")) { - const std::string minChainWorkStr = args.GetArg("-minimumchainwork", ""); - if (!IsHexNumber(minChainWorkStr)) { - return InitError(strprintf(Untranslated("Invalid non-hex (%s) minimum chain work value specified"), minChainWorkStr)); - } - nMinimumChainWork = UintToArith256(uint256S(minChainWorkStr)); - } else { - nMinimumChainWork = UintToArith256(chainparams.GetConsensus().nMinimumChainWork); - } - // block pruning; get the amount of disk space (in MiB) to allot for block & undo files int64_t nPruneArg = args.GetIntArg("-prune", 0); if (nPruneArg < 0) { @@ -1366,8 +1355,6 @@ bool AppInitParameterInteraction(const ArgsManager& args) if (args.GetBoolArg("-peerbloomfilters", DEFAULT_PEERBLOOMFILTERS)) nLocalServices = ServiceFlags(nLocalServices | NODE_BLOOM); - nMaxTipAge = args.GetIntArg("-maxtipage", DEFAULT_MAX_TIP_AGE); - if (args.GetBoolArg("-reindex-chainstate", false)) { // indexes that must be deactivated to prevent index corruption, see #24630 if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) { @@ -1418,6 +1405,16 @@ bool AppInitParameterInteraction(const ArgsManager& args) Untranslated(""))); } + // Also report errors from parsing before daemonization + { + ChainstateManager::Options chainman_opts_dummy{ + .chainparams = chainparams, + }; + if (const auto error{ApplyArgsManOptions(args, chainman_opts_dummy)}) { + return InitError(*error); + } + } + return true; } @@ -1515,7 +1512,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) LogPrintf("Script verification uses %d additional threads\n", script_threads); if (script_threads >= 1) { - g_parallel_script_checks = true; StartScriptCheckWorkerThreads(script_threads); } @@ -1915,6 +1911,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) fReindex = args.GetBoolArg("-reindex", false); bool fReindexChainState = args.GetBoolArg("-reindex-chainstate", false); + ChainstateManager::Options chainman_opts{ + .chainparams = chainparams, + }; + Assert(!ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction // cache size calculations CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size()); @@ -1984,9 +1984,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return {node::ChainstateLoadStatus::SUCCESS, {}}; }); - const ChainstateManager::Options chainman_opts{ - .chainparams = chainparams, - }; node.chainman = std::make_unique(chainman_opts); ChainstateManager& chainman = *node.chainman; diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index 5c7b9cce63f2..f1074a2cbde3 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -5,10 +5,18 @@ #ifndef BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H #define BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H +#include +#include +#include + #include +#include class CChainParams; +static constexpr bool DEFAULT_CHECKPOINTS_ENABLED{true}; +static constexpr auto DEFAULT_MAX_TIP_AGE{6h}; // ~144 blocks behind -> 2 x fork detection time, was 24h in bitcoin + namespace kernel { /** @@ -18,6 +26,14 @@ namespace kernel { */ struct ChainstateManagerOpts { const CChainParams& chainparams; + std::optional check_block_index{}; + bool checkpoints_enabled{DEFAULT_CHECKPOINTS_ENABLED}; + //! If set, it will override the minimum work we will assume exists on some valid chain. + std::optional minimum_chain_work; + //! If set, it will override the block hash whose ancestors we will assume to have valid scripts without checking them. + std::optional assumed_valid_block; + //! If the tip is older than this, the node is considered to be in initial block download. + std::chrono::seconds max_tip_age{DEFAULT_MAX_TIP_AGE}; }; } // namespace kernel diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e25a9a1230c5..740a4e966d27 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1455,7 +1455,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int co // Make sure pindexBestKnownBlock is up to date, we'll need it. ProcessBlockAvailability(peer.m_id); - if (state->pindexBestKnownBlock == nullptr || state->pindexBestKnownBlock->nChainWork < m_chainman.ActiveChain().Tip()->nChainWork || state->pindexBestKnownBlock->nChainWork < nMinimumChainWork) { + if (state->pindexBestKnownBlock == nullptr || state->pindexBestKnownBlock->nChainWork < m_chainman.ActiveChain().Tip()->nChainWork || state->pindexBestKnownBlock->nChainWork < m_chainman.MinimumChainWork()) { // This peer has nothing interesting. return; } @@ -3207,14 +3207,14 @@ void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, if (m_chainman.ActiveChainstate().IsInitialBlockDownload() && !may_have_more_headers) { // If the peer has no more headers to give us, then we know we have // their tip. - if (nodestate->pindexBestKnownBlock && nodestate->pindexBestKnownBlock->nChainWork < nMinimumChainWork) { + if (nodestate->pindexBestKnownBlock && nodestate->pindexBestKnownBlock->nChainWork < m_chainman.MinimumChainWork()) { // This peer has too little work on their headers chain to help // us sync -- disconnect if it is an outbound disconnection // candidate. - // Note: We compare their tip to nMinimumChainWork (rather than + // Note: We compare their tip to the minumum chain work (rather than // m_chainman.ActiveChain().Tip()) because we won't start block download // until we have a headers chain that has at least - // nMinimumChainWork, even if a peer has a chain past our tip, + // the minimum chain work, even if a peer has a chain past our tip, // as an anti-DoS measure. if (pfrom.IsOutboundOrBlockRelayConn()) { LogPrintf("Disconnecting outbound peer %d -- headers chain has insufficient work\n", pfrom.GetId()); @@ -4671,12 +4671,12 @@ void PeerManagerImpl::ProcessMessage( // Note that if we were to be on a chain that forks from the checkpointed // chain, then serving those headers to a peer that has seen the // checkpointed chain would cause that peer to disconnect us. Requiring - // that our chainwork exceed nMinimumChainWork is a protection against + // that our chainwork exceed the mimimum chain work is a protection against // being fed a bogus chain when we started up for the first time and // getting partitioned off the honest network for serving that chain to // others. if (m_chainman.ActiveTip() == nullptr || - (m_chainman.ActiveTip()->nChainWork < nMinimumChainWork && !pfrom.HasPermission(NetPermissionFlags::Download))) { + (m_chainman.ActiveTip()->nChainWork < m_chainman.MinimumChainWork() && !pfrom.HasPermission(NetPermissionFlags::Download))) { LogPrint(BCLog::NET, "Ignoring %s from peer=%d because active chain has too little work; sending empty response\n", msg_type, pfrom.GetId()); // Just respond with an empty headers message, to tell the peer to // go away but not treat us as unresponsive. @@ -5153,7 +5153,7 @@ void PeerManagerImpl::ProcessMessage( // (eg disk space). Because we only try to reconstruct blocks when // we're close to caught up (via the CanDirectFetch() requirement // above, combined with the behavior of not requesting blocks until - // we have a chain with at least nMinimumChainWork), and we ignore + // we have a chain with at least the minimum chain work), and we ignore // compact blocks with less work than our tip, it is safe to treat // reconstructed compact blocks as having been requested. ProcessBlock(pfrom, pblock, /*force_processing=*/true); @@ -6232,7 +6232,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!state.fSyncStarted && CanServeBlocks(*peer) && !fImporting && !fReindex && pto->CanRelay()) { // Only actively request headers from a single peer, unless we're close to end of initial download. - if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->Time() > NodeClock::now() - std::chrono::seconds{nMaxTipAge}) { + if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->Time() > NodeClock::now() - m_chainman.m_options.max_tip_age) { const CBlockIndex* pindexStart = m_chainman.m_best_header; /* If possible, start at the block preceding the currently best known header. This ensures that we always get a @@ -6647,7 +6647,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Check for headers sync timeouts if (state.fSyncStarted && state.m_headers_sync_timeout < std::chrono::microseconds::max()) { // Detect whether this is a stalling initial-headers-sync peer - if (m_chainman.m_best_header->Time() <= NodeClock::now() - std::chrono::seconds{nMaxTipAge}) { + if (m_chainman.m_best_header->Time() <= NodeClock::now() - m_chainman.m_options.max_tip_age) { if (current_time > state.m_headers_sync_timeout && nSyncStarted == 1 && (m_num_preferred_download_peers - state.fPreferredDownload >= 1)) { // Disconnect a peer (without NetPermissionFlags::NoBan permission) if it is our only sync peer, // and we have others we could be using instead. diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index b0ceaf9dea67..e9dbde777940 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -4,11 +4,13 @@ #include +#include #include #include #include #include #include +#include #include #include #include @@ -32,6 +34,7 @@ #include #include +#include #include #include @@ -301,13 +304,13 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize assert(options.chainlocks); assert(options.mn_sync); - if (!hashAssumeValid.IsNull()) { - LogPrintf("Assuming ancestors of block %s have valid signatures.\n", hashAssumeValid.GetHex()); + if (!chainman.AssumedValidBlock().IsNull()) { + LogPrintf("Assuming ancestors of block %s have valid signatures.\n", chainman.AssumedValidBlock().GetHex()); } else { LogPrintf("Validating signatures for all blocks.\n"); } - LogPrintf("Setting nMinimumChainWork=%s\n", nMinimumChainWork.GetHex()); - if (nMinimumChainWork < UintToArith256(chainman.GetConsensus().nMinimumChainWork)) { + LogPrintf("Setting nMinimumChainWork=%s\n", chainman.MinimumChainWork().GetHex()); + if (chainman.MinimumChainWork() < UintToArith256(chainman.GetConsensus().nMinimumChainWork)) { LogPrintf("Warning: nMinimumChainWork set below default value of %s\n", chainman.GetConsensus().nMinimumChainWork.GetHex()); } if (nPruneTarget == std::numeric_limits::max()) { diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp new file mode 100644 index 000000000000..b0d929626bde --- /dev/null +++ b/src/node/chainstatemanager_args.cpp @@ -0,0 +1,39 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace node { +std::optional ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts) +{ + if (auto value{args.GetBoolArg("-checkblockindex")}) opts.check_block_index = *value; + + if (auto value{args.GetBoolArg("-checkpoints")}) opts.checkpoints_enabled = *value; + + if (auto value{args.GetArg("-minimumchainwork")}) { + if (!IsHexNumber(*value)) { + return strprintf(Untranslated("Invalid non-hex (%s) minimum chain work value specified"), *value); + } + opts.minimum_chain_work = UintToArith256(uint256S(*value)); + } + + if (auto value{args.GetArg("-assumevalid")}) opts.assumed_valid_block = uint256S(*value); + + if (auto value{args.GetIntArg("-maxtipage")}) opts.max_tip_age = std::chrono::seconds{*value}; + + return std::nullopt; +} +} // namespace node diff --git a/src/node/chainstatemanager_args.h b/src/node/chainstatemanager_args.h new file mode 100644 index 000000000000..6c46b998f213 --- /dev/null +++ b/src/node/chainstatemanager_args.h @@ -0,0 +1,19 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_CHAINSTATEMANAGER_ARGS_H +#define BITCOIN_NODE_CHAINSTATEMANAGER_ARGS_H + +#include + +#include + +class ArgsManager; +struct bilingual_str; + +namespace node { +std::optional ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts); +} // namespace node + +#endif // BITCOIN_NODE_CHAINSTATEMANAGER_ARGS_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 7a1d2adb99d7..4fe1a6e8f168 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -215,8 +215,6 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve m_node.connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); // Deterministic randomness for tests. - fCheckBlockIndex = true; - m_node.mn_metaman = std::make_unique(); m_node.netfulfilledman = std::make_unique(); m_node.sporkman = std::make_unique(); @@ -274,6 +272,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, + .check_block_index = true, }; m_node.chainman = std::make_unique(chainman_opts); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(m_cache_sizes.block_tree_db, true); @@ -282,10 +281,8 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve m_node.clhandler = std::make_unique(*m_node.chainlocks, *m_node.chainman, *m_node.mempool, *m_node.mn_sync); - // Start script-checking threads. Set g_parallel_script_checks to true so they are used. constexpr int script_check_threads = 2; StartScriptCheckWorkerThreads(script_check_threads); - g_parallel_script_checks = true; } ChainTestingSetup::~ChainTestingSetup() diff --git a/src/validation.cpp b/src/validation.cpp index bd0599561fd4..48274750aac2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -128,13 +128,6 @@ RecursiveMutex cs_main; GlobalMutex g_best_block_mutex; std::condition_variable g_best_block_cv; uint256 g_best_block; -bool g_parallel_script_checks{false}; -bool fCheckBlockIndex = false; -bool fCheckpointsEnabled = DEFAULT_CHECKPOINTS_ENABLED; -int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE; - -uint256 hashAssumeValid; -arith_uint256 nMinimumChainWork; // Forward declaration to break dependency over node/transaction.h namespace node @@ -1713,10 +1706,12 @@ bool Chainstate::IsInitialBlockDownload() const return true; if (m_chain.Tip() == nullptr) return true; - if (m_chain.Tip()->nChainWork < nMinimumChainWork) + if (m_chain.Tip()->nChainWork < m_chainman.MinimumChainWork()) { return true; - if (m_chain.Tip()->GetBlockTime() < (GetTime() - nMaxTipAge)) + } + if (m_chain.Tip()->Time() < NodeClock::now() - m_chainman.m_options.max_tip_age) { return true; + } LogPrintf("Leaving InitialBlockDownload (latching to false)\n"); m_cached_finished_ibd.store(true, std::memory_order_relaxed); return false; @@ -2205,6 +2200,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, uint256 block_hash{block.GetHash()}; assert(*pindex->phashBlock == block_hash); + const bool parallel_script_checks{scriptcheckqueue.HasThreads()}; assert(m_chain_helper); @@ -2266,17 +2262,17 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, } bool fScriptChecks = true; - if (!hashAssumeValid.IsNull()) { + if (!m_chainman.AssumedValidBlock().IsNull()) { // We've been configured with the hash of a block which has been externally verified to have a valid history. // A suitable default value is included with the software and updated from time to time. Because validity // relative to a piece of software is an objective fact these defaults can be easily reviewed. // This setting doesn't force the selection of any particular chain but makes validating some faster by // effectively caching the result of part of the verification. - BlockMap::const_iterator it = m_blockman.m_block_index.find(hashAssumeValid); + BlockMap::const_iterator it{m_blockman.m_block_index.find(m_chainman.AssumedValidBlock())}; if (it != m_blockman.m_block_index.end()) { if (it->second.GetAncestor(pindex->nHeight) == pindex && m_chainman.m_best_header->GetAncestor(pindex->nHeight) == pindex && - m_chainman.m_best_header->nChainWork >= nMinimumChainWork) { + m_chainman.m_best_header->nChainWork >= m_chainman.MinimumChainWork()) { // This block is a member of the assumed verified chain and an ancestor of the best header. // Script verification is skipped when connecting blocks under the // assumevalid block. Assuming the assumevalid block is valid this @@ -2289,7 +2285,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // it hard to hide the implication of the demand. This also avoids having release candidates // that are hardly doing any signature verification at all in testing without having to // artificially set the default assumed verified block further back. - // The test against nMinimumChainWork prevents the skipping when denied access to any chain at + // The test against the minimum chain work prevents the skipping when denied access to any chain at // least as good as the expected chain. fScriptChecks = (GetBlockProofEquivalentTime(*m_chainman.m_best_header, *pindex, *m_chainman.m_best_header, params.GetConsensus()) <= 60 * 60 * 24 * 7 * 2); } @@ -2372,7 +2368,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // doesn't invalidate pointers into the vector, and keep txsdata in scope // for as long as `control`. std::vector txsdata(block.vtx.size()); - CCheckQueueControl control(fScriptChecks && g_parallel_script_checks ? &scriptcheckqueue : nullptr); + CCheckQueueControl control(fScriptChecks && parallel_script_checks ? &scriptcheckqueue : nullptr); std::vector prevheights; CAmount nFees = 0; @@ -2446,7 +2442,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, std::vector vChecks; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ TxValidationState tx_state; - if (fScriptChecks && !CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], g_parallel_script_checks ? &vChecks : nullptr)) { + if (fScriptChecks && !CheckInputScripts(tx, tx_state, view, flags, fCacheResults, fCacheResults, txsdata[i], parallel_script_checks ? &vChecks : nullptr)) { // Any transaction validation failure in ConnectBlock is a block consensus failure state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), tx_state.GetDebugMessage()); @@ -4176,7 +4172,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio } // Check against checkpoints - if (fCheckpointsEnabled) { + if (chainman.m_options.checkpoints_enabled) { // Don't accept any forks from the main chain prior to last checkpoint. // GetLastCheckpoint finds the last checkpoint in MapCheckpoints that's in our // BlockIndex(). @@ -4497,7 +4493,7 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr& pblock, // If our tip is behind, a peer could try to send us // low-work blocks on a fake chain that we would never // request; don't process these. - if (pindex->nChainWork < nMinimumChainWork) return true; + if (pindex->nChainWork < MinimumChainWork()) return true; } const CChainParams& params{GetParams()}; @@ -5243,7 +5239,7 @@ void ChainstateManager::LoadExternalBlockFile( void ChainstateManager::CheckBlockIndex() { - if (!fCheckBlockIndex) { + if (!ShouldCheckBlockIndex()) { return; } @@ -6395,6 +6391,23 @@ void ChainstateManager::ResetChainstates() m_active_chainstate = nullptr; } +/** + * Apply default chain params to nullopt members. + * This helps to avoid coding errors around the accidental use of the compare + * operators that accept nullopt, thus ignoring the intended default value. + */ +static ChainstateManager::Options&& Flatten(ChainstateManager::Options&& opts) +{ + if (!opts.check_block_index.has_value()) opts.check_block_index = opts.chainparams.DefaultConsistencyChecks(); + if (!opts.minimum_chain_work.has_value()) opts.minimum_chain_work = UintToArith256(opts.chainparams.GetConsensus().nMinimumChainWork); + if (!opts.assumed_valid_block.has_value()) opts.assumed_valid_block = opts.chainparams.GetConsensus().defaultAssumeValid; + return std::move(opts); +} + +ChainstateManager::ChainstateManager(Options options) + : m_options{Flatten(std::move(options))}, + m_blockman{{m_options.chainparams}} {} + ChainstateManager::~ChainstateManager() { LOCK(::cs_main); diff --git a/src/validation.h b/src/validation.h index a180a4aa2f37..4525e41efac8 100644 --- a/src/validation.h +++ b/src/validation.h @@ -84,10 +84,6 @@ static const int DEFAULT_SCRIPTCHECK_THREADS = 0; static const unsigned int MAX_HEADERS_UNCOMPRESSED_RESULT = 2000; static const unsigned int MAX_HEADERS_COMPRESSED_RESULT = 8000; -static const int64_t DEFAULT_MAX_TIP_AGE = 6 * 60 * 60; // ~144 blocks behind -> 2 x fork detection time, was 24 * 60 * 60 in bitcoin - -static const bool DEFAULT_CHECKPOINTS_ENABLED = true; - /** Default for -stopatheight */ static const int DEFAULT_STOPATHEIGHT = 0; /** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of ActiveChain().Tip() will not be pruned. */ @@ -117,20 +113,6 @@ extern GlobalMutex g_best_block_mutex; extern std::condition_variable g_best_block_cv; /** Used to notify getblocktemplate RPC of new tips. */ extern uint256 g_best_block; -/** Whether there are dedicated script-checking threads running. - * False indicates all script checking is done on the main threadMessageHandler thread. - */ -extern bool g_parallel_script_checks; -extern bool fCheckBlockIndex; -extern bool fCheckpointsEnabled; -/** If the tip is older than this (in seconds), the node is considered to be in initial block download. */ -extern int64_t nMaxTipAge; - -/** Block hash whose ancestors we will assume to have valid scripts without checking them. */ -extern uint256 hashAssumeValid; - -/** Minimum work we will assume exists on some valid chain. */ -extern arith_uint256 nMinimumChainWork; /** Documentation for argument 'checklevel'. */ extern const std::vector CHECKLEVEL_DOC; @@ -948,17 +930,18 @@ class ChainstateManager public: using Options = kernel::ChainstateManagerOpts; - explicit ChainstateManager(Options options) - : m_options{std::move(options)}, - m_blockman{{m_options.chainparams}} {} + explicit ChainstateManager(Options options); const CChainParams& GetParams() const { return m_options.chainparams; } const Consensus::Params& GetConsensus() const { return m_options.chainparams.GetConsensus(); } + bool ShouldCheckBlockIndex() const { return *Assert(m_options.check_block_index); } + const arith_uint256& MinimumChainWork() const { return *Assert(m_options.minimum_chain_work); } + const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); } /** * Make various assertions about the state of the block index. * - * By default this only executes fully when using the Regtest chain; see: fCheckBlockIndex. + * By default this only executes fully when using the Regtest chain; see: m_options.check_block_index. */ void CheckBlockIndex(); diff --git a/test/functional/feature_maxtipage.py b/test/functional/feature_maxtipage.py index 4a9da54378c7..696bb41cd8c6 100755 --- a/test/functional/feature_maxtipage.py +++ b/test/functional/feature_maxtipage.py @@ -2,10 +2,10 @@ # Copyright (c) 2022 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Test logic for setting nMaxTipAge on command line. +"""Test logic for setting -maxtipage on command line. Nodes don't consider themselves out of "initial block download" as long as -their best known block header time is more than nMaxTipAge in the past. +their best known block header time is more than -maxtipage in the past. """ import time From ffa31938a8b43675cbbce7d51e9a156f1590f9ca Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 28 Oct 2022 15:19:05 -0400 Subject: [PATCH 7/9] Merge bitcoin/bitcoin#26409: refactor: Silence GCC Wmissing-field-initializers in ChainstateManagerOpts fa29ef00adac6f0842acdd38344820a1ce0e3087 refactor: Silence GCC Wmissing-field-initializers in ChainstateManagerOpts (MacroFake) Pull request description: The `std::optional` fields in the struct that fall back to chain param defaults if not provided should be initialized to `std::nullopt`. This already happens with the current code. However, for consistency with `check_block_index` and to silence a GCC warning, add the "missing" `{}`. ACKs for top commit: achow101: ACK fa29ef00adac6f0842acdd38344820a1ce0e3087 hebasto: ACK fa29ef00adac6f0842acdd38344820a1ce0e3087, tested on Ubuntu 22.04 + GCC 11.3. jonatack: ACK fa29ef00adac6f0842acdd38344820a1ce0e3087 Tree-SHA512: bdec9c56df5d601a5616e107fed48737b13b0a7242b6526092fb682b5016544a4bc08666b60304c668d44c6f7ac69d3788093d921382c1d6c577c1f9fe31fc50 Co-authored-by: Andrew Chow --- src/kernel/chainstatemanager_opts.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index f1074a2cbde3..a353414d8e99 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -29,9 +29,9 @@ struct ChainstateManagerOpts { std::optional check_block_index{}; bool checkpoints_enabled{DEFAULT_CHECKPOINTS_ENABLED}; //! If set, it will override the minimum work we will assume exists on some valid chain. - std::optional minimum_chain_work; + std::optional minimum_chain_work{}; //! If set, it will override the block hash whose ancestors we will assume to have valid scripts without checking them. - std::optional assumed_valid_block; + std::optional assumed_valid_block{}; //! If the tip is older than this, the node is considered to be in initial block download. std::chrono::seconds max_tip_age{DEFAULT_MAX_TIP_AGE}; }; From 02f26d452a72d9c697404f7b65046cd909a6c409 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 4 Aug 2022 16:40:56 +0100 Subject: [PATCH 8/9] Merge bitcoin/bitcoin#25527: [kernel 3c/n] Decouple validation cache initialization from `ArgsManager` 0f3a2532c38074dd9789d1c4c667db6ca46ff0ab validationcaches: Use size_t for sizes (Carl Dong) 41c5201a90bbc2893333e334e8945759ef24e7dd validationcaches: Add and use ValidationCacheSizes (Carl Dong) 82d3058539f54ebad745e2b02b61df01aa832a54 cuckoocache: Check for uint32 overflow in setup_bytes (Carl Dong) b370164b319df1a500b70694b077f92265a777fb validationcaches: Abolish arbitrary limit (Carl Dong) 08dbc6ef72db48168dc03991f5f838dae42c8dfd cuckoocache: Return approximate memory size (Carl Dong) 0dbce4b1034b53d19b88af332385a006098b6d48 tests: Reduce calls to InitS*Cache() (Carl Dong) Pull request description: This is part of the `libbitcoinkernel` project: #24303, https://github.com/bitcoin/bitcoin/projects/18 This PR is **_NOT_** dependent on any other PRs. ----- a.k.a. "Stop calling `gArgs.GetIntArg("-maxsigcachesize")` from validation code" This PR introduces the `ValidationCacheSizes` struct and its corresponding `ApplyArgsManOptions` function, removing the need to call `gArgs` from `Init{Signature,ScriptExecution}Cache()`. This serves to further decouple `ArgsManager` from `libbitcoinkernel` code. More context can be gleaned from the commit messages. ACKs for top commit: glozow: re ACK 0f3a2532c3 theStack: Code-review ACK 0f3a2532c38074dd9789d1c4c667db6ca46ff0ab ryanofsky: Code review ACK 0f3a2532c38074dd9789d1c4c667db6ca46ff0ab. Rebase and comment tweak since last Tree-SHA512: a492ca608466979807cac25ae3d8ef75d2f1345de52a156aa0d222c5a940f79f1b65db40090de69183cccdb12297ec060f6c64e57a26a155a94fec80e07ea0f7 Co-authored-by: glozow --- ci/dash/lint-tidy.sh | 1 + src/Makefile.am | 3 +++ src/bitcoin-chainstate.cpp | 6 +++-- src/cuckoocache.h | 23 ++++++++++++++----- src/init.cpp | 15 +++++++++--- src/kernel/validation_cache_sizes.h | 20 ++++++++++++++++ src/node/validation_cache_args.cpp | 34 ++++++++++++++++++++++++++++ src/node/validation_cache_args.h | 17 ++++++++++++++ src/script/sigcache.cpp | 18 ++++++++------- src/script/sigcache.h | 11 ++++----- src/test/fuzz/script_sigcache.cpp | 10 +++++--- src/test/txvalidationcache_tests.cpp | 5 ---- src/test/util/setup_common.cpp | 12 ++++++++-- src/validation.cpp | 17 ++++++++------ src/validation.h | 2 +- 15 files changed, 151 insertions(+), 43 deletions(-) create mode 100644 src/kernel/validation_cache_sizes.h create mode 100644 src/node/validation_cache_args.cpp create mode 100644 src/node/validation_cache_args.h diff --git a/ci/dash/lint-tidy.sh b/ci/dash/lint-tidy.sh index 09300327eea6..0d2aa9383428 100755 --- a/ci/dash/lint-tidy.sh +++ b/ci/dash/lint-tidy.sh @@ -104,6 +104,7 @@ iwyu_tool.py \ "src/node/chainstate.cpp" \ "src/node/chainstatemanager_args.cpp" \ "src/node/mempool_args.cpp" \ + "src/node/validation_cache_args.cpp" \ "src/node/minisketchwrapper.cpp" \ "src/policy/feerate.cpp" \ "src/policy/packages.cpp" \ diff --git a/src/Makefile.am b/src/Makefile.am index 706ba0778b9f..60fc74caf3c2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -293,6 +293,7 @@ BITCOIN_CORE_H = \ kernel/mempool_limits.h \ kernel/mempool_options.h \ kernel/mempool_persist.h \ + kernel/validation_cache_sizes.h \ key.h \ key_io.h \ limitedmap.h \ @@ -357,6 +358,7 @@ BITCOIN_CORE_H = \ node/transaction.h \ node/txreconciliation.h \ node/utxo_snapshot.h \ + node/validation_cache_args.h \ noui.h \ outputtype.h \ policy/feerate.h \ @@ -629,6 +631,7 @@ libbitcoin_node_a_SOURCES = \ node/transaction.cpp \ node/txreconciliation.cpp \ node/utxo_snapshot.cpp \ + node/validation_cache_args.cpp \ noui.cpp \ policy/fees.cpp \ policy/fees_args.cpp \ diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 1ebd86bae1c5..204c4701f08d 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -72,8 +73,9 @@ int main(int argc, char* argv[]) // Necessary for CheckInputScripts (eventually called by ProcessNewBlock), // which will try the script cache first and fall back to actually // performing the check with the signature cache. - InitSignatureCache(); - InitScriptExecutionCache(); + kernel::ValidationCacheSizes validation_cache_sizes{}; + Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes)); + Assert(InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes)); // SETUP: Scheduling and Background Signals diff --git a/src/cuckoocache.h b/src/cuckoocache.h index 833254b40b36..9adcd7e32499 100644 --- a/src/cuckoocache.h +++ b/src/cuckoocache.h @@ -12,7 +12,9 @@ #include #include #include +#include #include +#include #include #include @@ -325,7 +327,7 @@ class cache } /** setup initializes the container to store no more than new_size - * elements. + * elements and no less than 2 elements. * * setup should only be called once. * @@ -335,8 +337,8 @@ class cache uint32_t setup(uint32_t new_size) { // depth_limit must be at least one otherwise errors can occur. - depth_limit = static_cast(std::log2(static_cast(std::max((uint32_t)2, new_size)))); size = std::max(2, new_size); + depth_limit = static_cast(std::log2(static_cast(size))); table.resize(size); collection_flags.setup(size); epoch_flags.resize(size); @@ -356,12 +358,21 @@ class cache * * @param bytes the approximate number of bytes to use for this data * structure - * @returns the maximum number of elements storable (see setup() - * documentation for more detail) + * @returns A pair of the maximum number of elements storable (see setup() + * documentation for more detail) and the approxmiate total size of these + * elements in bytes or std::nullopt if the size requested is too large. */ - uint32_t setup_bytes(size_t bytes) + std::optional> setup_bytes(size_t bytes) { - return setup(bytes/sizeof(Element)); + size_t requested_num_elems = bytes / sizeof(Element); + if (std::numeric_limits::max() < requested_num_elems) { + return std::nullopt; + } + + auto num_elems = setup(bytes/sizeof(Element)); + + size_t approx_size_bytes = num_elems * sizeof(Element); + return std::make_pair(num_elems, approx_size_bytes); } /** insert loops at most depth_limit times trying to insert a hash diff --git a/src/init.cpp b/src/init.cpp index cae85f3d70e7..46bfb1156341 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include @@ -56,6 +57,7 @@ #include #include #include +#include #include #include #include @@ -156,7 +158,9 @@ using kernel::CoinStatsHashType; using kernel::DumpMempool; +using kernel::ValidationCacheSizes; +using node::ApplyArgsManOptions; using node::CacheSizes; using node::CalculateCacheSizes; using node::DEFAULT_PERSIST_MEMPOOL; @@ -806,7 +810,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-watchquorums=", strprintf("Watch and validate quorum communication (default: %u)", llmq::DEFAULT_WATCH_QUORUMS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-capturemessages", "Capture all P2P messages to disk", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-disablegovernance", strprintf("Disable governance validation (0-1, default: %u)", 0), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-maxsigcachesize=", strprintf("Limit sum of signature cache and script execution cache sizes to MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_SIZE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-maxsigcachesize=", strprintf("Limit sum of signature cache and script execution cache sizes to MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_BYTES >> 20), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-maxtipage=", strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", Ticks(DEFAULT_MAX_TIP_AGE)), @@ -1494,8 +1498,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) args.GetArg("-datadir", ""), fs::PathToString(fs::current_path())); } - InitSignatureCache(); - InitScriptExecutionCache(); + ValidationCacheSizes validation_cache_sizes{}; + ApplyArgsManOptions(args, validation_cache_sizes); + if (!InitSignatureCache(validation_cache_sizes.signature_cache_bytes) + || !InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes)) + { + return InitError(strprintf(_("Unable to allocate memory for -maxsigcachesize: '%s' MiB"), args.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_BYTES >> 20))); + } int script_threads = args.GetIntArg("-par", DEFAULT_SCRIPTCHECK_THREADS); if (script_threads <= 0) { diff --git a/src/kernel/validation_cache_sizes.h b/src/kernel/validation_cache_sizes.h new file mode 100644 index 000000000000..72e4d1a52cc6 --- /dev/null +++ b/src/kernel/validation_cache_sizes.h @@ -0,0 +1,20 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_KERNEL_VALIDATION_CACHE_SIZES_H +#define BITCOIN_KERNEL_VALIDATION_CACHE_SIZES_H + +#include