From 8bdab4d4fec89cff77f2a01d58045c429e02532d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 1 Oct 2024 12:21:30 +0000 Subject: [PATCH] merge bitcoin#23437: AcceptToMemoryPool --- src/test/fuzz/tx_pool.cpp | 8 ++++---- src/validation.cpp | 29 +++++++++++------------------ src/validation.h | 19 ++++++++++++------- 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index d42b42122c..4f90d4e5af 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -86,7 +86,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, con options.nBlockMaxSize = fuzzed_data_provider.ConsumeIntegralInRange(0U, MaxBlockSize(true)); options.blockMinFeeRate = CFeeRate{ConsumeMoney(fuzzed_data_provider, /* max */ COIN)}; - auto assembler = BlockAssembler{chainstate, node, *static_cast(&tx_pool), ::Params(), options}; + auto assembler = BlockAssembler{chainstate, node, *static_cast(&tx_pool), chainstate.m_params, options}; auto block_template = assembler.CreateNewBlock(CScript{} << OP_TRUE); Assert(block_template->block.vtx.size() >= 1); } @@ -222,13 +222,13 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool) // Make sure ProcessNewPackage on one transaction works and always fully validates the transaction. // The result is not guaranteed to be the same as what is returned by ATMP. const auto result_package = WITH_LOCK(::cs_main, - return ProcessNewPackage(node.chainman->ActiveChainstate(), tx_pool, {tx}, true)); + return ProcessNewPackage(chainstate, tx_pool, {tx}, true)); auto it = result_package.m_tx_results.find(tx->GetHash()); Assert(it != result_package.m_tx_results.end()); 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(chainstate, tx_pool, tx, bypass_limits)); + const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(tx_pool, chainstate, tx, GetTime(), bypass_limits, /* test_accept= */ false)); const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID; SyncWithValidationInterfaceQueue(); UnregisterSharedValidationInterface(txr); @@ -328,7 +328,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(node.chainman->ActiveChainstate(), tx_pool, tx, bypass_limits)); + const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(tx_pool, 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 87079c5baf..54e5977423 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -354,7 +354,9 @@ void CChainState::MaybeUpdateMempoolForReorg( while (it != disconnectpool.queuedTx.get().rend()) { // ignore validation errors in resurrected transactions if (!fAddToMempool || (*it)->IsCoinBase() || - AcceptToMemoryPool(*this, *m_mempool, *it, true /* bypass_limits */).m_result_type != MempoolAcceptResult::ResultType::VALID) { + AcceptToMemoryPool(*m_mempool, *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). m_mempool->removeRecursive(**it, MemPoolRemovalReason::REORG); @@ -978,16 +980,13 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: } // anon namespace -/** (try to) add transaction to memory pool with a specified acceptance time **/ -static MempoolAcceptResult AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CChainState& active_chainstate, - const CTransactionRef &tx, int64_t nAcceptTime, - bool bypass_limits, bool test_accept) +MempoolAcceptResult AcceptToMemoryPool(CTxMemPool& pool, CChainState& active_chainstate, const CTransactionRef& tx, + int64_t accept_time, bool bypass_limits, bool test_accept) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - AssertLockHeld(::cs_main); + const CChainParams& chainparams{active_chainstate.m_params}; std::vector coins_to_uncache; - MemPoolAccept::ATMPArgs args { chainparams, nAcceptTime, bypass_limits, coins_to_uncache, test_accept }; - + MemPoolAccept::ATMPArgs args { chainparams, accept_time, bypass_limits, coins_to_uncache, test_accept }; const MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args); if (result.m_result_type != MempoolAcceptResult::ResultType::VALID || test_accept) { if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { @@ -1008,11 +1007,6 @@ static MempoolAcceptResult AcceptToMemoryPoolWithTime(const CChainParams& chainp return result; } -MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, CTxMemPool& pool, const CTransactionRef &tx, bool bypass_limits, bool test_accept) -{ - return AcceptToMemoryPoolWithTime(Params(), pool, active_chainstate, tx, GetTime(), bypass_limits, test_accept); -} - PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTxMemPool& pool, const Package& package, bool test_accept) { @@ -1232,12 +1226,12 @@ CChainState::CChainState(CTxMemPool* mempool, const std::unique_ptr& isman, std::optional from_snapshot_blockhash) : m_mempool(mempool), - m_params(::Params()), m_chain_helper(chain_helper), m_clhandler(clhandler), m_isman(isman), m_evoDb(evoDb), m_blockman(blockman), + m_params(::Params()), m_chainman(chainman), m_from_snapshot_blockhash(from_snapshot_blockhash) {} @@ -4001,7 +3995,7 @@ MempoolAcceptResult ChainstateManager::ProcessTransaction(const CTransactionRef& state.Invalid(TxValidationResult::TX_NO_MEMPOOL, "no-mempool"); return MempoolAcceptResult::Failure(state); } - auto result = AcceptToMemoryPool(active_chainstate, *active_chainstate.m_mempool, tx, bypass_limits, test_accept); + auto result = AcceptToMemoryPool(*active_chainstate.m_mempool, active_chainstate, tx, GetTime(), bypass_limits, test_accept); active_chainstate.m_mempool->check(active_chainstate.CoinsTip(), active_chainstate.m_chain.Height() + 1); return result; } @@ -4966,7 +4960,6 @@ static const uint64_t MEMPOOL_DUMP_VERSION = 1; bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function) { - const CChainParams& chainparams = Params(); int64_t nExpiryTimeout = gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60; FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat", "rb")}; CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); @@ -5005,8 +4998,8 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka } if (nTime > nNow - nExpiryTimeout) { LOCK(cs_main); - if (AcceptToMemoryPoolWithTime(chainparams, pool, active_chainstate, tx, nTime, false /* bypass_limits */, - false /* test_accept */).m_result_type == MempoolAcceptResult::ResultType::VALID) { + if (AcceptToMemoryPool(pool, active_chainstate, tx, nTime, /* bypass_limits= */ false, + /* test_accept= */ false).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 18df160a3b..951d6e5f39 100644 --- a/src/validation.h +++ b/src/validation.h @@ -240,19 +240,23 @@ 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() + * 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] active_chainstate Reference to the active chainstate. * @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. + * 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(CChainState& active_chainstate, CTxMemPool& pool, const CTransactionRef& tx, - bool bypass_limits, bool test_accept=false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +MempoolAcceptResult AcceptToMemoryPool(CTxMemPool& pool, CChainState& active_chainstate, const CTransactionRef& tx, + int64_t accept_time, bool bypass_limits, bool test_accept) + EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** * Atomically test acceptance of a package. If the package only contains one tx, package rules still @@ -486,8 +490,6 @@ protected: //! Only the active chainstate has a mempool. CTxMemPool* m_mempool; - const CChainParams& m_params; - //! Manages the UTXO set, which is a reflection of the contents of `m_chain`. std::unique_ptr m_coins_views; @@ -502,6 +504,9 @@ public: //! CChainState instances. BlockManager& m_blockman; + /** Chain parameters for this chainstate */ + const CChainParams& m_params; + //! The chainstate manager that owns this chainstate. The reference is //! necessary so that this instance can check whether it is the active //! chainstate within deeply nested method calls.