From bc64013e6fad2d054bc5a31630c09f33a62b8f4f Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Sat, 25 Jan 2025 11:04:58 -0500 Subject: [PATCH 01/26] Remove unused variable (cacheMap) in mempool --- src/txmempool.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/txmempool.h b/src/txmempool.h index 1fc94c6db70..c305d5d448a 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -281,9 +281,6 @@ public: std::vector GetParents(const CTxMemPoolEntry &entry) const; private: - typedef std::map cacheMap; - - std::vector GetSortedScoreWithTopology() const EXCLUSIVE_LOCKS_REQUIRED(cs); /** From 01d8520038eafa0e00eeddcea29cba2b1b87917e Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Tue, 16 Apr 2024 13:37:46 -0400 Subject: [PATCH 02/26] Remove unused argument to RemoveStaged --- src/txmempool.cpp | 12 ++++++------ src/txmempool.h | 4 +--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index df90683a406..7f259369357 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -198,7 +198,7 @@ void CTxMemPool::Apply(ChangeSet* changeset) AssertLockHeld(cs); m_txgraph->CommitStaging(); - RemoveStaged(changeset->m_to_remove, false, MemPoolRemovalReason::REPLACED); + RemoveStaged(changeset->m_to_remove, MemPoolRemovalReason::REPLACED); for (size_t i=0; im_entry_vec.size(); ++i) { auto tx_entry = changeset->m_entry_vec[i]; @@ -336,7 +336,7 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso CalculateDescendants(it, setAllRemoves); } - RemoveStaged(setAllRemoves, false, reason); + RemoveStaged(setAllRemoves, reason); } void CTxMemPool::removeForReorg(CChain& chain, std::function check_final_and_mature) @@ -354,7 +354,7 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function check for (txiter it : txToRemove) { CalculateDescendants(it, setAllRemoves); } - RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG); + RemoveStaged(setAllRemoves, MemPoolRemovalReason::REORG); for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { assert(TestLockPointValidity(chain, it->GetLockPoints())); } @@ -392,7 +392,7 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne setEntries stage; stage.insert(it); txs_removed_for_block.emplace_back(*it); - RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK); + RemoveStaged(stage, MemPoolRemovalReason::BLOCK); } removeConflicts(*tx); ClearPrioritisation(tx->GetHash()); @@ -744,7 +744,7 @@ void CTxMemPool::RemoveUnbroadcastTx(const Txid& txid, const bool unchecked) { } } -void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) { +void CTxMemPool::RemoveStaged(setEntries &stage, MemPoolRemovalReason reason) { AssertLockHeld(cs); for (txiter it : stage) { removeUnchecked(it, reason); @@ -776,7 +776,7 @@ int CTxMemPool::Expire(std::chrono::seconds time) for (txiter removeit : toremove) { CalculateDescendants(removeit, stage); } - RemoveStaged(stage, false, MemPoolRemovalReason::EXPIRY); + RemoveStaged(stage, MemPoolRemovalReason::EXPIRY); return stage.size(); } diff --git a/src/txmempool.h b/src/txmempool.h index c305d5d448a..064ea9ae8db 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -578,10 +578,8 @@ private: * If a transaction is in this set, then all in-mempool descendants must * also be in the set, unless this transaction is being removed for being * in a block. - * Set updateDescendants to true when removing a tx that was in a block, so - * that any in-mempool descendants have their ancestor state updated. */ - void RemoveStaged(setEntries& stage, bool updateDescendants, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); + void RemoveStaged(setEntries& stage, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Before calling removeUnchecked for a given transaction, * UpdateForRemoveFromMempool must be called on the entire (dependent) set From a5a7905d83dfa8a5173f886f7007132e18b53e3a Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Wed, 5 Feb 2025 09:11:40 -0500 Subject: [PATCH 03/26] Simplify removeRecursive --- src/txmempool.cpp | 52 ++++++++++++++++++++++++++--------------------- src/txmempool.h | 8 ++++++++ 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 7f259369357..fec111fda1c 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -308,35 +308,41 @@ CTxMemPool::txiter CTxMemPool::CalculateDescendants(const CTxMemPoolEntry& entry return mapTx.iterator_to(entry); } +void CTxMemPool::removeRecursive(CTxMemPool::txiter to_remove, MemPoolRemovalReason reason) +{ + AssertLockHeld(cs); + Assume(!m_have_changeset); + auto descendants = m_txgraph->GetDescendants(*to_remove, TxGraph::Level::MAIN); + for (auto tx: descendants) { + removeUnchecked(mapTx.iterator_to(static_cast(*tx)), reason); + } +} + void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason) { // Remove transaction from memory pool AssertLockHeld(cs); Assume(!m_have_changeset); - setEntries txToRemove; - txiter origit = mapTx.find(origTx.GetHash()); - if (origit != mapTx.end()) { - txToRemove.insert(origit); - } else { - // When recursively removing but origTx isn't in the mempool - // be sure to remove any children that are in the pool. This can - // happen during chain re-orgs if origTx isn't re-accepted into - // the mempool for any reason. - for (unsigned int i = 0; i < origTx.vout.size(); i++) { - auto it = mapNextTx.find(COutPoint(origTx.GetHash(), i)); - if (it == mapNextTx.end()) - continue; - txiter nextit = it->second; - assert(nextit != mapTx.end()); - txToRemove.insert(nextit); - } + txiter origit = mapTx.find(origTx.GetHash()); + if (origit != mapTx.end()) { + removeRecursive(origit, reason); + } else { + // When recursively removing but origTx isn't in the mempool + // be sure to remove any descendants that are in the pool. This can + // happen during chain re-orgs if origTx isn't re-accepted into + // the mempool for any reason. + auto iter = mapNextTx.lower_bound(COutPoint(origTx.GetHash(), 0)); + std::vector to_remove; + while (iter != mapNextTx.end() && iter->first->hash == origTx.GetHash()) { + to_remove.emplace_back(&*(iter->second)); + ++iter; } - setEntries setAllRemoves; - for (txiter it : txToRemove) { - CalculateDescendants(it, setAllRemoves); + auto all_removes = m_txgraph->GetDescendantsUnion(to_remove, TxGraph::Level::MAIN); + for (auto ref : all_removes) { + auto tx = mapTx.iterator_to(static_cast(*ref)); + removeUnchecked(tx, reason); } - - RemoveStaged(setAllRemoves, reason); + } } void CTxMemPool::removeForReorg(CChain& chain, std::function check_final_and_mature) @@ -372,7 +378,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) if (Assume(txConflict.GetHash() != tx.GetHash())) { ClearPrioritisation(txConflict.GetHash()); - removeRecursive(txConflict, MemPoolRemovalReason::CONFLICT); + removeRecursive(it->second, MemPoolRemovalReason::CONFLICT); } } } diff --git a/src/txmempool.h b/src/txmempool.h index 064ea9ae8db..e8bead51219 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -320,6 +320,11 @@ public: */ void check(const CCoinsViewCache& active_coins_tip, int64_t spendheight) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** + * Remove a transaction from the mempool along with any descendants. + * If the transaction is not already in the mempool, find any descendants + * and remove them. + */ void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); /** After reorg, filter the entries that would no longer be valid in the next block, and update * the entries' cached LockPoints if needed. The mempool does not have any knowledge of @@ -581,6 +586,9 @@ private: */ void RemoveStaged(setEntries& stage, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); + /* Helper for the public removeRecursive() */ + void removeRecursive(txiter to_remove, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); + /** Before calling removeUnchecked for a given transaction, * UpdateForRemoveFromMempool must be called on the entire (dependent) set * of transactions being removed at the same time. We use each From a3c31dfd71def7ce4414c627261fa4516f943547 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Tue, 4 Feb 2025 16:54:20 -0500 Subject: [PATCH 04/26] scripted-diff: rename AddToMempool -> TryAddToMempool -BEGIN VERIFY SCRIPT- find src/test -type f -exec sed -i 's/AddToMempool/TryAddToMempool/g' {} + find src/bench -type f -exec sed -i 's/AddToMempool/TryAddToMempool/g' {} + -END VERIFY SCRIPT- --- src/bench/blockencodings.cpp | 2 +- src/bench/mempool_ephemeral_spends.cpp | 2 +- src/bench/mempool_eviction.cpp | 2 +- src/bench/mempool_stress.cpp | 2 +- src/bench/rpc_mempool.cpp | 2 +- src/test/blockencodings_tests.cpp | 8 +-- src/test/fuzz/mini_miner.cpp | 2 +- src/test/fuzz/partially_downloaded_block.cpp | 2 +- src/test/fuzz/rbf.cpp | 8 +-- src/test/mempool_tests.cpp | 60 +++++++++---------- src/test/miner_tests.cpp | 62 ++++++++++---------- src/test/miniminer_tests.cpp | 42 ++++++------- src/test/policyestimator_tests.cpp | 12 ++-- src/test/rbf_tests.cpp | 52 ++++++++-------- src/test/txvalidation_tests.cpp | 24 ++++---- src/test/util/txmempool.cpp | 2 +- src/test/util/txmempool.h | 2 +- 17 files changed, 143 insertions(+), 143 deletions(-) diff --git a/src/bench/blockencodings.cpp b/src/bench/blockencodings.cpp index c92ade60d2d..f12d22fae80 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; - AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); + TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), 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 ce17650ee4b..c973c783cbe 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; - AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), + TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, fee, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); } diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index 72d2356a0b0..dcbe1245eb2 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; - AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), + TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, nFee, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); } diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index 5f0a20ecd6c..8bdeb75c46a 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; - AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, det_rand.randrange(10000)+1000, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); + TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), 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 4ad1f0b28a0..069230f0dfb 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; - AddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); + TryAddToMempool(pool, CTxMemPoolEntry(TxGraph::Ref(), 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/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 641617b724e..096449b944f 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -67,7 +67,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) CBlock block(BuildBlockTestCase(rand_ctx)); LOCK2(cs_main, pool.cs); - AddToMempool(pool, entry.FromTx(block.vtx[2])); + TryAddToMempool(pool, entry.FromTx(block.vtx[2])); BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); // Do a simple ShortTxIDs RT @@ -151,7 +151,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) CBlock block(BuildBlockTestCase(rand_ctx)); LOCK2(cs_main, pool.cs); - AddToMempool(pool, entry.FromTx(block.vtx[2])); + TryAddToMempool(pool, entry.FromTx(block.vtx[2])); BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); Txid txhash; @@ -222,7 +222,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) CBlock block(BuildBlockTestCase(rand_ctx)); LOCK2(cs_main, pool.cs); - AddToMempool(pool, entry.FromTx(block.vtx[1])); + TryAddToMempool(pool, entry.FromTx(block.vtx[1])); BOOST_CHECK_EQUAL(pool.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); Txid txhash; @@ -322,7 +322,7 @@ BOOST_AUTO_TEST_CASE(ReceiveWithExtraTransactions) { extra_txn.resize(10); LOCK2(cs_main, pool.cs); - AddToMempool(pool, entry.FromTx(block.vtx[2])); + TryAddToMempool(pool, entry.FromTx(block.vtx[2])); BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); // Ensure the non_block_tx is actually not in the block for (const auto &block_tx : block.vtx) { diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp index 226c47d827e..6a5c164985a 100644 --- a/src/test/fuzz/mini_miner.cpp +++ b/src/test/fuzz/mini_miner.cpp @@ -67,7 +67,7 @@ FUZZ_TARGET(mini_miner, .init = initialize_miner) TestMemPoolEntryHelper entry; const CAmount fee{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/100000)}; assert(MoneyRange(fee)); - AddToMempool(pool, entry.Fee(fee).FromTx(tx)); + TryAddToMempool(pool, entry.Fee(fee).FromTx(tx)); // All outputs are available to spend for (uint32_t n{0}; n < num_outputs; ++n) { diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp index a39b51d71f9..41e7c9060d0 100644 --- a/src/test/fuzz/partially_downloaded_block.cpp +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -81,7 +81,7 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb) if (add_to_mempool && !pool.exists(tx->GetHash())) { LOCK2(cs_main, pool.cs); - AddToMempool(pool, ConsumeTxMemPoolEntry(fuzzed_data_provider, *tx)); + TryAddToMempool(pool, ConsumeTxMemPoolEntry(fuzzed_data_provider, *tx)); available.insert(i); } } diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 02ec3cb0e8a..a2a345ab168 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -75,14 +75,14 @@ FUZZ_TARGET(rbf, .init = initialize_rbf) } LOCK2(cs_main, pool.cs); if (!pool.GetIter(another_tx.GetHash())) { - AddToMempool(pool, ConsumeTxMemPoolEntry(fuzzed_data_provider, another_tx)); + TryAddToMempool(pool, ConsumeTxMemPoolEntry(fuzzed_data_provider, another_tx)); } } const CTransaction tx{*mtx}; if (fuzzed_data_provider.ConsumeBool()) { LOCK2(cs_main, pool.cs); if (!pool.GetIter(tx.GetHash())) { - AddToMempool(pool, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx)); + TryAddToMempool(pool, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx)); } } { @@ -143,7 +143,7 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) break; } assert(!pool.GetIter(parent_entry.GetTx().GetHash())); - AddToMempool(pool, parent_entry); + TryAddToMempool(pool, parent_entry); // It's possible that adding this to the mempool failed due to cluster // size limits; if so bail out. @@ -162,7 +162,7 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) break; } if (!pool.GetIter(child_entry.GetTx().GetHash())) { - AddToMempool(pool, child_entry); + TryAddToMempool(pool, child_entry); // Adding this transaction to the mempool may fail due to cluster // size limits; if so bail out. if(!pool.GetIter(child_entry.GetTx().GetHash())) { diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index d0cbdea717e..f618640686b 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -72,17 +72,17 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) BOOST_CHECK_EQUAL(testPool.size(), poolSize); // Just the parent: - AddToMempool(testPool, entry.FromTx(txParent)); + TryAddToMempool(testPool, entry.FromTx(txParent)); poolSize = testPool.size(); testPool.removeRecursive(CTransaction(txParent), REMOVAL_REASON_DUMMY); BOOST_CHECK_EQUAL(testPool.size(), poolSize - 1); // Parent, children, grandchildren: - AddToMempool(testPool, entry.FromTx(txParent)); + TryAddToMempool(testPool, entry.FromTx(txParent)); for (int i = 0; i < 3; i++) { - AddToMempool(testPool, entry.FromTx(txChild[i])); - AddToMempool(testPool, entry.FromTx(txGrandChild[i])); + TryAddToMempool(testPool, entry.FromTx(txChild[i])); + TryAddToMempool(testPool, entry.FromTx(txGrandChild[i])); } // Remove Child[0], GrandChild[0] should be removed: poolSize = testPool.size(); @@ -104,8 +104,8 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) // Add children and grandchildren, but NOT the parent (simulate the parent being in a block) for (int i = 0; i < 3; i++) { - AddToMempool(testPool, entry.FromTx(txChild[i])); - AddToMempool(testPool, entry.FromTx(txGrandChild[i])); + TryAddToMempool(testPool, entry.FromTx(txChild[i])); + TryAddToMempool(testPool, entry.FromTx(txGrandChild[i])); } // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be // put into the mempool (maybe because it is non-standard): @@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx1.vout.resize(1); tx1.vout[0].scriptPubKey = CScript() << OP_1 << OP_EQUAL; tx1.vout[0].nValue = 10 * COIN; - AddToMempool(pool, entry.Fee(1000LL).FromTx(tx1)); + TryAddToMempool(pool, entry.Fee(1000LL).FromTx(tx1)); CMutableTransaction tx2 = CMutableTransaction(); tx2.vin.resize(1); @@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx2.vout.resize(1); tx2.vout[0].scriptPubKey = CScript() << OP_2 << OP_EQUAL; tx2.vout[0].nValue = 10 * COIN; - AddToMempool(pool, entry.Fee(500LL).FromTx(tx2)); + TryAddToMempool(pool, entry.Fee(500LL).FromTx(tx2)); pool.TrimToSize(pool.DynamicMemoryUsage()); // should do nothing BOOST_CHECK(pool.exists(tx1.GetHash())); @@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(pool.exists(tx1.GetHash())); BOOST_CHECK(!pool.exists(tx2.GetHash())); - AddToMempool(pool, entry.FromTx(tx2)); + TryAddToMempool(pool, entry.FromTx(tx2)); CMutableTransaction tx3 = CMutableTransaction(); tx3.vin.resize(1); tx3.vin[0].prevout = COutPoint(tx2.GetHash(), 0); @@ -153,7 +153,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx3.vout.resize(1); tx3.vout[0].scriptPubKey = CScript() << OP_3 << OP_EQUAL; tx3.vout[0].nValue = 10 * COIN; - AddToMempool(pool, entry.Fee(2000LL).FromTx(tx3)); + TryAddToMempool(pool, entry.Fee(2000LL).FromTx(tx3)); pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4); // tx3 should pay for tx2 (CPFP) BOOST_CHECK(!pool.exists(tx1.GetHash())); @@ -216,11 +216,11 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) tx7.vout[1].scriptPubKey = CScript() << OP_7 << OP_EQUAL; tx7.vout[1].nValue = 10 * COIN; - AddToMempool(pool, entry.Fee(700LL).FromTx(tx4)); + TryAddToMempool(pool, entry.Fee(700LL).FromTx(tx4)); auto usage_with_tx4_only = pool.DynamicMemoryUsage(); - AddToMempool(pool, entry.Fee(100LL).FromTx(tx5)); - AddToMempool(pool, entry.Fee(110LL).FromTx(tx6)); - AddToMempool(pool, entry.Fee(900LL).FromTx(tx7)); + TryAddToMempool(pool, entry.Fee(100LL).FromTx(tx5)); + TryAddToMempool(pool, entry.Fee(110LL).FromTx(tx6)); + TryAddToMempool(pool, entry.Fee(900LL).FromTx(tx7)); // From the topology above, tx7 must be sorted last, so it should // definitely evicted first if we must trim. tx4 should definitely remain @@ -234,10 +234,10 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) // tx7, but this behavior need not be guaranteed. if (!pool.exists(tx5.GetHash())) - AddToMempool(pool, entry.Fee(100LL).FromTx(tx5)); + TryAddToMempool(pool, entry.Fee(100LL).FromTx(tx5)); if (!pool.exists(tx6.GetHash())) - AddToMempool(pool, entry.Fee(110LL).FromTx(tx6)); - AddToMempool(pool, entry.Fee(900LL).FromTx(tx7)); + TryAddToMempool(pool, entry.Fee(110LL).FromTx(tx6)); + TryAddToMempool(pool, entry.Fee(900LL).FromTx(tx7)); // If we trim sufficiently, everything but tx4 should be removed. pool.TrimToSize(usage_with_tx4_only + 1); @@ -246,9 +246,9 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) BOOST_CHECK(!pool.exists(tx6.GetHash())); BOOST_CHECK(!pool.exists(tx7.GetHash())); - AddToMempool(pool, entry.Fee(100LL).FromTx(tx5)); - AddToMempool(pool, entry.Fee(110LL).FromTx(tx6)); - AddToMempool(pool, entry.Fee(900LL).FromTx(tx7)); + TryAddToMempool(pool, entry.Fee(100LL).FromTx(tx5)); + TryAddToMempool(pool, entry.Fee(110LL).FromTx(tx6)); + TryAddToMempool(pool, entry.Fee(900LL).FromTx(tx7)); std::vector vtx; SetMockTime(42); @@ -307,7 +307,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) // [tx1] // CTransactionRef tx1 = make_tx(/*output_values=*/{10 * COIN}); - AddToMempool(pool, entry.Fee(10000LL).FromTx(tx1)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(tx1)); // Ancestors / clustersize should be 1 / 1 (itself / itself) pool.GetTransactionAncestry(tx1->GetHash(), ancestors, clustersize); @@ -319,7 +319,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) // [tx1].0 <- [tx2] // CTransactionRef tx2 = make_tx(/*output_values=*/{495 * CENT, 5 * COIN}, /*inputs=*/{tx1}); - AddToMempool(pool, entry.Fee(10000LL).FromTx(tx2)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(tx2)); // Ancestors / clustersize should be: // transaction ancestors clustersize @@ -338,7 +338,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) // [tx1].0 <- [tx2].0 <- [tx3] // CTransactionRef tx3 = make_tx(/*output_values=*/{290 * CENT, 200 * CENT}, /*inputs=*/{tx2}); - AddToMempool(pool, entry.Fee(10000LL).FromTx(tx3)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(tx3)); // Ancestors / clustersize should be: // transaction ancestors clustersize @@ -363,7 +363,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) // \---1 <- [tx4] // CTransactionRef tx4 = make_tx(/*output_values=*/{290 * CENT, 250 * CENT}, /*inputs=*/{tx2}, /*input_indices=*/{1}); - AddToMempool(pool, entry.Fee(10000LL).FromTx(tx4)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(tx4)); // Ancestors / clustersize should be: // transaction ancestors clustersize @@ -400,13 +400,13 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests) CTransactionRef& tyi = *ty[i]; tyi = make_tx(/*output_values=*/{v}, /*inputs=*/i > 0 ? std::vector{*ty[i - 1]} : std::vector{}); v -= 50 * CENT; - AddToMempool(pool, entry.Fee(10000LL).FromTx(tyi)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(tyi)); pool.GetTransactionAncestry(tyi->GetHash(), ancestors, clustersize); BOOST_CHECK_EQUAL(ancestors, i+1); BOOST_CHECK_EQUAL(clustersize, i+1); } CTransactionRef ty6 = make_tx(/*output_values=*/{5 * COIN}, /*inputs=*/{tx3, ty5}); - AddToMempool(pool, entry.Fee(10000LL).FromTx(ty6)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(ty6)); // Ancestors / clustersize should be: // transaction ancestors clustersize @@ -472,10 +472,10 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTestsDiamond) tb = make_tx(/*output_values=*/{5 * COIN, 3 * COIN}, /*inputs=*/ {ta}); tc = make_tx(/*output_values=*/{2 * COIN}, /*inputs=*/{tb}, /*input_indices=*/{1}); td = make_tx(/*output_values=*/{6 * COIN}, /*inputs=*/{tb, tc}, /*input_indices=*/{0, 0}); - AddToMempool(pool, entry.Fee(10000LL).FromTx(ta)); - AddToMempool(pool, entry.Fee(10000LL).FromTx(tb)); - AddToMempool(pool, entry.Fee(10000LL).FromTx(tc)); - AddToMempool(pool, entry.Fee(10000LL).FromTx(td)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(ta)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(tb)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(tc)); + TryAddToMempool(pool, entry.Fee(10000LL).FromTx(td)); // Ancestors / descendants should be: // transaction ancestors descendants diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 3b1472199d1..b148c806db8 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -149,21 +149,21 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const // This tx has a low fee: 1000 satoshis Txid hashParentTx = tx.GetHash(); // save this txid for later use const auto parent_tx{entry.Fee(1000).Time(Now()).SpendsCoinbase(true).FromTx(tx)}; - AddToMempool(tx_mempool, parent_tx); + TryAddToMempool(tx_mempool, parent_tx); // This tx has a medium fee: 10000 satoshis tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vout[0].nValue = 5000000000LL - 10000; Txid hashMediumFeeTx = tx.GetHash(); const auto medium_fee_tx{entry.Fee(10000).Time(Now()).SpendsCoinbase(true).FromTx(tx)}; - AddToMempool(tx_mempool, medium_fee_tx); + TryAddToMempool(tx_mempool, medium_fee_tx); // This tx has a high fee, but depends on the first transaction tx.vin[0].prevout.hash = hashParentTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 50k satoshi fee Txid hashHighFeeTx = tx.GetHash(); const auto high_fee_tx{entry.Fee(50000).Time(Now()).SpendsCoinbase(false).FromTx(tx)}; - AddToMempool(tx_mempool, high_fee_tx); + TryAddToMempool(tx_mempool, high_fee_tx); block_template = mining->createNewBlock(options); BOOST_REQUIRE(block_template); @@ -192,7 +192,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const tx.vin[0].prevout.hash = hashHighFeeTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 0 fee Txid hashFreeTx = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(0).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(0).FromTx(tx)); uint64_t freeTxSize{::GetSerializeSize(TX_WITH_WITNESS(tx))}; // Calculate a fee on child transaction that will put the package just @@ -202,7 +202,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const tx.vin[0].prevout.hash = hashFreeTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000 - feeToUse; Txid hashLowFeeTx = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(feeToUse).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(feeToUse).FromTx(tx)); // waitNext() should return nullptr because there is no better template should_be_nullptr = block_template->waitNext({.timeout = MillisecondsDouble{0}, .fee_threshold = 1}); @@ -221,7 +221,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const tx_mempool.removeRecursive(CTransaction(tx), MemPoolRemovalReason::REPLACED); tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee hashLowFeeTx = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(feeToUse + 2).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(feeToUse + 2).FromTx(tx)); // waitNext() should return if fees for the new template are at least 1 sat up block_template = block_template->waitNext({.fee_threshold = 1}); @@ -243,7 +243,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const // hashFreeTx2 + hashLowFeeTx2. BulkTransaction(tx, 4000); Txid hashFreeTx2 = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); // This tx can't be mined by itself tx.vin[0].prevout.hash = hashFreeTx2; @@ -251,7 +251,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const feeToUse = blockMinFeeRate.GetFee(freeTxSize); tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse; Txid hashLowFeeTx2 = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); block_template = mining->createNewBlock(options); BOOST_REQUIRE(block_template); block = block_template->getBlock(); @@ -266,7 +266,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const // as well. tx.vin[0].prevout.n = 1; tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee - AddToMempool(tx_mempool, entry.Fee(10000).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(10000).FromTx(tx)); block_template = mining->createNewBlock(options); BOOST_REQUIRE(block_template); block = block_template->getBlock(); @@ -350,7 +350,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: for (auto& t : txs) { // If we don't set the number of sigops in the CTxMemPoolEntry, // template creation fails during sanity checks. - AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(true).FromTx(t)); + TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(true).FromTx(t)); legacy_sigops += GetLegacySigOpCount(*t); BOOST_CHECK(tx_mempool.GetIter(t->GetHash()).has_value()); } @@ -375,7 +375,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: int64_t legacy_sigops = 0; for (auto& t : txs) { - AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(true).SigOpsCost(GetLegacySigOpCount(*t)*WITNESS_SCALE_FACTOR).FromTx(t)); + TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(true).SigOpsCost(GetLegacySigOpCount(*t)*WITNESS_SCALE_FACTOR).FromTx(t)); legacy_sigops += GetLegacySigOpCount(*t); BOOST_CHECK(tx_mempool.GetIter(t->GetHash()).has_value()); } @@ -408,7 +408,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vout[0].nValue -= LOWFEE; hash = tx.GetHash(); bool spendsCoinbase = i == 0; // only first tx spends coinbase - AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); BOOST_CHECK(tx_mempool.GetIter(hash).has_value()); tx.vin[0].prevout.hash = hash; } @@ -421,7 +421,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: // orphan in tx_mempool, template creation fails hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).FromTx(tx)); BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); } @@ -434,7 +434,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE; hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); tx.vin[0].prevout.hash = hash; tx.vin.resize(2); tx.vin[1].scriptSig = CScript() << OP_1; @@ -442,7 +442,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vin[1].prevout.n = 0; tx.vout[0].nValue = tx.vout[0].nValue + BLOCKSUBSIDY - HIGHERFEE; // First txn output + fresh coinbase - new txn fee hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(HIGHERFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(HIGHERFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); BOOST_REQUIRE(mining->createNewBlock(options)); } @@ -457,7 +457,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vout[0].nValue = 0; hash = tx.GetHash(); // give it a fee so it'll get mined - AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); // Should throw bad-cb-multiple BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-cb-multiple")); } @@ -472,10 +472,10 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE; tx.vout[0].scriptPubKey = CScript() << OP_1; hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); tx.vout[0].scriptPubKey = CScript() << OP_2; hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); } @@ -518,12 +518,12 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: CScript script = CScript() << OP_0; tx.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(script)); hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); tx.vin[0].prevout.hash = hash; tx.vin[0].scriptSig = CScript() << std::vector(script.begin(), script.end()); tx.vout[0].nValue -= LOWFEE; hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now()).SpendsCoinbase(false).FromTx(tx)); BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("block-script-verify-flag-failed")); // Delete the dummy blocks again. @@ -559,7 +559,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vout[0].scriptPubKey = CScript() << OP_1; tx.nLockTime = 0; hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail @@ -573,7 +573,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | (((m_node.chainman->ActiveChain().Tip()->GetMedianTimePast()+1-m_node.chainman->ActiveChain()[1]->GetMedianTimePast()) >> CTxIn::SEQUENCE_LOCKTIME_GRANULARITY) + 1); // txFirst[1] is the 3rd block prevheights[0] = baseheight + 2; hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Time(Now()).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Time(Now()).FromTx(tx)); BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail @@ -596,7 +596,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: prevheights[0] = baseheight + 3; tx.nLockTime = m_node.chainman->ActiveChain().Tip()->nHeight + 1; hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Time(Now()).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Time(Now()).FromTx(tx)); BOOST_CHECK(!CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime fails BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // 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 @@ -611,7 +611,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: prevheights.resize(1); prevheights[0] = baseheight + 4; hash = tx.GetHash(); - AddToMempool(tx_mempool, entry.Time(Now()).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Time(Now()).FromTx(tx)); BOOST_CHECK(!CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime fails BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // 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 @@ -675,7 +675,7 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const tx.vout.resize(1); tx.vout[0].nValue = 5000000000LL; // 0 fee Txid hashFreePrioritisedTx = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(0).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(0).Time(Now()).SpendsCoinbase(true).FromTx(tx)); tx_mempool.PrioritiseTransaction(hashFreePrioritisedTx, 5 * COIN); tx.vin[0].prevout.hash = txFirst[1]->GetHash(); @@ -683,20 +683,20 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const tx.vout[0].nValue = 5000000000LL - 1000; // This tx has a low fee: 1000 satoshis Txid hashParentTx = tx.GetHash(); // save this txid for later use - AddToMempool(tx_mempool, entry.Fee(1000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(1000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); // This tx has a medium fee: 10000 satoshis tx.vin[0].prevout.hash = txFirst[2]->GetHash(); tx.vout[0].nValue = 5000000000LL - 10000; Txid hashMediumFeeTx = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(10000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(10000).Time(Now()).SpendsCoinbase(true).FromTx(tx)); tx_mempool.PrioritiseTransaction(hashMediumFeeTx, -5 * COIN); // This tx also has a low fee, but is prioritised tx.vin[0].prevout.hash = hashParentTx; tx.vout[0].nValue = 5000000000LL - 1000 - 1000; // 1000 satoshi fee Txid hashPrioritsedChild = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(1000).Time(Now()).SpendsCoinbase(false).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(1000).Time(Now()).SpendsCoinbase(false).FromTx(tx)); tx_mempool.PrioritiseTransaction(hashPrioritsedChild, 2 * COIN); // Test that transaction selection properly updates ancestor fee calculations as prioritised @@ -708,19 +708,19 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const tx.vin[0].prevout.hash = txFirst[3]->GetHash(); tx.vout[0].nValue = 5000000000LL; // 0 fee Txid hashFreeParent = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(true).FromTx(tx)); tx_mempool.PrioritiseTransaction(hashFreeParent, 10 * COIN); tx.vin[0].prevout.hash = hashFreeParent; tx.vout[0].nValue = 5000000000LL; // 0 fee Txid hashFreeChild = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); tx_mempool.PrioritiseTransaction(hashFreeChild, 1 * COIN); tx.vin[0].prevout.hash = hashFreeChild; tx.vout[0].nValue = 5000000000LL; // 0 fee Txid hashFreeGrandchild = tx.GetHash(); - AddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); + TryAddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(false).FromTx(tx)); auto block_template = mining->createNewBlock(options); BOOST_REQUIRE(block_template); diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index 45c932c1f03..95b83dfa655 100644 --- a/src/test/miniminer_tests.cpp +++ b/src/test/miniminer_tests.cpp @@ -84,7 +84,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup) const CAmount negative_modified_fees{positive_base_fee + negative_fee_delta}; BOOST_CHECK(negative_modified_fees < 0); const auto tx_mod_negative = make_tx({COutPoint{m_coinbase_txns[4]->GetHash(), 0}}, /*num_outputs=*/1); - AddToMempool(pool, entry.Fee(positive_base_fee).FromTx(tx_mod_negative)); + TryAddToMempool(pool, entry.Fee(positive_base_fee).FromTx(tx_mod_negative)); pool.PrioritiseTransaction(tx_mod_negative->GetHash(), negative_fee_delta); const COutPoint only_outpoint{tx_mod_negative->GetHash(), 0}; @@ -114,21 +114,21 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) // Create a parent tx0 and child tx1 with normal fees: const auto tx0 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(med_fee).FromTx(tx0)); + TryAddToMempool(pool, entry.Fee(med_fee).FromTx(tx0)); const auto tx1 = make_tx({COutPoint{tx0->GetHash(), 0}}, /*num_outputs=*/1); - AddToMempool(pool, entry.Fee(med_fee).FromTx(tx1)); + TryAddToMempool(pool, entry.Fee(med_fee).FromTx(tx1)); // Create a low-feerate parent tx2 and high-feerate child tx3 (cpfp) const auto tx2 = make_tx({COutPoint{m_coinbase_txns[1]->GetHash(), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx2)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx2)); const auto tx3 = make_tx({COutPoint{tx2->GetHash(), 0}}, /*num_outputs=*/1); - AddToMempool(pool, entry.Fee(high_fee).FromTx(tx3)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(tx3)); // Create a parent tx4 and child tx5 where both have low fees const auto tx4 = make_tx({COutPoint{m_coinbase_txns[2]->GetHash(), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx4)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx4)); const auto tx5 = make_tx({COutPoint{tx4->GetHash(), 0}}, /*num_outputs=*/1); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx5)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx5)); const CAmount tx5_delta{CENT/100}; // Make tx5's modified fee much higher than its base fee. This should cause it to pass // the fee-related checks despite being low-feerate. @@ -137,9 +137,9 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) // Create a high-feerate parent tx6, low-feerate child tx7 const auto tx6 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(high_fee).FromTx(tx6)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(tx6)); const auto tx7 = make_tx({COutPoint{tx6->GetHash(), 0}}, /*num_outputs=*/1); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx7)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx7)); std::vector all_unspent_outpoints({ COutPoint{tx0->GetHash(), 1}, @@ -405,23 +405,23 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup) // Create 3 parents of different feerates, and 1 child spending outputs from all 3 parents. const auto tx0 = make_tx({COutPoint{m_coinbase_txns[0]->GetHash(), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx0)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx0)); const auto tx1 = make_tx({COutPoint{m_coinbase_txns[1]->GetHash(), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(med_fee).FromTx(tx1)); + TryAddToMempool(pool, entry.Fee(med_fee).FromTx(tx1)); const auto tx2 = make_tx({COutPoint{m_coinbase_txns[2]->GetHash(), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(high_fee).FromTx(tx2)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(tx2)); const auto tx3 = make_tx({COutPoint{tx0->GetHash(), 0}, COutPoint{tx1->GetHash(), 0}, COutPoint{tx2->GetHash(), 0}}, /*num_outputs=*/3); - AddToMempool(pool, entry.Fee(high_fee).FromTx(tx3)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(tx3)); // Create 1 grandparent and 1 parent, then 2 children. const auto tx4 = make_tx({COutPoint{m_coinbase_txns[3]->GetHash(), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(high_fee).FromTx(tx4)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(tx4)); const auto tx5 = make_tx({COutPoint{tx4->GetHash(), 0}}, /*num_outputs=*/3); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx5)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx5)); const auto tx6 = make_tx({COutPoint{tx5->GetHash(), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(med_fee).FromTx(tx6)); + TryAddToMempool(pool, entry.Fee(med_fee).FromTx(tx6)); const auto tx7 = make_tx({COutPoint{tx5->GetHash(), 1}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(high_fee).FromTx(tx7)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(tx7)); std::vector all_transactions{tx0, tx1, tx2, tx3, tx4, tx5, tx6, tx7}; std::vector tx_vsizes; @@ -608,7 +608,7 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup) lasttx = m_coinbase_txns[cluster_count]; for (auto i{0}; i < 50; ++i) { const auto tx = make_tx({COutPoint{lasttx->GetHash(), 0}}, /*num_outputs=*/1); - AddToMempool(pool, entry.Fee(CENT).FromTx(tx)); + TryAddToMempool(pool, entry.Fee(CENT).FromTx(tx)); chain_txids.push_back(tx->GetHash()); lasttx = tx; } @@ -622,7 +622,7 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup) // GatherClusters stops at 500 transactions. const auto tx_501 = make_tx({COutPoint{lasttx->GetHash(), 0}}, /*num_outputs=*/1); - AddToMempool(pool, entry.Fee(CENT).FromTx(tx_501)); + TryAddToMempool(pool, entry.Fee(CENT).FromTx(tx_501)); const auto cluster_501 = pool.GatherClusters(last_txs); BOOST_CHECK_EQUAL(cluster_501.size(), 0); @@ -635,12 +635,12 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup) std::vector zigzag_txids; for (auto p{0}; p < 32; ++p) { const auto txp = make_tx({COutPoint{Txid::FromUint256(GetRandHash()), 0}}, /*num_outputs=*/2); - AddToMempool(pool, entry.Fee(CENT).FromTx(txp)); + TryAddToMempool(pool, entry.Fee(CENT).FromTx(txp)); zigzag_txids.push_back(txp->GetHash()); } for (auto c{0}; c < 31; ++c) { const auto txc = make_tx({COutPoint{zigzag_txids[c], 1}, COutPoint{zigzag_txids[c+1], 0}}, /*num_outputs=*/1); - AddToMempool(pool, entry.Fee(CENT).FromTx(txc)); + TryAddToMempool(pool, entry.Fee(CENT).FromTx(txc)); zigzag_txids.push_back(txc->GetHash()); } const auto vec_iters_zigzag = pool.GetIterVec(zigzag_txids); diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 0e1bfec24a8..d13a0d1ebf6 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -63,9 +63,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) tx.vin[0].prevout.n = 10000*blocknum+100*j+k; // make transaction unique { LOCK2(cs_main, mpool.cs); - AddToMempool(mpool, entry.Fee(feeV[j]).Time(Now()).Height(blocknum).FromTx(tx)); + TryAddToMempool(mpool, entry.Fee(feeV[j]).Time(Now()).Height(blocknum).FromTx(tx)); // Since TransactionAddedToMempool callbacks are generated in ATMP, - // not AddToMempool, we cheat and create one manually here + // not TryAddToMempool, we cheat and create one manually here const int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx)); const NewMempoolTransactionInfo tx_info{NewMempoolTransactionInfo(MakeTransactionRef(tx), feeV[j], @@ -163,9 +163,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) tx.vin[0].prevout.n = 10000*blocknum+100*j+k; { LOCK2(cs_main, mpool.cs); - AddToMempool(mpool, entry.Fee(feeV[j]).Time(Now()).Height(blocknum).FromTx(tx)); + TryAddToMempool(mpool, entry.Fee(feeV[j]).Time(Now()).Height(blocknum).FromTx(tx)); // Since TransactionAddedToMempool callbacks are generated in ATMP, - // not AddToMempool, we cheat and create one manually here + // not TryAddToMempool, we cheat and create one manually here const int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx)); const NewMempoolTransactionInfo tx_info{NewMempoolTransactionInfo(MakeTransactionRef(tx), feeV[j], @@ -226,9 +226,9 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) tx.vin[0].prevout.n = 10000*blocknum+100*j+k; { LOCK2(cs_main, mpool.cs); - AddToMempool(mpool, entry.Fee(feeV[j]).Time(Now()).Height(blocknum).FromTx(tx)); + TryAddToMempool(mpool, entry.Fee(feeV[j]).Time(Now()).Height(blocknum).FromTx(tx)); // Since TransactionAddedToMempool callbacks are generated in ATMP, - // not AddToMempool, we cheat and create one manually here + // not TryAddToMempool, we cheat and create one manually here const int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx)); const NewMempoolTransactionInfo tx_info{NewMempoolTransactionInfo(MakeTransactionRef(tx), feeV[j], diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index 45dd8bcb120..4527ea5e8fd 100644 --- a/src/test/rbf_tests.cpp +++ b/src/test/rbf_tests.cpp @@ -47,7 +47,7 @@ static CTransactionRef add_descendants(const CTransactionRef& tx, int32_t num_de auto tx_to_spend = tx; for (int32_t i{0}; i < num_descendants; ++i) { auto next_tx = make_tx(/*inputs=*/{tx_to_spend}, /*output_values=*/{(50 - i) * CENT}); - AddToMempool(pool, entry.FromTx(next_tx)); + TryAddToMempool(pool, entry.FromTx(next_tx)); BOOST_CHECK(pool.GetIter(next_tx->GetHash()).has_value()); tx_to_spend = next_tx; } @@ -67,40 +67,40 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup) // Create a parent tx1 and child tx2 with normal fees: const auto tx1 = make_tx(/*inputs=*/ {m_coinbase_txns[0]}, /*output_values=*/ {10 * COIN}); - AddToMempool(pool, entry.Fee(normal_fee).FromTx(tx1)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(tx1)); const auto tx2 = make_tx(/*inputs=*/ {tx1}, /*output_values=*/ {995 * CENT}); - AddToMempool(pool, entry.Fee(normal_fee).FromTx(tx2)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(tx2)); // Create a low-feerate parent tx3 and high-feerate child tx4 (cpfp) const auto tx3 = make_tx(/*inputs=*/ {m_coinbase_txns[1]}, /*output_values=*/ {1099 * CENT}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx3)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx3)); const auto tx4 = make_tx(/*inputs=*/ {tx3}, /*output_values=*/ {999 * CENT}); - AddToMempool(pool, entry.Fee(high_fee).FromTx(tx4)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(tx4)); // Create a parent tx5 and child tx6 where both have very low fees const auto tx5 = make_tx(/*inputs=*/ {m_coinbase_txns[2]}, /*output_values=*/ {1099 * CENT}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx5)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx5)); const auto tx6 = make_tx(/*inputs=*/ {tx5}, /*output_values=*/ {1098 * CENT}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx6)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx6)); // Make tx6's modified fee much higher than its base fee. This should cause it to pass // the fee-related checks despite being low-feerate. pool.PrioritiseTransaction(tx6->GetHash(), 1 * COIN); // Two independent high-feerate transactions, tx7 and tx8 const auto tx7 = make_tx(/*inputs=*/ {m_coinbase_txns[3]}, /*output_values=*/ {999 * CENT}); - AddToMempool(pool, entry.Fee(high_fee).FromTx(tx7)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(tx7)); const auto tx8 = make_tx(/*inputs=*/ {m_coinbase_txns[4]}, /*output_values=*/ {999 * CENT}); - AddToMempool(pool, entry.Fee(high_fee).FromTx(tx8)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(tx8)); // Will make these two parents of single child const auto tx11 = make_tx(/*inputs=*/ {m_coinbase_txns[7]}, /*output_values=*/ {995 * CENT}); - AddToMempool(pool, entry.Fee(normal_fee).FromTx(tx11)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(tx11)); const auto tx12 = make_tx(/*inputs=*/ {m_coinbase_txns[8]}, /*output_values=*/ {995 * CENT}); - AddToMempool(pool, entry.Fee(normal_fee).FromTx(tx12)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(tx12)); // Will make two children of this single parent const auto tx13 = make_tx(/*inputs=*/ {m_coinbase_txns[9]}, /*output_values=*/ {995 * CENT, 995 * CENT}); - AddToMempool(pool, entry.Fee(normal_fee).FromTx(tx13)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(tx13)); const auto entry1_normal = pool.GetIter(tx1->GetHash()).value(); const auto entry2_normal = pool.GetIter(tx2->GetHash()).value(); @@ -179,8 +179,8 @@ BOOST_FIXTURE_TEST_CASE(rbf_conflicts_calculator, TestChain100Setup) const auto parent_tx_1 = make_tx(/*inputs=*/ {m_coinbase_txns[0]}, /*output_values=*/ output_values); const auto parent_tx_2 = make_tx(/*inputs=*/ {m_coinbase_txns[1]}, /*output_values=*/ output_values); - AddToMempool(pool, entry.Fee(normal_fee).FromTx(parent_tx_1)); - AddToMempool(pool, entry.Fee(normal_fee).FromTx(parent_tx_2)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(parent_tx_1)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(parent_tx_2)); std::vector direct_children; @@ -190,7 +190,7 @@ BOOST_FIXTURE_TEST_CASE(rbf_conflicts_calculator, TestChain100Setup) auto pretx = make_tx(/*inputs=*/ {parent_tx}, /*output_values=*/ {995 * CENT}); CMutableTransaction tx(*pretx); tx.vin[0].prevout.n = i; - AddToMempool(pool, entry.Fee(normal_fee).FromTx(tx)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(tx)); BOOST_CHECK(pool.GetIter(tx.GetHash()).has_value()); direct_children.push_back(MakeTransactionRef(tx)); } @@ -257,9 +257,9 @@ BOOST_FIXTURE_TEST_CASE(improves_feerate, TestChain100Setup) // low feerate parent with normal feerate child const auto tx1 = make_tx(/*inputs=*/ {m_coinbase_txns[0], m_coinbase_txns[1]}, /*output_values=*/ {10 * COIN}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(tx1)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(tx1)); const auto tx2 = make_tx(/*inputs=*/ {tx1}, /*output_values=*/ {995 * CENT}); - AddToMempool(pool, entry.Fee(normal_fee).FromTx(tx2)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(tx2)); const auto entry1 = pool.GetIter(tx1->GetHash()).value(); const auto tx1_fee = entry1->GetModifiedFee(); @@ -323,7 +323,7 @@ BOOST_FIXTURE_TEST_CASE(improves_feerate, TestChain100Setup) // Adding a grandchild makes the cluster size 3, which is also calculable const auto tx5 = make_tx(/*inputs=*/ {tx2}, /*output_values=*/ {995 * CENT}); - AddToMempool(pool, entry.Fee(normal_fee).FromTx(tx5)); + TryAddToMempool(pool, entry.Fee(normal_fee).FromTx(tx5)); const auto entry5 = pool.GetIter(tx5->GetHash()).value(); changeset = pool.GetChangeSet(); @@ -348,7 +348,7 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) // low -> high -> medium fee transactions that would result in two chunks together since they // are all same size const auto low_tx = make_tx(/*inputs=*/ {m_coinbase_txns[0]}, /*output_values=*/ {10 * COIN}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(low_tx)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(low_tx)); const auto entry_low = pool.GetIter(low_tx->GetHash()).value(); const auto low_size = entry_low->GetAdjustedWeight(); @@ -384,7 +384,7 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) // Add a second transaction to the cluster that will make a single chunk, to be evicted in the RBF const auto high_tx = make_tx(/*inputs=*/ {low_tx}, /*output_values=*/ {995 * CENT}); - AddToMempool(pool, entry.Fee(high_fee).FromTx(high_tx)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(high_tx)); const auto entry_high = pool.GetIter(high_tx->GetHash()).value(); const auto high_size = entry_high->GetAdjustedWeight(); @@ -416,12 +416,12 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) // Make a size 2 cluster that is itself two chunks; evict both txns const auto high_tx_2 = make_tx(/*inputs=*/ {m_coinbase_txns[1]}, /*output_values=*/ {10 * COIN}); - AddToMempool(pool, entry.Fee(high_fee).FromTx(high_tx_2)); + TryAddToMempool(pool, entry.Fee(high_fee).FromTx(high_tx_2)); const auto entry_high_2 = pool.GetIter(high_tx_2->GetHash()).value(); const auto high_size_2 = entry_high_2->GetAdjustedWeight(); const auto low_tx_2 = make_tx(/*inputs=*/ {high_tx_2}, /*output_values=*/ {9 * COIN}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(low_tx_2)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(low_tx_2)); const auto entry_low_2 = pool.GetIter(low_tx_2->GetHash()).value(); const auto low_size_2 = entry_low_2->GetAdjustedWeight(); @@ -440,15 +440,15 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) // You can have more than two direct conflicts const auto conflict_1 = make_tx(/*inputs=*/ {m_coinbase_txns[2]}, /*output_values=*/ {10 * COIN}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(conflict_1)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(conflict_1)); const auto conflict_1_entry = pool.GetIter(conflict_1->GetHash()).value(); const auto conflict_2 = make_tx(/*inputs=*/ {m_coinbase_txns[3]}, /*output_values=*/ {10 * COIN}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(conflict_2)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(conflict_2)); const auto conflict_2_entry = pool.GetIter(conflict_2->GetHash()).value(); const auto conflict_3 = make_tx(/*inputs=*/ {m_coinbase_txns[4]}, /*output_values=*/ {10 * COIN}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(conflict_3)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(conflict_3)); const auto conflict_3_entry = pool.GetIter(conflict_3->GetHash()).value(); { @@ -465,7 +465,7 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) // Add a child transaction to conflict_1 and make it cluster size 2, two chunks due to same feerate const auto conflict_1_child = make_tx(/*inputs=*/{conflict_1}, /*output_values=*/ {995 * CENT}); - AddToMempool(pool, entry.Fee(low_fee).FromTx(conflict_1_child)); + TryAddToMempool(pool, entry.Fee(low_fee).FromTx(conflict_1_child)); const auto conflict_1_child_entry = pool.GetIter(conflict_1_child->GetHash()).value(); { diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 3271838e788..4bd5d7e62a8 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -225,7 +225,7 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Add first grandparent to mempool and fetch entry - AddToMempool(pool, entry.FromTx(grandparent_tx_1)); + TryAddToMempool(pool, entry.FromTx(grandparent_tx_1)); // Ignores ancestors that aren't direct parents BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_wtxid)); @@ -248,7 +248,7 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Add second grandparent to mempool - AddToMempool(pool, entry.FromTx(grandparent_tx_2)); + TryAddToMempool(pool, entry.FromTx(grandparent_tx_2)); // Only spends single dust out of two direct parents BOOST_CHECK(!CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool, child_state, child_wtxid)); @@ -263,7 +263,7 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) BOOST_CHECK_EQUAL(child_wtxid, Wtxid()); // Now add dusty parent to mempool - AddToMempool(pool, entry.FromTx(parent_with_dust)); + TryAddToMempool(pool, entry.FromTx(parent_with_dust)); // Passes dust checks even with non-parent ancestors BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_wtxid)); @@ -281,9 +281,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) std::vector empty_parents; auto mempool_tx_v3 = make_tx(random_outpoints(1), /*version=*/3); - AddToMempool(pool, entry.FromTx(mempool_tx_v3)); + TryAddToMempool(pool, entry.FromTx(mempool_tx_v3)); auto mempool_tx_v2 = make_tx(random_outpoints(1), /*version=*/2); - AddToMempool(pool, entry.FromTx(mempool_tx_v2)); + TryAddToMempool(pool, entry.FromTx(mempool_tx_v2)); // Cannot spend from an unconfirmed TRUC transaction unless this tx is also TRUC. { @@ -389,7 +389,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) package_multi_parents.emplace_back(mempool_tx_v3); for (size_t i{0}; i < 2; ++i) { auto mempool_tx = make_tx(random_outpoints(i + 1), /*version=*/3); - AddToMempool(pool, entry.FromTx(mempool_tx)); + TryAddToMempool(pool, entry.FromTx(mempool_tx)); mempool_outpoints.emplace_back(mempool_tx->GetHash(), 0); package_multi_parents.emplace_back(mempool_tx); } @@ -414,7 +414,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto last_outpoint{random_outpoints(1)[0]}; for (size_t i{0}; i < 2; ++i) { auto mempool_tx = make_tx({last_outpoint}, /*version=*/3); - AddToMempool(pool, entry.FromTx(mempool_tx)); + TryAddToMempool(pool, entry.FromTx(mempool_tx)); last_outpoint = COutPoint{mempool_tx->GetHash(), 0}; package_multi_gen.emplace_back(mempool_tx); if (i == 1) middle_tx = mempool_tx; @@ -501,7 +501,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) BOOST_CHECK(GetTransactionWeight(*tx_mempool_v3_child) <= TRUC_CHILD_MAX_VSIZE * WITNESS_SCALE_FACTOR); auto parents{pool.GetParents(entry.FromTx(tx_mempool_v3_child))}; BOOST_CHECK(SingleTRUCChecks(pool, tx_mempool_v3_child, parents, empty_conflicts_set, GetVirtualTransactionSize(*tx_mempool_v3_child)) == std::nullopt); - AddToMempool(pool, entry.FromTx(tx_mempool_v3_child)); + TryAddToMempool(pool, entry.FromTx(tx_mempool_v3_child)); Package package_v3_1p1c{mempool_tx_v3, tx_mempool_v3_child}; BOOST_CHECK(PackageTRUCChecks(pool, tx_mempool_v3_child, GetVirtualTransactionSize(*tx_mempool_v3_child), package_v3_1p1c, empty_parents) == std::nullopt); @@ -528,7 +528,7 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) expected_error_str); // Configuration where parent already has 2 other children in mempool (no sibling eviction allowed). This may happen as the result of a reorg. - AddToMempool(pool, entry.FromTx(tx_v3_child2)); + TryAddToMempool(pool, entry.FromTx(tx_v3_child2)); auto tx_v3_child3 = make_tx({COutPoint{mempool_tx_v3->GetHash(), 24}}, /*version=*/3); auto entry_mempool_parent = pool.GetIter(mempool_tx_v3->GetHash()).value(); BOOST_CHECK_EQUAL(pool.GetDescendantCount(entry_mempool_parent), 3); @@ -547,9 +547,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) auto tx_mempool_nibling = make_tx({COutPoint{tx_mempool_sibling->GetHash(), 0}}, /*version=*/3); auto tx_to_submit = make_tx({COutPoint{tx_mempool_grandparent->GetHash(), 1}}, /*version=*/3); - AddToMempool(pool, entry.FromTx(tx_mempool_grandparent)); - AddToMempool(pool, entry.FromTx(tx_mempool_sibling)); - AddToMempool(pool, entry.FromTx(tx_mempool_nibling)); + TryAddToMempool(pool, entry.FromTx(tx_mempool_grandparent)); + TryAddToMempool(pool, entry.FromTx(tx_mempool_sibling)); + TryAddToMempool(pool, entry.FromTx(tx_mempool_nibling)); auto parents_3gen{pool.GetParents(entry.FromTx(tx_to_submit))}; const auto expected_error_str{strprintf("tx %s (wtxid=%s) would exceed descendant count limit", diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index 49f3be43994..b158713d340 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -212,7 +212,7 @@ void CheckMempoolTRUCInvariants(const CTxMemPool& tx_pool) } } -void AddToMempool(CTxMemPool& tx_pool, const CTxMemPoolEntry& entry) +void TryAddToMempool(CTxMemPool& tx_pool, const CTxMemPoolEntry& entry) { LOCK2(cs_main, tx_pool.cs); auto changeset = tx_pool.GetChangeSet(); diff --git a/src/test/util/txmempool.h b/src/test/util/txmempool.h index 69b21a2b4df..f0b66debea0 100644 --- a/src/test/util/txmempool.h +++ b/src/test/util/txmempool.h @@ -65,7 +65,7 @@ void CheckMempoolTRUCInvariants(const CTxMemPool& tx_pool); /** One-line wrapper for creating a mempool changeset with a single transaction * and applying it if the policy limits are respected. */ -void AddToMempool(CTxMemPool& tx_pool, const CTxMemPoolEntry& entry); +void TryAddToMempool(CTxMemPool& tx_pool, const CTxMemPoolEntry& entry); /** Mock the mempool minimum feerate by adding a transaction and calling TrimToSize(0), * simulating the mempool "reaching capacity" and evicting by descendant feerate. Note that From 3e39ea8c307010bc0132615ecef55b39851f7437 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Wed, 5 Feb 2025 17:58:57 -0500 Subject: [PATCH 05/26] Rewrite removeForReorg to avoid using sets Also improve test coverage for removeForReorg by creating a scenario where there are in-mempool descendants that are only invalidated due to an in-mempool parent no longer spending a mature coin. --- src/txmempool.cpp | 18 +++++++++++------- test/functional/mempool_reorg.py | 22 ++++++++++++++++++++-- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index fec111fda1c..5fc1f3e85ca 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -352,15 +352,19 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function check AssertLockHeld(::cs_main); Assume(!m_have_changeset); - setEntries txToRemove; - for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { - if (check_final_and_mature(it)) txToRemove.insert(it); + std::vector to_remove; + for (txiter it = mapTx.begin(); it != mapTx.end(); it++) { + if (check_final_and_mature(it)) { + to_remove.emplace_back(&*it); + } } - setEntries setAllRemoves; - for (txiter it : txToRemove) { - CalculateDescendants(it, setAllRemoves); + + auto all_to_remove = m_txgraph->GetDescendantsUnion(to_remove, TxGraph::Level::MAIN); + + for (auto ref : all_to_remove) { + auto it = mapTx.iterator_to(static_cast(*ref)); + removeUnchecked(it, MemPoolRemovalReason::REORG); } - RemoveStaged(setAllRemoves, MemPoolRemovalReason::REORG); for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { assert(TestLockPointValidity(chain, it->GetLockPoints())); } diff --git a/test/functional/mempool_reorg.py b/test/functional/mempool_reorg.py index 818dd2cae86..71f34b4a177 100755 --- a/test/functional/mempool_reorg.py +++ b/test/functional/mempool_reorg.py @@ -146,7 +146,7 @@ class MempoolCoinbaseTest(BitcoinTestFramework): assert_raises_rpc_error(-26, "non-final", self.nodes[0].sendrawtransaction, timelock_tx) self.log.info("Broadcast and mine spend_2 and spend_3") - wallet.sendrawtransaction(from_node=self.nodes[0], tx_hex=spend_2['hex']) + spend_2_id = wallet.sendrawtransaction(from_node=self.nodes[0], tx_hex=spend_2['hex']) wallet.sendrawtransaction(from_node=self.nodes[0], tx_hex=spend_3['hex']) self.log.info("Generate a block") self.generate(self.nodes[0], 1) @@ -154,7 +154,7 @@ class MempoolCoinbaseTest(BitcoinTestFramework): assert_raises_rpc_error(-26, 'non-final', self.nodes[0].sendrawtransaction, timelock_tx) self.log.info("Create spend_2_1 and spend_3_1") - spend_2_1 = wallet.create_self_transfer(utxo_to_spend=spend_2["new_utxo"]) + spend_2_1 = wallet.create_self_transfer(utxo_to_spend=spend_2["new_utxo"], version=1) spend_3_1 = wallet.create_self_transfer(utxo_to_spend=spend_3["new_utxo"]) self.log.info("Broadcast and mine spend_3_1") @@ -181,6 +181,24 @@ class MempoolCoinbaseTest(BitcoinTestFramework): self.log.info("spend_3_1 has been re-orged out of the chain and is back in the mempool") assert_equal(set(self.nodes[0].getrawmempool()), {spend_1_id, spend_2_1_id, spend_3_1_id}) + self.log.info("Reorg out enough blocks to get spend_2 back in the mempool, along with its child") + + while (spend_2_id not in self.nodes[0].getrawmempool()): + b = self.nodes[0].getbestblockhash() + for node in self.nodes: + node.invalidateblock(b) + + assert(spend_2_id in self.nodes[0].getrawmempool()) + assert(spend_2_1_id in self.nodes[0].getrawmempool()) + + # Chain 10 more transactions off of spend_2_1 + self.log.info("Give spend_2 some more descendants by creating a chain of 10 transactions spending from it") + parent_utxo = spend_2_1["new_utxo"] + for i in range(10): + tx = wallet.create_self_transfer(utxo_to_spend=parent_utxo, version=1) + self.nodes[0].sendrawtransaction(tx['hex']) + parent_utxo = tx["new_utxo"] + self.log.info("Use invalidateblock to re-org back and make all those coinbase spends immature/invalid") b = self.nodes[0].getblockhash(first_block + 100) for node in self.nodes: From 9292570f4cb85fc6690dfeeb55ea867d575ebba3 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Wed, 5 Feb 2025 18:13:55 -0500 Subject: [PATCH 06/26] Rewrite GetChildren without sets --- src/txmempool.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 5fc1f3e85ca..1162634b5b7 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -58,13 +58,12 @@ std::vector CTxMemPool::GetChildren(const C { LOCK(cs); std::vector ret; - setEntries children; + WITH_FRESH_EPOCH(m_epoch); auto iter = mapNextTx.lower_bound(COutPoint(entry.GetTx().GetHash(), 0)); for (; iter != mapNextTx.end() && iter->first->hash == entry.GetTx().GetHash(); ++iter) { - children.insert(iter->second); - } - for (const auto& child : children) { - ret.emplace_back(*child); + if (!visited(iter->second)) { + ret.emplace_back(*(iter->second)); + } } return ret; } From 80d8df2d47c25851b51fe3319605fe41c34ca9f8 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Sat, 25 Jan 2025 11:41:06 -0500 Subject: [PATCH 07/26] Invoke removeUnchecked() directly in removeForBlock() --- src/txmempool.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 1162634b5b7..ee4ced3b0c8 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -398,10 +398,8 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne for (const auto& tx : vtx) { txiter it = mapTx.find(tx->GetHash()); if (it != mapTx.end()) { - setEntries stage; - stage.insert(it); txs_removed_for_block.emplace_back(*it); - RemoveStaged(stage, MemPoolRemovalReason::BLOCK); + removeUnchecked(it, MemPoolRemovalReason::BLOCK); } removeConflicts(*tx); ClearPrioritisation(tx->GetHash()); From ed8e819121d7065c6e34a6ae422842369c4a1659 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 9 Oct 2025 20:20:35 -0400 Subject: [PATCH 08/26] Warn user if using -limitancestorsize/-limitdescendantsize that the options have no effect Also update the help text for -limitancestorcount/-limitdescendantcount to explain they no longer affect the mempool, and are only used by the wallet for coin selection. --- src/init.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index d1c6a5cad0e..58c97dd3ac8 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -633,10 +633,13 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-deprecatedrpc=", "Allows deprecated RPC method(s) to be used", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-stopafterblockimport", strprintf("Stop running after importing blocks from disk (default: %u)", DEFAULT_STOPAFTERBLOCKIMPORT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-stopatheight", strprintf("Stop running after reaching the given height in the main chain (default: %u)", DEFAULT_STOPATHEIGHT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-limitancestorcount=", strprintf("Do not accept transactions if number of in-mempool ancestors is or more (default: %u)", DEFAULT_ANCESTOR_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-limitancestorsize=", strprintf("Do not accept transactions whose size with all in-mempool ancestors exceeds kilobytes (default: %u)", DEFAULT_ANCESTOR_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-limitdescendantcount=", strprintf("Do not accept transactions if any ancestor would have or more in-mempool descendants (default: %u)", DEFAULT_DESCENDANT_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-limitdescendantsize=", strprintf("Do not accept transactions if any ancestor would have more than kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-limitancestorcount=", strprintf("Deprecated setting to not accept transactions if number of in-mempool ancestors is or more (default: %u); replaced by cluster limits (see -limitclustercount) and only used by wallet for coin selection", DEFAULT_ANCESTOR_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + // Ancestor and descendant size limits were removed. We keep + // -limitancestorsize/-limitdescendantsize as hidden args to display a more + // user friendly error when set. + argsman.AddArg("-limitancestorsize", "", ArgsManager::ALLOW_ANY, OptionsCategory::HIDDEN); + argsman.AddArg("-limitdescendantsize", "", ArgsManager::ALLOW_ANY, OptionsCategory::HIDDEN); + argsman.AddArg("-limitdescendantcount=", strprintf("Deprecated setting to not accept transactions if any ancestor would have or more in-mempool descendants (default: %u); replaced by cluster limits (see -limitclustercount) and only used by wallet for coin selection", DEFAULT_DESCENDANT_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-test=