From b014668e27b496bd6ad30985294f3d6971311910 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 5 Jun 2017 20:46:23 -0400 Subject: [PATCH] Add CheckInputs wrapper CCoinsViewMemPool -> non-consensus-critical This wraps CheckInputs in ATMP's cache-inputs call to check that each scriptPubKey the CCoinsViewCache provides is the one which was committed to by the input's transaction hash. --- src/validation.cpp | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 621077bcba..463c16c11f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -399,6 +399,42 @@ void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool f LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); } +// 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, CTxMemPool& pool, + unsigned int flags, bool cacheSigStore, PrecomputedTransactionData& txdata) { + 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 CheckInputs + LOCK(pool.cs); + + assert(!tx.IsCoinBase()); + for (const CTxIn& txin : tx.vin) { + const Coin& coin = view.AccessCoin(txin.prevout); + + // At this point we haven't actually checked if the coins are all + // available (or shouldn't assume we have, since CheckInputs does). + // So we just return failure if the inputs are not available here, + // and then only have to check equivalence for available inputs. + if (coin.IsSpent()) return false; + + 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 = pcoinsTip->AccessCoin(txin.prevout); + assert(!coinFromDisk.IsSpent()); + assert(coinFromDisk.out == coin.out); + } + } + + return CheckInputs(tx, state, view, true, flags, cacheSigStore, true, txdata); +} + static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, bool fLimitFree, bool* pfMissingInputs, int64_t nAcceptTime, std::list* plTxnReplaced, bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector& coins_to_uncache) @@ -782,7 +818,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // invalid blocks (using TestBlockValidity), however allowing such // transactions into the mempool can be exploited as a DoS attack. unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(chainActive.Tip(), Params().GetConsensus()); - if (!CheckInputs(tx, state, view, true, currentBlockScriptVerifyFlags, true, true, txdata)) + if (!CheckInputsFromMempoolAndCache(tx, state, view, pool, currentBlockScriptVerifyFlags, true, txdata)) { // If we're using promiscuousmempoolflags, we may hit this normally // Check if current block has some flags that scriptVerifyFlags