Skip to content

feat: deep link spending hw sign - #1176

Draft
guzino wants to merge 7 commits into
synonymdev:masterfrom
guzino:feat/spending-hw-sign-deeplink
Draft

guzino wants to merge 7 commits into
synonymdev:masterfrom
guzino:feat/spending-hw-sign-deeplink

Conversation

@guzino

@guzino guzino commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Refs #1126
Refs #1119

This PR opens the hardware-wallet transfer Sign screen from bitkit://screen/spending-hw-sign/{walletId}/{orderId}.

Description

#1119 left six transfer destinations InternalOnly because they read activity-scoped TransferViewModel state. SpendingHwSign was the closest: it already took a wallet id (the issue still says deviceId) and bounced home when spendingUiState.order was null. A generated link therefore could not reconstruct the Blocktank order.

ContentView now parses that URI and calls prepareSpendingHwSign before navController.handleDeepLink. Unknown wallet or missing order is refused with the existing Unhandled screen deeplink warning and does not navigate. A matching in-memory order is reused; otherwise blocktankRepo.getOrder(orderId, refresh = true) loads it and adoptSpendingOrder writes the same state onOrderCreated already wrote, without emitting TransferEffect.OnOrderCreated. The pending URI is consumed after that suspend, so the LaunchedEffect is not cancelled mid-fetch.

OnOrderCreated now carries orderId. Amount → Sign navigates Routes.SpendingHwSign(walletId, orderId) from the effect. The dest reads both route args and matches state.order to orderId.

  • Promotes Routes.SpendingHwSign to DeepLinkable with path bitkit://screen/spending-hw-sign/{walletId}/{orderId}.
  • Parses that path in ScreenDeepLinks.spendingHwSignLink via kebabId(Routes.SpendingHwSign::class).
  • Keeps SavingsProgress, SettingUp, SpendingAdvanced, SpendingConfirm, and SpendingHwSigned as InternalOnly. The rest of feat: deep link the late transfer screens #1126 stays a follow-up.

Preview

N/A

QA Notes

Dev mode is on by default on debug builds (Settings ▸ Advanced ▸ Dev Settings). The app must be past onboarding. The Sign path needs a paired hardware wallet and a live Blocktank order id.

Manual Tests

  • 1. Hardware Wallet detail → Transfer To Spending → Amount → Continue → Sign: lands on Sign With Your Device with the created order.
  • 2. From that Sign screen, adb shell am start -a android.intent.action.VIEW -d "bitkit://screen/spending-hw-sign/<walletId>/<orderId>" to.bitkit.dev → Sign opens with the same order.
  • 3. Cold start the same URI → Sign opens with that order, not Home.
  • 4. Same path with an unknown wallet id or a missing order → screen unchanged, logcat carries Unhandled screen deeplink.
  • 5. regression: bitkit://screen/spending-amount-hw/<walletId> → Amount still opens.

Automated Checks

  • Unit tests added in ScreenDeepLinksTest.kt: path pattern, wallet and order segments, missing order id.
  • Unit tests added in TransferViewModelTest.kt: known wallet loads the named order, unknown wallet and missing order are refused, in-memory order is reused without a second fetch.
  • Local: just compile, just test, just lint all pass, no new detekt findings.

Comment thread app/src/main/java/to/bitkit/ui/utils/ScreenDeepLinks.kt Fixed
@greptile-apps

greptile-apps Bot commented Aug 22, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds debug-only deep-link navigation into the hardware-wallet spending-sign flow, restoring the requested Blocktank order before navigation.

  • Extends the sign route and order-created effect with an order ID.
  • Validates the wallet, loads or reuses the requested order, and adopts it into transfer state.
  • Adds focused parsing, route-contract, preparation, and regression tests.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified.

The deep link remains restricted by the existing debug and Dev Mode gates, prepares the exact requested order before navigation, rejects unavailable prerequisites, and keeps internal navigation synchronized through the new order ID.

Important Files Changed

Filename Overview
app/src/main/java/to/bitkit/ui/ContentView.kt Coordinates order preparation before deep-link navigation and registers the sign destination with both route arguments.
app/src/main/java/to/bitkit/viewmodels/TransferViewModel.kt Adds validated order restoration, centralizes spending-order adoption, and includes the order ID in creation effects.
app/src/main/java/to/bitkit/ui/utils/ScreenDeepLinks.kt Parses the hardware sign route's wallet and order path segments while retaining existing screen-link gating.
app/src/main/java/to/bitkit/ui/screens/transfer/hardware/SpendingHwSignScreen.kt Ensures the in-memory order matches the route order before rendering the signing flow.
app/src/test/java/to/bitkit/viewmodels/TransferViewModelTest.kt Covers successful restoration, unknown wallets, missing orders, and reuse of a matching in-memory order.
app/src/test/java/to/bitkit/ui/utils/ScreenDeepLinksTest.kt Covers the generated URI contract and extraction of both required route identifiers.

Sequence Diagram

sequenceDiagram
    participant Intent as Screen deep link
    participant AppVM as AppViewModel
    participant Content as ContentView
    participant TransferVM as TransferViewModel
    participant Blocktank as BlocktankRepo
    participant Nav as NavController
    Intent->>AppVM: queue URI when debug runtime and Dev Mode permit
    AppVM-->>Content: pendingScreenDeepLink
    Content->>TransferVM: prepareSpendingHwSign(walletId, orderId)
    alt matching order already in memory
        TransferVM-->>Content: true
    else order must be restored
        TransferVM->>Blocktank: "getOrder(orderId, refresh = true)"
        Blocktank-->>TransferVM: order or missing
        TransferVM-->>Content: preparation result
    end
    alt prepared
        Content->>Nav: handleDeepLink(uri)
    else rejected
        Content->>Content: log unhandled link
    end
    Content->>AppVM: consumeScreenDeepLink()
Loading

Reviews (1): Last reviewed commit: "fix: consume deeplink after prepare" | Re-trigger Greptile

@jvsena42 jvsena42 self-assigned this Aug 27, 2026

@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.

Reviewed and validated on a regtest emulator against the deterministic Trezor Bridge emulator from bitkit-docker (T2T1, seed all all ..., paired as BITKIT TEST TREZOR with 26,890,661 sats).

The deep link itself works. Both branches of prepareSpendingHwSign were exercised end to end:

  • fresh order id → refreshOrders runs, order is adopted, isAdvanced resets (Advanced button flips back from "Use Defaults"), sign screen opens with the right amounts;
  • already-current order id → short-circuits on current?.id == orderId and opens directly;
  • unknown wallet and unknown order are both refused, no navigation.

One blocking issue and three smaller ones inline.

Comment thread app/src/main/java/to/bitkit/ui/screens/transfer/hardware/SpendingHwSignScreen.kt Outdated
Comment thread app/src/main/java/to/bitkit/viewmodels/TransferViewModel.kt Outdated
Comment thread app/src/main/java/to/bitkit/viewmodels/TransferViewModel.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/ContentView.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/utils/ScreenDeepLinks.kt
@ovitrif

ovitrif commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

General note from briefly looking over review comments: it may not have been specified in the issues or past comments but this screen deeplinks work is more intended to aid in development with ai agents, for example: it could add the possibility to open a specific screen and continue from there. Maybe it should not be a requirement that the rest of the flow(s) would work correctly, or even if it tries to, it should only add it in logic specific to the handler of that screen deeplink; while the impact on production code should try to stay limited to parametrizing the screen inputs (strings, bools, etc, whatever is needed as starting state / to mutate UI)

@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.

Reviewed this as a key-management change, and the trust boundary holds up: the link carries only {walletId, orderId}, both validated against local state (hwWalletRepo.wallets, and getOrder searching this client's own _blocktankState.orders). It cannot supply a PSBT, address, amount, fee rate or derivation path, the Trezor still confirms on-device, and shouldQueue = isEnabled && devMode keeps the whole surface out of release.

One substantive item, posted as a reply on the existing thread since the first half was already raised there.

Comment thread app/src/test/java/to/bitkit/viewmodels/TransferViewModelTest.kt Outdated
@jvsena42
jvsena42 marked this pull request as draft September 10, 2026 10:43
@jvsena42

Copy link
Copy Markdown
Member

Went through my six open review threads against origin/master (6d23c632) to check which still hold. Three are resolved, three are left open — details are in each thread.

The short version: #1247 (5620bef6 fix: create order only on swipe) landed and removed the concept this PR is keyed on. There is no order at the sign screen any more:

  • SpendingHwSignScreen no longer takes an orderId; it gates on state.feeSat == 0uL.
  • TransferEffect.OnOrderCreated(orderId) → TransferEffect.OnQuoteReady.
  • prepareSpendingHwSign and adoptSpendingOrder are gone from TransferViewModel.

So bitkit://screen/spending-hw-sign/{walletId}/{orderId} has no order id to name. I merged origin/master into the branch locally to see the damage: 7 files conflict — ContentView.kt, SpendingAdvancedScreen.kt, SpendingAmountScreen.kt, SpendingAmountHwScreen.kt, SpendingHwSignScreen.kt, TransferViewModel.kt, ContentViewTest.kt — and the TransferViewModel ones are all the same order-vs-quote split. I did not push that merge: resolving it is a redesign of the link's payload, which is your call, not a conflict fix.

Two things from the old review are worth carrying into whatever replaces it:

  • Master already added the in-flight guard I asked for, at the flow entry points rather than the link: if (confirmPayJob?.isActive == true || hwTransferSignJob?.isActive == true) return heads both onConfirmAmount and onSpendingAdvancedContinue. The redesigned link should sit behind the same guard.
  • ScreenDeepLinks.isScreenDeepLink is still not routed through ScreenDeepLinkRuntime/isEnabled on master, so AppViewModel.processDeeplink's shouldQueue(...) remains the only thing keeping a release build from acting on a dev-only URI. Worth making the new parse fail safe on its own.

Not blocking anything of mine — flagging it so the rebase isn't a surprise.

Master's synonymdev#1247 removed the order this link named, so the conflicts could
only be resolved by reworking the link onto the quote flow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jvsena42

Copy link
Copy Markdown
Member

Rebased this onto origin/master and reworked the link, since #1247 (5620bef6 fix: create order only on swipe) removed the order the old design named. Pushed as 105486a8a; all six of my review threads are now resolved.

What changed

The link was spending-hw-sign/{walletId}/{orderId}. There is no order at the sign screen any more, so it is now spending-hw-sign/{walletId}/{amountSats} and TransferViewModel.prepareSpendingHwSign produces the same quote the amount screen does:

loadHwLimits(walletId)
if (!quoteSpendingAmount(amountSats)) { … return false }

To make that awaitable, onConfirmAmount and updateHwLimits keep their fire-and-forget signatures but delegate to suspend cores (quoteSpendingAmount, loadHwLimits), and onEstimateReady returns whether it applied. Nothing is bought until the user swipes, so the link cannot strand an order.

Three review points folded in:

  • In-flight guard. The link is refused while confirmPayJob/hwTransferSignJob is active or pendingHwFundingBroadcast != null, so it cannot discard a signed-but-unbroadcast funding tx.
  • Fail-safe parsing. spendingHwSignLink returns null when !isEnabled, so a release build cannot reach transfer state through a dev-only URI even if a call site forgets shouldQueue(...).
  • One consume, one warn. The deep-link effect in ContentView consumes pendingScreenDeepLink exactly once, at the end (consuming early cancels the coroutine mid-prepareSpendingHwSign), and no longer emits a second generic "Unhandled screen deeplink" for a link it recognised and deliberately refused.

A malformed link is now refused rather than navigated to. The old parser returned null for a bad amount, which fell through to handleDeepLink, matched the route anyway, and bounced the user to the wallet home on feeSat == 0uL. The parse result is a sealed type now: null means "not this screen", Malformed means "this screen, refuse it".

QA

Pixel_9 emulator, dev build, paired Trezor emulator (device balance 24 831 128 sats, LSP cap 16 935 sats).

…/spending-hw-sign/trezor:8df665ab…81be5/10000 opens the sign screen on a live quote, with no Buying channel with lspBalanceSat line in the app log:

Sign screen opened by the deep link, showing TO SPENDING 10 000 and TOTAL 11 158

Every refusal keeps the wallet overview up and logs exactly one line, with no duplicate "Unhandled screen deeplink":

link result
…/not-a-wallet/10000 Refused … unknown wallet 'not-a-wallet'
…/<walletId>/0 Refused … malformed
…/<walletId>/abc Refused … malformed
…/<walletId>/999999999 Refused … no quote for '999999999' sats

The in-app path still works: HW amount screen → 25% → Continue reaches the sign screen with TO SPENDING ₿ 4 197.

One pre-existing issue, not from this PR. On the Advanced screen reached from the sign screen, the liquidity fee stays — and Continue does nothing. I re-checked it on a baseline build with ContentView.kt, ScreenDeepLinks.kt, TransferViewModel.kt and SpendingHwSignScreen.kt reverted to origin/master, and it stalls identically, so it is master behaviour with this emulator's LSP state. Probably worth its own issue.

just compile, just test and just lint are green.

Design

N/A — no design available.

QA Notes

New cover in ScreenDeepLinksTest.kt (valid link, other-screen fall-through, malformed arguments, disabled gate) and TransferViewModelTest.kt (quotes the linked amount without creating an order; refuses unknown wallet, non-positive amount, unfundable amount; keeps a pending hardware broadcast). journeys/deeplinks/spending-hw-sign-deeplink.xml walks the same cases on a device.

🤖 Generated with Claude Code

This branch has not been deployed

No deployments
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.

4 participants