feat: passkey wallet — onboarding UI & PIN-less integration - #897
feat: passkey wallet — onboarding UI & PIN-less integration#897pedroferreira1 wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
8b21323 to
00833bf
Compare
cf15061 to
2ce7c1c
Compare
00833bf to
330d8d7
Compare
raul-oliveira
left a comment
raul-oliveira
left a comment
There was a problem hiding this comment.
Approving — the integration diff itself looks solid and internally consistent. Two non-blocking nits inline.
github-project-automation
Bot
moved this from In Progress (Done)
to In Review (WIP)
in Hathor Network
330d8d7 to
e2c8037
Compare
Wires the passkey core into the app (stacked on feat/passkey-core): - Onboarding button + passkey lock screen; InitWallet/App mounting & routing. - xpub-only wallet start + external-signer registration; push-notification and wallet-service gating for passkey wallets. - PIN-less send / create-token / swap / reown; Security screen hides PIN & biometry for passkey wallets. - Re-adds the ttag wrapping to passkeySigner's error strings and the extracted translations (pt-br filled; da/ru-ru English fallback). All behind the passkey-onboarding.rollout flag (off by default).
e2c8037 to
6625648
Compare
The passkey shortcut -- if (STORE.isPasskeyWallet()) run the operation directly (no PIN), else open the PinScreen -- plus its multi-line rationale was duplicated verbatim across SendConfirmScreen, CreateTokenConfirm and TokenSwapReview. The copies had already started to drift (the comment said "signTx" in two screens and "create-token" in the third), which is exactly the maintenance hazard that duplication invites. Extract src/passkey/authorizeTransaction.js so the PIN-less contract and its rationale live in one place, and rewire all three screens to call it. The refactor is behavior-preserving: - SendConfirmScreen / CreateTokenConfirm keep setIsSending(true) unconditionally before the call (button disabled on both the passkey and PIN paths, as before). - TokenSwapReview keeps setIsSending(true) ONLY on the passkey path (the PIN path never set it), by wrapping it inside the `execute` callback the helper runs for passkey wallets. Each screen still owns its own PinScreen params (screen text differs per operation); the helper only centralizes the wallet-type branch. The now-unused STORE import is dropped from all three.
| if (isPasskeyWallet) { | ||
| // Must happen BEFORE start(): flips isSignedExternally so the read-only guards allow | ||
| // sending, with signatures produced by the passkey ceremony instead of a stored key. | ||
| wallet.setExternalTxSigningMethod(makePasskeyTxSigner()); |
There was a problem hiding this comment.
issue(blocking): the pinned wallet-lib has none of the APIs this line switches on
Pinned here for delivery — the actual concern is package.json:29, which pins @hathor/wallet-lib: 3.1.1 and is not touched by this PR.
I unpacked the published 3.1.1 tarball: transactionUtils.signTxInputs — what makePasskeyTxSigner delegates to — exists nowhere in the package. Separately, every signing entry point in 3.1.1 rejects a missing PIN unconditionally, with no external-signer escape: signTx, prepareCreateNewToken, sendManyOutputsSendTransaction, SendTransaction.signTx. isSignedExternally short-circuits isReadonly() only, never the PIN check.
So on a clean npm install every AC involving a transaction fails: Send shows "Pin code is required to sign a transaction", Create Token shows "Pin is required.", and no biometric sheet ever appears.
To be clear about where the fault is: the app code is right. I checked the lib branches, and feat/pin-optional-external-signer implements exactly this relaxation at exactly these call sites — it just is not released. Land the @hathor/wallet-lib bump with this PR rather than as a follow-up; the dependency contract belongs with the code that needs it.
| biometryLoadingText: t`Building transaction`, | ||
| }; | ||
| navigation.navigate('PinScreen', pinParams); | ||
| authorizeTransaction({ |
There was a problem hiding this comment.
issue(blocking): the swap path is unreachable for a passkey wallet, even after the lib bump
Pinned here for delivery — the actual concern is TokenSwapReview.js:97 (outside the diff), the wallet.createNanoContractTransaction call at screen mount.
That method guards on await this.storage.isReadonly() — the raw storage flag — instead of the wallet-level this.isReadonly() that short-circuits on isSignedExternally. generateAccessDataFromXpub sets WALLET_FLAGS.READONLY unconditionally, so the guard is always true for a passkey wallet. I confirmed this is still the case in the newest published 4.1.0, so the bump in my other comment does not fix it.
The call runs with signTx: false well before any authorization, so the screen dies in PHASE.ERROR and this whole authorizeTransaction block plus the new isSending state never execute. The reown htr_sendNanoContractTx / htr_createNanoContractCreateTokenTx paths fail identically, which also makes part of the new PinConfirmationPrompt branch dead.
feat/pin-optional-external-signer already converts both lib call sites to this.isReadonly(). This PR needs the release that carries it, or the swap AC should come out until it lands.
| // A cancelled passkey ceremony is "not now", not an error (the rpc-handler re-wraps our | ||
| // typed error, so we use a consumable flag — same pattern as pinWasCancelled). Retry the | ||
| // request: the dapp consent modal shows again and the user can accept or reject. | ||
| if (consumePasskeySigningCancelled()) { |
There was a problem hiding this comment.
issue(blocking): a stale signingCancelled flag silently hijacks unrelated failures here
signingCancelled (passkeySigner.js:83) is module-global. It is set by any cancelled signing ceremony and cleared only by the start of the next ceremony, or by consumePasskeySigningCancelled() — whose sole caller is this line.
So an in-app cancel on SendConfirmScreen / CreateTokenConfirm / TokenSwapReview leaves it true indefinitely. The next dapp request that fails for an unrelated reason before any ceremony runs — insufficient funds, invalid params, PromptRejectedError, network — reads that stale true right here, before the switch (e.constructor) below. The saga retries instead of reporting: the user's explicit reject is ignored, the consent modal reappears for no visible reason, and the InsufficientFundsModal / REQUEST_ERROR modal that should have explained the failure never renders.
Reset signingCancelled = false at the top of processRequest, before handleRpcRequest, so the flag can only ever reflect a ceremony from this attempt.
| // cleanup at the end of processRequest, so without this the fork leaks (and a new one is | ||
| // forked per retry). Mirrors the SendNanoContractTxError retry path. | ||
| yield cancel(pendingPollTask); | ||
| const result = yield* processRequest(action); |
There was a problem hiding this comment.
issue(blocking): the retry lands on a screen the user cannot act on
This breaks the "a cancelled ceremony is retryable" AC rather than fulfilling it.
The rpc-handler fires its *LoadingTrigger before signing (sendTransaction.js:141 then :146), so by the time the passkey ceremony runs, setSendTxStatusLoading() has already dispatched via reown.js:1070. This branch returns before the switch below, so unlike every sibling retry path — setNewNanoContractStatusFailure() before the recursion at :725, setSendTxStatusFailure() at :794 — it resets nothing.
SendTransactionRequest.js:485 then renders the full-screen "Sending transaction / Please wait." spinner and hides its own main content at :497 for as long as the status is LOADING. The recursion pushes a fresh consent prompt onto a screen stuck on a spinner that will never resolve, so the user can neither see nor accept the retry.
That is the concrete difference from the pinWasCancelled pattern this mirrors: a PIN cancel happens during the prompt phase, before any loading status exists. Dispatch the matching *StatusFailure for params.request.method before recursing.
| dispatch(unlockScreen()); | ||
| dispatch(startWalletRequested({ walletType: 'passkey' })); | ||
| NavigationService.resetToMain(); | ||
| } catch (e) { |
There was a problem hiding this comment.
issue(blocking): a failed write here strands the user on a PIN screen for a PIN-less wallet
STORE.initPasskeyStorage writes in two awaited steps — storage.saveAccessData(accessData) then setItem(WALLET_META_KEY, ...) (store.js:362-366) — and this catch performs no rollback.
If the second write rejects, walletIsLoaded() returns true (access data exists) but isPasskeyWallet() returns false (no meta), so App.js:754 renders PinScreen instead of PasskeyLockScreen. The app is permanently stuck asking for a PIN that was never set, on access data holding no encrypted words. The only escape is "Reset wallet" — while the alert the user just saw said "Passkey unavailable", implying nothing was created.
Worth noting the timing: setItem populates the memory cache before awaiting AsyncStorage, so the current session behaves normally and the breakage only appears on the next cold start.
Make initPasskeyStorage atomic — write the meta first, or clear the access data on failure — and give this case its own message.
| // wallet metadata, and start the wallet read-only. No PIN, no stored secret. | ||
| const xpub = derivePasskeyXpub(words); | ||
| await STORE.initPasskeyStorage(xpub, { | ||
| passkeyLabel: label || 'Passkey wallet', |
There was a problem hiding this comment.
nitpick: these default names are user-facing but not translated
'Passkey wallet' here and 'Hathor Wallet' at :105 both reach the user: the label is rendered by PasskeyLockScreen ("Unlock with the passkey ...") and by PasskeyXpubMismatchError, and the name becomes the OS passkey displayName in the sign-in picker.
Wrapping both in t would bring them in line with the rest of the PR's i18n, which is otherwise complete.
| const wallet = useSelector((state) => state.wallet); | ||
| const [verifying, setVerifying] = useState(false); | ||
| const [error, setError] = useState(null); | ||
| // Ceremony must fire exactly once on mount even if the component re-renders (the OS shows |
There was a problem hiding this comment.
nitpick: the stated reason for startedRef is not what it guards
A useEffect(..., []) does not re-run on re-render, so "even if the component re-renders" is not the hazard.
What the ref actually protects against is React 18 StrictMode's double-mount and Fast Refresh remounts.
Small thing, but a maintainer who correctly reasons "empty deps already cover that" and deletes startedRef reintroduces a double credential sheet in dev.
| }; | ||
| }, []); | ||
|
|
||
| if (!enabled) return null; |
There was a problem hiding this comment.
praise: this toggle design avoids the rollback trap we hit before
Gating only wallet creation on the flag, and keying every runtime branch afterwards off the persisted walletType rather than the toggle, is the right call.
It is the opposite of what bit us in the amount-format work, where a toggle rollback stranded users with the behavior but no UI to change it. Here a rollback cannot leave an existing passkey user facing a PIN prompt they can never satisfy.
The i18n is also complete, for what it's worth: all new strings extracted to .pot, propagated to all three .po files, pt-br filled, and zero fuzzy entries across every catalog.
| #: src/passkey/passkeySigner.js:46 | ||
| msgid "" | ||
| "This passkey opens a different wallet. Use the passkey that created this " | ||
| "wallet." |
There was a problem hiding this comment.
suggestion(non-blocking): Translate this word
| "wallet." | |
| "carteira." |
| "This passkey opens a different wallet. Use the passkey that created this " | ||
| "wallet." | ||
| msgstr "" | ||
| "Essa passkey é de uma carteira diferente. Use a passkey que criou essa wallet" |
There was a problem hiding this comment.
suggestion(non-blocking): Translate this word
| "Essa passkey é de uma carteira diferente. Use a passkey que criou essa wallet" | |
| "Essa passkey é de uma carteira diferente. Use a passkey que criou essa carteira" |
…g error Addresses PR #897 review (core-owned files): - initPasskeyStorage: persist the wallet metadata BEFORE the access data. The app gates walletIsLoaded() on the access data, so a crash between the two flushes now leaves meta-without-access-data (a clean re-onboarding) instead of access-data-without-meta, which stranded a PIN-less wallet on the PinScreen with no PIN that could ever unlock it. - Split a MISSING stored xpub (corrupted/incomplete metadata) into a distinct PasskeyMetadataMissingError, separate from PasskeyXpubMismatchError (wrong passkey), at both the signer and unlock-verify guards — the two need different guidance (reset vs "use the other passkey") and should be diagnosed differently. - Soften the signer header comment: wallet-lib re-checks isReadonly() per operation, so registering the external signer before start() is a safe convention, not a hard requirement.
…UI gating - reown: reset the per-method tx status to READY before the passkey-cancel retry (the retry no longer lands on a full-screen spinner the user can't dismiss); clear the stale module-global signingCancelled flag at the top of processRequest so it can't hijack a later unrelated failure; reject unsupported passkey methods with UNAUTHORIZED_METHODS (3001) instead of USER_REJECTED; correct the gated-methods comment. - TokenSwapReview: a cancelled/busy passkey ceremony keeps the reviewed swap intact; real failures (incl. the actionable xpub-mismatch message) surface through the error modal + onExceptionCaptured instead of silently popping the screen. - startWallet saga: report the missing-xpub corruption via onExceptionCaptured; soften the before-start() comment. - PasskeyLockScreen: log + report non-cancel unlock failures and always show a (translated) message. - PasskeyOnboardingButton: a dismissed OS sheet is a silent cancel, not "Passkey unavailable"; use the shared logger; narrow the try so post-persist nav failures aren't mislabeled; generic backup copy (the storing password manager isn't reliably detectable). - pushNotification: log the defense-in-depth guard when it fires. - InitWallet: gate the New/Import button reorder behind the passkey feature flag (unchanged with the flag off). - Drop the unread walletType dispatch payload; wrap the default wallet names in ttag; authorizeTransaction defaults onPasskey to pinParams.cb; pt-br wording fix.
…UI gating Batched fixes for the round-2 review on the passkey integration: reown saga: - reset the per-method tx status to READY before the passkey-cancel retry, so the retry no longer lands on a full-screen "Sending transaction" spinner the user can't dismiss (READY, not *StatusFailure, which would pop an error modal and contradict the retry); - clear the stale module-global signingCancelled flag at the top of processRequest so an earlier in-app cancel can't hijack a later unrelated dapp failure; - reject unsupported passkey methods with UNAUTHORIZED_METHODS (3001), not USER_REJECTED; - correct the gated-methods comment. Screens / components: - TokenSwapReview: a cancelled/busy passkey ceremony keeps the reviewed swap intact; real failures (incl. the actionable xpub-mismatch message) surface through the error modal + onExceptionCaptured instead of a silent goBack that destroyed the reviewed transaction; - PasskeyLockScreen: log + report non-cancel unlock failures and always show a translated message; - PasskeyOnboardingButton: a dismissed OS sheet is a silent cancel, not "Passkey unavailable"; use the shared logger; narrow the try so post-persist nav failures aren't mislabeled; generic backup copy (the storing password manager isn't reliably detectable); - InitWallet: gate the New/Import button reorder behind the passkey feature flag; - drop the unread walletType dispatch payload; wrap default wallet names in ttag; authorizeTransaction defaults onPasskey to pinParams.cb. Storage / signer / saga (core-owned, mirrored from feat/passkey-core): - initPasskeyStorage persists walletMeta before the access data (crash-atomic in the safe direction); - a missing stored xpub throws a distinct PasskeyMetadataMissingError (vs xpub mismatch) and the start saga reports it via onExceptionCaptured; softened the before-start() comments; - pushNotification logs the defense-in-depth guard when it fires. i18n: new strings extracted; pt-br translated, da/ru-ru English fallback.
Bring the passkey-core review fixes (crash-atomic initPasskeyStorage, the distinct PasskeyMetadataMissingError, softened before-start header) into the stacked integration branch so #897 sits on top of its current base and merges cleanly. Integration already carries the ttag-wrapped equivalents of these changes, so the passkeySigner.js conflict is resolved in favor of integration's version.
f6f315c to
b088691
Compare
ac817ce to
0e63b48
Compare
3 participants
Summary
Wires the passkey core into the app — stacked on
feat/passkey-core(review/merge that first). A passkey wallet replaces both the seed phrase and the PIN: create or sign in with a passkey, persist only the account xpub + label, and run every operation that used to need a PIN through a passkey ceremony (Face ID / fingerprint) that derives keys in memory, signs, and discards them.What's here:
InitWallet/Appmounting and routing (kill/relaunch → lock screen, not the PIN pad).ttagwrapping topasskeySigner's error strings and adds the extracted translations (pt-br filled; da/ru-ru English fallback).All behind the
passkey-onboarding.rolloutflag (off by default).Acceptance Criteria
htr_sendTransaction/htr_sendNanoContractTxwork with one ceremony each;htr_signWithAddress/htr_signOracleDataare cleanly rejected ("not yet supported") pending the external private-key follow-up; a cancelled ceremony is retryable.msgfmt -cpasses.Security Checklist