fix(react-ui): treat LOADABLE wallets as connectable in the wallet modal - #101
Merged
Conversation
Wallets reporting WalletReadyState.LOADABLE (e.g. Shield with the remote relay fallback configured and no injected provider) were grouped with not-detected wallets, so the modal showed the 'Get an Aleo wallet to continue' install redirect and connect() was never invoked - even though AleoWalletProvider.connect() already supports LOADABLE. Group them with INSTALLED wallets instead so they appear under 'Connect an Aleo wallet' and run the normal select-and-connect flow.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dms120
previously approved these changes
Aug 19, 2026
…list Review follow-up: keep the always-connectable grouping, but order installed extensions first so a loadable (remote-capable) wallet never displaces an extension the user actually has.
esren0x
force-pushed
the
esren0x/react-ui-loadable-connect
branch
from
August 19, 2026 14:42
9a060ec to
35c5882
Compare
dms120
previously approved these changes
Aug 20, 2026
…dead LOADABLE fallback Review follow-through on this PR (raised in #100's review): - getStartedWallet's standalone LOADABLE find could never run: every LOADABLE wallet is already in connectableWallets, so reaching the fallback implies none exist. Deleted. - isWalletConnectable(state) now lives next to WalletReadyState in aleo-wallet-standard and replaces the three hand-rolled INSTALLED-or-LOADABLE checks: the modal's grouping (filter + stable INSTALLED-first sort) and both WalletProvider copies (autoConnect, connect).
esren0x
added a commit
that referenced
this pull request
Aug 21, 2026
… pairing surface Review follow-through (dms120 + veria-ai on #100), on top of merged adapter/react-ui fixes from #99/#101: - Vendoring is now script-owned: scripts/sync-shield-relay.sh regenerates src/lib/shieldRelay/ verbatim from a PINNED shield-relay commit (refuses to run against the wrong sha or a dirty checkout; the only transforms are concatenation, the internal import-path rewrite, and eslint --fix). Re-vendored at shield-relay@1c390ae, which also fixes both security findings upstream (secret moved to the URL fragment on http(s) links; wallet key pinned after first authenticated handshake - shield-relay#12) and drops the dead nextId, so no hand edits remain. - codeExamples.remoteConnect imports the vendor path it actually runs, with a comment pointing real dapps at '@shield/relay-dapp-client', and teaches additive onConnectUrl instead of the old XOR behavior. - App.tsx onConnectUrl only writes the URL into UI state - the adapter owns deeplink firing now. IS_MOBILE_UA is gone from shieldRemoteConfig; the banner keeps a local UA check strictly for QR presentation. - /remote page drops its duplicate pairing-URL dump; the banner owns that surface. Verified: example vite build + lint green; headless smoke test 11/11 against a live relay (pair via secret-carrying URL, connect, signMessage, two-phase execute, reload-resume without re-pair, disconnect clears the session, fresh re-pair). EXT-272
Collaborator
Author
|
Follow-through from the #100 review landed here in 4ed2fa8 (this is its subject matter — re-review needed since it stales the approval, sorry):
|
dms120
previously approved these changes
Aug 21, 2026
dms120
approved these changes
Aug 21, 2026
kpandl
approved these changes
Aug 25, 2026
esren0x
added a commit
that referenced
this pull request
Aug 28, 2026
…72) (#100) * feat(shield): opt-in remote (relay) fallback in ShieldWalletAdapter On plain mobile browsers there is no injected window.shield, so Shield stayed NotDetected exactly where the relay matters. With remote config the adapter now reports Loadable and connects to the Shield app over the deeplink + E2E-encrypted relay (ProvableHQ/shield-relay). Backwards compatible by construction: - zero-arg construction and all injected behavior unchanged - injected provider always wins (Loadable upgrades to Installed on detect) - the remote path is a lazily-imported facade implementing the existing ShieldWallet interface, so every adapter method flows through untouched - the relay client is not a dependency: structural types + runtime dynamic import, with a remote.transport factory for bundler-resolved apps Verified end-to-end against a local Centrifugo relay + fake wallet using the built dist: NotDetected/Loadable readiness, connect, signMessage byte round-trip, two-phase execute (temp id -> accepted at1 id), networkChanged event surfacing, disconnect. Workspace build + lint green. EXT-271 * fix(shield): resolve the optional relay client via a variable specifier A literal dynamic import of the optional package broke consumers' builds: tsup strips ignore-comments from the published dist, so Rollup/webpack try to statically resolve a package that opted-out dapps never install. A variable specifier is unanalyzable, leaving resolution to runtime (the documented behavior). Bundled apps keep using remote.transport. Drops the now-unneeded ambient module declaration. EXT-271 * feat(example): Shield remote (relay) connect flow in react-app Turns the example into the Stage-1 POC dapp for shield-relay: - App.tsx: with VITE_SHIELD_RELAY_URL set, the Shield adapter is constructed with the remote fallback; deeplink auto-fires on mobile, and the pending connect URL surfaces in an app-level banner (open/copy - copy feeds the fake wallet on desktop) - new Remote Connect page (/remote): live readyState/connection status, LAN test recipe, code example; registered per repo conventions (page, route, sidebar, codeExamples) - vendored relay dapp transport + protocol (src/lib/shieldRelay/, verbatim from ProvableHQ/shield-relay@9a31463 incl. replay protection) because the relay clients use workspace:* deps and cannot be consumed via file:/git until published - tracked as Linear WS-92 - deps: centrifuge, eciesjs (used by the vendored transport) Verified: vite build + lint green; dev server serves /remote with the relay configured. EXT-272 * feat(example): render the remote connect URL as a QR code in the pairing banner Desktop browsers now show a scannable QR (qrcode.react) for the Shield app alongside the open/copy actions; mobile keeps the raw link since the deeplink fires automatically. Closes the QR acceptance criterion of EXT-272. * feat(example): re-vendor shield-relay transport at 34f1a97 (authenticated handshake) Sync the vendored relay files with shield-relay PR #11: - connect URL gains a 32-byte pairing secret (out-of-band only; never travels over the relay) - key handshake carries an HMAC over the pairing transcript; forged handshakes are dropped (handshakeRejected event) rather than fatal - every sealed message is signed by the sender's key; unsigned or wrongly-signed payloads are rejected - persisted sessions without a secret are discarded (fresh channel) - deps: @noble/curves + @noble/hashes pinned to the exact versions the upstream protocol package uses Vendored verbatim apart from formatting; upstream remains the source of truth until the clients are published (WS-92). * docs(shield): mark the remote (relay) fallback @experimental TSDoc @experimental tags on ShieldRemoteConfig and the adapter's remote option, plus a README callout: the fallback needs a Shield app build with relay support that is not yet generally available, so dapp developers should not enable it in production until Shield announces support. No behavioral change. * docs(example): sync vendored relay comments with merged shield-relay main (27b4c2e) Comment-only upstream additions from the merged PR #11: the rationale for excluding the relay URL from the handshake MAC transcript, and the planned fragment-based connect URL for universal links. No code changes. * refactor(shield): require remote.transport; one lifecycle for the remote wallet Address review on the remote fallback: - remote.transport is now required. The default runtime dynamic import of '@shield/relay-dapp-client' (variable specifier so bundlers could not see it) is deleted: it only worked where bare specifiers resolve at runtime, which excludes the bundled browser dapps this feature targets, and it already needed a workaround commit. The package now never names the relay client; the dapp's bundler resolves a literal import via the factory. Types no longer claim an 'optional peer dependency' that package.json never declared - the transport-like types are the factory contract. - Detection no longer writes the live wallet. _checkAvailability only reports readyState; connect() binds _shieldWallet via _resolveWallet(), which returns a definite instance. A provider injected mid-session can no longer hijack an active remote connection or leak its listeners. - The remote facade is no longer cached across connects (_remoteWallet deleted): pairing persistence lives in the transport's localStorage session, which a fresh instance resumes. The facade drops its transport on disconnect instead of reusing an instance whose keys were forgotten, and the 'forwarded' Set + generic event loop are replaced by three explicit typed forwards. README, TSDoc, and the changeset updated to the required-factory contract. * test(example): headless smoke test for the Shield remote (relay) flow Drives the real ShieldWalletAdapter -> RemoteShieldWallet facade -> vendored RemoteShieldTransport against a local Centrifugo relay and shield-relay's fake wallet, with browser globals stubbed. Covers pairing (secret-carrying connect URL), connect, signMessage, two-phase executeTransaction/transactionStatus, reload-resume from the persisted session without re-pairing, disconnect clearing the session, and a fresh re-pair afterwards. Run from a shield-relay checkout (relay up via pnpm relay:up): pnpm --filter fake-wallet exec tsx <repo>/examples/react-app/scripts/smoke-remote.ts * fix(shield): bind the live wallet only after connect() succeeds A thrown wallet.connect() (timeout, user cancel, empty address) left the new remote facade assigned to _shieldWallet and never disconnected, so retries allocated another live relay session. Bind after success; tear the unused facade down in the catch so the next retry starts clean. * chore: drop unrelated scopeProgram docs from the remote-fallback branch website/docs/privacy-preserving-dapps.md was swept into the original feature commit; it has nothing to do with the relay work. Restore the master version so this PR only ships the Shield adapter change. * chore(example): discover the shield-relay checkout instead of hardcoding a path SHIELD_RELAY_DIR env override, with cwd-based fallbacks for the documented pnpm --filter invocation. RELAY_URL is also overridable for hosted-relay runs. Fails with a clear message when no checkout is found. * fix(shield): full teardown on wallet-initiated disconnect; additive onConnectUrl; self-cleaning remote connect; typed transport events Addresses the remaining dms120 review items on this PR and the adapter halves of the #100 review: - Wallet-initiated disconnect now tears down both layers: the facade drops its transport (keys are forgotten) before emitting, and the adapter's _onDisconnect nulls _shieldWallet after _cleanupListeners — the same teardown on both lifecycle edges, so a dead facade or its live Centrifugo session can never leak into the next connect(). - onConnectUrl is additive, not XOR: always called when set; the mobile deeplink still fires unless the new fireDeeplink: false opts out. Desktop still requires the callback. README + changeset aligned. - RemoteShieldWallet.connect() cleans up its own relay session on any failure. The adapter's inner catch is gone: resolve, connect, bind — no injected disconnect-on-reject, no double-wrapped WalletConnectionError (the one remaining cleanup, empty-address after a successful wallet connect, stays at the adapter as adapter policy). - Transport events are a closed union (ShieldRemoteTransportEvent): the three wallet-RPC events the facade forwards plus the transport lifecycle events, with walletDisconnected documented as deliberately NOT a disconnect — the Shield app drops its socket on every background and the session stays valid; only the explicit disconnect RPC ends it. EXT-271 * fix(example): honest pinned vendoring, adapter-owned deeplink, single pairing surface Review follow-through (dms120 + veria-ai on #100), on top of merged adapter/react-ui fixes from #99/#101: - Vendoring is now script-owned: scripts/sync-shield-relay.sh regenerates src/lib/shieldRelay/ verbatim from a PINNED shield-relay commit (refuses to run against the wrong sha or a dirty checkout; the only transforms are concatenation, the internal import-path rewrite, and eslint --fix). Re-vendored at shield-relay@1c390ae, which also fixes both security findings upstream (secret moved to the URL fragment on http(s) links; wallet key pinned after first authenticated handshake - shield-relay#12) and drops the dead nextId, so no hand edits remain. - codeExamples.remoteConnect imports the vendor path it actually runs, with a comment pointing real dapps at '@shield/relay-dapp-client', and teaches additive onConnectUrl instead of the old XOR behavior. - App.tsx onConnectUrl only writes the URL into UI state - the adapter owns deeplink firing now. IS_MOBILE_UA is gone from shieldRemoteConfig; the banner keeps a local UA check strictly for QR presentation. - /remote page drops its duplicate pairing-URL dump; the banner owns that surface. Verified: example vite build + lint green; headless smoke test 11/11 against a live relay (pair via secret-carrying URL, connect, signMessage, two-phase execute, reload-resume without re-pair, disconnect clears the session, fresh re-pair). EXT-272 * docs(shield): tell the truth about the pre-publish install path The README and changeset sent dapps to a file:/git install that cannot work - the relay clients declare workspace:* deps, exactly what the example's sync script documents. Until WS-92 publishes the clients, the working integration path is vendoring the client source from a pinned shield-relay commit (examples/react-app/scripts/sync-shield-relay.sh is the reference); the snippet's published-package import is labeled as the post-publish shape. * docs(example): banner comment reflects adapter-owned deeplink; merge doc/changeset fixes forward Merges the #99 install-story doc fix and the #101 wallet-standard patch changeset into this stack, and corrects the stale banner comment that still credited App.tsx with firing the deeplink - the adapter owns it.
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
3 participants
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.
Problem
With the Shield remote (relay) fallback configured and no extension installed, Shield reports
WalletReadyState.LOADABLE— but the wallet modal groupedLOADABLEwallets with not-detected ones. The modal rendered the Get an Aleo wallet to continue install screen and every click redirected to the wallet's install page, so the relayconnect()path was unreachable from the modal.AleoWalletProvider.connect()already acceptsLOADABLEwallets; the modal was the only place gating onINSTALLED.Change
WalletModalnow groupsLOADABLEwallets withINSTALLEDones (connectableWallets), so they're listed under Connect an Aleo wallet and clicking them runs the normal select-and-connect flow.NOT_DETECTEDwallets remain behind the install-redirect path.@provablehq/aleo-wallet-adaptor-react-ui.Testing
pnpm --filter @provablehq/aleo-wallet-adaptor-react-ui build lintgreen.VITE_SHIELD_RELAY_URLset and no extension, Shield now appears in the connect list and pairing completes against shield-relay's fake wallet; with the extension installed the injected path is unchanged.Relates to EXT-271 (Shield remote relay fallback).
Review follow-up (35c5882)
Kept the always-connectable grouping (per review) and added ordering: INSTALLED wallets are listed before LOADABLE ones, so a remote-capable wallet never displaces an extension the user actually has.
NOT_DETECTEDremains the install funnel.