[jvsena42]

Closes #1786
Twin: synonymdev/bitkit-ios#1221

This PR cuts the hardware composes for a payment request funded from a hardware wallet down to one before the confirm screen is ready. Today it runs a send-max compose per wallet, then up to four exact composes (one per fee speed) on the single core queue, plus composes from a refresh that the confirm navigation cancels. Same problem as synonymdev/bitkit-ios#1187.

Description

• AppViewModel.selectHardwareFundingSourceForAmount: tries hardware wallets largest balance first and composes the exact payment at the selected speed. A successful compose proves the wallet can pay. When it fails, the old max-spendable compose still runs, so the insufficient-funds toast is unchanged.
• HwWalletRepo.estimateFundingMiningFee: one compose per wallet, address, amount, fee rate and funding balance (standing in for the UTXO set), shared by concurrent callers and owned by the repository scope, so a cancelled caller leaves its result for the next one. Failures are not kept; entries for an older balance are dropped.
• refreshOnchainFeeUi: hardware sends estimate only the selected speed and leave estimates empty, so SendEvent.SpeedAndFee estimates the other speeds when the speed picker opens. Signing then does not queue behind three extra composes.
• Source selection no longer starts its own fee refresh; onScanOnchain starts one after navigating to Confirm.
• The initial hardware available amount on this path is balance minus this payment's fee, which can be slightly above the max-spendable amount; it feeds the balance warning only.

Review round 1

• Fee compose cache keyed on the funding account's balance plus its transaction ids, so different coins at the same total compose again (2f09e74).
• A failed compose drops its own cache entry when it finishes, so a cancelled caller no longer leaves a failure cached; the compose starts only once its entry is stored (46d849d).

Review round 2

• A speed change no longer treats a payment request's amount as send-max when it equals the hardware available amount, which a change-free exact compose can produce (5929075).

Out of Scope

• bitkit-core onchain_compose_transaction: each call opens a new Electrum client and fully syncs a fresh wallet; reusing a synced wallet is a separate bitkit-core follow-up.
• Software (Savings) sends: their fee estimates are local and still compute all speeds.
• AppViewModel quoted requests (USD with fixed rate): the quoted path (AppViewModel.kt:4000-4043) never calls selectHardwareFundingSourceForAmount, so a quoted request Savings cannot cover opens on Savings with the swipe disabled instead of falling back to the hardware wallet. Seen in the device run; the path is unchanged here.
• SendFeeViewModel.getFeeLimit(): the fee cap uses the Savings balance (totalOnchainSats), so with empty Savings every preset speed is disabled on a hardware send and cannot be selected. Seen in the device run; unchanged here.

Design

N/A — no UI changes.

Preview

N/A

QA Notes

Setup

• Regtest against bitkit-docker, Trezor emulator over Bridge as in hardware-wallet.
• A contact that can send payment requests; Savings smaller than the request, the Trezor wallet larger.

Journeys

N/A — not drivable; see Manual Tests.

Manual Tests

• 1. Open a BTC payment request (1,000,000 sats) from a contact → logcat shows one composeTransactionOffline before Confirm shows a fee, and the swipe is enabled about one compose after Confirm appears — compose count from logcat is not in Capabilities
• 2. Same as 1 with a USD-quoted request → one compose
• 3. On that Confirm, open the speed picker → each speed shows a fee; pick another speed → Confirm shows its fee — compose count from logcat is not in Capabilities
• 4. Pay the request from 1 → the device signs and the payment broadcasts — regression check

Automated Checks

• added HwWalletRepoTest.kt — the same payment composes once and a different rate composes again; a failed compose is retried
• added HwWalletRepoTest.kt — coins changing at the same balance compose again; a cancelled caller's failed compose is retried by the next caller
• added AppViewModelSendFlowTest.kt — a hardware-funded request keeps its amount when the speed changes
• added AppViewModelSendFlowTest.kt — a request Savings cannot cover picks the Trezor without a send-max compose and estimates only the selected speed

Evidence

• HwWalletRepoTest (115) and AppViewModelSendFlowTest (583) at a471907: 0 failures.

• ./gradlew detekt at a471907: passed.

• Device run at c7e51ba (devDebug, staging regtest, TREZOR_BRIDGE=true), Android emulator (arm64), Trezor emulator over Bridge (Savings 0, Trezor 23,242,326 sats), requests from tools/paykit-fixture-sender (synonymdev/bitkit-e2e-tests). Composes counted from the cold Electrum sync each offline compose runs (one found tx_details for the account's oldest tx per compose):

  • 1. BTC 1,000,000 sats: one compose before Confirm; Confirm showed Normal (209 sats) with Swipe To Pay enabled. A second request (200,000 sats) also ran one compose and showed Normal (141 sats).
  • 2. USD 600 at a fixed rate (150,000 sats): opened on Savings with the swipe disabled and ran no hardware compose; see Out of Scope.
  • 3. Opening the speed picker ran one more compose (Fast, 423 sats); Normal and Slow share a rate and reused the stored fee (141 sats). Choosing Fast was blocked because every preset row is disabled; see Out of Scope.
  • 4. Swiped the request from 1 and approved on the emulator: the signing compose ran, the device signed, and the send screen showed success to the requested address.

• Round 1 at 2f09e74: HwWalletRepoTest 117/117 and AppViewModelSendFlowTest 583/583; the two new repo tests fail against c7e51ba. Full just test at the previous round-1 head: 4855 tests, the one failure fixed since. detekt, lintDevDebug and the androidTest compile pass.

• Round 2 at 5929075: AppViewModelSendFlowTest 584/584, HwWalletRepoTest 117/117; the new test fails without the fix (expected:<2500> but was:<48000>). compile, just lint and the androidTest compile pass.

Gaps

• Case 2 did not exercise the change: quoted requests never reach hardware source selection (pre-existing).
• Case 3 could not change the speed because of the pre-existing fee cap; the fee update after a speed change is unverified on device.
• Master was not re-run in the same session.

Models used

• Planning/scoping: claude-opus-5-5 (reasoning: Not exposed)
• Implementation: claude-opus-5-5 (reasoning: Not exposed)
• Review: Not performed

🤖 Generated with Claude Code

[github-actions]

Regtest APK

Built from 3fdea9c (run).

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

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issues were found.

Summary

Hardware-funded requests now reuse one exact-payment fee compose while preparing confirmation.

• Hardware-funded requests reuse one payment compose before Confirm.
• Payment requests keep their amount when fee speed changes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Request exceeds Savings] --> B[Try largest hardware balance]
  B --> C[Compose exact payment fee]
  C --> D{Compose succeeds?}
  D -->|Yes| E[Cache shared fee result]
  D -->|No| F[Check maximum spendable amount]
  E --> G[Open confirmation]
  G --> H[Reuse selected speed fee]
  H --> I{Open speed picker?}
  I -->|Yes| J[Estimate remaining speeds]
  H --> K[User confirms]
  K --> L[Compose transaction for signing]

Loading

_{Reviews (4) · Last reviewed commit: "fix: keep request amounts off the hardwa..." · Reviewed by Greptile}

[ovitrif]

Placed in 3.2.0 with its twin: a non-blocking fix found after the 3.1 cut. I filed #1786 to track it under the milestone and added Closes #1786 to this PR. The twin synonymdev/bitkit-ios#1221 closes synonymdev/bitkit-ios#1187, already in 3.2.0.

[pwltr]

Reviewed c7e51ba against 21f0b40.

Looks good. Fee composes for the same hardware payment are now shared and survive a cancelled caller, failures are not cached, and entries are dropped when the wallet balance changes. Choosing a hardware wallet proves the exact payment once, and the confirm screen reuses that fee instead of composing every speed up front. No findings.

Review details

Claude Opus 5.5, effort high

[talosmachina]

No findings. Composes a hardware-funded payment request once: the exact-amount compose that picks the funding wallet is cached in HwWalletRepo (keyed by wallet, payment, fee rate and funding balance) and reused by the confirm screen, and a hardware send now estimates only the selected speed until the picker opens. Reviewed c7e51ba, full tier. Static review: on this track I do not build the Android branch, so this is reasoned from the code and from CI. Twin: synonymdev/bitkit-ios#1221.

What I checked, and 5 candidates I ruled out

Read: the HwWalletRepo diff with composeFundingOffline, getFundingAccount and TrezorRepo.composeTransactionOffline. In AppViewModel: selectHardwareFundingSourceForAmount and its only caller in the payment-request path, hardwareAvailableForPayment, refreshOnchainSendIfNeeded, refreshOnchainFeeUi, getFeeEstimate, prepareHardwareSendFee, the SpeedAndFee handler, the speed-change path, validateAmount and handleSanityChecks. Also SendFeeViewModel.init and SendAmountScreen.
CI at review time: build, build-and-test, lint, detekt green. compose-ui and the local e2e shards still running.

Ruled out

• Dropping refreshOnchainSendIfNeeded from the selection loses the confirm fee: the one caller runs refreshOnchainSendIfNeeded(walletGeneration) right after navigateToSendRoute(... Confirm ...), and the toast path returns before navigating.
• Speed picker opens with no other speeds: SendEvent.SpeedAndFee runs the full refreshOnchainFeeUi whenever estimates is empty, which is the state selectedSpeedOnly leaves. SendFeeViewModel is the only reader of estimates.
• hardwareAvailableSats set to balance minus this payment's fee instead of the max spendable: the compose succeeding means the inputs cover amount plus fee, so it is never below amount. A speed change recomputes it through hardwareMaxSpendable, and wasHardwareMax only matches when the amount equals it.
• Cache serving a fee for a changed UTXO set: entries for the wallet are dropped as soon as its funding balance moves. The balance standing in for the UTXO set is the documented design, not an accident.
• A failed compose cached after its only waiter is cancelled: already raised in Greptile's open thread on HwWalletRepo.kt:805, so not repeated here.

Merge confidence: 4/5. The new tests cover the shared compose and the single selection, but the e2e shards had not finished, and the cancelled-waiter failure path in the open Greptile thread has no test.

Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (the runner does not report the setting to the review session)

[pwltr]

Reviewed commit c7e51ba.

Medium: invalidate hardware fee estimates when spending inputs change. The new cache keys successful estimates by total wallet balance, so spending and replenishing an account to the same total can reuse a fee calculated for different inputs. Confirmation and app fee warnings can then use the stale estimate, while signing freshly composes a transaction with a different fee. This is introduced by retaining completed estimates across payment preparations. Invalidate on transaction/input changes or scope reuse to a single payment preparation. Hardware-device transaction approval still occurs; this is an app fee-preview and warning correctness issue.

Review details

General review: GPT-6.1 Sol / high.
Finding verification: GPT-6.1 Sol / high.

[jvsena42]

@pwltr on the fee estimates keyed by balance: fixed in 2f09e74. The cache key is now the funding account's state, its balance plus the sorted txids of the native-segwit watcher's on-chain activities, not the balance alone. Any coin change (a spend, a top-up, a replaced or dropped transaction) changes the key and composes again, even when the total ends up the same.

Proof: new test estimateFundingMiningFee composes again when coins change at the same balance. The same coins at 50,000 sats reuse the compose. A spend and top-up back to 50,000 sats with new txids composes again. It fails against the code before this fix and passes with it. HwWalletRepoTest 117/117 and AppViewModelSendFlowTest 583/583 pass, along with detekt, lint and the androidTest compile.

Ported to synonymdev/bitkit-ios#1221 in 1f8643f45, which keys the stored preview on balance plus txids the same way.

The fix for Greptile's cancelled-caller finding is in 46d849d (reply in that thread).

[pwltr]

Reviewed commit 2f09e74.

Medium candidate: changing transaction speed after automatic hardware funding selection may change a fixed payment request’s amount. hardwareAvailableForPayment stores the balance minus the exact-payment fee; setTransactionSpeed treats equality with that value as send-max intent and replaces the amount with the newly computed maximum. For a valid no-change exact payment, the fixed request can satisfy that equality even when it is below the true maximum, and request validation then rejects the changed amount.

The native-composer behavior needed for this case remains unverified. Can the composer return that no-change exact-payment result? If so, fixed request amounts need to stay independent of the send-max heuristic.

Review details

General: GPT-6.1 Sol / High.

[jvsena42]

@pwltr on the speed change rewriting a fixed request amount: it reproduces, and it is fixed in 5929075.

Yes, the composer can return that result. Branch-and-bound accepts a change-free selection and adds the leftover below the change cost to the fee, so for an exact payment that spends every coin, balance - fee == amount. hardwareAvailableForPayment stores that value, and setTransactionSpeed read the equality as send-max, replacing the amount with the new max. A request amount within that dust-sized margin of the account's maximum triggers it, and request amounts come from the counterparty. Before this PR the same path needed the amount to equal the send-max result exactly.

Fix: wasHardwareMax now also requires activeIncomingPaymentRequest() == null, the same guard isMaxAmount already uses. A request's amount never follows send-max.

Proof: new test hardware funded request keeps its amount when the speed changes. The exact compose spends the whole 50,000 sat balance (available == 2,500 == amount), then the speed changes to Fast. Without the guard it fails with expected:<2500> but was:<48000>; with it, it passes. AppViewModelSendFlowTest 584/584 and HwWalletRepoTest 117/117 pass, along with compile, just lint (detekt and lintDevDebug) and the androidTest compile.

iOS (synonymdev/bitkit-ios#1221) does not have this: SendFeeRate and SendFeeCustom replace the amount only when wallet.isMaxAmountSend is set by an explicit Max choice, never by comparing amounts.

[pwltr]

Reviewed commit 5929075. Static review found no supported correctness findings in the hardware fee-cache and incoming-payment preparation changes.

Review details

General review
Model: GPT-6.1 Sol
Reasoning effort: high

[jvsena42]

Rebased on master at 3fdea9c to resolve a conflict with #1763.

Both changed the hardware send-max guard in setTransactionSpeed. #1763 excludes scanned amounts with fixedOnchainAmount == null, but fixedOnchainAmount is only set when incomingPaymentRequest == null (AppViewModel.kt:5310), so it doesn't cover payment requests. The resolved guard keeps both conditions. Master's new scan path also calls selectHardwareFundingSourceForAmount, so a scanned amount covered by a hardware wallet now gets this PR's single compose too.

Local: just compile, just test (4884 passed, 0 failed, including hardware funded request keeps its amount when the speed changes and master's speed change keeps a scanned hardware amount that equals the hardware maximum), just lint, androidTest compile. No device rerun after the rebase.

[ben-kaufman]

Reviewed 3fdea9c708f906b755bc0fc19d0ed15bddd765d8. No new material findings. Independent security passes remain incomplete (HTTP 401), and there was no device run after the rebase.

Reviewer model: claude-opus-5-5, gpt-6.1-sol
Reasoning effort: high for both