[validation] make CheckSequenceLocks context-free

Allow CheckSequenceLocks to use heights and coins from any CoinsView and
CBlockIndex provided. This means that CheckSequenceLocks() doesn't need
to hold the mempool lock or cs_main. The caller is responsible for
ensuring the CoinsView and CBlockIndex are consistent before passing
them in. The typical usage is still to create a CCoinsViewMemPool from
the mempool and grab the CBlockIndex from the chainstate tip.
This commit is contained in:
glozow 2021-02-11 09:36:20 -08:00
parent 710c8ba829
commit 42cf8b25df
4 changed files with 21 additions and 21 deletions

View File

@ -28,7 +28,8 @@ struct MinerTestingSetup : public TestingSetup {
void TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& 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);
};

View File

@ -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);

View File

@ -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<int> 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));

View File

@ -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