Skip to content

Fix intermittent "fee cannot be less than vSize" errors and Firo EX address sends - #1426

Open
reubenyap wants to merge 3 commits into
cypherstack:stagingfrom
firoorg:claude/fee-vsize-consistency
Open

Fix intermittent "fee cannot be less than vSize" errors and Firo EX address sends#1426
reubenyap wants to merge 3 commits into
cypherstack:stagingfrom
firoorg:claude/fee-vsize-consistency

Conversation

@reubenyap

Copy link
Copy Markdown
Contributor

Fixes several inconsistencies between estimated fees and the final signed transaction that cause the intermittent Error in fee calculation: Transaction fee cannot be less than vSize exception in prepareSend, plus a regression that broke sends to Firo EX (exchange) addresses through the default coin selection path.

Background

prepareSend rejects any prepared transaction whose fee is below its vSize. The legacy fee paths (coin control, send-all) estimate the fee from a dummy build with different output amounts, then re-sign the final transaction. ECDSA signature length varies between signings (~70 vs occasionally 69 byte DER), so the final vSize can exceed the dummy-based estimate by a byte or two. At the 1 sat/vB floor — the normal state for Firo — that trips the guard intermittently. The codebase already knew about this: the two-output branch has a one-shot vSize - fee == 1 adjustment (which re-signs without re-checking, and misses gaps ≥ 2), and _sendAllBuilder has a proper recheck loop, but only for MWEB peg-ins ("Signing can change vSize...").

Simulating both paths with realistic signature-size variance (200k trials per rate) shows the old code trips the guard at 1000 sats/kvB in ~0.09% of two-output builds and ~4% of send-all builds; the fixed code never trips it and converges within ≤ 3 rebuilds while conserving input value = recipient + change + fee.

Changes

  • _sendAllBuilder: use the existing "recalculate fee from the final tx" loop for all transaction types, not just mwebPegIn. Fee-overridden and plain mweb/mwebPegOut sends (whose fee is recalculated by the caller) break out after the first build exactly as before, so MWEB behavior is unchanged.
  • Two-output branch of coinSelection: replace the one-shot == 1 adjustment with a loop that takes the shortfall from change and rebuilds until the fee covers the final signed size (bounded: the fee increases strictly each pass). If the adjusted change would be dust, the change output is dropped and it reverts to a single-output transaction.
  • singleOutputTxn: report the actual fee paid (difference, the whole input excess) instead of the smaller rate-based estimate — without a change output the excess is the fee, and the previous value both understated the fee in the UI and could trip the guard spuriously.
  • get fees: clamp server fee estimates below the coin's defaultFeeRate, consistent with the -1 fallback already in ElectrumXClient.estimateFee one layer down. Previously any low positive response (e.g. a misconfigured server returning under 0.00001 FIRO/kB) made every send fail the guard deterministically while the -1 case was handled.
  • _optimalCoinSelection: parse the recipient with the same Firo EX (exchange) address fallback buildTransaction uses — now shared as _addressFromString. Firo.validateAddress accepts EX addresses, but the optimal-selection path (default for normal sends) parsed with bare coinlib.Address.fromString, which throws on them — so transparent sends to exchange addresses failed. This also makes the selection size the larger EX output correctly.
  • BigIntExtensions.atLeast: small helper for the two clamp sites above.

Complementary to #1422, which fixes the remaining path (OP_RETURN output missing from optimal-selection sizing); the two do not conflict.

Verification

  • dart format --set-exit-if-changed clean on both changed files.
  • flutter analyze on the changed files: identical findings before and after (10 pre-existing lines_longer_than_80_chars infos, none introduced).
  • Full flutter test suite run locally (Flutter 3.38.5, configured via build_app.sh -p linux -a stack_wallet -d -s per the Test workflow).
  • Monte-Carlo + adversarial simulation of both rebuild loops: termination (≤ 3 iterations), fee ≥ final vSize, change > dust, and value conservation hold across rates 1000–25000 sats/kvB; the pre-fix logic reproduces the reported exception at 1 sat/vB.

🤖 Generated with Claude Code

claude and others added 3 commits August 6, 2026 12:26
The legacy fee paths estimate fees from a dummy build, then re-sign the
final transaction. Signature length can differ between signings, so the
final vSize occasionally exceeds the estimate and prepareSend throws
"Transaction fee cannot be less than vSize" at 1 sat/vB rates.

- _sendAllBuilder: apply the existing recalculate-from-final-tx loop to
  all tx types, not just mwebPegIn. Overridden and mweb/mwebPegOut fees
  (recalculated by the caller) still build once as before.
- coinSelection two-output branch: replace the one-shot vSize - fee == 1
  change adjustment with a loop that rebuilds until the fee covers the
  final signed size, dropping change and reverting to a single output
  if it would become dust.
- singleOutputTxn: report the whole input excess as the fee instead of
  the smaller rate-based estimate.
- fees getter: clamp server estimates below the coin's defaultFeeRate,
  consistent with the -1 fallback in ElectrumXClient.estimateFee, so a
  broken estimate cannot force the guard failure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NFAYSbQoTjdbLmGwNJWzCj
_optimalCoinSelection parsed the recipient with bare
coinlib.Address.fromString, which throws on Firo EX (exchange)
addresses, so normal sends to them failed since the switch to coinlib
coin selection. Share buildTransaction's EX fallback as
_addressFromString and use it in both places.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NFAYSbQoTjdbLmGwNJWzCj
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants