Merge #20834: locks and docs in ATMP and CheckInputsFromMempoolAndCache

2f463f57e3 [doc] for CheckInputsFromMempoolAndCache (gzhao408)
85cc6bed64 lock annotations for MemPoolAccept functions (gzhao408)

Pull request description:

  This is a very small PR that adds some lock annotations to clarify that, now, the `pool.cs` lock is held throughout tx validation for mempool.  The comments in `CheckInputsFromMempoolAndCache` were unclear/outdated so I updated those as well.

  ~This PR is a cleanup. It removes unnecessary code that doesn't do much.~

ACKs for top commit:
  sdaftuar:
    utACK 2f463f57e3
  jnewbery:
    utACK 2f463f57e3
  MarcoFalke:
    cr ACK 2f463f57e3
  fanquake:
    ACK 2f463f57e3

Tree-SHA512: 5863a546b00ef02eba8f208c2c04c32f64671f17c967a5e3c51332fc0f472e5e9addddae075d0b91c77caebe1be9331317646b0bec802e86d9550773fd9621a9
This commit is contained in:
fanquake 2021-01-14 23:02:44 +08:00
commit f91587f050
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1

View File

@ -404,36 +404,41 @@ static void UpdateMempoolForReorg(CTxMemPool& mempool, DisconnectedBlockTransact
LimitMempoolSize(mempool, 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, TxValidationState& state, const CCoinsViewCache& view, const CTxMemPool& pool,
unsigned int flags, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
/**
* Checks to avoid mempool polluting consensus critical paths since cached
* signature and script validity results will be reused if we validate this
* transaction again during block validation.
* */
static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, TxValidationState& state,
const CCoinsViewCache& view, const CTxMemPool& pool,
unsigned int flags, PrecomputedTransactionData& txdata)
EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs)
{
AssertLockHeld(cs_main);
// pool.cs should be locked already, but go ahead and re-take the lock here
// to enforce that mempool doesn't change between when we check the view
// and when we actually call through to CheckInputScripts
LOCK(pool.cs);
AssertLockHeld(pool.cs);
assert(!tx.IsCoinBase());
for (const CTxIn& txin : tx.vin) {
const Coin& coin = view.AccessCoin(txin.prevout);
// AcceptToMemoryPoolWorker has already checked that the coins are
// available, so this shouldn't fail. If the inputs are not available
// here then return false.
// This coin was checked in PreChecks and MemPoolAccept
// has been holding cs_main since then.
Assume(!coin.IsSpent());
if (coin.IsSpent()) return false;
// Check equivalence for available inputs.
// If the Coin is available, there are 2 possibilities:
// it is available in our current ChainstateActive UTXO set,
// or it's a UTXO provided by a transaction in our mempool.
// Ensure the scriptPubKeys in Coins from CoinsView are correct.
const CTransactionRef& txFrom = pool.get(txin.prevout.hash);
if (txFrom) {
assert(txFrom->GetHash() == txin.prevout.hash);
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(!coinFromDisk.IsSpent());
assert(coinFromDisk.out == coin.out);
const Coin& coinFromUTXOSet = ::ChainstateActive().CoinsTip().AccessCoin(txin.prevout);
assert(!coinFromUTXOSet.IsSpent());
assert(coinFromUTXOSet.out == coin.out);
}
}
@ -502,13 +507,13 @@ private:
// Run the script checks using our policy flags. As this can be slow, we should
// only invoke this on transactions that have otherwise passed policy checks.
bool PolicyScriptChecks(ATMPArgs& args, const Workspace& ws, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool PolicyScriptChecks(ATMPArgs& args, const Workspace& ws, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);
// Re-run the script checks, using consensus flags, and try to cache the
// result in the scriptcache. This should be done after
// PolicyScriptChecks(). This requires that all inputs either be in our
// utxo set or in the mempool.
bool ConsensusScriptChecks(ATMPArgs& args, const Workspace& ws, PrecomputedTransactionData &txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool ConsensusScriptChecks(ATMPArgs& args, const Workspace& ws, PrecomputedTransactionData &txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);
// Try to add the transaction to the mempool, removing any conflicts first.
// Returns true if the transaction is in the mempool after any size
@ -516,7 +521,7 @@ private:
bool Finalize(ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);
// Compare a package's feerate against minimum allowed.
bool CheckFeeRate(size_t package_size, CAmount package_fee, TxValidationState& state)
bool CheckFeeRate(size_t package_size, CAmount package_fee, TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs)
{
CAmount mempoolRejectFee = m_pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(package_size);
if (mempoolRejectFee > 0 && package_fee < mempoolRejectFee) {