fix: settle spending transfer maximums - #686
Draft
jvsena42 wants to merge 6 commits into
Draft
Conversation
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.
This PR:
Ports synonymdev/bitkit-android#1179 and synonymdev/bitkit-android#1180.
Description
Sizing a max transfer takes two LSP fee quotes: the first prices the whole available balance, the second re-prices at the balance left after that fee. The maximum was then set to the available balance minus the second quote — a balance that quote never priced. The resulting order can cost more than the user has. On Android it was one satoshi over, and the transfer failed with an insufficient funds error. The same derivation is here, and it also backs the hardware wallet limits.
Capping the maximum at the quoted balance is not enough, because the service fee moves with the client/LSP split and the direction differs by deployment: in production it rises with the client balance, on staging and regtest it falls. That belongs to the LSP rather than the app, so the maximum is now verified instead of assumed — each round re-quotes its own candidate, and only an amount whose own quote fits the budget is offered. Where the fee rises the first check passes immediately, so that path keeps its current latency.
The advanced screen has the same gap: Min, Default and Max come straight from the LSP's liquidity options, which know nothing about the client balance already committed. Since the LSP prices both sides, raising the receiving capacity raises the order fee, so Max on a transfer sized near the balance produced an order that only failed later on the confirm screen. The offered maximum is now settled before it is shown, and settling it below what is already entered brings the entered amount down with it.
Both maximums are backstopped by a live re-check before the order is created, since a balance can drain after the limits were sized. A hardware transfer reads the device account, never on-chain savings, which would reject every hardware transfer. An unreadable balance or a missing quote deliberately does not block the user — the confirm step stays the authority — and both cases are logged.
Two differences from Android, for anyone comparing: the fee is read as network plus service fee rather than the estimate's total, since the bindings document neither convention and the split is what the existing calculation already uses; and Android's fee-quote race fix is not ported, because the advanced screen here already cancels its in-flight quote correctly.
Linked Issues/Tasks
Screenshot / Video
QA Notes
Needs the whole on-chain balance transferred at once — the gap only appears at exactly Max. Regtest is regression coverage only: its vulnerable window is roughly 2 satoshis wide versus about 37 in production, and its channel limits drift on their own. The unit tests are the gate for the fix itself.
Manual Tests
regression:Spending Advanced → Default → Continue: the order is created normally.regression:Spending Advanced → Min → Continue: the order is created normally.regression:Spending Amount → ¼ → Continue → swipe: funds normally.Automated Checks
BitkitTests/TransferViewModelTests.swift(4 → 24): both fee directions, each re-quote pricing the split the order will use, a failed re-quote, exhausted rounds, six advanced-capacity cases, and the funding guards.BitkitTests/TransferViewModelHwTests.swift(32 → 35): the budget reads the device account, and is nil when the device is unreachable so the guard stays non-blocking.xcodebuild testpasses (975 tests; only the regtest-dependentUtxoSelectionTests.testUtxoSelectionfails, on an unreachable Blocktank endpoint), plusswiftformat --lintandnode scripts/validate-translations.js.