From f1f10c0514fe81318c8b064f9dad0e2f9a2cd037 Mon Sep 17 00:00:00 2001 From: lsilva01 Date: Thu, 2 Dec 2021 01:50:27 -0300 Subject: [PATCH] Remove CTxMemPool params from ATMP Co-authored-by: John Newbery <1063656+jnewbery@users.noreply.github.com> Co-authored-by: Jon Atack --- src/test/fuzz/tx_pool.cpp | 17 ++++++++++++++--- src/validation.cpp | 19 +++++++++++-------- src/validation.h | 12 ++++++++---- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index c32c965ab0..fe1b9c7c0c 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -29,6 +29,15 @@ struct MockedTxPool : public CTxMemPool { } }; +class DummyChainState final : public CChainState +{ +public: + void SetMempool(CTxMemPool* mempool) + { + m_mempool = mempool; + } +}; + void initialize_tx_pool() { static const auto testing_setup = MakeNoLogFileContext(); @@ -114,7 +123,7 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); const auto& node = g_setup->m_node; - auto& chainstate = node.chainman->ActiveChainstate(); + auto& chainstate{static_cast(node.chainman->ActiveChainstate())}; MockTime(fuzzed_data_provider, chainstate); SetMempoolConstraints(*node.args, fuzzed_data_provider); @@ -134,6 +143,8 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool) CTxMemPool tx_pool_{/*estimator=*/nullptr, /*check_ratio=*/1}; MockedTxPool& tx_pool = *static_cast(&tx_pool_); + chainstate.SetMempool(&tx_pool); + // Helper to query an amount const CCoinsViewMemPool amount_view{WITH_LOCK(::cs_main, return &chainstate.CoinsTip()), tx_pool}; const auto GetAmount = [&](const COutPoint& outpoint) { @@ -230,7 +241,7 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool) Assert(it->second.m_result_type == MempoolAcceptResult::ResultType::VALID || it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID); - const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(tx_pool, chainstate, tx, GetTime(), bypass_limits, /* test_accept= */ false)); + const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false)); const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID; SyncWithValidationInterfaceQueue(); UnregisterSharedValidationInterface(txr); @@ -330,7 +341,7 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) const auto tx = MakeTransactionRef(mut_tx); const bool bypass_limits = fuzzed_data_provider.ConsumeBool(); ::fRequireStandard = fuzzed_data_provider.ConsumeBool(); - const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(tx_pool, chainstate, tx, GetTime(), bypass_limits, /* test_accept= */ false)); + const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false)); const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID; if (accepted) { txids.push_back(tx->GetHash()); diff --git a/src/validation.cpp b/src/validation.cpp index 6ed5e7a548..38037f1012 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -331,8 +331,8 @@ void CChainState::MaybeUpdateMempoolForReorg( while (it != disconnectpool.queuedTx.get().rend()) { // ignore validation errors in resurrected transactions if (!fAddToMempool || (*it)->IsCoinBase() || - AcceptToMemoryPool(*m_mempool, *this, *it, GetTime(), - /* bypass_limits= */true, /* test_accept= */ false).m_result_type != + AcceptToMemoryPool(*this, *it, GetTime(), + /*bypass_limits=*/true, /*test_accept=*/false).m_result_type != MempoolAcceptResult::ResultType::VALID) { // If the transaction doesn't make it in to the mempool, remove any // transactions that depend on it (which would now be orphans). @@ -1091,11 +1091,14 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: } // anon namespace -MempoolAcceptResult AcceptToMemoryPool(CTxMemPool& pool, CChainState& active_chainstate, const CTransactionRef& tx, +MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, const CTransactionRef& tx, int64_t accept_time, bool bypass_limits, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { const CChainParams& chainparams{active_chainstate.m_params}; + assert(active_chainstate.GetMempool() != nullptr); + CTxMemPool& pool{*active_chainstate.GetMempool()}; + std::vector coins_to_uncache; auto args = MemPoolAccept::ATMPArgs::SingleAccept(chainparams, accept_time, bypass_limits, coins_to_uncache, test_accept); const MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args); @@ -3500,13 +3503,13 @@ bool ChainstateManager::ProcessNewBlock(const CChainParams& chainparams, const s MempoolAcceptResult ChainstateManager::ProcessTransaction(const CTransactionRef& tx, bool test_accept) { CChainState& active_chainstate = ActiveChainstate(); - if (!active_chainstate.m_mempool) { + if (!active_chainstate.GetMempool()) { TxValidationState state; state.Invalid(TxValidationResult::TX_NO_MEMPOOL, "no-mempool"); return MempoolAcceptResult::Failure(state); } - auto result = AcceptToMemoryPool(*active_chainstate.m_mempool, active_chainstate, tx, GetTime(), /* bypass_limits= */ false, test_accept); - active_chainstate.m_mempool->check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1); + auto result = AcceptToMemoryPool(active_chainstate, tx, GetTime(), /*bypass_limits=*/ false, test_accept); + active_chainstate.GetMempool()->check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1); return result; } @@ -4572,8 +4575,8 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka } if (nTime > nNow - nExpiryTimeout) { LOCK(cs_main); - if (AcceptToMemoryPool(pool, active_chainstate, tx, nTime, /* bypass_limits= */ false, - /* test_accept= */ false).m_result_type == MempoolAcceptResult::ResultType::VALID) { + const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false); + if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) { ++count; } else { // mempool may contain the transaction already, e.g. from diff --git a/src/validation.h b/src/validation.h index 881438f37a..9e79adfd72 100644 --- a/src/validation.h +++ b/src/validation.h @@ -211,18 +211,16 @@ struct PackageMempoolAcceptResult * Try to add a transaction to the mempool. This is an internal function and is exposed only for testing. * Client code should use ChainstateManager::ProcessTransaction() * - * @param[in] pool Reference to the node's mempool. * @param[in] active_chainstate Reference to the active chainstate. * @param[in] tx The transaction to submit for mempool acceptance. - * @param[in] accept_time The timestamp for adding the transaction to the mempool. Usually - * the current system time, but may be different. + * @param[in] accept_time The timestamp for adding the transaction to the mempool. * It is also used to determine when the entry expires. * @param[in] bypass_limits When true, don't enforce mempool fee and capacity limits. * @param[in] test_accept When true, run validation checks but don't submit to mempool. * * @returns a MempoolAcceptResult indicating whether the transaction was accepted/rejected with reason. */ -MempoolAcceptResult AcceptToMemoryPool(CTxMemPool& pool, CChainState& active_chainstate, const CTransactionRef& tx, +MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, const CTransactionRef& tx, int64_t accept_time, bool bypass_limits, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -656,6 +654,12 @@ public: return m_coins_views->m_dbview; } + //! @returns A pointer to the mempool. + CTxMemPool* GetMempool() + { + return m_mempool; + } + //! @returns A reference to a wrapped view of the in-memory UTXO set that //! handles disk read errors gracefully. CCoinsViewErrorCatcher& CoinsErrorCatcher() EXCLUSIVE_LOCKS_REQUIRED(cs_main)