fix: show the amount that leaves on spending confirm - #772
Conversation
|
jvsena42
left a comment
There was a problem hiding this comment.
No HIGH or MEDIUM findings. The displayed total matches what leaves the wallet on both paths. Two LOWs inline.
Traced:
- Normal path. The display is
feeSat + transactionFee, andorderForDisplayedFeekeepsorder.feeSat <= uiState.feeSat, otherwise it bumps the fee, throws and re-sizes. The mining fee is recomputed at the displayed rate right beforepayOrder, which sendsorder.feeSatwith the selected UTXOs. What leaves is at most what is shown. - Send-all.
maxSendable + sendAllFeereconstructs the spendable balance, guarded when the balance is below the fee, andsendAllToAddress(retainReserve: true)spends exactly that.onConfirmre-sizes immediately before broadcast. - Re-sizing.
sizeFundingdoesn't clear state first, so the previous total stays visible instead of flashing zero. A stalemaxSendableAmountafter a normal-path re-size is ignored becauseshouldUseSendAllis false, whichtestNormalPath_IgnoresStaleMaxSendablepins. On failure everything resets and the swipe is disabled. - Hardware. Untouched. The grid is
feeSat + miningFeeSats, the PSBT is an exact-amount compose fororder.feeSat, and there is no send-all there. - Stacking. This sits on top of #741 (merged) and relies on its
orderForDisplayedFeeand.task(id: feeSat). No conflict.
Pre-existing and out of scope, worth a follow-up issue on both platforms: shouldUseSendAllToAvoidDust decides on the selected UTXO subset, but sendAllToAddress then drains every spendable UTXO and ignores utxosToSpend. With several UTXOs, if the largest one alone covers the order and leaves dust change, the whole wallet is swept to the LSP address and the order is overpaid. This PR makes the screen show that honestly; the swipe behaviour is unchanged. synonymdev/bitkit-android#1323 has the same shape.
jvsena42
left a comment
There was a problem hiding this comment.
Follow-up from the android twin synonymdev/bitkit-android#1323, same mechanism here and pre-existing, not introduced by this PR.
onConfirm re-runs calculateTransactionFee against the order address and order.feeSat and pays immediately, while orderForDisplayedFee stops only when the order costs more than the estimate. When it costs the same or less, or a UTXO arrives in between, the recomputed plan can flip to send-all through shouldUseSendAllToAvoidDust, which decides on the selected subset while the sweep is wallet-wide. The new numbers are published to the view, but only for the instant before payment, so there is no second swipe.
Pinning confirmedFeeRate (from #741) removes the fee-rate trigger, which android still has. Worth the same guard: if the recomputed leaving amount differs from the displayed one, update and require another swipe.
|
QA reviewed on 4f67926. Spending confirm now shows the amount that leaves. On a send-all transfer that is the spendable balance. On the normal path it is still the order fee plus the miner fee. Ran on iPhone 17. MAX showed network fee 143, service fee 596, to spending 40 046, total 40 785. A 39 900 transfer showed network fee 143, service fee 597, to spending 39 900, total 40 640. Both totals match the three rows added together, so this wallet did not hit send-all. Did not swipe. Twin bitkit-android#1323 showed the send-all case: total 20 000 while the rows added up to 19 895. FindingsNo findings. Checked and clean
QA LGTM |
|
Pushed b7c5ba8. The send-all transfer record now uses the same leaving amount as the confirm screen, and that helper saturates instead of trapping.
@jvsena42 That rebuild is unchanged from #741. This push only makes the recorded total match the amount on screen. A second swipe when the rebuilt plan differs is tracked in #783. |
piotr-iohk
left a comment
There was a problem hiding this comment.
Spending confirm shows the amount that leaves. On send-all that is the savings balance. On the normal path it is the order fee plus the miner fee. The transfer record uses that same amount.
Ran on iPhone 17 / C0525820-E7A8-44DD-8912-9ABF7A825153, regtest, 200 000 in savings.
- 10 000: network fees 422, service fees 2 260, to spending 10 000, total 12 682.
- MAX: network fees 329, service fees 2 048, to spending 197 623, total 200 000. The network-fee row stayed the miner fee.
- Swiped MAX. The total stayed 200 000 through the spinner. Savings went to 0, spending to 197 623, and activity showed a 200 000 transfer from savings.
Twin bitkit-android#1323 showed the same totals.
The swipe that can pay a different amount than the screen is still bitkit-ios#783. This MAX swipe did not hit it.
Findings
No findings.
QA LGTM
jvsena42
left a comment
There was a problem hiding this comment.
Delta since 4f679264d (b7c5ba870): no findings. Both LOWs are fixed.
payOrdernow recordstxTotalSatsthroughSpendingConfirmTotal.leavingAmountwithisMaxAmountandmaxSendableAmount, both of which are its own parameters, so a send-all sweep is no longer under-stated in the transfer record thatBalanceManagersubtracts. The send-all fallback whenmaxSendableAmountis nil keeps the old formula, andpayOrderalready rejects that case earlier when the max is below the order fee.- The helper saturates in both branches, with an
UInt64.maxtest for each.
Fixes #773
Description
Out of Scope
Design
Transfer to Spending Balance From Savings (Channel Opening) › Transfer to Spending Confirm. Same frame as the Android screen.
Preview
N/A — screenshot needs a funded send-all confirm, not captured in this pass.
QA Notes
Journeys
N/A
Manual Tests
Automated Checks
SpendingConfirmTotalTests.swift— overflow saturation on both pathsSpendingConfirmTotalTests.swift— 6 passed, 0 failed