From 158623b8e0726dff7eae4288138f1710e727db9c Mon Sep 17 00:00:00 2001 From: glozow Date: Mon, 27 Nov 2023 16:29:59 +0000 Subject: [PATCH] [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid It's preferable to use type-safe transaction identifiers to avoid confusing txid and wtxid. The next commit will add a reference to this set; we use this opportunity to change it to Txid ahead of time instead of adding new uses of uint256. --- src/policy/rbf.cpp | 4 ++-- src/policy/rbf.h | 2 +- src/test/rbf_tests.cpp | 2 -- src/txmempool.cpp | 4 ++-- src/txmempool.h | 2 +- src/validation.cpp | 2 +- 6 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 76ab2b1a96..f0830d8f22 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -115,11 +115,11 @@ std::optional HasNoNewUnconfirmed(const CTransaction& tx, } std::optional EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors, - const std::set& direct_conflicts, + const std::set& direct_conflicts, const uint256& txid) { for (CTxMemPool::txiter ancestorIt : ancestors) { - const uint256& hashAncestor = ancestorIt->GetTx().GetHash(); + const Txid& hashAncestor = ancestorIt->GetTx().GetHash(); if (direct_conflicts.count(hashAncestor)) { return strprintf("%s spends conflicting transaction %s", txid.ToString(), diff --git a/src/policy/rbf.h b/src/policy/rbf.h index fff9828482..5a33ed64a3 100644 --- a/src/policy/rbf.h +++ b/src/policy/rbf.h @@ -80,7 +80,7 @@ std::optional HasNoNewUnconfirmed(const CTransaction& tx, const CTx * @returns error message if the sets intersect, std::nullopt if they are disjoint. */ std::optional EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors, - const std::set& direct_conflicts, + const std::set& direct_conflicts, const uint256& txid); /** Check that the feerate of the replacement transaction(s) is higher than the feerate of each diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index fb6a3614c0..e6c135eed9 100644 --- a/src/test/rbf_tests.cpp +++ b/src/test/rbf_tests.cpp @@ -135,8 +135,6 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup) // Tests for EntriesAndTxidsDisjoint BOOST_CHECK(EntriesAndTxidsDisjoint(empty_set, {tx1->GetHash()}, unused_txid) == std::nullopt); BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx3->GetHash()}, unused_txid) == std::nullopt); - // EntriesAndTxidsDisjoint uses txids, not wtxids. - BOOST_CHECK(EntriesAndTxidsDisjoint({entry2}, {tx2->GetWitnessHash()}, unused_txid) == std::nullopt); BOOST_CHECK(EntriesAndTxidsDisjoint({entry2}, {tx2->GetHash()}, unused_txid).has_value()); BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx1->GetHash()}, unused_txid).has_value()); BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx2->GetHash()}, unused_txid).has_value()); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index b783181bb8..9fb4db330b 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -454,7 +454,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces cachedInnerUsage += entry.DynamicMemoryUsage(); const CTransaction& tx = newit->GetTx(); - std::set setParentTransactions; + std::set setParentTransactions; for (unsigned int i = 0; i < tx.vin.size(); i++) { mapNextTx.insert(std::make_pair(&tx.vin[i].prevout, &tx)); setParentTransactions.insert(tx.vin[i].prevout.hash); @@ -969,7 +969,7 @@ std::optional CTxMemPool::GetIter(const uint256& txid) const return std::nullopt; } -CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set& hashes) const +CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set& hashes) const { CTxMemPool::setEntries ret; for (const auto& h : hashes) { diff --git a/src/txmempool.h b/src/txmempool.h index bac7a445d6..591aee0156 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -522,7 +522,7 @@ public: /** Translate a set of hashes into a set of pool iterators to avoid repeated lookups. * Does not require that all of the hashes correspond to actual transactions in the mempool, * only returns the ones that exist. */ - setEntries GetIterSet(const std::set& hashes) const EXCLUSIVE_LOCKS_REQUIRED(cs); + setEntries GetIterSet(const std::set& hashes) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Translate a list of hashes into a list of mempool iterators to avoid repeated lookups. * The nth element in txids becomes the nth element in the returned vector. If any of the txids diff --git a/src/validation.cpp b/src/validation.cpp index 0501499004..5c0c92114f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -582,7 +582,7 @@ private: struct Workspace { explicit Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {} /** Txids of mempool transactions that this transaction directly conflicts with. */ - std::set m_conflicts; + std::set m_conflicts; /** Iterators to mempool entries that this transaction directly conflicts with. */ CTxMemPool::setEntries m_iters_conflicting; /** Iterators to all mempool entries that would be replaced by this transaction, including