Feat - Implement WalletConnect - #64
Conversation
- Desktop: dedicated WalletConnect tile in Pay with Wallet, routes to a custom QR + copy-to-clipboard page (ConnectWalletConnect) instead of WalletConnect's bundled modal. - walletConnectProjectId on getDefaultConfig() defaults to Rozo's shared WalletConnect Cloud project ID (same one used by Stellar's WalletConnectModule) so the option is available without extra config. - Connector hardened against provider-init failures (private browsing, storage issues) surfacing as uncaught errors instead of a catchable rejection. - Fixed a token-list flash on wallet switch (useTokenOptions trusted stale sortedOptionsList across address changes) and a related stuck- loading regression when the new address genuinely has zero tokens. - Mobile: WalletConnect tile intentionally not shown (native deeplinking already covers installed wallets; WC's own bundled modal didn't reliably surface there).
- add walletConnect connector + shared CONNECT_WALLETCONNECT QR page - desktop no-extension fallback tiles route through the WC QR (any WC wallet can scan it) when no injected EVM provider is present - EIP-6963 injected connector for direct browser-extension connect - brand WC/Stellar icons when connectors carry no icon payload - resolve real walletConnect connector regardless of which tile clicked - narrow hydrated order before reading txHash fields in Confirmation - add settlementMode/payin/payout txHash to zod metadata schema - pin @reown/appkit to avoid custom-element collision; workspace:* deps
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d1bf75bbc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| hydrated?.payoutTransactionHash ?? | ||
| hydrated?.destFastFinishTxHash ?? | ||
| hydrated?.destClaimTxHash; | ||
| return !!sourceTx && !!destTx && sourceTx === destTx; |
There was a problem hiding this comment.
The settlementMode value is read but no longer checked, so any non-Stellar order whose source and destination hash fields happen to match is classified as isStellarDirectSameTx. When showProcessingPayout is enabled, this suppresses the payout-processing state and its Pusher/polling path even though the order was not routed through stellar_direct; restore the settlementMode === "stellar_direct" guard before comparing hashes.
Useful? React with 👍 / 👎.
| withConnectorId( | ||
| walletConnect({ | ||
| projectId: walletConnectProjectId, | ||
| showQrModal: true, | ||
| }), | ||
| "walletConnectModal" |
There was a problem hiding this comment.
Although this creates a walletConnectModal connector specifically for mobile, useWallets(true) never appends either WalletConnect connector: it only adds Coinbase and connectors whose type is injected, while WalletConnect is not injected. Consequently mobile users never see or invoke the promised WalletConnect app picker, particularly for compatible wallets outside the curated deeplink list.
Useful? React with 👍 / 👎.
| }`; | ||
| }, | ||
| showInMobileConnectors: true, | ||
| walletConnectFallback: true, |
There was a problem hiding this comment.
Replacing showInMobileConnectors: true with walletConnectFallback: true removes Family from the mobile “Other Wallets” page, because MobileConnectors explicitly filters out every config without showInMobileConnectors. The new desktop fallback flag should be added without deleting the existing mobile flag.
Useful? React with 👍 / 👎.
| }, | ||
| okx: { | ||
| name: "OKX", | ||
| walletConnectFallback: true, |
There was a problem hiding this comment.
This replacement drops name: "OKX", leaving the OKX config with neither a name nor a short name. Both the existing mobile connector row and the new desktop WalletConnect fallback render their labels from those fields, so users now see an unlabeled OKX icon; retain the name alongside walletConnectFallback.
Useful? React with 👍 / 👎.
walletConnectFallback:false hid MetaMask entirely in incognito/no-extension. The fallback block is already gated on !window.ethereum, so with the extension present no stubs render (MetaMask connects directly); without it the stub shows and routes through the WC QR.
walletConnectFallback was inherited by the real injected wallet (config spread), so the click handler sent the live MetaMask extension tile to the WalletConnect QR instead of connecting directly. Gate the QR branch on the wallet having no connector — only the no-extension fallback stub lacks one.
…ension stub okx config lacked 'name' (fallback stub showed logo only, no label) and the prior edit had dropped walletConnectFallback:true (would suppress the stub entirely). Restore both so the OKX WC fallback tile renders a label and is generated in no-extension mode.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // connector names itself after the wallet (e.g. "io.metamask") and connects | ||
| // directly. Without extension, generic "injected" name is filtered by useWallets. | ||
| connectors.push( | ||
| injected({ shimDisconnect: true }) |
There was a problem hiding this comment.
P1 — potential duplicate wallet tiles. This unconditionally adds a generic injected() connector. getDefaultConfig sets no multiInjectedProviderDiscovery, so wagmi defaults it to true and also discovers each EIP-6963 wallet as its own connector. The useWallets filter only drops the generic injected connector when connector.name.toLowerCase().includes("injected") — but wagmi's injected() names itself after the detected provider (e.g. "MetaMask"), so with an extension installed you can end up with two MetaMask tiles (the EIP-6963 one + this generic one). The comment here even claims the generic name is "filtered by useWallets", which won't hold once the provider is name-detectable. Please confirm dedupe behavior with an extension installed, or gate/remove this connector.
| // Check window.ethereum, not the connector list — defaultConnectors() never | ||
| // adds an injected() connector, so wagmi's list is empty even when a wallet | ||
| // extension IS installed (then it arrives via EIP-6963 / additionalConnectors). |
There was a problem hiding this comment.
P2 — stale/contradictory comment. This says "defaultConnectors() never adds an injected() connector", but this same PR now adds injected({ shimDisconnect: true }) in defaultConnectors.ts:152. The window.ethereum detection still works, but the stated justification is now false and directly contradicts the comment at defaultConnectors.ts:148-150. Please reconcile the two comments.
| if (!isLoading) { | ||
| settledAddressKey.current = currentAddressKey; | ||
| } |
There was a problem hiding this comment.
P1 (plausible) — the settled-address guard can settle prematurely. settledAddressKey.current is updated whenever !isLoading. But the fix's own premise is that on address change the new fetch is debounced 300ms before it starts — during that window isLoading is still false (the underlying walletPaymentOptions.isLoading hasn't flipped yet). So on the first render after the address changes to B, this immediately sets settledAddressKey.current = B while the options still belong to A, making optionsAreForCurrentAddress true and hiding the loading state — the exact flash you're trying to prevent. Consider gating the settle on the fetch actually having run for the new address (e.g. only advance the key when hasAnyData corresponds to the current key, or when isLoading has been observed true→false for this address), rather than any !isLoading render.
| // Error Alert changes content height — poke the modal so it re-measures. | ||
| useEffect(() => { | ||
| if (error) setTimeout(context.triggerResize, 10); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [error]); |
There was a problem hiding this comment.
P2 — redirects even when already connected via a different wallet. This effect navigates to SELECT_TOKEN whenever isConnected is true. If the user already has an EVM wallet connected and then clicks the WalletConnect tile, isConnected is already true on mount, so this fires immediately and bounces to token selection using the existing connection — the WC pairing initiated in the effect above never completes/matters. Likely harmless, but worth gating on the WC connector actually being the active one (or on a fresh connection) to avoid confusing behavior.
| const isStellarDirectSameTx = useMemo(() => { | ||
| if (!order) return false; | ||
| const settlementMode = (order.metadata as any)?.settlementMode; | ||
| if (settlementMode !== "stellar_direct") return false; | ||
| const sourceTx = order.sourceStartTxHash ?? order.metadata?.payinTransactionHash; | ||
| const destTx = order.payoutTransactionHash ?? order.destFastFinishTxHash ?? order.destClaimTxHash; | ||
| const settlementMode = order.metadata?.settlementMode; | ||
| // txHash fields only exist on hydrated orders — narrow first. | ||
| const hydrated = isHydrated(order) ? order : null; | ||
| const sourceTx = | ||
| hydrated?.sourceStartTxHash ?? order.metadata?.payinTransactionHash; | ||
| const destTx = | ||
| order.metadata?.payoutTransactionHash ?? | ||
| hydrated?.payoutTransactionHash ?? | ||
| hydrated?.destFastFinishTxHash ?? | ||
| hydrated?.destClaimTxHash; | ||
| return !!sourceTx && !!destTx && sourceTx === destTx; | ||
| }, [order]); |
There was a problem hiding this comment.
P1 — test gap on payout-status logic. isStellarDirectSameTx decides whether to skip Pusher/polling entirely for the payout confirmation (per the comment above). This is payout-status-affecting logic and its inputs changed in this PR (new settlementMode / payinTransactionHash / payoutTransactionHash metadata fields, hydrated-order narrowing), yet there's no test covering it. A wrong true here would short-circuit payout polling; a wrong false would keep polling forever. Please add unit coverage for the same-tx and different-tx cases (hydrated vs metadata-only sources).
Review Summary — PR #64 (WalletConnect)Pre-merge review. No P0 blockers found. Solid, well-commented feature work; a few P1/P2 items worth addressing before merge. Security check (all clear)
P1 — should fix
P2 — nice to have
Inline comments left on each. Not approving/merging — verdict only. |
intent-common@0.1.29-beta.0 is published; pin intent-pay's workspace dep so the published package depends on the exact beta.
….29-beta.0, intent-pay 0.1.41-beta.0)
1 participant
Summary
Adds WalletConnect as an EVM wallet option, with a desktop QR flow and a
no-extension fallback, plus connector hardening and a couple of related fixes.
WalletConnect (EVM)
defaultConnectors.ts, both backed by wagmi'swalletConnect()but relabeled viawithConnectorIdso they don't collide:walletConnect— desktop; WC's own modal suppressed (showQrModal: false),rendered through our own
ConnectWalletConnectQR + copyable-URI page.walletConnectModal— mobile; WC's bundled modal (showQrModal: true),routed through the generic ConnectUsing page.
CONNECT_WALLETCONNECTroute +ConnectWalletConnectpage (desktop-only):listens for the connector's
display_urievent, renders the QR, brands itself"Scan with WalletConnect" (or "Scan with " for fallback stubs), advances
to
SELECT_TOKENon connect.walletConnectProjectIdprop ongetDefaultConfig(defaults to Rozo's sharedReown project ID; consumers can pass their own).
No-extension desktop fallback
useWallets: when no injected EVM provider is present (incognito / no extension),curated tiles would vanish — instead, major wallets surface as
wc-fallback-*stubs that route through the WC QR (any WC wallet can scan it). Desktop-only;
gated after the mobile early-return and only created when
window.ethereumisabsent, so an installed extension still connects directly.
Connector hardening (
hardenConnector)indexedDB/localStorageat init and can resolve without alive provider (private browsing, quota, stale session). wagmi calls
provider.on(...)with no null-check, so an undefined provider throws synchronously as an uncaught
error.
getProvider/connect/setupare wrapped to rethrow as a realError,and
setup()is swallowed (fire-and-forget) — both now surface as normal promiserejections our
onErrorhandlers catch.typeof window).projectIdto avoid "Core already initialized" noise on HMR.Branding
isWalletConnectConnector()util;OrderHeaderandSelectMethodshow theWalletConnect logo / name when a connector carries no icon/name payload.
WalletConnecttile inuseWalletsuses the WC logo + squircle.Other fixes
useTokenOptions: tracks the settled address so switching wallets no longerflashes a stale "no tokens" state or gets stuck on loading (also handles a wallet
with zero tokens settling correctly).
Confirmation: narrows to the hydratedRozoPayOrderbefore reading txHashfields; zod metadata gains
settlementMode,payinTransactionHash,payoutTransactionHash.Bug fixes (follow-up commits)
walletConnectFallbackwasinherited by the real injected wallet (config spread in
useWallets), so theConnectorListclick handler sent the live MetaMask/OKX/etc. extension tile to theWalletConnect QR instead of connecting. The QR branch now also requires the wallet
to have no connector — only the no-extension fallback stub lacks one.
okxconfig was missing aname, so thestub rendered only the logo. Added
name: "OKX"(keptwalletConnectFallback: true).Deps / tooling
@reown/appkit(and subpackages) to1.8.23to avoid custom-element collision.@rozoai/*deps switched toworkspace:*inexamples+connectkit.Test plan
pnpm --filter @rozoai/intent-pay exec tsc --noEmitpasses.