diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 9ba004cc38..c47d0eae1e 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -28,7 +28,8 @@ 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); bool TestSequenceLocks(const CTransaction& tx, int flags) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs) { - return CheckSequenceLocks(::ChainstateActive(), *m_node.mempool, tx, flags); + CCoinsViewMemPool viewMempool(&m_node.chainman->ActiveChainstate().CoinsTip(), *m_node.mempool); + return CheckSequenceLocks(m_node.chainman->ActiveChain().Tip(), viewMempool, tx, flags); } BlockAssembler AssemblerForTest(const CChainParams& params); }; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 5957637e81..8bd6e2a6a6 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -515,7 +515,9 @@ void CTxMemPool::removeForReorg(CChainState& active_chainstate, int flags) LockPoints lp = it->GetLockPoints(); 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)) { + CCoinsViewMemPool viewMempool(&active_chainstate.CoinsTip(), *this); + if (!CheckFinalTx(active_chainstate.m_chain.Tip(), tx, flags) + || !CheckSequenceLocks(active_chainstate.m_chain.Tip(), viewMempool, 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); diff --git a/src/validation.cpp b/src/validation.cpp index 4f9b8687b7..ac02cc0515 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -245,18 +245,13 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp) return true; } -bool CheckSequenceLocks(CChainState& active_chainstate, - const CTxMemPool& pool, +bool CheckSequenceLocks(CBlockIndex* tip, + const CCoinsView& coins_view, 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 = active_chainstate.m_chain.Tip(); assert(tip != nullptr); CBlockIndex index; @@ -276,14 +271,12 @@ bool CheckSequenceLocks(CChainState& active_chainstate, lockPair.second = lp->time; } else { - // 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++) { const CTxIn& txin = tx.vin[txinIndex]; Coin coin; - if (!viewMemPool.GetCoin(txin.prevout, coin)) { + if (!coins_view.GetCoin(txin.prevout, coin)) { return error("%s: Missing input", __func__); } if (coin.nHeight == MEMPOOL_HEIGHT) { @@ -686,10 +679,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Only accept BIP68 sequence locked 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. - // Must keep pool.cs for this unless we change CheckSequenceLocks to take a - // CoinsViewCache instead of create its own + // Pass in m_view which has all of the relevant inputs cached. Note that, since m_view's + // backend was removed, it no longer pulls coins from the mempool. assert(std::addressof(::ChainstateActive()) == std::addressof(m_active_chainstate)); - if (!CheckSequenceLocks(m_active_chainstate, m_pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) + if (!CheckSequenceLocks(m_active_chainstate.m_chain.Tip(), m_view, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-BIP68-final"); assert(std::addressof(g_chainman.m_blockman) == std::addressof(m_active_chainstate.m_blockman)); diff --git a/src/validation.h b/src/validation.h index 1b50644185..4deaf1830f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -226,9 +226,13 @@ bool CheckFinalTx(const CBlockIndex* active_chain_tip, const CTransaction &tx, i 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. - * - * Simulates calling SequenceLocks() with data from the tip of the current active chain. + * Check if transaction will be BIP68 final in the next block to be created on top of tip. + * @param[in] tip Chain tip to check tx sequence locks against. For example, + * the tip of the current active chain. + * @param[in] coins_view Any CCoinsView that provides access to the relevant coins + * for checking sequence locks. Any CCoinsView can be passed in; + * it is assumed to be consistent with the tip. + * Simulates calling SequenceLocks() with data from the tip passed in. * Optionally stores in LockPoints the resulting height and time calculated and the hash * of the block needed for calculation or skips the calculation and uses the LockPoints * passed in for evaluation. @@ -236,12 +240,12 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp) EXCLUSIVE * * See consensus/consensus.h for flag definitions. */ -bool CheckSequenceLocks(CChainState& active_chainstate, - const CTxMemPool& pool, +bool CheckSequenceLocks(CBlockIndex* tip, + const CCoinsView& coins_view, const CTransaction& tx, int flags, LockPoints* lp = nullptr, - bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs); + bool useExistingLockPoints = false); /** * Closure representing one script verification