Merge #20750: [Bundle 2/n] Prune g_chainman usage in mempool-related validation functions

e8ae1db style-only: Make AcceptToMemoryPool signature readable (Carl Dong)
8f5c100 style-only: Make CheckSequenceLock signature readable (Carl Dong)
8c82481 validation: Use *this in CChainState::LoadMempool (Carl Dong)
0a9a24d validation: Pass in chainstate to UpdateMempoolForReorg (Carl Dong)
7142018 validation: Pass in chainstate to CTxMemPool::removeForReorg (Carl Dong)
71734c6 validation: Pass in chain to ::TestLockPointValidity (Carl Dong)
120aaba tree-wide: Fix erroneous AcceptToMemoryPool replacements (Carl Dong)
417dafc validation: Remove old AcceptToMemoryPool w/o chainstate param (Carl Dong)
3704433 scripted-diff: Invoke ::AcceptToMemoryPool with chainstate (Carl Dong)
229bc37 validation: Pass in chainstate to ::AcceptToMemoryPool (Carl Dong)
d0da7ea validation: Pass in chainstate to ::LoadMempool (Carl Dong)
3a205c4 validation: Pass in chainstate to AcceptToMemoryPoolWithTime (Carl Dong)
d8a8163 validation: Add chainstate member to MemPoolAccept (Carl Dong)
4c15942 validation: Pass in chainstate to ::CheckSequenceLocks (Carl Dong)
577b774 validation: Remove old CheckFinalTx w/o chain tip param (Carl Dong)
7031cf8 scripted-diff: Invoke ::CheckFinalTx with chain tip (Carl Dong)
d015eaa validation: Pass in chain tip to ::CheckFinalTx (Carl Dong)
252b489 validation: Pass in coins tip to CheckInputsFromMempoolAndCache (Carl Dong)
73a6d2b validation: Pass in chainstate to IsCurrentForFeeEstimation (Carl Dong)
d1f932b validation: Pass in coins cache to ::LimitMempoolSize (Carl Dong)

Pull request description:

  Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)

  Note to reviewers:
  1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
  	1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
  	2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
  	3. Remove `old_function`

ACKs for top commit:
  glozow:
    reACK bitcoin/bitcoin@e8ae1db via `git range-diff 15f0042...e8ae1db`, only change is fixing ATMP call from conflict
  MarcoFalke:
    ACK e8ae1db 📣

Tree-SHA512: 6af50f04940a69c5c3d3796a24f32f963fa02503cdc1155cc11fff832a99172b407cd163a19793080a5af98580f051b48195b62ec4a797ba2763b4883174153d

Author: MarcoFalke

src/bench/block_assemble.cpp

@@ -48,7 +48,7 @@ static void AssembleBlock(benchmark::Bench& bench)
         LOCK(::cs_main); // Required for ::AcceptToMemoryPool.
 
         for (const auto& txr : txs) {
-            const MempoolAcceptResult res = ::AcceptToMemoryPool(*test_setup.m_node.mempool, txr, false /* bypass_limits */);
+            const MempoolAcceptResult res = ::AcceptToMemoryPool(::ChainstateActive(), *test_setup.m_node.mempool, txr, false /* bypass_limits */);
             assert(res.m_result_type == MempoolAcceptResult::ResultType::VALID);
         }
     }

src/net_processing.cpp

@@ -2236,7 +2236,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
         if (orphan_it == mapOrphanTransactions.end()) continue;
 
         const CTransactionRef porphanTx = orphan_it->second.tx;
-        const MempoolAcceptResult result = AcceptToMemoryPool(m_mempool, porphanTx, false /* bypass_limits */);
+        const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), m_mempool, porphanTx, false /* bypass_limits */);
         const TxValidationState& state = result.m_state;
 
         if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
@@ -3258,7 +3258,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             return;
         }
 
-        const MempoolAcceptResult result = AcceptToMemoryPool(m_mempool, ptx, false /* bypass_limits */);
+        const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), m_mempool, ptx, false /* bypass_limits */);
         const TxValidationState& state = result.m_state;
 
         if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {

src/node/interfaces.cpp

@@ -441,7 +441,7 @@ class ChainImpl : public Chain
     bool checkFinalTx(const CTransaction& tx) override
     {
         LOCK(cs_main);
-        return CheckFinalTx(tx);
+        return CheckFinalTx(::ChainActive().Tip(), tx);
     }
     Optional<int> findLocatorFork(const CBlockLocator& locator) override
     {

src/node/transaction.cpp

@@ -53,7 +53,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
         if (max_tx_fee > 0) {
             // First, call ATMP with test_accept and check the fee. If ATMP
             // fails here, return error immediately.
-            const MempoolAcceptResult result = AcceptToMemoryPool(*node.mempool, tx, false /* bypass_limits */,
+            const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), *node.mempool, tx, false /* bypass_limits */,
                                                                   true /* test_accept */);
             if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
                 return HandleATMPError(result.m_state, err_string);
@@ -62,7 +62,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
             }
         }
         // Try to submit the transaction to the mempool.
-        const MempoolAcceptResult result = AcceptToMemoryPool(*node.mempool, tx, false /* bypass_limits */,
+        const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), *node.mempool, tx, false /* bypass_limits */,
                                                               false /* test_accept */);
         if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
             return HandleATMPError(result.m_state, err_string);

src/rpc/rawtransaction.cpp

@@ -946,7 +946,7 @@ static RPCHelpMan testmempoolaccept()
     result_0.pushKV("txid", tx->GetHash().GetHex());
     result_0.pushKV("wtxid", tx->GetWitnessHash().GetHex());
 
-    const MempoolAcceptResult accept_result = WITH_LOCK(cs_main, return AcceptToMemoryPool(mempool, std::move(tx),
+    const MempoolAcceptResult accept_result = WITH_LOCK(cs_main, return AcceptToMemoryPool(::ChainstateActive(), mempool, std::move(tx),
                                                   false /* bypass_limits */, /* test_accept */ true));
 
     // Only return the fee and vsize if the transaction would pass ATMP.

src/test/miner_tests.cpp

@@ -28,7 +28,7 @@ struct MinerTestingSetup : public TestingSetup {
     void TestPackageSelection(const CChainParams& chainparams, const CScript& scriptPubKey, const std::vector<CTransactionRef>& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs);
     bool TestSequenceLocks(const CTransaction& tx, int flags) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs)
     {
-        return CheckSequenceLocks(*m_node.mempool, tx, flags);
+        return CheckSequenceLocks(::ChainstateActive(), *m_node.mempool, tx, flags);
     }
     BlockAssembler AssemblerForTest(const CChainParams& params);
 };
@@ -437,7 +437,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(CheckFinalTx(CTransaction(tx), flags)); // Locktime passes
+    BOOST_CHECK(CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime passes
     BOOST_CHECK(!TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks fail
     BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, prevheights, CreateBlockIndex(::ChainActive().Tip()->nHeight + 2))); // Sequence locks pass on 2nd block
 
@@ -447,7 +447,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(CheckFinalTx(CTransaction(tx), flags)); // Locktime passes
+    BOOST_CHECK(CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime passes
     BOOST_CHECK(!TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks fail
 
     for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
@@ -463,7 +463,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
     tx.nLockTime = ::ChainActive().Tip()->nHeight + 1;
     hash = tx.GetHash();
     m_node.mempool->addUnchecked(entry.Time(GetTime()).FromTx(tx));
-    BOOST_CHECK(!CheckFinalTx(CTransaction(tx), flags)); // Locktime fails
+    BOOST_CHECK(!CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime fails
     BOOST_CHECK(TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks pass
     BOOST_CHECK(IsFinalTx(CTransaction(tx), ::ChainActive().Tip()->nHeight + 2, ::ChainActive().Tip()->GetMedianTimePast())); // Locktime passes on 2nd block
 
@@ -474,7 +474,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(!CheckFinalTx(CTransaction(tx), flags)); // Locktime fails
+    BOOST_CHECK(!CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime fails
     BOOST_CHECK(TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks pass
     BOOST_CHECK(IsFinalTx(CTransaction(tx), ::ChainActive().Tip()->nHeight + 2, ::ChainActive().Tip()->GetMedianTimePast() + 1)); // Locktime passes 1 second later
 
@@ -483,7 +483,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
     prevheights[0] = ::ChainActive().Tip()->nHeight + 1;
     tx.nLockTime = 0;
     tx.vin[0].nSequence = 0;
-    BOOST_CHECK(CheckFinalTx(CTransaction(tx), flags)); // Locktime passes
+    BOOST_CHECK(CheckFinalTx(::ChainActive().Tip(), CTransaction(tx), flags)); // Locktime passes
     BOOST_CHECK(TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks pass
     tx.vin[0].nSequence = 1;
     BOOST_CHECK(!TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks fail

src/test/txvalidation_tests.cpp

@@ -33,7 +33,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup)
     LOCK(cs_main);
 
     unsigned int initialPoolSize = m_node.mempool->size();
-    const MempoolAcceptResult result = AcceptToMemoryPool(*m_node.mempool, MakeTransactionRef(coinbaseTx),
+    const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), *m_node.mempool, MakeTransactionRef(coinbaseTx),
                 true /* bypass_limits */);
 
     BOOST_CHECK(result.m_result_type == MempoolAcceptResult::ResultType::INVALID);

src/test/txvalidationcache_tests.cpp

@@ -28,7 +28,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup)
     const auto ToMemPool = [this](const CMutableTransaction& tx) {
         LOCK(cs_main);
 
-        const MempoolAcceptResult result = AcceptToMemoryPool(*m_node.mempool, MakeTransactionRef(tx),
+        const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), *m_node.mempool, MakeTransactionRef(tx),
             true /* bypass_limits */);
         return result.m_result_type == MempoolAcceptResult::ResultType::VALID;
     };

src/test/util/setup_common.cpp

@@ -303,7 +303,7 @@ CMutableTransaction TestChain100Setup::CreateValidMempoolTransaction(CTransactio
     // Add transaction to the mempool
     {
         LOCK(cs_main);
-        const MempoolAcceptResult result = AcceptToMemoryPool(*m_node.mempool.get(), MakeTransactionRef(mempool_txn), /* bypass_limits */ false);
+        const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), *m_node.mempool.get(), MakeTransactionRef(mempool_txn), /* bypass_limits */ false);
         assert(result.m_result_type == MempoolAcceptResult::ResultType::VALID);
     }
 

src/test/validation_block_tests.cpp

@@ -273,7 +273,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
         {
             LOCK(cs_main);
             for (const auto& tx : txs) {
-                const MempoolAcceptResult result = AcceptToMemoryPool(*m_node.mempool, tx, false /* bypass_limits */);
+                const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), *m_node.mempool, tx, false /* bypass_limits */);
                 BOOST_REQUIRE(result.m_result_type == MempoolAcceptResult::ResultType::VALID);
             }
         }