#34698 wallet: handle MiniMiner bump fee calculation failures

ReviewabilityReviewsAgreementSize
#34698 shuv-amp wallet: handle MiniMiner bump fee calculation failuresWaiting on author1Strong33 + 114 tests
  • Return clear user-facing errors when wallet fee calculation fails on large unconfirmed transaction clusters
  • Prevent transaction creation RPCs from failing with a cryptic internal exception
  • Ready to review
  • Strong support with bug reproduction and validation across releases (DevaanshPathak)
  • Approved after earlier error propagation concerns were addressed (achow101)
  • Nonblocking suggestion to document or assert MiniMiner return invariants (DevaanshPathak)
  • Open question asking to confirm RPC error code mapping for send (DevaanshPathak)
  • 33 lines added or modified outside tests
  • 114 lines added or modified in tests
  • 37 lines removed in total
  • 11 files, 1 commits
Wallet#39 of 92
P3 · bug fix
  • P3 because it fixes transaction creation failing with a cryptic internal exception during an edge case
  • Only affects spending unconfirmed coins whose mempool cluster exceeds 500 transactions
Ranking notes
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

This change updates wallet coin selection and coin fetching to detect when MiniMiner fails to calculate individual bump fees for unconfirmed UTXOs. It changes AvailableCoins to return util::Result<CoinsResult> and handles empty fee calculation results in FetchSelectedInputs and AvailableCoins. Instead of letting map::at throw an internal exception when cluster limits are reached, the wallet now returns a descriptive user-facing error.

Problem: When spending unconfirmed UTXOs whose cluster in the mempool exceeds MiniMiner's cluster limit (500 transactions), bump fee calculation returns an empty map. The wallet previously looked up requested outpoints in that empty map using map::at, crashing transaction construction with a cryptic map::at exception exposed over RPC.

Evidence of importance:

  • Issue #29711 describes wallet funding failing with the nondescript error message 'map::at' when creating a 502nd unconfirmed transaction.
  • PR diff updates mini_miner fuzz testing, miniminer unit tests, and adds a regression test in wallet_spend_unconfirmed.py reproducing the 501-unconfirmed-transaction cluster limit failure across send and sendall.

Scope: Changes the signature of wallet::AvailableCoins to return util::Result<CoinsResult>, requiring Assert or CHECK_NONFATAL unwrapping across call sites in RPCs, unit tests, and benchmarks.

Reviewability

Paused · Waiting on author

Open objections from DevaanshPathak with no author reply for 23 days; review would repeat what has been asked.

Waiting on (from the thread read):

  • author: reply to DevaanshPathak's suggestion and question regarding MiniMiner contract documentation/assertion and send RPC error mapping 2026-10-09

Agreement

Strong

substantive support, no open objection (DevaanshPathak)

Thread notes: DevaanshPathak tested reproduction on master and v30.2 and gave supportive review notes with two non-blocking suggestions/questions.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
achow101 (member)suggestion, interfacekeep the map return type and check !empty() instead of returning std::optionalfixed · fix pushed · author kept the map return type and simplified failure detectionansweredyes2026-03-24: 'Changing this to be `std::optional` seems a bit unnecessary to me as it's equivalently easy to check for `!empty()`.'
Status from: 2026-08-19: shuv-amp: 'Added a comment at both call sites for why an empty bump-fee map means the calculation gave up'
achow101 (member)objection, interfaceAvailableCoins propagates errors through an out-parameter instead of returning a result type or extending CoinsResultfixed · fix pushed · AvailableCoins changed to return util::Result<CoinsResult>changedyes2026-04-30: 'I don't think this is the right way to propagate errors out of `AvailableCoins`.'
Status from: 2026-08-19: shuv-amp: 'AvailableCoins returns `util::Result<CoinsResult>` rather than reporting through an out-parameter'
DevaanshPathak (none)suggestion, maintenancedocument or assert that MiniMiner returns either a full map or empty so empty() && !outpoints.empty() remains a sound failure detectoropenansweredno2026-10-09: 'Please keep documenting (or asserting) that MiniMiner returns either a full map or empty — never partial — so `empty() && !outpoints.empty()` stays a sound failure detector.'
DevaanshPathak (none)question, interfaceconfirm that send maps util::Error to RPC_WALLET_ERROR rather than -1openansweredno2026-10-09: 'Confirm `send` maps the new `util::Error` to `RPC_WALLET_ERROR` (not `-1`), same as the `sendall` path in this PR.'

Support:

  • achow101 (member) (ACK, 2026-10-02): (no reason given) [not substantive]
  • DevaanshPathak (none) (2026-10-09) [both: GatherClusters, MiniMiner, CalculateBumpFees, AvailableCoins, miniminer_tests]: independently reproduced the bug and root cause on master and v30.2, and validated the approach aligning individual bump-fee failures via util::Result<CoinsResult>

Participants:

  • achow101 (member, support, 3 statements 2026-03-24 to 2026-10-02): ACKed after earlier objections regarding AvailableCoins error handling and MiniMiner return type were resolved
  • DevaanshPathak (none, support, 1 statements 2026-10-09 to 2026-10-09): reproduced the bug, verified the root cause, supported the approach, and left non-blocking suggestions

Files

135 lines under test/bench/ci.

  • test/functional/wallet_spend_unconfirmed.py +70/-0
  • src/wallet/spend.cpp +22/-9
  • src/test/fuzz/mini_miner.cpp +15/-9
  • src/bench/wallet_create_tx.cpp +8/-6
  • src/wallet/test/wallet_tests.cpp +9/-5
  • src/test/miniminer_tests.cpp +11/-0
  • src/wallet/spend.h +4/-4
  • src/wallet/rpc/spend.cpp +5/-1
  • src/wallet/interfaces.cpp +1/-1
  • src/wallet/rpc/coins.cpp +1/-1
  • src/wallet/test/spend_tests.cpp +1/-1

Card

The short text the ranking pass compares across PRs.

This PR handles MiniMiner bump fee calculation failures during wallet transaction creation and sendall. When unconfirmed transactions exceed mempool cluster limits (500 transactions), bump fee calculations return an empty map, which previously triggered an uncaught map::at exception returned to users. The PR converts AvailableCoins to return a util::Result and surfaces a descriptive error explaining that unconfirmed UTXOs depend on an enormous cluster of unconfirmed transactions. It fixes issue #29711 and includes unit, fuzz, and functional regression tests.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-10T01:21, confidence high, input hash 5ac32632d052ffc1
thread read through 2026-10-09 (7 statements), head a34afcd436
labels: Wallet

Categories

Wallet

P3 · bug fix

Fixes an unhandled internal exception exposed over RPC when spending unconfirmed UTXOs in large mempool clusters. As described in issue #29711, transaction construction currently crashes with a cryptic 'map::at' exception when MiniMiner cluster limits (500 transactions) are exceeded; returning a descriptive error fixes a confusing failure in an uncommon edge case.

Membership: Labeled Wallet and modifies wallet coin selection and transaction creation logic in src/wallet/spend.cpp and spend.h.

Factors: security/stability 1, bug 1, performance 0, user value 1, leverage 0