ZIP-321 standard support — open Vizor from zcash: links (all platforms)#280
Draft
piatoss3612 wants to merge 16 commits into
Draft
ZIP-321 standard support — open Vizor from zcash: links (all platforms)#280piatoss3612 wants to merge 16 commits into
piatoss3612 wants to merge 16 commits into
Conversation
Mirror the existing desktop com.zcash.wallet/payment_uri channel contract (takePendingUris / ready / onUris, buffer-then-flush) on mobile so a zcash:<address>?amount=... link opens Vizor's send screen prefilled. - Android: VIEW intent-filter (scheme=zcash, DEFAULT+BROWSABLE) on the singleTop MainActivity; capture the launch intent (cold start) in configureFlutterEngine and warm links in onNewIntent, buffering until Dart calls ready. - iOS: this app uses the UIScene lifecycle, so URLs arrive via the scene delegate, not application(_:open:). SceneDelegate forwards cold-start (willConnectTo) and warm (openURLContexts) zcash: URLs to a PaymentUriChannelBridge in AppDelegate. Info.plist registers the zcash URL scheme and sets FlutterDeepLinkingEnabled=false so Flutter's router does not also try to route the link. - PaymentUriService now treats android/iOS as supported platforms.
A zcash: link opened while the wallet is locked routed to /unlock and kept the parsed prefill in _PaymentUriLinkListener, then drained to /send when the wallet unlocked. But the unlock screens unconditionally navigate to /home on success, which ran inside routerRefresh.pauseWhile and overrode the drain's /send — so the payment intent was silently lost on the locked path (every platform; the unlocked/warm path was unaffected). Hoist the pending prefill into paymentUriPrefillProvider. The unlock flow claims it (take()) right after a successful unlock, before the post-unlock work can clear it, and routes to /send with the prefill when one is pending (else /home). _PaymentUriLinkListener now reads and clears the same provider, so the already-unlocked path is unchanged. Both desktop (unlock_screen) and mobile (mobile_unlock_screen) unlock paths are covered. Verified live on Linux: zcash: link while locked -> /unlock -> unlock -> lands on the prefilled Send screen (address + amount) instead of Home.
Drives the ZIP-321 payment-URI feature end to end on the live regtest chain: imports a faucet-funded wallet, injects a zcash:<addr>?amount= link over the com.zcash.wallet/payment_uri channel (the contract all five native runners implement), asserts the send screen is prefilled from the URI (address + amount, not typed), then reviews/confirms a real shielded send. Verifies the recipient account observes the pending receive, the funds mine in, and the sender shows Sent -0.25 Completed. Runner: scripts/e2e/flutter-macos-regtest-payment-uri-send.sh (mirrors the existing macos regtest send runners). Heavy; run only on request.
Regression guard for commit 7054d00: a zcash: link opened while the wallet is locked must survive the unlock screen and land on a prefilled /send (not the default /home). The test imports a faucet-funded wallet, signs out to lock it, injects the URI over com.zcash.wallet/payment_uri while locked, asserts the unlock screen stays up, then unlocks and asserts the send screen is prefilled (address + amount) before driving a real shielded send and verifying the recipient mines it in. Adds stable test keys the flow needs: unlock_password_field, unlock_submit_button (unlock_screen) and sidebar_sign_out_button (app_main_sidebar). Runner: scripts/e2e/flutter-macos-regtest-payment-uri-locked-send.sh.
The mobile router's /send route only read a bare String recipient from state.extra, so a payment URI (which arrives as SendPrefillArgs, like the desktop /send route receives) was dropped entirely on mobile — neither the address nor the amount/memo prefilled. Desktop worked because it uses a separate route set (SendScreen(prefill:)). Unpack SendPrefillArgs into MobileSendScreen's existing initialRecipient/ initialAmount/initialMemo params. MobileSendScreen.initState already lands on the amount step (with the address filled) when an amount is present, and stays on the address step otherwise. Bare-string recipients still work. Covered by a mobile-lane mobile_routes_test case.
Adversarial review of the locked-path fix found a data-loss bug and related races. Fix all four: - #1 (data loss): the unlock screens claimed the parked prefill via take() BEFORE the post-unlock awaits (restoreAfterUnlock / refreshAfterUnlock / startSyncAnyway). If any threw or the screen unmounted, the prefill was already cleared with no recovery — the payment was silently lost, exactly in the cold-launch-into-locked scenario this feature targets. Now claim only AFTER the awaits succeed. - #3 (stale): a parked link left unclaimed could fire as a payment on a much later unrelated unlock. paymentUriPrefillProvider now stamps the park time and takeIfFresh() drops anything older than a 10-minute TTL. - #4 (race): _PaymentUriLinkListener listened to appSecurityProvider and drained on unlock, racing the unlock screen's own navigation. Drop that listener (the unlock screens own post-unlock nav) and have the drain defer while matchedLocation is /unlock, so a link arriving mid-unlock is delivered once by the unlock flow. - #7: a failed parse of a second link no longer clear()s a prefill already parked from an earlier valid link. The wallet-loading listener and the warm (already-unlocked) drain are unchanged.
…view fixes) - #5: a ZIP-321 URI carrying an amount jumps the mobile send flow straight to the amount step, which bypasses the recipient step's address-validity gate (_amountReady never checks the address). If the prefilled address validates as 'invalid', fall back to the recipient step so the error is shown instead of letting the user continue to review/send. Only a definitive 'invalid' triggers the fallback — a transient validation 'error' (e.g. offline) is left alone and re-checked at review/send. - #6: add a PaymentUriService unit test covering the cold-start contract (initialize -> takePendingUris drains the buffered URI -> ready), plus a later onUris push, which the regtest tests (onUris-only) did not exercise.
Codex review (Medium): main.cpp registered the zcash: protocol handler unconditionally on every launch, and payment_uri_protocol.cpp wrote HKCU\...\zcash\shell\open\command without an ownership check — so simply opening Vizor reclaimed the handler from another wallet (or another Vizor channel) the user had selected. The Velopack install/update hooks already register it, making the per-launch register both redundant and aggressive. Add RegisterZcashProtocolHandlerIfUnclaimed(): register only when the handler is unset or already points at this install, reusing the same ownership check UnregisterZcashProtocolHandler already does. Startup calls this variant; install/update hooks keep the unconditional register (the intended moment to claim the handler). Not compile-verified — no local Windows build environment (already flagged in the PR).
Codex review (Low): the new send-domain parser (lib/src/features/send/domain/zip321_payment_request.dart, added in 084c65c) was byte-identical to the pre-existing core parser (lib/src/core/zcash/zip321_payment_request.dart) that address-scan and swap already use. Standards parsing should not drift across two copies. Point app.dart and the send parser test at the core copy and delete the duplicate. The send test's cases now exercise the core parser too (extra coverage). Both parser test suites (send + swap, 17 tests) pass; analyze clean.
A payment-URI deep link makes /send the navigation root, so the amount step's back button called context.pop() with nothing to pop and did nothing (the user was stuck on the amount screen). - amount back now steps to the recipient step. recipient -> amount is a same-route _step change (not a push), so back mirrors it instead of popping the whole /send route; the user can review/edit the prefilled address. - recipient (first step) back pops if possible, else routes to /home, so a deep-link root has somewhere to go. - _routePopAllowed intercepts the amount step (system back gesture also steps to recipient) and only lets recipient pop when there's something to pop. - Use GoRouter.maybeOf(...)?.canPop() instead of the throwing context.canPop() extension so widgetbook galleries that render this screen without a GoRouter don't crash. analyze clean; mobile-lane use-case + routes suites pass (the pre-existing 'recipient focused' failure is unrelated).
piatoss3612
force-pushed
the
rowan/vzr-91
branch
2 times, most recently
from
78689af to
e777cb1
Compare
…eep link The earlier back-nav fix (e777cb1) was wrong: it made the amount step always do a _step transition. But in the normal route-step flow amount is a pushed /send/amount PAGE (recipient -> amount is context.push via _continueToAmount, not a _step change), so that broke page-pop and two mobile_send_screen_test cases (route pop / pop as pages). The real bug only happens when a payment-URI deep link lands on the amount step of the ROOT /send route (initialAmount makes _step=amount with no page to pop). So: - amount back pops the /send/amount page when there is one (_canPopRoute), else steps back to recipient in place (deep-link root). - recipient back pops if possible, else routes to /home. - _routePopAllowed restored to the original (useRouteSteps || recipient). mobile_send_screen_test green (+26); full mobile lane has only the pre-existing recipient-focused failure; desktop lane +1385 green.
windows/runner/payment_uri_protocol.cpp called the nonexistent ::ShellChangeNotify and did not include <shlobj.h>, leaving SHChangeNotify and SHCNE_ASSOCCHANGED / SHCNF_IDLIST undeclared. The Windows ZIP-321 URL-protocol registration never compiled (there is no Windows build lane in CI to catch it). Use the real Win32 API ::SHChangeNotify and include <shlobj.h>. Verified on a Win11 x64 debug build: the handler registers HKCU\Software\Classes\zcash (URL Protocol marker, DefaultIcon, and shell\open\command = "<exe>" "%1"), and firing `start "" "zcash:...?amount=...&message=..."` launches the app with the full payment URI passed through as argv[1].
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Clicking a
zcash:payment link (zcash:<address>?amount=...&memo=...) opens Vizor with the send screen prefilled (address, amount, memo). ZIP-321 is the standard Zcash payment-request URI, interoperable with other wallets (Zashi, Ywallet, …).Linear: VZR-91
What changed
com.zcash.wallet/payment_uri, buffer-then-flush:takePendingUris→ready→onUris).SceneDelegate(willConnectTocold /openURLContextswarm), notapplication(_:open:), withFlutterDeepLinkingEnabled=false./sendafter unlock instead of/home. (Hardened after review: claim only after the post-unlock work succeeds, 10-min TTL, no drain↔unlock race.)/sendunpacks the prefill intoMobileSendScreen: the amount step (with the address filled) when an amount is present, or a fallback to the address step on an invalid address.How to test
Manual (link → prefill):
→ the send screen should be prefilled with the address + amount.
Locked path: lock the wallet (sidebar → Sign out), open the link, enter your password on the unlock screen → you should land on the prefilled send screen, not
/home.Automated:
zcash:scheme registration ✓ only. The actual mobile-form-factor link → prefill → Send flow is unverified (mobile/sendprefill is covered only by the widget routing test).Verified: macOS live end-to-end · Linux (UTM) live (locked / no-wallet / unlock→send branches) · regtest E2E (warm + locked) with real shielded sends mining in.
Review
A 6-dimension multi-agent adversarial review confirmed 8 findings → all fixed (incl. a locked-path data-loss bug; 3 false-positives refuted). See the VZR-91 comment for details.
Out of scope (follow-up)