fix: settle spending transfer maximums - #686
Conversation
|
Heads-up for whoever lands this: a
If the journeys branch merges first, this PR should update them. The relevant change is in if maxLspBalance > 0, maxLspBalance < amountViewModel.amountSats {
amountViewModel.updateFromSats(maxLspBalance, currency: currency)
}That brings the receiving-capacity screen in line with what
A row for the advanced screen should be added once this lands, since it will move from the Send-style rejection to the Transfer-style snap. If this PR merges first instead, the journeys branch picks the change up and nothing is needed here. |
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.