validation: Consolidate AccessCoins calls in PreChecks

This should introduce no behaviour change, but avoids a few extra vector allocations and coins lookups

Author: sedited

src/validation.cpp

@@ -896,40 +896,43 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
                              strprintf("%s: inputs missing/spent", __func__));
     }
 
-    // The mempool holds txs for the next block, so pass height+1 to CheckTxInputs
-    auto check_tx_inputs_res = m_view.AccessCoins(tx, [&tx, &state, &ws, this](auto&& coins) {
-        return Consensus::CheckTxInputs(tx, state, coins, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees);
-    });
-    if (!check_tx_inputs_res) {
-        return false; // state filled in by CheckTxInputs
-    }
+    int64_t nSigOpsCost = 0;
+    bool fSpendsCoinbase = false;
+    // Wrap access to required coins in their own scope to avoid coin lifetime extension issues
+    auto res = m_view.AccessCoins(tx, [&, this](auto&& coins) {
+        // The mempool holds txs for the next block, so pass height+1 to CheckTxInputs
+        auto check_tx_inputs_res =  Consensus::CheckTxInputs(tx, state, coins, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees);
+        if (!check_tx_inputs_res) {
+            return false; // state filled in by CheckTxInputs
+        }
 
-    if (m_pool.m_opts.require_standard) {
-        state = ValidateInputsStandardness(tx, m_view);
-        if (state.IsInvalid()) {
-            return false;
+        if (m_pool.m_opts.require_standard) {
+            state = ValidateInputsStandardness(tx, m_view);
+            if (state.IsInvalid()) {
+                return false;
+            }
         }
-    }
 
-    // Check for non-standard witnesses.
-    if (tx.HasWitness() && m_pool.m_opts.require_standard && !IsWitnessStandard(tx, m_view)) {
-        return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, "bad-witness-nonstandard");
-    }
+        // Check for non-standard witnesses.
+        if (tx.HasWitness() && m_pool.m_opts.require_standard && !IsWitnessStandard(tx, m_view)) {
+            return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, "bad-witness-nonstandard");
+        }
 
-    int64_t nSigOpsCost = m_view.AccessCoins(tx, [&tx](auto&& coins) {
-        return GetTransactionSigOpCost(tx, coins, STANDARD_SCRIPT_VERIFY_FLAGS);
-    });
+        nSigOpsCost = GetTransactionSigOpCost(tx, coins, STANDARD_SCRIPT_VERIFY_FLAGS);
 
-    // Keep track of transactions that spend a coinbase, which we re-scan
-    // during reorgs to ensure COINBASE_MATURITY is still met.
-    bool fSpendsCoinbase = false;
-    for (const CTxIn &txin : tx.vin) {
-        const Coin &coin = m_view.AccessCoin(txin.prevout);
-        if (coin.IsCoinBase()) {
-            fSpendsCoinbase = true;
-            break;
+        // Keep track of transactions that spend a coinbase, which we re-scan
+        // during reorgs to ensure COINBASE_MATURITY is still met.
+        for (const Coin& coin: coins) {
+            if (coin.IsCoinBase()) {
+                fSpendsCoinbase = true;
+                break;
+            }
         }
-    }
+
+        return true;
+    });
+
+    if (!res) return res;
 
     // Set entry_sequence to 0 when bypass_limits is used; this allows txs from a block
     // reorg to be marked earlier than any child txs that were already in the mempool.