Conversation
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.
|
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
left a comment
There was a problem hiding this comment.
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.
|
Went through my six open review threads against The short version: #1247 (
So Two things from the old review are worth carrying into whatever replaces it:
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>
|
Rebased this onto What changedThe link was loadHwLimits(walletId)
if (!quoteSpendingAmount(amountSats)) { … return false }To make that awaitable, Three review points folded in:
A malformed link is now refused rather than navigated to. The old parser returned QAPixel_9 emulator, dev build, paired Trezor emulator (device balance 24 831 128 sats, LSP cap 16 935 sats).
Every refusal keeps the wallet overview up and logs exactly one line, with no duplicate "Unhandled screen deeplink":
The in-app path still works: HW amount screen → 25% → Continue reaches the sign screen with One pre-existing issue, not from this PR. On the Advanced screen reached from the sign screen, the liquidity fee stays
DesignN/A — no design available. QA NotesNew cover in 🤖 Generated with Claude Code |
# Conflicts: # journeys/README.md
# Conflicts: # journeys/README.md

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.