policy, refactor: Convert uint256 to Txid
What changed, and why it matters
This commit is a straightforward code cleanup that swaps the generic uint256 type for the more specific Txid type in transaction-related policy code. It does not change behavior, fix a bug, or introduce a security vulnerability. It is purely a type-safety refactor.
No security action needed. Treat as normal refactoring code review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit replaces uses of uint256 with the strongly-typed Txid alias/class in the fee estimator, package validation, RBF policy, and mempool acceptance paths. It also updates test and fuzz harness call sites to use Txid::FromUint256 where raw uint256 values were previously passed. No logic, control flow, or validation rules are altered.
Changed components
src/policy/fees.cppsrc/policy/fees.hsrc/policy/packages.cppsrc/policy/rbf.cppsrc/policy/rbf.hsrc/test/fuzz/policy_estimator.cppsrc/test/rbf_tests.cppsrc/validation.cppInspect captured patch +22 / −23
diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
index e385f548..a650a132 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<uint256, TxStatsInfo>::iterator pos = mapMemPoolTxs.find(hash);
+ std::map<Txid, TxStatsInfo>::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 a95cc19d..b355b65a 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<uint256, TxStatsInfo> mapMemPoolTxs GUARDED_BY(m_cs_fee_estimator);
+ std::map<Txid, TxStatsInfo> mapMemPoolTxs GUARDED_BY(m_cs_fee_estimator);
/** Classes to track historical data on transaction confirmations */
std::unique_ptr<TxConfirmStats> 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 693adcdf..187bd467 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<uint256, SaltedTxidHasher>& later_txids)
+bool IsTopoSortedPackage(const Package& txns, std::unordered_set<Txid, SaltedTxidHasher>& 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<uint256, Salted
bool IsTopoSortedPackage(const Package& txns)
{
- std::unordered_set<uint256, SaltedTxidHasher> later_txids;
+ std::unordered_set<Txid, SaltedTxidHasher> 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<uint256, SaltedTxidHasher> later_txids;
+ std::unordered_set<Txid, SaltedTxidHasher> 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<uint256, SaltedTxidHasher> input_txids;
+ std::unordered_set<Txid, SaltedTxidHasher> 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<uint256, SaltedTxidHasher> parent_txids;
+ std::unordered_set<Txid, SaltedTxidHasher> 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 dc74f43b..210b9491 100644
--- a/src/policy/rbf.cpp
+++ b/src/policy/rbf.cpp
@@ -62,7 +62,6 @@ std::optional<std::string> 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<std::string> 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<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
const CTxMemPool::setEntries& iters_conflicting)
{
AssertLockHeld(pool.cs);
- std::set<uint256> parents_of_conflicts;
+ std::set<Txid> 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<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
const std::set<Txid>& 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<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries&
std::optional<std::string> 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<std::string> 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 3cc0fc31..cc397b46 100644
--- a/src/policy/rbf.h
+++ b/src/policy/rbf.h
@@ -90,7 +90,7 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTx
*/
std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
const std::set<Txid>& 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<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries&
* @returns error message if fees insufficient, otherwise std::nullopt.
*/
std::optional<std::string> 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<std::string> 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 37e37964..8455a232 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 0052276e..cbbea61a 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 69cc84bb..0d68475a 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);
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.