#36367 mempool: keep prioritisetransaction deltas within MAX_MONEY
| Reviewability | Reviews | Agreement | Size | |||||||
|---|---|---|---|---|---|---|---|---|---|---|
| #36367 mempool: keep prioritisetransaction deltas within MAX_MONEY | Ready | 0 | Crickets | 11 + 29 tests | ||||||
|
|
|
|
| ||||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | ||||||||||
Goal
Bound fee deltas in the `prioritisetransaction` RPC and mempool prioritisation tracking to the range between `-MAX_MONEY` and `MAX_MONEY`. CTxMemPool::PrioritiseTransaction clamps accumulated deltas to this range and adjusts modified fees by the difference. This prevents transaction fee sums across cluster mempool chunks from overflowing 64-bit integer limits.
Problem: CTxMemPool::PrioritiseTransaction allowed accumulated transaction fee deltas to reach INT64_MAX. When cluster mempool aggregates modified fees across multiple transactions in FeeFrac, the calculation experiences signed integer overflow, resulting in negative chunk fees and causing getblocktemplate to omit affected transactions.
Evidence of importance:
- PR description documents reproducible UBSan failure: "feefrac.h:108:13: runtime error: signed integer overflow, reached from linearization via getrawmempool".
- PR description demonstrates that prioritising a parent by 2^62 and a child by INT64_MAX causes getmempoolcluster to report "chunkfee: -46116860184.27356705" and getblocktemplate to exclude both transactions.
Risk of a mistake: medium (mempool_policy, rpc_api). A flaw in delta clamping or modified fee accounting could cause incorrect chunk fee calculations in the cluster mempool or corrupt block template construction for miners.
Changed since the previous assessment: No changes to the patch or approach; full diff is now provided.
Reviewability
Ready
Nothing known that would invalidate a review now.
Agreement
Crickets
no substantive comment either way
Objections, suggestions, and questions: none recorded.
Files
29 lines under test/bench/ci.
- test/functional/mining_prioritisetransaction.py +29/-0
- src/txmempool.cpp +7/-3
- src/rpc/mining.cpp +4/-0
Card
The short text the ranking pass compares across PRs.
Clamps prioritisetransaction fee deltas and accumulated mempool deltas to the range [-MAX_MONEY, MAX_MONEY]. Unbounded int64 fee deltas previously allowed stacked transaction fees to cause signed integer overflow in cluster mempool FeeFrac arithmetic, producing negative chunk fees and causing getblocktemplate to exclude prioritised transactions from mined blocks. Restricting individual and accumulated deltas ensures cluster fee calculations stay within int64 limits, preventing undefined behaviour and broken block templates for miners.
Data
labels: Mempool
Categories
Mempool and policy
P2 · bug fix
Clamping transaction fee deltas resolves a signed integer overflow in FeeFrac during cluster mempool linearization. The PR description notes a reproducible UBSan failure ("feefrac.h:108:13: runtime error: signed integer overflow") and cluster calculation corruption, which is an important correctness fix for cluster mempool data structures.
Membership: Carries the Mempool label and edits src/txmempool.cpp to prevent FeeFrac integer overflows during cluster mempool linearization.
Factors: security/stability 1, bug 2, performance 0, user value 0, leverage 1
Mining
P3 · bug fix (ranking pass: Moved up: signed-overflow UB that silently drops prioritised transactions from templates is a template-correctness issue, ranking above cleanups and tests despite the unrealistic trigger.)
Bounding prioritisetransaction deltas prevents template construction from omitting prioritized transactions due to corrupted chunk fees. The PR shows extreme fee deltas caused getblocktemplate to omit affected transactions, though triggering the issue requires unrealistic deltas exceeding MAX_MONEY.
Membership: Modifies prioritisetransaction handling in src/rpc/mining.cpp and tests in test/functional/mining_prioritisetransaction.py to fix getblocktemplate transaction omission.
Factors: security/stability 1, bug 2, performance 0, user value 0, leverage 1