From 812ef53685b76eb7b2f72e5aa0c09a89f1d3ded6 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Fri, 12 Aug 2022 08:31:41 +0200 Subject: [PATCH] Merge bitcoin/bitcoin#25677: refactor: make active_chain_tip a reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 9376a6dae41022874df3b9302667796a9fb7b81d refactor: make active_chain_tip a reference (Aurèle Oulès) Pull request description: This PR fixes a TODO introduced in #21055. Makes `active_chain_tip` argument in `CheckFinalTxAtTip` function a reference instead of a pointer. ACKs for top commit: dongcarl: ACK 9376a6dae41022874df3b9302667796a9fb7b81d Tree-SHA512: c36d1769e0b9598b7f79334704b26b73e958d54caa3bd7e4eff954f3964fcf3f5e3a44a5a760497afad51b76e1614c86314fe035e4083c855e3574a620de7f4d --- src/node/interfaces.cpp | 2 +- src/test/miner_tests.cpp | 10 +++++----- src/validation.cpp | 11 +++++------ src/validation.h | 2 +- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index e1418aa442..f15026cf7b 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -767,7 +767,7 @@ public: bool checkFinalTx(const CTransaction& tx) override { LOCK(cs_main); - return CheckFinalTxAtTip(m_node.chainman->ActiveChain().Tip(), tx); + return CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), tx); } bool isInstantSendLockedTx(const uint256& hash) override { diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 76017966b0..7d2cce7a1b 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -448,7 +448,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.nLockTime = 0; hash = tx.GetHash(); m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); - BOOST_CHECK(CheckFinalTxAtTip(m_node.chainman->ActiveChain().Tip(), CTransaction{tx})); // Locktime passes + BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail { @@ -462,7 +462,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) prevheights[0] = baseheight + 2; hash = tx.GetHash(); m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx)); - BOOST_CHECK(CheckFinalTxAtTip(m_node.chainman->ActiveChain().Tip(), CTransaction{tx})); // Locktime passes + BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++) @@ -483,7 +483,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.nLockTime = m_node.chainman->ActiveChain().Tip()->nHeight + 1; hash = tx.GetHash(); m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx)); - BOOST_CHECK(!CheckFinalTxAtTip(m_node.chainman->ActiveChain().Tip(), CTransaction{tx})); // Locktime fails + BOOST_CHECK(!CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime fails BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass BOOST_CHECK(IsFinalTx(CTransaction(tx), m_node.chainman->ActiveChain().Tip()->nHeight + 2, m_node.chainman->ActiveChain().Tip()->GetMedianTimePast())); // Locktime passes on 2nd block @@ -494,7 +494,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) prevheights[0] = baseheight + 4; hash = tx.GetHash(); m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx)); - BOOST_CHECK(!CheckFinalTxAtTip(m_node.chainman->ActiveChain().Tip(), CTransaction{tx})); // Locktime fails + BOOST_CHECK(!CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime fails BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass BOOST_CHECK(IsFinalTx(CTransaction(tx), m_node.chainman->ActiveChain().Tip()->nHeight + 2, m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1)); // Locktime passes 1 second later @@ -503,7 +503,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) prevheights[0] = m_node.chainman->ActiveChain().Tip()->nHeight + 1; tx.nLockTime = 0; tx.vin[0].nSequence = 0; - BOOST_CHECK(CheckFinalTxAtTip(m_node.chainman->ActiveChain().Tip(), CTransaction{tx})); // Locktime passes + BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes BOOST_CHECK(TestSequenceLocks(CTransaction{tx})); // Sequence locks pass tx.vin[0].nSequence = 1; BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail diff --git a/src/validation.cpp b/src/validation.cpp index b4f0c1810b..2b3ad4da53 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -145,10 +145,9 @@ const CBlockIndex* CChainState::FindForkInGlobalIndex(const CBlockLocator& locat bool CheckInputScripts(const CTransaction& tx, TxValidationState &state, const CCoinsViewCache &inputs, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector *pvChecks = nullptr); -bool CheckFinalTxAtTip(const CBlockIndex* active_chain_tip, const CTransaction& tx) +bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx) { AssertLockHeld(cs_main); - assert(active_chain_tip); // TODO: Make active_chain_tip a reference // CheckFinalTxAtTip() uses active_chain_tip.Height()+1 to evaluate // nLockTime because when IsFinalTx() is called within @@ -156,14 +155,14 @@ bool CheckFinalTxAtTip(const CBlockIndex* active_chain_tip, const CTransaction& // evaluated is what is used. Thus if we want to know if a // transaction can be part of the *next* block, we need to call // IsFinalTx() with one more than active_chain_tip.Height(). - const int nBlockHeight = active_chain_tip->nHeight + 1; + const int nBlockHeight = active_chain_tip.nHeight + 1; // BIP113 requires that time-locked transactions have nLockTime set to // less than the median time of the previous block they're contained in. // When the next block is created its previous block will be the current // chain tip, so we use that to calculate the median time passed to // IsFinalTx(). - const int64_t nBlockTime{active_chain_tip->GetMedianTimePast()}; + const int64_t nBlockTime{active_chain_tip.GetMedianTimePast()}; return IsFinalTx(tx, nBlockHeight, nBlockTime); } @@ -380,7 +379,7 @@ void CChainState::MaybeUpdateMempoolForReorg( const CTransaction& tx = it->GetTx(); // The transaction must be final. - if (!CheckFinalTxAtTip(m_chain.Tip(), tx)) return true; + if (!CheckFinalTxAtTip(*Assert(m_chain.Tip()), tx)) return true; LockPoints lp = it->GetLockPoints(); const bool validLP{TestLockPointValidity(m_chain, lp)}; CCoinsViewMemPool view_mempool(&CoinsTip(), *m_mempool); @@ -632,7 +631,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // Only accept nLockTime-using 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. - if (!CheckFinalTxAtTip(m_active_chainstate.m_chain.Tip(), tx)) { + if (!CheckFinalTxAtTip(*Assert(m_active_chainstate.m_chain.Tip()), tx)) { return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-final"); } diff --git a/src/validation.h b/src/validation.h index cf041cf99d..fd9bf01d89 100644 --- a/src/validation.h +++ b/src/validation.h @@ -280,7 +280,7 @@ int GetUTXOConfirmations(CChainState& active_chainstate, const COutPoint& outpoi /** * Check if transaction will be final in the next block to be created. */ -bool CheckFinalTxAtTip(const CBlockIndex* active_chain_tip, const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); +bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** * Check if transaction will be BIP68 final in the next block to be created on top of tip.