From d2ecd6815d89c9b089b55bc96fdf93b023be8dda Mon Sep 17 00:00:00 2001 From: marcofleon Date: Tue, 1 Apr 2025 15:26:25 +0100 Subject: [PATCH] policy, refactor: Convert uint256 to Txid --- src/policy/fees.cpp | 6 +++--- src/policy/fees.h | 6 +++--- src/policy/packages.cpp | 10 +++++----- src/policy/rbf.cpp | 11 +++++------ src/policy/rbf.h | 6 +++--- src/test/fuzz/policy_estimator.cpp | 2 +- src/test/rbf_tests.cpp | 2 +- src/validation.cpp | 2 +- 8 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index e385f548d30..a650a1326b0 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -519,16 +519,16 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe } } -bool CBlockPolicyEstimator::removeTx(uint256 hash) +bool CBlockPolicyEstimator::removeTx(Txid hash) { LOCK(m_cs_fee_estimator); return _removeTx(hash, /*inBlock=*/false); } -bool CBlockPolicyEstimator::_removeTx(const uint256& hash, bool inBlock) +bool CBlockPolicyEstimator::_removeTx(const Txid& hash, bool inBlock) { AssertLockHeld(m_cs_fee_estimator); - std::map::iterator pos = mapMemPoolTxs.find(hash); + std::map::iterator pos = mapMemPoolTxs.find(hash); if (pos != mapMemPoolTxs.end()) { feeStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex, inBlock); shortStats->removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex, inBlock); diff --git a/src/policy/fees.h b/src/policy/fees.h index a95cc19dd45..b355b65a346 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -212,7 +212,7 @@ public: EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); /** Remove a transaction from the mempool tracking stats for non BLOCK removal reasons*/ - bool removeTx(uint256 hash) + bool removeTx(Txid hash) EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator); /** DEPRECATED. Return a feerate estimate */ @@ -287,7 +287,7 @@ private: }; // map of txids to information about that transaction - std::map mapMemPoolTxs GUARDED_BY(m_cs_fee_estimator); + std::map mapMemPoolTxs GUARDED_BY(m_cs_fee_estimator); /** Classes to track historical data on transaction confirmations */ std::unique_ptr feeStats PT_GUARDED_BY(m_cs_fee_estimator); @@ -315,7 +315,7 @@ private: unsigned int MaxUsableEstimate() const EXCLUSIVE_LOCKS_REQUIRED(m_cs_fee_estimator); /** A non-thread-safe helper for the removeTx function */ - bool _removeTx(const uint256& hash, bool inBlock) + bool _removeTx(const Txid& hash, bool inBlock) EXCLUSIVE_LOCKS_REQUIRED(m_cs_fee_estimator); }; diff --git a/src/policy/packages.cpp b/src/policy/packages.cpp index 693adcdfd0b..187bd467579 100644 --- a/src/policy/packages.cpp +++ b/src/policy/packages.cpp @@ -16,7 +16,7 @@ /** IsTopoSortedPackage where a set of txids has been pre-populated. The set is assumed to be correct and * is mutated within this function (even if return value is false). */ -bool IsTopoSortedPackage(const Package& txns, std::unordered_set& later_txids) +bool IsTopoSortedPackage(const Package& txns, std::unordered_set& later_txids) { // Avoid misusing this function: later_txids should contain the txids of txns. Assume(txns.size() == later_txids.size()); @@ -42,7 +42,7 @@ bool IsTopoSortedPackage(const Package& txns, std::unordered_set later_txids; + std::unordered_set later_txids; std::transform(txns.cbegin(), txns.cend(), std::inserter(later_txids, later_txids.end()), [](const auto& tx) { return tx->GetHash(); }); @@ -91,7 +91,7 @@ bool IsWellFormedPackage(const Package& txns, PackageValidationState& state, boo return state.Invalid(PackageValidationResult::PCKG_POLICY, "package-too-large"); } - std::unordered_set later_txids; + std::unordered_set later_txids; std::transform(txns.cbegin(), txns.cend(), std::inserter(later_txids, later_txids.end()), [](const auto& tx) { return tx->GetHash(); }); @@ -123,7 +123,7 @@ bool IsChildWithParents(const Package& package) // The package is expected to be sorted, so the last transaction is the child. const auto& child = package.back(); - std::unordered_set input_txids; + std::unordered_set input_txids; std::transform(child->vin.cbegin(), child->vin.cend(), std::inserter(input_txids, input_txids.end()), [](const auto& input) { return input.prevout.hash; }); @@ -136,7 +136,7 @@ bool IsChildWithParents(const Package& package) bool IsChildWithParentsTree(const Package& package) { if (!IsChildWithParents(package)) return false; - std::unordered_set parent_txids; + std::unordered_set parent_txids; std::transform(package.cbegin(), package.cend() - 1, std::inserter(parent_txids, parent_txids.end()), [](const auto& ptx) { return ptx->GetHash(); }); // Each parent must not have an input who is one of the other parents. diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index dc74f43b354..210b94915cc 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -62,7 +62,6 @@ std::optional GetEntriesForConflicts(const CTransaction& tx, CTxMemPool::setEntries& all_conflicts) { AssertLockHeld(pool.cs); - const uint256 txid = tx.GetHash(); uint64_t nConflictingCount = 0; for (const auto& mi : iters_conflicting) { nConflictingCount += mi->GetCountWithDescendants(); @@ -72,7 +71,7 @@ std::optional GetEntriesForConflicts(const CTransaction& tx, // times), but we just want to be conservative to avoid doing too much work. if (nConflictingCount > MAX_REPLACEMENT_CANDIDATES) { return strprintf("rejecting replacement %s; too many potential replacements (%d > %d)", - txid.ToString(), + tx.GetHash().ToString(), nConflictingCount, MAX_REPLACEMENT_CANDIDATES); } @@ -89,7 +88,7 @@ std::optional HasNoNewUnconfirmed(const CTransaction& tx, const CTxMemPool::setEntries& iters_conflicting) { AssertLockHeld(pool.cs); - std::set parents_of_conflicts; + std::set parents_of_conflicts; for (const auto& mi : iters_conflicting) { for (const CTxIn& txin : mi->GetTx().vin) { parents_of_conflicts.insert(txin.prevout.hash); @@ -118,7 +117,7 @@ std::optional HasNoNewUnconfirmed(const CTransaction& tx, std::optional EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors, const std::set& direct_conflicts, - const uint256& txid) + const Txid& txid) { for (CTxMemPool::txiter ancestorIt : ancestors) { const Txid& hashAncestor = ancestorIt->GetTx().GetHash(); @@ -133,7 +132,7 @@ std::optional EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& std::optional PaysMoreThanConflicts(const CTxMemPool::setEntries& iters_conflicting, CFeeRate replacement_feerate, - const uint256& txid) + const Txid& txid) { for (const auto& mi : iters_conflicting) { // Don't allow the replacement to reduce the feerate of the mempool. @@ -161,7 +160,7 @@ std::optional PaysForRBF(CAmount original_fees, CAmount replacement_fees, size_t replacement_vsize, CFeeRate relay_fee, - const uint256& txid) + const Txid& txid) { // Rule #3: The replacement fees must be greater than or equal to fees of the // transactions it replaces, otherwise the bandwidth used by those conflicting transactions diff --git a/src/policy/rbf.h b/src/policy/rbf.h index 3cc0fc3149f..cc397b463dc 100644 --- a/src/policy/rbf.h +++ b/src/policy/rbf.h @@ -90,7 +90,7 @@ std::optional HasNoNewUnconfirmed(const CTransaction& tx, const CTx */ std::optional EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors, const std::set& direct_conflicts, - const uint256& txid); + const Txid& txid); /** Check that the feerate of the replacement transaction(s) is higher than the feerate of each * of the transactions in iters_conflicting. @@ -98,7 +98,7 @@ std::optional EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& * @returns error message if fees insufficient, otherwise std::nullopt. */ std::optional PaysMoreThanConflicts(const CTxMemPool::setEntries& iters_conflicting, - CFeeRate replacement_feerate, const uint256& txid); + CFeeRate replacement_feerate, const Txid& txid); /** The replacement transaction must pay more fees than the original transactions. The additional * fees must pay for the replacement's bandwidth at or above the incremental relay feerate. @@ -113,7 +113,7 @@ std::optional PaysForRBF(CAmount original_fees, CAmount replacement_fees, size_t replacement_vsize, CFeeRate relay_fee, - const uint256& txid); + const Txid& txid); /** * The replacement transaction must improve the feerate diagram of the mempool. diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index 37e37964182..8455a23284d 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -85,7 +85,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) block_policy_estimator.processBlock(txs, current_height); }, [&] { - (void)block_policy_estimator.removeTx(ConsumeUInt256(fuzzed_data_provider)); + (void)block_policy_estimator.removeTx(Txid::FromUint256(ConsumeUInt256(fuzzed_data_provider))); }, [&] { block_policy_estimator.FlushUnconfirmed(); diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index 0052276eac4..cbbea61a53c 100644 --- a/src/test/rbf_tests.cpp +++ b/src/test/rbf_tests.cpp @@ -196,7 +196,7 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup) entry5_low, entry6_low_prioritised, entry7_high, entry8_high}; CTxMemPool::setEntries empty_set; - const auto unused_txid{GetRandHash()}; + const auto unused_txid = Txid::FromUint256(GetRandHash()); // Tests for PaysMoreThanConflicts // These tests use feerate, not absolute fee. diff --git a/src/validation.cpp b/src/validation.cpp index 69cc84bba22..0d68475aa6c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1088,7 +1088,7 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) AssertLockHeld(m_pool.cs); const CTransaction& tx = *ws.m_ptx; - const uint256& hash = ws.m_hash; + const Txid& hash = ws.m_hash; TxValidationState& state = ws.m_state; CFeeRate newFeeRate(ws.m_modified_fees, ws.m_vsize);