From 30191be0b10ff7ba4e7b926908b4e3f19be2f3f9 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Thu, 23 Mar 2023 07:47:33 +0000 Subject: [PATCH] merge bitcoin#20750: Prune g_chainman usage in mempool-related validation functions --- src/bench/block_assemble.cpp | 2 +- src/coinjoin/coinjoin.cpp | 2 +- src/coinjoin/server.cpp | 4 +- src/interfaces/chain.cpp | 2 +- src/net_processing.cpp | 4 +- src/node/transaction.cpp | 2 +- src/rpc/rawtransaction.cpp | 2 +- src/test/miner_tests.cpp | 12 +-- src/test/txvalidation_tests.cpp | 2 +- src/test/txvalidationcache_tests.cpp | 2 +- src/test/validation_block_tests.cpp | 1 + src/txmempool.cpp | 10 +- src/txmempool.h | 3 +- src/validation.cpp | 138 +++++++++++++++++---------- src/validation.h | 15 ++- 15 files changed, 121 insertions(+), 80 deletions(-) diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 222f57c8c2..1afedc9963 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -41,7 +41,7 @@ static void AssembleBlock(benchmark::Bench& bench) for (const auto& txr : txs) { CValidationState state; - bool ret{::AcceptToMemoryPool(*test_setup.m_node.mempool, state, txr, nullptr /* pfMissingInputs */, false /* bypass_limits */, /* nAbsurdFee */ 0)}; + bool ret{::AcceptToMemoryPool(::ChainstateActive(), *test_setup.m_node.mempool, state, txr, nullptr /* pfMissingInputs */, false /* bypass_limits */, /* nAbsurdFee */ 0)}; assert(ret); } } diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 92acfa664c..c18c72d148 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -352,7 +352,7 @@ bool CCoinJoin::IsCollateralValid(CTxMemPool& mempool, const CTransaction& txCol { LOCK(cs_main); CValidationState validationState; - if (!AcceptToMemoryPool(mempool, validationState, MakeTransactionRef(txCollateral), /*pfMissingInputs=*/nullptr, /*bypass_limits=*/false, /*nAbsurdFee=*/DEFAULT_MAX_RAW_TX_FEE, /*test_accept=*/true)) { + if (!AcceptToMemoryPool(::ChainstateActive(), mempool, validationState, MakeTransactionRef(txCollateral), /*pfMissingInputs=*/nullptr, /*bypass_limits=*/false, /*nAbsurdFee=*/DEFAULT_MAX_RAW_TX_FEE, /*test_accept=*/true)) { LogPrint(BCLog::COINJOIN, "CCoinJoin::IsCollateralValid -- didn't pass AcceptToMemoryPool()\n"); return false; } diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 52dc6fea22..1ebcc36b35 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -322,7 +322,7 @@ void CCoinJoinServer::CommitFinalTransaction() TRY_LOCK(cs_main, lockMain); CValidationState validationState; mempool.PrioritiseTransaction(hashTx, 0.1 * COIN); - if (!lockMain || !AcceptToMemoryPool(mempool, validationState, finalTransaction, nullptr /* pfMissingInputs */, false /* bypass_limits */, DEFAULT_MAX_RAW_TX_FEE /* nAbsurdFee */)) { + if (!lockMain || !AcceptToMemoryPool(::ChainstateActive(), mempool, validationState, finalTransaction, nullptr /* pfMissingInputs */, false /* bypass_limits */, DEFAULT_MAX_RAW_TX_FEE /* nAbsurdFee */)) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- AcceptToMemoryPool() error: Transaction not valid\n"); WITH_LOCK(cs_coinjoin, SetNull()); // not much we can do in this case, just notify clients @@ -455,7 +455,7 @@ void CCoinJoinServer::ConsumeCollateral(const CTransactionRef& txref) const { LOCK(cs_main); CValidationState validationState; - if (!AcceptToMemoryPool(mempool, validationState, txref, nullptr /* pfMissingInputs */, false /* bypass_limits */, 0 /* nAbsurdFee */)) { + if (!AcceptToMemoryPool(::ChainstateActive(), mempool, validationState, txref, nullptr /* pfMissingInputs */, false /* bypass_limits */, 0 /* nAbsurdFee */)) { LogPrint(BCLog::COINJOIN, "%s -- AcceptToMemoryPool failed\n", __func__); } else { connman.RelayTransaction(*txref); diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index c8ae63cb2f..e568448564 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -239,7 +239,7 @@ public: bool checkFinalTx(const CTransaction& tx) override { LOCK(cs_main); - return CheckFinalTx(tx); + return CheckFinalTx(::ChainActive().Tip(), tx); } bool findBlock(const uint256& hash, CBlock* block, int64_t* time, int64_t* time_max) override { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f8223a41d5..5b9882a6dc 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2272,7 +2272,7 @@ void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::set& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs); bool TestSequenceLocks(const CTransaction& tx, int flags) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs) { - return CheckSequenceLocks(*m_node.mempool, tx, flags); + return CheckSequenceLocks(::ChainstateActive(), *m_node.mempool, tx, flags); } BlockAssembler AssemblerForTest(const CChainParams& params); }; @@ -456,7 +456,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.nLockTime = 0; hash = tx.GetHash(); m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - BOOST_CHECK(CheckFinalTx(CTransaction(tx), flags)); // Locktime passes + BOOST_CHECK(CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime passes BOOST_CHECK(!TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks fail BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, prevheights, CreateBlockIndex(::ChainActive().Tip()->nHeight + 2))); // Sequence locks pass on 2nd block @@ -466,7 +466,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) prevheights[0] = baseheight + 2; hash = tx.GetHash(); m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx)); - BOOST_CHECK(CheckFinalTx(CTransaction(tx), flags)); // Locktime passes + BOOST_CHECK(CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime passes BOOST_CHECK(!TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks fail for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++) @@ -482,7 +482,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.nLockTime = ::ChainActive().Tip()->nHeight + 1; hash = tx.GetHash(); m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx)); - BOOST_CHECK(!CheckFinalTx(CTransaction(tx), flags)); // Locktime fails + BOOST_CHECK(!CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime fails BOOST_CHECK(TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks pass BOOST_CHECK(IsFinalTx(CTransaction(tx), ::ChainActive().Tip()->nHeight + 2, ::ChainActive().Tip()->GetMedianTimePast())); // Locktime passes on 2nd block @@ -493,7 +493,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) prevheights[0] = baseheight + 4; hash = tx.GetHash(); m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx)); - BOOST_CHECK(!CheckFinalTx(CTransaction(tx), flags)); // Locktime fails + BOOST_CHECK(!CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime fails BOOST_CHECK(TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks pass BOOST_CHECK(IsFinalTx(CTransaction(tx), ::ChainActive().Tip()->nHeight + 2, ::ChainActive().Tip()->GetMedianTimePast() + 1)); // Locktime passes 1 second later @@ -502,7 +502,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) prevheights[0] = ::ChainActive().Tip()->nHeight + 1; tx.nLockTime = 0; tx.vin[0].nSequence = 0; - BOOST_CHECK(CheckFinalTx(CTransaction(tx), flags)); // Locktime passes + BOOST_CHECK(CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime passes BOOST_CHECK(TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks pass tx.vin[0].nSequence = 1; BOOST_CHECK(!TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks fail diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 7c597fe2b6..fdb2b4a8c0 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -38,7 +38,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup) BOOST_CHECK_EQUAL( false, - AcceptToMemoryPool(*m_node.mempool, state, MakeTransactionRef(coinbaseTx), + AcceptToMemoryPool(::ChainstateActive(), *m_node.mempool, state, MakeTransactionRef(coinbaseTx), nullptr /* pfMissingInputs */, true /* bypass_limits */, 0 /* nAbsurdFee */)); diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 3c07470663..39f74e4acf 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -28,7 +28,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) LOCK(cs_main); CValidationState state; - return AcceptToMemoryPool(*m_node.mempool, state, MakeTransactionRef(tx), nullptr /* pfMissingInputs */, + return AcceptToMemoryPool(::ChainstateActive(), *m_node.mempool, state, MakeTransactionRef(tx), nullptr /* pfMissingInputs */, true /* bypass_limits */, 0 /* nAbsurdFee */); }; diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 01be24a24a..b6cd6db433 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -299,6 +299,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) CValidationState state; for (const auto& tx : txs) { BOOST_REQUIRE(AcceptToMemoryPool( + ::ChainstateActive(), *m_node.mempool, state, tx, diff --git a/src/txmempool.cpp b/src/txmempool.cpp index fd4fc5bb5c..a9a202991e 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -752,7 +752,7 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso RemoveStaged(setAllRemoves, false, reason); } -void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +void CTxMemPool::removeForReorg(CChainState& active_chainstate, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { // Remove transactions spending a coinbase which are now immature and no-longer-final transactions AssertLockHeld(cs); @@ -760,8 +760,9 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { const CTransaction& tx = it->GetTx(); LockPoints lp = it->GetLockPoints(); - bool validLP = TestLockPointValidity(&lp); - if (!CheckFinalTx(tx, flags) || !CheckSequenceLocks(*this, tx, flags, &lp, validLP)) { + assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); + bool validLP = TestLockPointValidity(active_chainstate.m_chain, &lp); + if (!CheckFinalTx(active_chainstate.m_chain.Tip(), tx, flags) || !CheckSequenceLocks(active_chainstate, *this, tx, flags, &lp, validLP)) { // Note if CheckSequenceLocks fails the LockPoints may still be invalid // So it's critical that we remove the tx and not depend on the LockPoints. txToRemove.insert(it); @@ -770,8 +771,9 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); if (it2 != mapTx.end()) continue; - const Coin &coin = pcoins->AccessCoin(txin.prevout); + const Coin &coin = active_chainstate.CoinsTip().AccessCoin(txin.prevout); if (m_check_ratio != 0) assert(!coin.IsSpent()); + unsigned int nMemPoolHeight = active_chainstate.m_chain.Tip()->nHeight + 1; if (coin.IsSpent() || (coin.IsCoinBase() && ((signed long)nMemPoolHeight) - coin.nHeight < COINBASE_MATURITY)) { txToRemove.insert(it); break; diff --git a/src/txmempool.h b/src/txmempool.h index 87efdeab93..e093a6733c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -36,6 +36,7 @@ class CBLSPublicKey; class CBlockIndex; +class CChainState; extern CCriticalSection cs_main; /** Fake height value used in Coin to signify they are only in the memory pool (since 0.8) */ @@ -618,7 +619,7 @@ public: bool removeSpentIndex(const uint256 txhash); void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); - void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); + void removeForReorg(CChainState& active_chainstate, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeProTxPubKeyConflicts(const CTransaction &tx, const CKeyID &keyId) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeProTxPubKeyConflicts(const CTransaction &tx, const CBLSPublicKey &pubKey) EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/validation.cpp b/src/validation.cpp index 968018a23c..f3a590bd7c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -205,9 +205,10 @@ static FILE* OpenUndoFile(const FlatFilePos &pos, bool fReadOnly = false); static FlatFileSeq BlockFileSeq(); static FlatFileSeq UndoFileSeq(); -bool CheckFinalTx(const CTransaction &tx, int flags) +bool CheckFinalTx(const CBlockIndex* active_chain_tip, const CTransaction &tx, int flags) { AssertLockHeld(cs_main); + assert(std::addressof(*::ChainActive().Tip()) == std::addressof(*active_chain_tip)); // By convention a negative value for flags indicates that the // current network-enforced consensus rules should be used. In @@ -223,7 +224,7 @@ bool CheckFinalTx(const CTransaction &tx, int flags) // evaluated is what is used. Thus if we want to know if a // transaction can be part of the *next* block, we need to call // IsFinalTx() with one more than ::ChainActive().Height(). - const int nBlockHeight = ::ChainActive().Height() + 1; + const int nBlockHeight = active_chain_tip->nHeight + 1; // BIP113 requires that time-locked transactions have nLockTime set to // less than the median time of the previous block they're contained in. @@ -231,13 +232,13 @@ bool CheckFinalTx(const CTransaction &tx, int flags) // chain tip, so we use that to calculate the median time passed to // IsFinalTx() if LOCKTIME_MEDIAN_TIME_PAST is set. const int64_t nBlockTime = (flags & LOCKTIME_MEDIAN_TIME_PAST) - ? ::ChainActive().Tip()->GetMedianTimePast() + ? active_chain_tip->GetMedianTimePast() : GetAdjustedTime(); return IsFinalTx(tx, nBlockHeight, nBlockTime); } -bool TestLockPointValidity(const LockPoints* lp) +bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp) { AssertLockHeld(cs_main); assert(lp); @@ -246,7 +247,8 @@ bool TestLockPointValidity(const LockPoints* lp) if (lp->maxInputBlock) { // Check whether ::ChainActive() is an extension of the block at which the LockPoints // calculation was valid. If not LockPoints are no longer valid - if (!::ChainActive().Contains(lp->maxInputBlock)) { + assert(std::addressof(::ChainActive()) == std::addressof(active_chain)); + if (!active_chain.Contains(lp->maxInputBlock)) { return false; } } @@ -255,22 +257,28 @@ bool TestLockPointValidity(const LockPoints* lp) return true; } -bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flags, LockPoints* lp, bool useExistingLockPoints) +bool CheckSequenceLocks(CChainState& active_chainstate, + const CTxMemPool& pool, + const CTransaction& tx, + int flags, + LockPoints* lp, + bool useExistingLockPoints) { AssertLockHeld(cs_main); AssertLockHeld(pool.cs); + assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); - CBlockIndex* tip = ::ChainActive().Tip(); + CBlockIndex* tip = active_chainstate.m_chain.Tip(); assert(tip != nullptr); CBlockIndex index; index.pprev = tip; - // CheckSequenceLocks() uses ::ChainActive().Height()+1 to evaluate + // CheckSequenceLocks() uses active_chainstate.m_chain.Height()+1 to evaluate // height based locks because when SequenceLocks() is called within // ConnectBlock(), the height of the block *being* // evaluated is what is used. // Thus if we want to know if a transaction can be part of the - // *next* block, we need to use one more than ::ChainActive().Height() + // *next* block, we need to use one more than active_chainstate.m_chain.Height() index.nHeight = tip->nHeight + 1; std::pair lockPair; @@ -280,8 +288,8 @@ bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flag lockPair.second = lp->time; } else { - // CoinsTip() contains the UTXO set for ::ChainActive().Tip() - CCoinsViewMemPool viewMemPool(&::ChainstateActive().CoinsTip(), pool); + // CoinsTip() contains the UTXO set for active_chainstate.m_chain.Tip() + CCoinsViewMemPool viewMemPool(&active_chainstate.CoinsTip(), pool); std::vector prevheights; prevheights.resize(tx.vin.size()); for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) { @@ -389,7 +397,8 @@ bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, // Returns the script flags which should be checked for a given block static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams); -static void LimitMempoolSize(CTxMemPool& pool, size_t limit, std::chrono::seconds age) EXCLUSIVE_LOCKS_REQUIRED(pool.cs, ::cs_main) +static void LimitMempoolSize(CTxMemPool& pool, CCoinsViewCache& coins_cache, size_t limit, std::chrono::seconds age) + EXCLUSIVE_LOCKS_REQUIRED(pool.cs, ::cs_main) { int expired = pool.Expire(GetTime() - age); if (expired != 0) { @@ -398,18 +407,20 @@ static void LimitMempoolSize(CTxMemPool& pool, size_t limit, std::chrono::second std::vector vNoSpendsRemaining; pool.TrimToSize(limit, &vNoSpendsRemaining); + assert(std::addressof(::ChainstateActive().CoinsTip()) == std::addressof(coins_cache)); for (const COutPoint& removed : vNoSpendsRemaining) - ::ChainstateActive().CoinsTip().Uncache(removed); + coins_cache.Uncache(removed); } -static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static bool IsCurrentForFeeEstimation(CChainState& active_chainstate) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); - if (::ChainstateActive().IsInitialBlockDownload()) + assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); + if (active_chainstate.IsInitialBlockDownload()) return false; - if (::ChainActive().Tip()->GetBlockTime() < count_seconds(GetTime() - MAX_FEE_ESTIMATION_TIP_AGE)) + if (active_chainstate.m_chain.Tip()->GetBlockTime() < count_seconds(GetTime() - MAX_FEE_ESTIMATION_TIP_AGE)) return false; - if (::ChainActive().Height() < pindexBestHeader->nHeight - 1) + if (active_chainstate.m_chain.Height() < pindexBestHeader->nHeight - 1) return false; return true; } @@ -427,10 +438,11 @@ static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main) * and instead just erase from the mempool as needed. */ -static void UpdateMempoolForReorg(CTxMemPool& mempool, DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, mempool.cs) +static void UpdateMempoolForReorg(CChainState& active_chainstate, CTxMemPool& mempool, DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, mempool.cs) { AssertLockHeld(cs_main); AssertLockHeld(mempool.cs); + assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); std::vector vHashUpdate; // disconnectpool's insertion_order index sorts the entries from // oldest to newest, but the oldest entry will be the last tx from the @@ -443,7 +455,7 @@ static void UpdateMempoolForReorg(CTxMemPool& mempool, DisconnectedBlockTransact // ignore validation errors in resurrected transactions CValidationState stateDummy; if (!fAddToMempool || (*it)->IsCoinBase() || - !AcceptToMemoryPool(mempool, stateDummy, *it, nullptr /* pfMissingInputs */, + !AcceptToMemoryPool(active_chainstate, mempool, stateDummy, *it, nullptr /* pfMissingInputs */, true /* bypass_limits */, 0 /* nAbsurdFee */)) { // If the transaction doesn't make it in to the mempool, remove any // transactions that depend on it (which would now be orphans). @@ -462,16 +474,18 @@ static void UpdateMempoolForReorg(CTxMemPool& mempool, DisconnectedBlockTransact mempool.UpdateTransactionsFromBlock(vHashUpdate); // We also need to remove any now-immature transactions - mempool.removeForReorg(&::ChainstateActive().CoinsTip(), ::ChainActive().Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); - + mempool.removeForReorg(active_chainstate, STANDARD_LOCKTIME_VERIFY_FLAGS); // Re-limit mempool size, in case we added any transactions - LimitMempoolSize(mempool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); + LimitMempoolSize(mempool, active_chainstate.CoinsTip(), gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); } // Used to avoid mempool polluting consensus critical paths if CCoinsViewMempool // were somehow broken and returning the wrong scriptPubKeys -static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, const CTxMemPool& pool, - unsigned int flags, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { +static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationState& state, + const CCoinsViewCache& view, const CTxMemPool& pool, + unsigned int flags, PrecomputedTransactionData& txdata, CCoinsViewCache& coins_tip) + EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ AssertLockHeld(cs_main); // pool.cs should be locked already, but go ahead and re-take the lock here @@ -495,7 +509,8 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationSt assert(txFrom->vout.size() > txin.prevout.n); assert(txFrom->vout[txin.prevout.n] == coin.out); } else { - const Coin& coinFromDisk = ::ChainstateActive().CoinsTip().AccessCoin(txin.prevout); + assert(std::addressof(::ChainstateActive().CoinsTip()) == std::addressof(coins_tip)); + const Coin& coinFromDisk = coins_tip.AccessCoin(txin.prevout); assert(!coinFromDisk.IsSpent()); assert(coinFromDisk.out == coin.out); } @@ -510,11 +525,13 @@ namespace { class MemPoolAccept { public: - MemPoolAccept(CTxMemPool& mempool) : m_pool(mempool), m_view(&m_dummy), m_viewmempool(&::ChainstateActive().CoinsTip(), m_pool), + explicit MemPoolAccept(CTxMemPool& mempool, CChainState& active_chainstate) : m_pool(mempool), m_view(&m_dummy), m_viewmempool(&active_chainstate.CoinsTip(), m_pool), m_active_chainstate(active_chainstate), m_limit_ancestors(gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT)), m_limit_ancestor_size(gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000), m_limit_descendants(gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT)), - m_limit_descendant_size(gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000) {} + m_limit_descendant_size(gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000) { + assert(std::addressof(::ChainstateActive()) == std::addressof(m_active_chainstate)); + } // We put the arguments we're handed into a struct, so we can pass them // around easier. @@ -593,6 +610,7 @@ private: CCoinsViewCache m_view; CCoinsViewMemPool m_viewmempool; CCoinsView m_dummy; + CChainState& m_active_chainstate; // The package limits in effect at the time of invocation. const size_t m_limit_ancestors; @@ -630,7 +648,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) if (!CheckTransaction(tx, state)) return false; // state filled in by CheckTransaction - if (!ContextualCheckTransaction(tx, state, chainparams.GetConsensus(), ::ChainActive().Tip())) + assert(std::addressof(::ChainstateActive()) == std::addressof(m_active_chainstate)); + if (!ContextualCheckTransaction(tx, state, chainparams.GetConsensus(), m_active_chainstate.m_chain.Tip())) return error("%s: ContextualCheckTransaction: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); if (tx.nVersion == 3 && tx.nType == TRANSACTION_QUORUM_COMMITMENT) { @@ -657,7 +676,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Only accept nLockTime-using transactions that can be mined in the next // block; we don't want our mempool filled up with transactions that can't // be mined yet. - if (!CheckFinalTx(tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) + if (!CheckFinalTx(m_active_chainstate.m_chain.Tip(), tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-final"); // is it already in the memory pool? @@ -695,7 +714,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) LockPoints lp; m_view.SetBackend(m_viewmempool); - CCoinsViewCache& coins_cache = ::ChainstateActive().CoinsTip(); + assert(std::addressof(::ChainstateActive().CoinsTip()) == std::addressof(m_active_chainstate.CoinsTip())); + CCoinsViewCache& coins_cache = m_active_chainstate.CoinsTip(); // do all inputs exist? for (const CTxIn& txin : tx.vin) { if (!coins_cache.HaveCoinInCache(txin.prevout)) { @@ -734,11 +754,13 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // be mined yet. // Must keep pool.cs for this unless we change CheckSequenceLocks to take a // CoinsViewCache instead of create its own - if (!CheckSequenceLocks(m_pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) + assert(std::addressof(::ChainstateActive()) == std::addressof(m_active_chainstate)); + if (!CheckSequenceLocks(m_active_chainstate, m_pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) return state.Invalid(ValidationInvalidReason::TX_PREMATURE_SPEND, false, REJECT_NONSTANDARD, "non-BIP68-final"); + assert(std::addressof(g_chainman.m_blockman) == std::addressof(m_active_chainstate.m_blockman)); CAmount nFees = 0; - if (!Consensus::CheckTxInputs(tx, state, m_view, g_chainman.m_blockman.GetSpendHeight(m_view), nFees)) { + if (!Consensus::CheckTxInputs(tx, state, m_view, m_active_chainstate.m_blockman.GetSpendHeight(m_view), nFees)) { return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); } @@ -763,7 +785,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } - entry.reset(new CTxMemPoolEntry(ptx, nFees, nAcceptTime, ::ChainActive().Height(), + assert(std::addressof(::ChainActive()) == std::addressof(m_active_chainstate.m_chain)); + entry.reset(new CTxMemPoolEntry(ptx, nFees, nAcceptTime, m_active_chainstate.m_chain.Height(), fSpendsCoinbase, nSigOps, lp)); unsigned int nSize = entry->GetTxSize(); @@ -806,7 +829,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // DoS scoring a node for non-critical errors, e.g. duplicate keys because a TX is received that was already // mined // NOTE: we use UTXO here and do NOT allow mempool txes as masternode collaterals - if (!CheckSpecialTx(tx, ::ChainActive().Tip(), state, ::ChainstateActive().CoinsTip(), true)) + if (!CheckSpecialTx(tx, m_active_chainstate.m_chain.Tip(), state, m_active_chainstate.CoinsTip(), true)) return false; if (m_pool.existsProviderTxConflict(tx)) { @@ -857,8 +880,10 @@ bool MemPoolAccept::ConsensusScriptChecks(ATMPArgs& args, Workspace& ws, Precomp // There is a similar check in CreateNewBlock() to prevent creating // invalid blocks (using TestBlockValidity), however allowing such // transactions into the mempool can be exploited as a DoS attack. - unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(::ChainActive().Tip(), chainparams.GetConsensus()); - if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, txdata)) { + assert(std::addressof(::ChainActive()) == std::addressof(m_active_chainstate.m_chain)); + unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(m_active_chainstate.m_chain.Tip(), chainparams.GetConsensus()); + assert(std::addressof(::ChainstateActive().CoinsTip()) == std::addressof(m_active_chainstate.CoinsTip())); + if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, txdata, m_active_chainstate.CoinsTip())) { return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed against latest-block but not STANDARD flags %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); } @@ -881,7 +906,8 @@ bool MemPoolAccept::Finalize(ATMPArgs& args, Workspace& ws) // - the node is not behind // - the transaction is not dependent on any other transactions in the mempool // - the transaction is not a zero fee transaction - bool validForFeeEstimation = (nModifiedFees != 0) && !bypass_limits && IsCurrentForFeeEstimation() && m_pool.HasNoInputsOf(tx); + assert(std::addressof(::ChainstateActive()) == std::addressof(m_active_chainstate)); + bool validForFeeEstimation = (nModifiedFees != 0) && !bypass_limits && IsCurrentForFeeEstimation(m_active_chainstate) && m_pool.HasNoInputsOf(tx); // Store transaction in memory m_pool.addUnchecked(*entry, setAncestors, validForFeeEstimation); @@ -905,7 +931,8 @@ bool MemPoolAccept::Finalize(ATMPArgs& args, Workspace& ws) } if (!bypass_limits) { - LimitMempoolSize(m_pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); + assert(std::addressof(::ChainstateActive().CoinsTip()) == std::addressof(m_active_chainstate.CoinsTip())); + LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip(), gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, std::chrono::hours{gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY)}); if (!m_pool.exists(hash)) return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool full"); } @@ -954,13 +981,15 @@ bool MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs } // anon namespace /** (try to) add transaction to memory pool with a specified acceptance time **/ -static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, +static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CChainState& active_chainstate, CValidationState &state, const CTransactionRef &tx, bool* pfMissingInputs, int64_t nAcceptTime, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::vector coins_to_uncache; MemPoolAccept::ATMPArgs args { chainparams, state, pfMissingInputs, nAcceptTime, bypass_limits, nAbsurdFee, coins_to_uncache, test_accept }; - bool res = MemPoolAccept(pool).AcceptSingleTransaction(tx, args); + + assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); + bool res = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args); if (!res || test_accept) { if (!res) LogPrint(BCLog::MEMPOOL, "%s: %s %s (%s)\n", __func__, tx->GetHash().ToString(), state.GetRejectReason(), state.GetDebugMessage()); @@ -970,19 +999,20 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo // (`CCoinsViewCache::cacheCoins`). for (const COutPoint& hashTx : coins_to_uncache) - ::ChainstateActive().CoinsTip().Uncache(hashTx); + active_chainstate.CoinsTip().Uncache(hashTx); } // After we've (potentially) uncached entries, ensure our coins cache is still within its size limits CValidationState stateDummy; - ::ChainstateActive().FlushStateToDisk(chainparams, stateDummy, FlushStateMode::PERIODIC); + active_chainstate.FlushStateToDisk(chainparams, stateDummy, FlushStateMode::PERIODIC); return res; } -bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, +bool AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool* pfMissingInputs, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept) { const CChainParams& chainparams = Params(); - return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, pfMissingInputs, GetTime(), bypass_limits, nAbsurdFee, test_accept); + assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); + return AcceptToMemoryPoolWithTime(chainparams, pool, active_chainstate, state, tx, pfMissingInputs, GetTime(), bypass_limits, nAbsurdFee, test_accept); } bool GetTimestampIndex(const unsigned int &high, const unsigned int &low, std::vector &hashes) @@ -3038,7 +3068,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar if (!DisconnectTip(state, chainparams, &disconnectpool)) { // This is likely a fatal error, but keep the mempool consistent, // just in case. Only remove from the mempool in this case. - UpdateMempoolForReorg(m_mempool, disconnectpool, false); + UpdateMempoolForReorg(::ChainstateActive(), m_mempool, disconnectpool, false); // If we're unable to disconnect a block during normal operation, // then that is a failure of our local system -- we should abort @@ -3082,7 +3112,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar // A system error occurred (disk space, database error, ...). // Make the mempool consistent with the current tip, just in case // any observers try to use it before shutdown. - UpdateMempoolForReorg(m_mempool, disconnectpool, false); + UpdateMempoolForReorg(::ChainstateActive(), m_mempool, disconnectpool, false); return false; } } else { @@ -3099,7 +3129,7 @@ bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainPar if (fBlocksDisconnected) { // If any blocks were disconnected, disconnectpool may be non empty. Add // any disconnected transactions back to the mempool. - UpdateMempoolForReorg(m_mempool, disconnectpool, true); + UpdateMempoolForReorg(::ChainstateActive(), m_mempool, disconnectpool, true); } m_mempool.check(&CoinsTip()); @@ -3347,7 +3377,7 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c // transactions back to the mempool if disconnecting was successful, // and we're not doing a very deep invalidation (in which case // keeping the mempool up to date is probably futile anyway). - UpdateMempoolForReorg(m_mempool, disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret); + UpdateMempoolForReorg(::ChainstateActive(), m_mempool, disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret); if (!ret) return false; assert(invalid_walk_tip->pprev == m_chain.Tip()); @@ -3493,7 +3523,7 @@ bool CChainState::MarkConflictingBlock(CValidationState& state, const CChainPara if (!DisconnectTip(state, chainparams, &disconnectpool)) { // It's probably hopeless to try to make the mempool consistent // here if DisconnectTip failed, but we can try. - UpdateMempoolForReorg(m_mempool, disconnectpool, false); + UpdateMempoolForReorg(::ChainstateActive(), m_mempool, disconnectpool, false); return false; } if (pindexOldTip == pindexBestHeader) { @@ -3515,7 +3545,7 @@ bool CChainState::MarkConflictingBlock(CValidationState& state, const CChainPara // DisconnectTip will add transactions to disconnectpool; try to add these // back to the mempool. - UpdateMempoolForReorg(m_mempool, disconnectpool, true); + UpdateMempoolForReorg(::ChainstateActive(), m_mempool, disconnectpool, true); } // m_mempool.cs // The resulting new best tip may not be in setBlockIndexCandidates anymore, so @@ -4662,7 +4692,8 @@ bool static LoadBlockIndexDB(ChainstateManager& chainman, const CChainParams& ch void CChainState::LoadMempool(const ArgsManager& args) { if (args.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - ::LoadMempool(m_mempool); + assert(std::addressof(::ChainstateActive()) == std::addressof(*this)); + ::LoadMempool(m_mempool, *this); } m_mempool.SetIsLoaded(!ShutdownRequested()); } @@ -5411,7 +5442,7 @@ int VersionBitsTipStateSinceHeight(const Consensus::Params& params, Consensus::D static const uint64_t MEMPOOL_DUMP_VERSION = 1; -bool LoadMempool(CTxMemPool& pool) +bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate) { const CChainParams& chainparams = Params(); int64_t nExpiryTimeout = gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60; @@ -5453,7 +5484,8 @@ bool LoadMempool(CTxMemPool& pool) CValidationState state; if (nTime + nExpiryTimeout > nNow) { LOCK(cs_main); - AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, nullptr /* pfMissingInputs */, nTime, + assert(std::addressof(::ChainstateActive()) == std::addressof(active_chainstate)); + AcceptToMemoryPoolWithTime(chainparams, pool, active_chainstate, state, tx, nullptr /* pfMissingInputs */, nTime, false /* bypass_limits */, 0 /* nAbsurdFee */, false /* test_accept */); if (state.IsValid()) { ++count; diff --git a/src/validation.h b/src/validation.h index f6cb13cce0..6e79d4a829 100644 --- a/src/validation.h +++ b/src/validation.h @@ -217,7 +217,7 @@ void UnlinkPrunedFiles(const std::set& setFilesToPrune); void PruneBlockFilesManual(int nManualPruneHeight); /** (try to) add transaction to memory pool */ -bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, +bool AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool* pfMissingInputs, bool bypass_limits, const CAmount nAbsurdFee, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -246,12 +246,12 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight); * * See consensus/consensus.h for flag definitions. */ -bool CheckFinalTx(const CTransaction &tx, int flags = -1) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +bool CheckFinalTx(const CBlockIndex* active_chain_tip, const CTransaction &tx, int flags = -1) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** * Test whether the LockPoints height and time are still valid on the current chain */ -bool TestLockPointValidity(const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** * Check if transaction will be BIP 68 final in the next block to be created. @@ -264,7 +264,12 @@ bool TestLockPointValidity(const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_mai * * See consensus/consensus.h for flag definitions. */ -bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +bool CheckSequenceLocks(CChainState& active_chainstate, + const CTxMemPool& pool, + const CTransaction& tx, + int flags, + LockPoints* lp = nullptr, + bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** * Closure representing one script verification @@ -1046,7 +1051,7 @@ CBlockFileInfo* GetBlockFileInfo(size_t n); bool DumpMempool(const CTxMemPool& pool); /** Load the mempool from disk. */ -bool LoadMempool(CTxMemPool& pool); +bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate); //! Check whether the block associated with this index entry is pruned or not. inline bool IsBlockPruned(const CBlockIndex* pblockindex)