From 1c6a73abbd1fb773c7d0036beb952b95dde8e38b Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Thu, 2 Nov 2023 15:50:45 +0100 Subject: [refactor] Add helper for retrieving mempool entry In places where the iterator is only needed for accessing the actual entry, it should not be required to first retrieve the iterator. --- src/test/miniminer_tests.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'src/test') diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index 76c079a6e7..2eb82ae748 100644 --- a/src/test/miniminer_tests.cpp +++ b/src/test/miniminer_tests.cpp @@ -94,7 +94,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup) const CFeeRate feerate_zero(0); mini_miner_target0.BuildMockTemplate(feerate_zero); // Check the quit condition: - BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(pool.GetIter(tx_mod_negative->GetHash()).value()->GetTxSize())); + BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(Assert(pool.GetEntry(tx_mod_negative->GetHash()))->GetTxSize())); BOOST_CHECK(mini_miner_target0.GetMockTemplateTxids().empty()); // With no target feerate, the template includes all transactions, even negative feerate ones. @@ -179,9 +179,9 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) }; std::map tx_dims; for (const auto& tx : all_transactions) { - const auto it = pool.GetIter(tx->GetHash()).value(); - tx_dims.emplace(tx->GetHash(), TxDimensions{it->GetTxSize(), it->GetModifiedFee(), - CFeeRate(it->GetModifiedFee(), it->GetTxSize())}); + const auto& entry{*Assert(pool.GetEntry(tx->GetHash()))}; + tx_dims.emplace(tx->GetHash(), TxDimensions{entry.GetTxSize(), entry.GetModifiedFee(), + CFeeRate(entry.GetModifiedFee(), entry.GetTxSize())}); } const std::vector various_normal_feerates({CFeeRate(0), CFeeRate(500), CFeeRate(999), @@ -447,15 +447,15 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup) // tx3's feerate is lower than tx2's. same fee, different weight. BOOST_CHECK(tx2_feerate > tx3_feerate); const auto tx3_anc_feerate = CFeeRate(low_fee + med_fee + high_fee + high_fee, tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]); - const auto tx3_iter = pool.GetIter(tx3->GetHash()); - BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_iter.value()->GetModFeesWithAncestors(), tx3_iter.value()->GetSizeWithAncestors())); + const auto& tx3_entry{*Assert(pool.GetEntry(tx3->GetHash()))}; + BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_entry.GetModFeesWithAncestors(), tx3_entry.GetSizeWithAncestors())); const auto tx4_feerate = CFeeRate(high_fee, tx_vsizes[4]); const auto tx6_anc_feerate = CFeeRate(high_fee + low_fee + med_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]); - const auto tx6_iter = pool.GetIter(tx6->GetHash()); - BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_iter.value()->GetModFeesWithAncestors(), tx6_iter.value()->GetSizeWithAncestors())); + const auto& tx6_entry{*Assert(pool.GetEntry(tx6->GetHash()))}; + BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_entry.GetModFeesWithAncestors(), tx6_entry.GetSizeWithAncestors())); const auto tx7_anc_feerate = CFeeRate(high_fee + low_fee + high_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[7]); - const auto tx7_iter = pool.GetIter(tx7->GetHash()); - BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_iter.value()->GetModFeesWithAncestors(), tx7_iter.value()->GetSizeWithAncestors())); + const auto& tx7_entry{*Assert(pool.GetEntry(tx7->GetHash()))}; + BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_entry.GetModFeesWithAncestors(), tx7_entry.GetSizeWithAncestors())); BOOST_CHECK(tx4_feerate > tx6_anc_feerate); BOOST_CHECK(tx4_feerate > tx7_anc_feerate); -- cgit v1.2.3 From f80909e7a31523d8f197fd650b138b9f228cd13f Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 30 Aug 2023 15:40:49 +0100 Subject: [refactor] remove access to mapTx in blockencodings_tests.cpp --- src/test/blockencodings_tests.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'src/test') diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 4348a20886..8709e0c623 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) LOCK2(cs_main, pool.cs); pool.addUnchecked(entry.FromTx(block.vtx[2])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); // Do a simple ShortTxIDs RT { @@ -80,7 +80,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) BOOST_CHECK(!partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); size_t poolSize = pool.size(); pool.removeRecursive(*block.vtx[2], MemPoolRemovalReason::REPLACED); @@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) LOCK2(cs_main, pool.cs); pool.addUnchecked(entry.FromTx(block.vtx[2])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; @@ -170,7 +170,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); // +1 because of partialBlock + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); // +1 because of partialBlock CBlock block2; { @@ -185,7 +185,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) partialBlock.FillBlock(block2, {block.vtx[1]}); // Current implementation doesn't check txn here, but don't require that partialBlock = tmp; } - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 2); // +2 because of partialBlock and block2 + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 2); // +2 because of partialBlock and block2 bool mutated; BOOST_CHECK(block.hashMerkleRoot != BlockMerkleRoot(block2, &mutated)); @@ -196,15 +196,15 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString()); BOOST_CHECK(!mutated); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 3); // +2 because of partialBlock and block2 and block3 + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 3); // +2 because of partialBlock and block2 and block3 txhash = block.vtx[2]->GetHash(); block.vtx.clear(); block2.vtx.clear(); block3.vtx.clear(); - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. } - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET - 1); // -1 because of block + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET - 1); // -1 because of block } BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) @@ -215,7 +215,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) LOCK2(cs_main, pool.cs); pool.addUnchecked(entry.FromTx(block.vtx[1])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; @@ -240,7 +240,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + BOOST_CHECK_EQUAL(pool.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); CBlock block2; PartiallyDownloadedBlock partialBlockCopy = partialBlock; @@ -253,9 +253,9 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) txhash = block.vtx[1]->GetHash(); block.vtx.clear(); block2.vtx.clear(); - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. } - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET - 1); // -1 because of block + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET - 1); // -1 because of block } BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) -- cgit v1.2.3 From dbc5bdbf595e9dd0330493645ebff0b8696192a3 Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 30 Aug 2023 15:48:51 +0100 Subject: [refactor] remove access to mapTx.find in mempool_tests.cpp --- src/test/mempool_tests.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'src/test') diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index db58a0baec..217e4a6d22 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) CheckSort(pool, sortedOrder); CTxMemPool::setEntries setAncestors; - setAncestors.insert(pool.mapTx.find(tx6.GetHash())); + setAncestors.insert(pool.GetIter(tx6.GetHash()).value()); CMutableTransaction tx7 = CMutableTransaction(); tx7.vin.resize(1); tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0); @@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx8.vout.resize(1); tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx8.vout[0].nValue = 10 * COIN; - setAncestors.insert(pool.mapTx.find(tx7.GetHash())); + setAncestors.insert(pool.GetIter(tx7.GetHash()).value()); pool.addUnchecked(entry.Fee(0LL).Time(NodeSeconds{2s}).FromTx(tx8), setAncestors); // Now tx8 should be sorted low, but tx6/tx both high @@ -247,8 +247,8 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) std::vector snapshotOrder = sortedOrder; - setAncestors.insert(pool.mapTx.find(tx8.GetHash())); - setAncestors.insert(pool.mapTx.find(tx9.GetHash())); + setAncestors.insert(pool.GetIter(tx8.GetHash()).value()); + setAncestors.insert(pool.GetIter(tx9.GetHash()).value()); /* tx10 depends on tx8 and tx9 and has a high fee*/ CMutableTransaction tx10 = CMutableTransaction(); tx10.vin.resize(2); @@ -291,11 +291,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) BOOST_CHECK_EQUAL(pool.size(), 10U); // Now try removing tx10 and verify the sort order returns to normal - pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); + pool.removeRecursive(*Assert(pool.get(tx10.GetHash())), REMOVAL_REASON_DUMMY); CheckSort(pool, snapshotOrder); - pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); - pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); + pool.removeRecursive(*Assert(pool.get(tx9.GetHash())), REMOVAL_REASON_DUMMY); + pool.removeRecursive(*Assert(pool.get(tx8.GetHash())), REMOVAL_REASON_DUMMY); } BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) -- cgit v1.2.3 From a03aef9cec35b0d03aa63d7e8093f0420cd4b40b Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 25 Aug 2023 17:01:51 +0100 Subject: [refactor] rewrite vTxHashes as a vector of CTransactionRef vTxHashes exposes a complex mapTx iterator type that its external users don't need. Directly populate it with CTransactionRef instead. --- src/test/blockencodings_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/test') diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 8709e0c623..33894fd076 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -51,8 +51,8 @@ static CBlock BuildBlockTestCase() { } // Number of shared use_counts we expect for a tx we haven't touched -// (block + mempool + our copy from the GetSharedTx call) -constexpr long SHARED_TX_OFFSET{3}; +// (block + mempool entry + mempool vTxHashes + our copy from the GetSharedTx call) +constexpr long SHARED_TX_OFFSET{4}; BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) { -- cgit v1.2.3 From 55b0939cab49d50ca5bc59105b669e379d5e7f6c Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Fri, 1 Sep 2023 21:36:54 +0200 Subject: scripted-diff: rename vTxHashes to txns_randomized -BEGIN VERIFY SCRIPT- git grep -l "vTxHashesIdx" src | xargs sed -i "s/vTxHashesIdx/idx_randomized/g" git grep -l "vTxHashes" src | xargs sed -i "s/vTxHashes/txns_randomized/g" -END VERIFY SCRIPT- --- src/test/blockencodings_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/test') diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 33894fd076..e4ef019daf 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -51,7 +51,7 @@ static CBlock BuildBlockTestCase() { } // Number of shared use_counts we expect for a tx we haven't touched -// (block + mempool entry + mempool vTxHashes + our copy from the GetSharedTx call) +// (block + mempool entry + mempool txns_randomized + our copy from the GetSharedTx call) constexpr long SHARED_TX_OFFSET{4}; BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) -- cgit v1.2.3 From 4dd94ca18f6fc843137ffca3e6d3e97e4f19377b Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 1 Nov 2023 20:21:43 +0100 Subject: [refactor] remove access to mapTx in validation_block_tests Use the helper function instead of reaching into the mapTx member object. --- src/test/validation_block_tests.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/test') diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 64cb5522eb..35e5c6a037 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -283,8 +283,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // Check that all txs are in the pool { - LOCK(m_node.mempool->cs); - BOOST_CHECK_EQUAL(m_node.mempool->mapTx.size(), txs.size()); + BOOST_CHECK_EQUAL(m_node.mempool->size(), txs.size()); } // Run a thread that simulates an RPC caller that is polling while @@ -295,7 +294,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // not some intermediate amount. while (true) { LOCK(m_node.mempool->cs); - if (m_node.mempool->mapTx.size() == 0) { + if (m_node.mempool->size() == 0) { // We are done with the reorg break; } @@ -304,7 +303,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // be atomic. So the caller assumes that the returned mempool // is consistent. That is, it has all txs that were there // before the reorg. - assert(m_node.mempool->mapTx.size() == txs.size()); + assert(m_node.mempool->size() == txs.size()); continue; } LOCK(cs_main); -- cgit v1.2.3