Fix intermittent "fee cannot be less than vSize" errors and Firo EX address sends - #1426
Open
reubenyap wants to merge 3 commits into
Open
Fix intermittent "fee cannot be less than vSize" errors and Firo EX address sends#1426reubenyap wants to merge 3 commits into
reubenyap wants to merge 3 commits into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 vSizeexception inprepareSend, plus a regression that broke sends to Firo EX (exchange) addresses through the default coin selection path.Background
prepareSendrejects 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-shotvSize - fee == 1adjustment (which re-signs without re-checking, and misses gaps ≥ 2), and_sendAllBuilderhas 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 justmwebPegIn. Fee-overridden and plainmweb/mwebPegOutsends (whose fee is recalculated by the caller) break out after the first build exactly as before, so MWEB behavior is unchanged.coinSelection: replace the one-shot== 1adjustment 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'sdefaultFeeRate, consistent with the-1fallback already inElectrumXClient.estimateFeeone 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-1case was handled._optimalCoinSelection: parse the recipient with the same Firo EX (exchange) address fallbackbuildTransactionuses — now shared as_addressFromString.Firo.validateAddressaccepts EX addresses, but the optimal-selection path (default for normal sends) parsed with barecoinlib.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-changedclean on both changed files.flutter analyzeon the changed files: identical findings before and after (10 pre-existinglines_longer_than_80_charsinfos, none introduced).flutter testsuite run locally (Flutter 3.38.5, configured viabuild_app.sh -p linux -a stack_wallet -d -sper the Test workflow).🤖 Generated with Claude Code