From fa3942fc4c66d7624bd3578d1e7f4a6a7721c11a Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 23 Nov 2021 18:03:11 +0100 Subject: [PATCH] Remove GetSpendHeight It is unclear what the goal of the helper is, as the caller already knows the spend height before calling the helper. Also, in case the coins view is corrupted, LookupBlockIndex will return nullptr. Dereferencing a nullptr is UB. Fix both issues by removing it. Also, add a sanity check, which aborts if the coins view is corrupted. --- src/validation.cpp | 13 ++++--------- src/validation.h | 7 ------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 6ed5e7a548c..0595cd8f4bf 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -723,6 +723,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // to coins_to_uncache) m_view.SetBackend(m_dummy); + assert(m_active_chainstate.m_blockman.LookupBlockIndex(m_view.GetBestBlock()) == m_active_chainstate.m_chain.Tip()); + // 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. @@ -731,7 +733,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) 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"); - if (!Consensus::CheckTxInputs(tx, state, m_view, m_active_chainstate.m_blockman.GetSpendHeight(m_view), ws.m_base_fees)) { + // The mempool holds txs for the next block, so pass height+1 to CheckTxInputs + if (!Consensus::CheckTxInputs(tx, state, m_view, m_active_chainstate.m_chain.Height() + 1, ws.m_base_fees)) { return false; // state filled in by CheckTxInputs } @@ -1311,14 +1314,6 @@ bool CScriptCheck::operator()() { return VerifyScript(scriptSig, m_tx_out.scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, m_tx_out.nValue, cacheStore, *txdata), &error); } -int BlockManager::GetSpendHeight(const CCoinsViewCache& inputs) -{ - AssertLockHeld(cs_main); - CBlockIndex* pindexPrev = LookupBlockIndex(inputs.GetBestBlock()); - return pindexPrev->nHeight + 1; -} - - static CuckooCache::cache g_scriptExecutionCache; static CSHA256 g_scriptExecutionCacheHasher; diff --git a/src/validation.h b/src/validation.h index 881438f37ae..9385dfdab47 100644 --- a/src/validation.h +++ b/src/validation.h @@ -478,13 +478,6 @@ public: //! Returns last CBlockIndex* that is a checkpoint CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - /** - * Return the spend height, which is one more than the inputs.GetBestBlock(). - * While checking, GetBestBlock() refers to the parent block. (protected by cs_main) - * This is also true for mempool checks. - */ - int GetSpendHeight(const CCoinsViewCache& inputs) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - ~BlockManager() { Unload(); }