diff --git a/src/bench/blockencodings.cpp b/src/bench/blockencodings.cpp index 3f6be56b464..ce968dbc651 100644 --- a/src/bench/blockencodings.cpp +++ b/src/bench/blockencodings.cpp @@ -22,7 +22,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { LockPoints lp; - TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); + TryAddToMempool(pool, CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); } namespace { diff --git a/src/bench/mempool_ephemeral_spends.cpp b/src/bench/mempool_ephemeral_spends.cpp index 2f89f0da208..f0d8eb0bea7 100644 --- a/src/bench/mempool_ephemeral_spends.cpp +++ b/src/bench/mempool_ephemeral_spends.cpp @@ -29,7 +29,7 @@ static void AddTx(const CTransactionRef& tx, CTxMemPool& pool) EXCLUSIVE_LOCKS_R unsigned int sigOpCost{4}; uint64_t fee{0}; LockPoints lp; - TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), + TryAddToMempool(pool, CTxMemPoolEntry( tx, fee, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); } diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index ad3ab08a994..37bb7215be0 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -27,7 +27,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& po bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; - TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), + TryAddToMempool(pool, CTxMemPoolEntry( tx, nFee, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); } diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index 9cddeb7815f..1f582081ea9 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -29,7 +29,7 @@ static void AddTx(const CTransactionRef& tx, CTxMemPool& pool, FastRandomContext bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; - TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, det_rand.randrange(10000)+1000, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); + TryAddToMempool(pool, CTxMemPoolEntry(tx, det_rand.randrange(10000)+1000, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); } struct Available { diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp index b27a7e72cc4..f319c961a04 100644 --- a/src/bench/rpc_mempool.cpp +++ b/src/bench/rpc_mempool.cpp @@ -22,7 +22,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { LockPoints lp; - TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); + TryAddToMempool(pool, CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); } static void RpcMempool(benchmark::Bench& bench) diff --git a/src/bench/txgraph.cpp b/src/bench/txgraph.cpp index 078f77b4256..7257a043777 100644 --- a/src/bench/txgraph.cpp +++ b/src/bench/txgraph.cpp @@ -67,7 +67,7 @@ void BenchTxGraphTrim(benchmark::Bench& bench) for (int chaintx = 0; chaintx < NUM_TX_PER_TOP_CHAIN; ++chaintx) { int64_t fee = rng.randbits<27>() + 100; FeePerWeight feerate{fee, 1}; - top_refs.push_back(graph->AddTransaction(feerate)); + graph->AddTransaction(top_refs.emplace_back(), feerate); // Add internal dependencies linking the chain transactions together. if (chaintx > 0) { graph->AddDependency(*(top_refs.rbegin()), *(top_refs.rbegin() + 1)); @@ -85,7 +85,8 @@ void BenchTxGraphTrim(benchmark::Bench& bench) // Construct the transaction. int64_t fee = rng.randbits<27>() + 100; FeePerWeight feerate{fee, 1}; - auto bottom_tx = graph->AddTransaction(feerate); + TxGraph::Ref bottom_tx; + graph->AddTransaction(bottom_tx, feerate); // Determine the number of dependencies this transaction will have. int deps = std::min(NUM_DEPS_PER_BOTTOM_TX, top_components.size()); for (int dep = 0; dep < deps; ++dep) { diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 58824f7a80c..dd0c23a2cde 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -84,12 +84,11 @@ private: public: virtual ~CTxMemPoolEntry() = default; - CTxMemPoolEntry(TxGraph::Ref&& ref, const CTransactionRef& tx, CAmount fee, + CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, int64_t time, unsigned int entry_height, uint64_t entry_sequence, bool spends_coinbase, int64_t sigops_cost, LockPoints lp) - : TxGraph::Ref(std::move(ref)), - tx{tx}, + : tx{tx}, nFee{fee}, nTxWeight{GetTransactionWeight(*tx)}, nUsageSize{RecursiveDynamicUsage(tx)}, diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp index 195c2e05025..6d5ac88218d 100644 --- a/src/test/fuzz/txgraph.cpp +++ b/src/test/fuzz/txgraph.cpp @@ -137,15 +137,16 @@ struct SimTxGraph return simmap[pos].get(); } - /** Add a new transaction to the simulation. */ - void AddTransaction(TxGraph::Ref&& ref, const FeePerWeight& feerate) + /** Add a new transaction to the simulation and the specified real graph. */ + void AddTransaction(TxGraph& txgraph, const FeePerWeight& feerate) { assert(graph.TxCount() < MAX_TRANSACTIONS); auto simpos = graph.AddTransaction(feerate); real_is_optimal = false; MakeModified(simpos); assert(graph.Positions()[simpos]); - simmap[simpos] = std::make_shared(std::move(ref)); + simmap[simpos] = std::make_shared(); + txgraph.AddTransaction(*simmap[simpos], feerate); auto ptr = simmap[simpos].get(); simrevmap[ptr] = simpos; // This may invalidate our cached oversized value. @@ -455,10 +456,8 @@ FUZZ_TARGET(txgraph) size = provider.ConsumeIntegralInRange(1, 0xff); } FeePerWeight feerate{fee, size}; - // Create a real TxGraph::Ref. - auto ref = real->AddTransaction(feerate); - // Create a shared_ptr place in the simulation to put the Ref in. - top_sim.AddTransaction(std::move(ref), feerate); + // Create the transaction in the simulation and the real graph. + top_sim.AddTransaction(*real, feerate); break; } else if ((block_builders.empty() || sims.size() > 1) && top_sim.GetTransactionCount() + top_sim.removed.size() > 1 && command-- == 0) { // AddDependency. diff --git a/src/test/fuzz/util/mempool.cpp b/src/test/fuzz/util/mempool.cpp index 241a4d559c7..a6a28f94006 100644 --- a/src/test/fuzz/util/mempool.cpp +++ b/src/test/fuzz/util/mempool.cpp @@ -27,5 +27,5 @@ CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const auto entry_height{fuzzed_data_provider.ConsumeIntegralInRange(0, max_height)}; const bool spends_coinbase = fuzzed_data_provider.ConsumeBool(); const unsigned int sig_op_cost = fuzzed_data_provider.ConsumeIntegralInRange(0, MAX_BLOCK_SIGOPS_COST); - return CTxMemPoolEntry{TxGraph::Ref(), MakeTransactionRef(tx), fee, time, entry_height, entry_sequence, spends_coinbase, sig_op_cost, {}}; + return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, entry_sequence, spends_coinbase, sig_op_cost, {}}; } diff --git a/src/test/txgraph_tests.cpp b/src/test/txgraph_tests.cpp index 26cf2b3a970..31fefd9ce18 100644 --- a/src/test/txgraph_tests.cpp +++ b/src/test/txgraph_tests.cpp @@ -46,11 +46,11 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_zigzag) refs.reserve(NUM_TOTAL_TX); // First all bottom transactions: the i'th bottom transaction is at position i. for (unsigned int i = 0; i < NUM_BOTTOM_TX; ++i) { - refs.push_back(graph->AddTransaction(FeePerWeight{200 - i, 100})); + graph->AddTransaction(refs.emplace_back(), FeePerWeight{200 - i, 100}); } // Then all top transactions: the i'th top transaction is at position NUM_BOTTOM_TX + i. for (unsigned int i = 0; i < NUM_TOP_TX; ++i) { - refs.push_back(graph->AddTransaction(FeePerWeight{100 - i, 100})); + graph->AddTransaction(refs.emplace_back(), FeePerWeight{100 - i, 100}); } // Create the zigzag dependency structure. @@ -109,9 +109,9 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_flower) refs.reserve(NUM_TOTAL_TX); // Add all transactions. They are in individual clusters. - refs.push_back(graph->AddTransaction({1, 100})); + graph->AddTransaction(refs.emplace_back(), {1, 100}); for (unsigned int i = 0; i < NUM_TOP_TX; ++i) { - refs.push_back(graph->AddTransaction(FeePerWeight{500 + i, 100})); + graph->AddTransaction(refs.emplace_back(), FeePerWeight{500 + i, 100}); } graph->SanityCheck(); @@ -196,8 +196,8 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_huge) // Use random fees, size 1. int64_t fee = rng.randbits<27>() + 100; FeePerWeight feerate{fee, 1}; - top_refs.push_back(graph->AddTransaction(feerate)); - // Add internal dependencies linked the chain transactions together. + graph->AddTransaction(top_refs.emplace_back(), feerate); + // Add internal dependencies linking the chain transactions together. if (chaintx > 0) { graph->AddDependency(*(top_refs.rbegin()), *(top_refs.rbegin() + 1)); } @@ -215,7 +215,8 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_huge) // Construct the transaction. int64_t fee = rng.randbits<27>() + 100; FeePerWeight feerate{fee, 1}; - auto bottom_tx = graph->AddTransaction(feerate); + TxGraph::Ref bottom_tx; + graph->AddTransaction(bottom_tx, feerate); // Determine the number of dependencies this transaction will have. int deps = std::min(NUM_DEPS_PER_BOTTOM_TX, top_components.size()); for (int dep = 0; dep < deps; ++dep) { @@ -271,7 +272,7 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_big_singletons) // The 88th transaction is oversized. // Every 20th transaction is oversized. const FeePerWeight feerate{500 + i, (i == 88 || i % 20 == 0) ? MAX_CLUSTER_SIZE + 1 : 100}; - refs.push_back(graph->AddTransaction(feerate)); + graph->AddTransaction(refs.emplace_back(), feerate); } graph->SanityCheck(); @@ -334,23 +335,23 @@ BOOST_AUTO_TEST_CASE(txgraph_chunk_chain) // everytime adding a transaction, test the chunk status // [A] - refs.push_back(graph->AddTransaction(feerateA)); + graph->AddTransaction(refs.emplace_back(), feerateA); BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 1); block_builder_checker({{&refs[0]}}); // [A, B] - refs.push_back(graph->AddTransaction(feerateB)); + graph->AddTransaction(refs.emplace_back(), feerateB); graph->AddDependency(/*parent=*/refs[0], /*child=*/refs[1]); BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 2); block_builder_checker({{&refs[0]}, {&refs[1]}}); // [A, BC] - refs.push_back(graph->AddTransaction(feerateC)); + graph->AddTransaction(refs.emplace_back(), feerateC); graph->AddDependency(/*parent=*/refs[1], /*child=*/refs[2]); BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 3); block_builder_checker({{&refs[0]}, {&refs[1], &refs[2]}}); // [ABCD] - refs.push_back(graph->AddTransaction(feerateD)); + graph->AddTransaction(refs.emplace_back(), feerateD); graph->AddDependency(/*parent=*/refs[2], /*child=*/refs[3]); BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 4); block_builder_checker({{&refs[0], &refs[1], &refs[2], &refs[3]}}); @@ -383,7 +384,7 @@ BOOST_AUTO_TEST_CASE(txgraph_staging) // everytime adding a transaction, test the chunk status // [A] - refs.push_back(graph->AddTransaction(feerateA)); + graph->AddTransaction(refs.emplace_back(), feerateA); BOOST_CHECK_EQUAL(graph->HaveStaging(), false); BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 1); @@ -392,7 +393,7 @@ BOOST_AUTO_TEST_CASE(txgraph_staging) BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 1); // [A, B] - refs.push_back(graph->AddTransaction(feerateB)); + graph->AddTransaction(refs.emplace_back(), feerateB); BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::MAIN), 1); BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 2); BOOST_CHECK_EQUAL(graph->Exists(refs[0], TxGraph::Level::TOP), true); diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index a019c830e1e..3c7eb87ccc2 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -38,7 +38,7 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) co CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) const { - return CTxMemPoolEntry{TxGraph::Ref(), tx, nFee, TicksSinceEpoch(time), nHeight, m_sequence, spendsCoinbase, sigOpCost, lp}; + return CTxMemPoolEntry{tx, nFee, TicksSinceEpoch(time), nHeight, m_sequence, spendsCoinbase, sigOpCost, lp}; } std::optional CheckPackageMempoolAcceptResult(const Package& txns, diff --git a/src/txgraph.cpp b/src/txgraph.cpp index f60d8ca2a66..d9f5900f306 100644 --- a/src/txgraph.cpp +++ b/src/txgraph.cpp @@ -753,7 +753,7 @@ public: // Implementations for the public TxGraph interface. - Ref AddTransaction(const FeePerWeight& feerate) noexcept final; + void AddTransaction(Ref& arg, const FeePerWeight& feerate) noexcept final; void RemoveTransaction(const Ref& arg) noexcept final; void AddDependency(const Ref& parent, const Ref& child) noexcept final; void SetTransactionFee(const Ref&, int64_t fee) noexcept final; @@ -2115,20 +2115,19 @@ void TxGraphImpl::MakeAllAcceptable(int level) noexcept GenericClusterImpl::GenericClusterImpl(uint64_t sequence) noexcept : Cluster{sequence} {} -TxGraph::Ref TxGraphImpl::AddTransaction(const FeePerWeight& feerate) noexcept +void TxGraphImpl::AddTransaction(Ref& arg, const FeePerWeight& feerate) noexcept { Assume(m_main_chunkindex_observers == 0 || GetTopLevel() != 0); Assume(feerate.size > 0); - // Construct a new Ref. - Ref ret; + Assume(GetRefGraph(arg) == nullptr); // Construct a new Entry, and link it with the Ref. auto idx = m_entries.size(); m_entries.emplace_back(); auto& entry = m_entries.back(); entry.m_main_chunkindex_iterator = m_main_chunkindex.end(); - entry.m_ref = &ret; - GetRefGraph(ret) = this; - GetRefIndex(ret) = idx; + entry.m_ref = &arg; + GetRefGraph(arg) = this; + GetRefIndex(arg) = idx; // Construct a new singleton Cluster (which is necessarily optimally linearized). bool oversized = uint64_t(feerate.size) > m_max_cluster_size; auto cluster = CreateEmptyCluster(1); @@ -2146,8 +2145,6 @@ TxGraph::Ref TxGraphImpl::AddTransaction(const FeePerWeight& feerate) noexcept clusterset.m_oversized = true; clusterset.m_group_data = std::nullopt; } - // Return the Ref. - return ret; } void TxGraphImpl::RemoveTransaction(const Ref& arg) noexcept diff --git a/src/txgraph.h b/src/txgraph.h index 0e1f2c79d07..385db0147cc 100644 --- a/src/txgraph.h +++ b/src/txgraph.h @@ -67,12 +67,14 @@ public: /** Virtual destructor, so inheriting is safe. */ virtual ~TxGraph() = default; - /** Construct a new transaction with the specified feerate, and return a Ref to it. + /** Initialize arg (which must be an empty Ref) to refer to a new transaction in this graph + * with the specified feerate. + * * If a staging graph exists, the new transaction is only created there. feerate.size must be - * strictly positive. In all further calls, only Refs created by AddTransaction() are allowed + * strictly positive. In all further calls, only Refs passed to AddTransaction() are allowed * to be passed to this TxGraph object (or empty Ref objects). Ref objects may outlive the - * TxGraph they were created for. */ - [[nodiscard]] virtual Ref AddTransaction(const FeePerWeight& feerate) noexcept = 0; + * TxGraph they were added to. */ + virtual void AddTransaction(Ref& arg, const FeePerWeight& feerate) noexcept = 0; /** Remove the specified transaction. If a staging graph exists, the removal only happens * there. This is a no-op if the transaction was already removed. * @@ -235,8 +237,7 @@ public: /** Index into the Graph's m_entries. Only used if m_graph != nullptr. */ GraphIndex m_index = GraphIndex(-1); public: - /** Construct an empty Ref. Non-empty Refs can only be created using - * TxGraph::AddTransaction. */ + /** Construct an empty Ref. It can be initialized through TxGraph::AddTransaction. */ Ref() noexcept = default; /** Destroy this Ref. If it is not empty, the corresponding transaction is removed (in both * main and staging, if it exists). */ diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 55c332769d4..58262e7f2f9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1000,8 +1000,9 @@ CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::StageAddition(const CTran CAmount delta{0}; m_pool->ApplyDelta(tx->GetHash(), delta); - TxGraph::Ref ref(m_pool->m_txgraph->AddTransaction(FeePerWeight(fee, GetSigOpsAdjustedWeight(GetTransactionWeight(*tx), sigops_cost, ::nBytesPerSigOp)))); - auto newit = m_to_add.emplace(std::move(ref), tx, fee, time, entry_height, entry_sequence, spends_coinbase, sigops_cost, lp).first; + FeePerWeight feerate(fee, GetSigOpsAdjustedWeight(GetTransactionWeight(*tx), sigops_cost, ::nBytesPerSigOp)); + auto newit = m_to_add.emplace(tx, fee, time, entry_height, entry_sequence, spends_coinbase, sigops_cost, lp).first; + m_pool->m_txgraph->AddTransaction(const_cast(*newit), feerate); if (delta) { newit->UpdateModifiedFee(delta); m_pool->m_txgraph->SetTransactionFee(*newit, newit->GetModifiedFee());