fix(bifrost): sign UTXO outbounds short of amount + MaxGas when the actual fee fits

Son of Odinrequested to merge
fix-inactive-vault-refund-maxgas into develop

Problem

test-simulation's InactiveVaultRefund-BTC.BTC actor times out intermittently: THORChain schedules REFUND:<inhash> out of the inactive vault, and every bifrost fails with

fail to get unspent UTXO: insufficient available UTXOs: need 2616478, only have 2581478 available from 1 UTXOs

Root cause

The two sides disagree about what MaxGas means:

  1. THORChain sizes the refund correctly at scheduling. Outbounds from an InactiveVault don't pay the outbound fee; they deduct exactly the current MaxGas instead (manager_txout_current.go, vault.Status == InactiveVault). In the sim run that was 49,000 sats (gas rate 49): refund 2,532,478 = inbound 2,581,478 − 49,000.
  2. THORChain then re-prices MaxGas without resizing the amount. TxOutStorage.EndBlock sets each outbound's MaxGas to the current gas details in the block a delayed outbound is released. The rate had risen to 84, so the released txout carried max_gas 84,000.
  3. bifrost treats MaxGas as a hard input requirement. getPaymentAmount returns amount + MaxGas and getUtxoToSpend refused to build unless the inputs covered it: 2,532,478 + 84,000 = 2,616,478 > 2,581,478. The transaction it would actually sign only needs amount + actual fee: about 2,532,478 + 18,732 (84 sat/vB × 223 vB).

The failure is intermittent because it depends on the gas rate rising between scheduling and release. The shortfall equals that rise × 1,000 vB: the three CI occurrences were short by 6,000, 35,000 and 50,000 sats.

Fix: bifrost selection gets a floor (no consensus change)

MaxGas only caps the fee. Selection still aims for amount + MaxGas exactly as before. When every eligible UTXO together falls short of that but still covers the amount, it now returns all of them and lets buildTx decide with the real fee:

  • getUtxoToSpend(pubkey, total, floor): shared by BTC, LTC, BCH and DOGE. buildTx passes floor = the coin amount. Consolidation passes 0, which keeps the old strict behaviour.
  • selectZecUtxos(..., target, floor, ...): ZEC's confirmed-input selector had the same amount + MaxGas requirement. ZEC/DOGE fee rates come from block averages, so their MaxGas moves as well. MaxGas outbounds pass floor = target − MaxGas. The no-MaxGas ZIP-317 path and consolidation pass 0 (unchanged).
  • The not enough balance to pay customer error now includes inputs, amount and gas, because shortfalls are now reported there.

Solvency and overspend guarantees are unchanged

Nothing in what buildTx signs changes:

  • The fee is still the estimate at the outbound's gas rate, capped by maxFeeSats and by MaxGas (applyFeeLimits). The gas rate is never lowered.
  • The customer gets exactly the scheduled amount. The remainder goes back to the vault as change.
  • If inputs < amount + fee (+ P2WPKH memo outputs), the existing balance < 0 check refuses to sign. So the vault never spends more than amount + actual fee, and never more than amount + MaxGas.
  • The input set stays deterministic across signers: it is the same eligible set the old loop walked, in the same order.

Trade-off vs. fixing it in THORChain

The other option was to make THORChain keep the refund and its MaxGas consistent. For example, it could skip re-pricing inactive-vault outbounds upward in EndBlock, or re-deduct the difference from the amount. I chose not to because:

  • It's a consensus change. It needs version gating and regression-fixture churn, and every node operator must upgrade before it takes effect.
  • Freezing MaxGas would leave a released refund priced at a stale (lower) fee ceiling while rates are rising. Re-deducting would change an outbound's amount after scheduling, which complicates voter/action matching.
  • It would only cover the inactive-vault case. Non-refund outbounds hit the same path. DiscoverOutbounds reserves only the scheduling-time MaxGas of headroom in active/retiring vaults. So a delayed outbound that leaves a vault nearly empty, released after a gas-rate rise, also needs amount + MaxGas_new > the vault's UTXOs. bifrost's UTXO view can also legitimately be smaller than THORChain's (unconfirmed/dust exclusions). The bifrost fix covers all of these.

What the bifrost fix gives up: when a vault is short of amount + MaxGas, it spends all of its eligible UTXOs in one transaction instead of failing. That only happens when the vault can't cover the full ceiling anyway, so those UTXOs are needed. The selection loop already went past MaxUTXOsToSpend when it needed more inputs to reach its target.

Not changed / residual edges

  • Migrations add the unused MaxGas gap to the amount paid to the new vault, so they still need amount + MaxGas. A short migration fails at the balance check instead of at selection, and still never overspends.
  • Near-dust change. Change can now be anywhere in [0, MaxGas − fee). A change below the relay dust limit (e.g. < 294 sats on BTC P2WPKH) would be rejected at broadcast. That is a narrow window, and the same edge already exists today whenever the fee lands just under MaxGas. I left it out to keep this fix minimal.
  • A gas-rate rise of roughly 4.5× or more pushes the actual fee past the deducted MaxGas. Then the refund still can't be paid, and bifrost refuses it rather than lowering the rate or eating into the amount. This is the correct outcome under the overspend rules.

Tests

New bifrost/pkg/chainclients/utxo/utxo_maxgas_shortfall_test.go. All 6 tests fail with the floor disabled (the old behaviour) and pass with it:

  • TestBuildTxRefundShortOfMaxGasPaysActualFee: the sim's exact numbers. Before the fix it fails with the sim's error (need 2616478, only have 2581478 available from 1 UTXOs). With the fix it pays exactly 2,532,478, the fee = 84 × estimated vsize (≪ MaxGas), and the rest goes back to the vault as change.
  • TestBuildTxShortOfMaxGasKeepsFeeWithinMaxGas: the estimate is above MaxGas, so the fee is capped at MaxGas. One sat short of amount + MaxGas is refused.
  • TestBuildTxShortOfActualFeeFails: inputs cover the amount but not the fee, so it is refused.
  • TestBuildTxShortOfAmountFailsSelection: the selection error now reports the amount it actually needs.
  • TestUTXOSelectionFloorSpendsAllEligibleWhenShortOfTotal: selector semantics. Strict mode is unchanged, the floor is reached / not reached, and a reachable total still returns the oldest covering prefix.
  • TestZECRefundShortOfMaxGasPaysActualFee: the same scenario through ZEC's confirmed-input selection, including the short-of-fee and short-of-amount cases.

go test -tags=mocknet ./bifrost/pkg/chainclients/utxo/... ./bifrost/pkg/chainclients/shared/... passes. scripts/lint.sh, go vet, gofmt and staticcheck are clean on the touched package. The change is bifrost-only, with no consensus paths touched.

Closes #3383

🤖 Generated with Claude Code

Merge request reports