Uh oh!
There was an error while loading. Please reload this page.
feat: deep link spending hw sign - #1176
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Greptile SummaryThe PR adds debug-only deep-link navigation into the hardware-wallet spending-sign flow, restoring the requested Blocktank order before navigation.
Confidence Score: 5/5The 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.
|
| 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()
Reviews (1): Last reviewed commit: "fix: consume deeplink after prepare" | Re-trigger Greptile
jvsena42
left a comment
There was a problem hiding this comment.
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 →
refreshOrdersruns, order is adopted,isAdvancedresets (Advanced button flips back from "Use Defaults"), sign screen opens with the right amounts; - already-current order id → short-circuits on
current?.id == orderIdand opens directly; - unknown wallet and unknown order are both refused, no navigation.
One blocking issue and three smaller ones inline.
| val state by viewModel.spendingUiState.collectAsStateWithLifecycle() | ||
| val order = state.order ?: run { | ||
| val order = state.order?.takeIf { it.id == orderId } ?: run { |
There was a problem hiding this comment.
Blocking — this guard breaks the existing Advanced flow.
orderId is a route arg, frozen when SpendingHwSign was pushed. But onAdvancedClick pushes Routes.SpendingAdvanced, and onSpendingAdvancedContinue calls blocktankRepo.createOrder(...) and stores a new order with a new id as spendingUiState.order (the old one moves to defaultOrder). SpendingAdvancedScreen's onOrderCreated is navController.popBackStack() — which lands back on SpendingHwSign still carrying the old id. takeIf yields null and the composable calls onCloseClick() → navigateToHome().
Reproduced on device: HW detail → Transfer To Spending → 25% → Continue → Sign → Advanced → MAX → Continue lands on the wallet home screen, and the freshly created order is stranded. Logcat:
INFO [BlocktankRepo.kt:285] Buying channel with lspBalanceSat: '341987', ...
DEBUG [BlocktankRepo.kt:222] Orders refreshed: 7 orders, 0 cjit entries, 2 paid orders
and blocktank.db then holds 3c10c207-… Created 341987 110227 with nothing pointing at it.
Suggest accepting defaultOrder?.id == orderId as a match too, or applying the id check only on first entry (deep-link admission) rather than on every recomposition.
| val current = _spendingUiState.value.order | ||
| if (current?.id == orderId) return true | ||
| val order = blocktankRepo.getOrder(orderId, refresh = true).getOrNull() |
There was a problem hiding this comment.
Worth confirming this is the intent: BlocktankRepo.getOrder refreshes and then searches _blocktankState.value.orders, i.e. only orders this install already tracks locally. An order that exists on the LSP but was never created on this device is always refused.
Verified on device — I created a valid order via the Blocktank API with this node's clientNodeId, then deep-linked to it:
WARN [TransferViewModel.kt:599] Refused spending hw sign deeplink, missing order 'c464a24c-…'
while a locally-created order id opened the sign screen fine. That's the right behaviour if the link is only ever meant to resume a transfer started on this device; it does mean a link handed over from another device or from support tooling can never resolve. Fine to leave as-is — just flagging it so the constraint is deliberate.
| setTransferEffect(TransferEffect.OnOrderCreated(order.id)) | ||
| } | ||
| suspend fun prepareSpendingHwSign(walletId: String, orderId: String): Boolean { |
There was a problem hiding this comment.
prepareSpendingHwSign → adoptSpendingOrder clears pendingHwFundingBroadcast and hasPendingHwBroadcast. If the user has already signed a HW funding tx for order X that failed to broadcast, onTransferToSpendingHwConfirm relies on pendingHwFundingBroadcast?.matches(...) to retry without re-prompting the device — a deep link naming a different order Y silently discards that in-memory signed transaction, making it unrecoverable.
Dev-mode only, so low severity, but cheap to guard: refuse the link (or skip the clobber) while hasPendingHwBroadcast is set.
| ScreenDeepLinks.spendingHwSignLink(uri)?.let { link -> | ||
| val prepared = transferViewModel.prepareSpendingHwSign(link.walletId, link.orderId) | ||
| if (!prepared) { | ||
| Logger.warn("Unhandled screen deeplink '$uri'", context = "ContentView") |
There was a problem hiding this comment.
This warn duplicates the specific reason already logged inside prepareSpendingHwSign, and reuses the exact wording of the generic !handled warn a few lines below. Observed on device — one refused link produces two WARN lines, the second of which says "Unhandled" when the link was in fact recognized and deliberately refused:
WARN [TransferViewModel.kt:591] Refused spending hw sign deeplink, unknown wallet 'foo'
WARN [ContentView.kt:328] Unhandled screen deeplink 'bitkit://screen/spending-hw-sign/foo/bar'
Per the repo rule (NEVER duplicate error logging in .onFailure {} if the called method already logs the same error internally), drop this line or reword it so it doesn't collide with the generic one.
Separately, consumeScreenDeepLink() now appears three times in this effect. It can't be hoisted to the top — that's what f405cd3 fixed, since the effect is keyed on pendingScreenDeepLink and consuming early cancels the coroutine mid-prepareSpendingHwSign — but the three calls can collapse into a single one at the end by turning the two early returns into an if/else chain.
| fun isScreenDeepLink(uri: Uri): Boolean = | ||
| uri.scheme?.lowercase() == SCHEME && uri.host?.lowercase() == HOST | ||
| fun spendingHwSignLink(uri: Uri): SpendingHwSignLink? { |
There was a problem hiding this comment.
Unlike linksFor/sheetFor, this isn't routed through ScreenDeepLinkRuntime/isEnabled, so it parses and returns a link in release builds too. Currently harmless because AppViewModel.processDeeplink gates queueing on ScreenDeepLinks.shouldQueue(...), but that single call site is the only thing stopping a release build from mutating live transfer state from a dev-only URI. An if (!isEnabled) return null here would make it fail safe.
ovitrif
commented
Aug 27, 2026
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) |
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
InternalOnlybecause they read activity-scopedTransferViewModelstate.SpendingHwSignwas the closest: it already took a wallet id (the issue still saysdeviceId) and bounced home whenspendingUiState.orderwas null. A generated link therefore could not reconstruct the Blocktank order.ContentViewnow parses that URI and callsprepareSpendingHwSignbeforenavController.handleDeepLink. Unknown wallet or missing order is refused with the existingUnhandled screen deeplinkwarning and does not navigate. A matching in-memory order is reused; otherwiseblocktankRepo.getOrder(orderId, refresh = true)loads it andadoptSpendingOrderwrites the same stateonOrderCreatedalready wrote, without emittingTransferEffect.OnOrderCreated. The pending URI is consumed after that suspend, so theLaunchedEffectis not cancelled mid-fetch.OnOrderCreatednow carriesorderId. Amount → Sign navigatesRoutes.SpendingHwSign(walletId, orderId)from the effect. The dest reads both route args and matchesstate.ordertoorderId.Routes.SpendingHwSigntoDeepLinkablewith pathbitkit://screen/spending-hw-sign/{walletId}/{orderId}.ScreenDeepLinks.spendingHwSignLinkviakebabId(Routes.SpendingHwSign::class).SavingsProgress,SettingUp,SpendingAdvanced,SpendingConfirm, andSpendingHwSignedasInternalOnly. 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
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.Unhandled screen deeplink.regression:bitkit://screen/spending-amount-hw/<walletId>→ Amount still opens.Automated Checks
ScreenDeepLinksTest.kt: path pattern, wallet and order segments, missing order id.TransferViewModelTest.kt: known wallet loads the named order, unknown wallet and missing order are refused, in-memory order is reused without a second fetch.just compile,just test,just lintall pass, no new detekt findings.