Skip to content

fix: show the amount that leaves on spending confirm - #1323

Merged
ovitrif merged 4 commits into
masterfrom
fix/spending-confirm-leaving-amount
Sep 24, 2026
Merged

ovitrif merged 4 commits into
masterfrom
fix/spending-confirm-leaving-amount

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1326

Description

  • Spending confirm total now shows the amount of bitcoin that leaves the wallet, including the send-all path where that figure is the spendable balance.
  • Refreshing that plan, including after swipe replaces the estimate with the order fee, keeps the previous total until the new plan arrives.

Out of Scope

  • bitkit-ios#772: the same confirm total, which already keeps the previous amount while funding is resized
  • app logs: unchanged
  • hardware confirmation: it sends the exact order amount, so its total stays order fee plus miner fee

Design

Transfer to Spending Balance From Savings (Channel Opening) › Transfer to Spending Confirm, mapped in docs/screens-map.md.

Preview

N/A — screenshot needs a funded send-all confirm, not captured in this pass.

QA Notes

Journeys

N/A

Manual Tests

  • Transfer to spending until Confirm → choose an amount that triggers send-all → total equals the spendable balance that will leave the wallet (network fee row stays the miner fee).
  • Transfer to spending Confirm on the normal path → total remains order fee + miner fee.
  • Send-all confirm → swipe, and if the created order costs more, the total stays the previous leaving amount until the refreshed plan replaces it.

Automated Checks

  • ran TransferViewModelTest.kt via just test file "to.bitkit.viewmodels.TransferViewModelTest" — passed, including the send-all total and repreparing send-all funding keeps the leaving amount until the next plan
  • reran that refresh test after merging master — passed

@ovitrif ovitrif self-assigned this Sep 22, 2026
@ovitrif
ovitrif marked this pull request as ready for review September 22, 2026 19:27
@ovitrif
ovitrif requested review from a team, piotr-iohk and pwltr and removed request for a team September 22, 2026 19:27
@greptile-apps

greptile-apps Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

The change appears safe to merge, with a non-blocking transient display issue during send-all fee recalculation.

Findings

  1. P2 Stale send-all state shows zero ▶

Summary

This PR changes the spending-transfer confirmation total to represent the full amount leaving savings during send-all funding.

  • Propagates the planner’s spendable balance into confirmation UI state.
  • Selects between spendable balance for send-all and order fee plus miner fee for normal funding.
  • Adds unit coverage for both calculation paths and documents the fix.

Reviews (1) · Last reviewed commit: "fix: show the amount that leaves on spen..."

Comment thread app/src/main/java/to/bitkit/viewmodels/TransferViewModel.kt Outdated
@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from c2122a2 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No regression. The displayed total now equals what is debited on both paths, verified by a second pass. One pre-existing LOW-MEDIUM inline, about the mechanism this PR touches.

Checked and clean:

  • Normal path. Display is feeSat + miningFeeSats; the pay call sends order.feeSat with the plan's UTXOs and records the same total.
  • Send-all. Display is spendableBalance; sendAllToAddress(retainReserve = true) spends exactly that, with the fee taken out of it, and the recorded total matches.
  • Rows. LSP fee is the saturating feeSat - clientBalance, so it cannot underflow, and all sat arithmetic goes through USat.
  • Mid-refresh hold. prepareSpendingConfirmFunding now only clears isConfirmFeeReady, so the previous numbers stay on screen instead of flashing zero, and the swipe is disabled throughout. The failure branch still clears all four fields together, which is what greptile's P2 asked for.
  • Advanced and Default. Both re-quote through the same re-plan, and a changed LSP balance drops the cached order.
  • Hardware. Untouched. The grid stays feeSat + hwMiningFeeSats, the PSBT is composed for exactly order.feeSat, and PendingHwFundingBroadcast.matches pins that amount. No send-all there.

Non-blocking, pre-existing and shared with iOS: when dust avoidance triggers send-all, the sweep is wallet-wide rather than limited to the selected inputs, so the overpayment to Blocktank is not bounded by the dust limit. This PR now shows that honestly. Whether Blocktank credits an on-chain overpayment is unverified.

Comment thread app/src/main/java/to/bitkit/viewmodels/TransferViewModel.kt
@piotr-iohk

Copy link
Copy Markdown
Collaborator

QA reviewed on c2122a2.

Spending confirm now shows the amount that leaves. On a send-all transfer the total is the spendable balance, and a refresh keeps the previous total on screen until the new plan arrives.

Ran on Pixel_6. With 20 000 sats in savings, an 18 600 transfer showed network fee 110, service fee 1 185, to spending 18 600, and total 20 000. Those three rows add up to 19 895, so the total is the whole balance that send-all spends, not the order fee plus the miner fee. Left the app and came back: the same total was still there. Did not swipe.

Twin bitkit-ios#772 uses the same total. Not run on a simulator this round.

Findings

No findings.

Checked and clean

  • Send-all total is the spendable balance. The network-fee row stays the miner fee.
  • While the plan refreshes, the previous total stays and the swipe stays disabled.
  • A failed plan clears the send-all flag and the balance together, so the total does not sit at zero.
  • The normal path is still order fee plus miner fee. That case is in TransferViewModelTest.

QA LGTM

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c2122a2

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. A refresh keeps the previous total until the new plan arrives.

Ran on Pixel_6 / emulator-5554, 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 047, to spending 197 624, total 200 000. The network-fee row stayed the miner fee.
  • Swiped MAX from the grab handle. The total stayed 200 000 until success. Savings went to 0 and spending to 197 624.

Twin bitkit-ios#772 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

@ovitrif
ovitrif merged commit 44c82ed into master Sep 24, 2026
19 checks passed
@ovitrif
ovitrif deleted the fix/spending-confirm-leaving-amount branch September 24, 2026 20:08
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.

bug: spending confirm shows less than what leaves

4 participants