Skip to content

feat(stellar): improve Stellar wallet UX - #58

Merged
akbarsaputrait merged 15 commits into
masterfrom
feat/stellar-wallet-ux
Jul 26, 2026
Merged

feat(stellar): improve Stellar wallet UX#58
akbarsaputrait merged 15 commits into
masterfrom
feat/stellar-wallet-ux

Conversation

@akbarsaputrait

@akbarsaputrait akbarsaputrait commented Jul 22, 2026

Copy link
Copy Markdown
Member

Summary

Improve Stellar wallet UX, add Direct Settlement support, and fix a set of payment-race and bundle-size issues found while testing this flow. Approach: keep Stellar wallet selection always visible (extension or WalletConnect fallback), auto-route same-chain Stellar payments through a direct settlement path, and close several races where a stale order or a duplicated init could send a payment down the wrong path.

Changes Made

Stellar Wallet UX

  • Always show Freighter & LOBSTR in the wallet list, even without the browser extension; route through WalletConnect when the extension isn't available
  • Detect WalletConnect QR modal close and reject properly instead of leaving a stuck spinner
  • Pre-fetch wallets in StellarContextProvider and cache the kit synchronously — removes the loading spinner and remount flash on the Connect Stellar page
  • Dynamic description text: "Scan QR" for WalletConnect vs "Open wallet" for extension

Stellar Direct Settlement

  • Auto-detects same-chain Stellar USDC/EURC payments (source chain+token === destination chain+token) and forces intent: "stellar_direct"
  • intent sent as a top-level field in POST /payments, separate from the display title
  • Handles settlementMode: "stellar_direct" in the response: source IS destination, fee "0.00", direct send
  • Added intent to CreateNewPaymentParams/CreatePaymentRequest, settlementMode to PaymentResponse

Race Fixes (payId-mode source-token switching)

  • PayWithStellarToken/PayWithSolanaToken: the auto-transfer effect could fire handleTransfer twice before either finished (Solana had no guard at all), sending duplicate checkout POSTs for the same payment. Replaced with a synchronously-claimed in-flight promise so a second call awaits the first instead of racing it.
  • usePaymentState.payWithToken (EVM) read a closure-captured pay.order instead of a fresh store read, so needRozoPayment could evaluate against stale order data.
  • needRozoPayment only compared chainId, so switching source tokens on the same chain (e.g. USDC Base → USDT ETH) skipped checkout and silently reused the stale order. Now compares chain and token address.
  • Checkout dedup cache was keyed by payId alone, so switching source tokens mid-flow could return a cached result for the wrong token. Rekeyed to payId:chainId:tokenAddress.

WalletConnect Double-Init

  • WalletConnectModule was creating two independent WalletConnect Cores (one via SignClient.init(), one via createAppKit()'s internal UniversalProvider init) — both always fired regardless of manualWCControl, which only affected close(). Deduped to a single Core.

Bundle Size / Perf

  • @stellar/stellar-sdk (~14MB) was statically imported in StellarContextProvider, usePaymentState, PayWithStellarToken, and (via chainAddress.ts) RozoPayButton itself, forcing it to parse/execute on every mount regardless of chain used. Converted to dynamic import(), deferred to actual use (wallet connect, quote, tx submit). Added validateAddressForChainAsync for RozoPayButton's internal validation path; public isValidStellarAddress/isValidSolanaAddress/validateAddressForChain stay sync for API compatibility.

Other

  • RozoPayModal/routes.ts: registered routes for the Stellar connect flow
  • ConnectorStellar: use WALLET_CONNECT_ID constant instead of a magic string
  • Workspace: examples/nextjs-app and packages/connectkit use workspace:* for local dev
  • Expanded test coverage: createPaymentPayload, derivePayIdPreferredTokens, isDNTEnabled, walletconnect.module

How to Test

Wallet UX

  1. Desktop with Freighter extension installed → tapping Freighter opens the extension popup
  2. Desktop without extension / incognito → tapping Freighter/LOBSTR opens WalletConnect QR
  3. Freighter Mobile Explorer / mobile browser → WalletConnect modal / QR
  4. Close the WalletConnect QR modal mid-connect → verify a proper rejection, no stuck spinner
  5. Open the Connect Stellar page → verify no loading spinner flash

Direct Settlement

  1. Pay Stellar USDC → Stellar USDC destination → verify intent: "stellar_direct" is sent and fee shows "0.00"
  2. Pay Stellar EURC → Stellar EURC destination → same as above
  3. Pay Stellar USDC → Stellar EURC destination (cross-token) → verify no intent sent, normal hub path used

Race fixes

  1. In payId mode, rapidly switch source token twice before the first checkout resolves → verify only one checkout POST fires
  2. Switch source token to a different token on the same chain (e.g. USDC Base → USDT Base... wait, use USDC Base → USDT ETH per commit) → verify checkout re-runs instead of reusing the stale order
  3. Check console for the WalletConnect Core double-init warning — should be gone

Perf

  1. Run pnpm build in packages/connectkit, confirm stellar-sdk chunk is not in the main render-critical bundle
  2. Load the checkout page for a non-Stellar payment and confirm stellar-sdk isn't eagerly fetched

Testing Checklist

  • Unit tests added/updated (createPaymentPayload, derivePayIdPreferredTokens, isDNTEnabled, walletconnect.module)
  • Manual testing completed (wallet UX matrix above)
  • Integration tests for direct settlement API path
  • Edge cases covered (modal close, stale order, same-chain token switch)
  • No regressions in existing tests

Reviewer Notes

  • Bundle-size fix note from the commit itself: webpack still inlines stellar-sdk into the main chunk because the SDK's index.ts barrel re-exports the sync isValidStellarAddress from the same entry graph — this reduces eager execution work but doesn't yet achieve true code-splitting. A follow-up (subpath export or consumer splitChunks config) is still needed if full code-splitting is required.
  • The three payId-mode races (duplicate checkout, stale pay.order, chain-only comparison) were fixed together since they all surface as variants of the same "switch source token quickly" bug — worth a closer look at usePaymentState.ts and both PayWithXToken components together rather than in isolation.

Related

  • N/A

- Always show Freighter and LOBSTR in wallet list (even without extension)
- When extension not installed, route through WalletConnect
- Add WalletConnect QR page for desktop without extension
- Add modal close detection to WalletConnect (rejects on user dismiss)
- Update description text for WalletConnect flow
- Pre-fetch supported wallets in StellarContextProvider (no loading spinner)
- Cache kit singleton synchronously to avoid flash
@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
intent-example Ready Ready Preview, Comment Jul 26, 2026 10:52am

Request Review

Comment thread packages/connectkit/src/components/Pages/Stellar/StellarWalletConnectQR/index.tsx Outdated
Comment thread packages/connectkit/src/utils/stellar/walletconnect.module.ts
Comment thread packages/connectkit/src/components/Pages/Stellar/ConnectStellar/index.tsx Outdated
Comment thread packages/connectkit/src/components/Pages/Stellar/ConnectStellar/index.tsx Outdated
Comment thread packages/connectkit/src/components/Pages/Stellar/ConnectorStellar/index.tsx Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 458b0636d3

ℹ️ 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".

Comment thread packages/connectkit/src/components/Pages/Stellar/ConnectStellar/index.tsx Outdated
- Remove dead StellarWalletConnectQR component (unreachable route, signing bug)
- Fix MutationObserver leak in walletconnect.module.ts (add finally block)
- Add walletsLoaded flag to StellarContextProvider (fix infinite spinner on error)
- Collapse duplicate isPlatformWrapper/else branches in ConnectStellar
- Use WALLET_CONNECT_ID constant in ConnectorStellar (no hardcoded string)
- Check hasWalletConnect before routing to WalletConnect (custom kit safety)
- Auto-detect Stellar→Stellar USDC/EURC same-chain payments
- Force intent: stellar_direct when source=destination chain+token
- Add settlementMode field to PaymentResponse
- Pass intent as top-level field in POST /payments
- Handle settlementMode in PayWithStellarToken (direct send)
- Add modal close detection to WalletConnect (MutationObserver)
- Update workspace deps for local dev mode
@shawnmuggle

Copy link
Copy Markdown
Member

Review — no P0, mergeable; 2 P1s worth a look first

Reviewed the signing / fund-routing path closely. No blocker. The stellar_direct auto-detection in createPaymentPayload.ts only sets a backend hint (intent: "stellar_direct"); it never mutates toAddress/toUnits, and settlementMode in PayWithStellarToken is log-only. So "source IS destination, fee 0.00" cannot misroute funds on the frontend. ✅

P1 — recommend before merge

  1. WalletConnect getAddress() blindly reuses live[0] (walletconnect.module.ts, getExistingSession/getAddress). Called with no address, it returns the first live session's publicKey without re-opening the modal. Multi-account scenario: user connected wallet A (session persisted), taps connect intending wallet B → silently re-bound to A, and signTransaction's wcSessionPaths[0] fallback signs from the unintended source account. Real mis-send risk for multi-account WC users. → match against an expected/requested address, or force a fresh connect when ambiguous.
  2. Modal-close observer race → stuck spinner (getAddress). document.querySelector(selector) is snapshotted once; if AppKit injects <appkit-modal> asynchronously or into a shadow root, the observer never fires. User closes the modal → Promise.race([approval(), modalClosed]) waits on approval() forever → spinner hangs, no cancel path. → prefer AppKit's subscribe/close callback over DOM-removal sniffing, or add a timeout.

P2 (non-blocking)

  • Forced intent override silently discards a consumer-supplied payParams.intent on same-token Stellar — intentional per comment, but no signal to the caller.
  • wcSessionPaths seeded from localStorage at module load before signClient exists; validated later via restorePersistedSessions, but any early signTransaction fallback would use an unvalidated entry. Add a guard.
  • Dropping payParams.intent from the title fallback is a behavioral change — confirm no consumer used intent as a display title.
  • supportedWallets: any[] loses type safety on id/isAvailable; settlementMode as a string-literal type will misrepresent any future backend mode.

Verdict: mergeable. Fund-safety path verified clean. The multi-account live[0] P1 has genuine mis-send potential for WC users — worth fixing now or as an immediate follow-up.

@shawnmuggle

Copy link
Copy Markdown
Member

AI review — incremental (1b4e69c4…960e2d16) — no P0

Scoped to the single commit in that compare range: 960e2d16 Update AnalyticsProvider.tsx, one file, +33/-12. Not a re-review of the full feat/stellar-wallet-ux diff.

The fix is correct, and it's a real bug

Verified the premise against posthog-js upstream (module.d.ts @ 1.407.2):

init(token: string, config?: ..., name?: string): PostHog;   // @returns The newly initialized PostHog instance

So both halves of this change check out:

  • The 3rd name arg is real and officially the multi-instance mechanism — upstream's own example is posthog.init(key, {}, 'project1').
  • init() returns the instance, so widening the local type from => void to => PostHogFull and assigning builtinRef.current = builtin is right. The previous code assigned mod.default — the shared singleton — which is exactly the collision being fixed.

The bug it fixes is genuine and not theoretical: import("posthog-js").then(m => m.default) resolves to the same module singleton the host app's posthog.init() touches. rozo-chat-ai calls posthog.init() itself, so before this commit an SDK embed there would either clobber the host's PostHog config or have its own telemetry silently dropped, depending on init order. Both directions are data-loss bugs and neither throws — the failure is invisible.

__loaded is declared on the PostHog class (module.d.ts:3286), not just on the default export, so the builtinRef.current?.__loaded guard still works on the named instance. Idempotency on remount / StrictMode double-invoke is fine too: init with an existing name returns the registered instance rather than constructing a second one.

The explanatory comment is unusually good — it states the failure mode and why the named form is required, which is exactly what stops someone "simplifying" this back into the bug later.

P2 — as unknown as PostHogFull widens an already-loose cast

The cast went from as PostHogFull to as unknown as PostHogFull, needed because the local PostHogFull now claims init returns PostHogFull and that no longer overlaps structurally with the real type. It works, but as unknown as disables the last structural check on this boundary — if posthog-js changes init's signature, this silently compiles and fails at runtime.

Since posthog-js is a declared (optional) peer dep, you can get the real type without importing the runtime:

type PostHogFull = import("posthog-js").PostHog;

That gives a type-only reference, keeps the lazy import() for the runtime, and makes a future upstream signature change a compile error instead of a runtime surprise. Not blocking — just noting the hand-rolled interface is now load-bearing enough to be worth deleting.

Note — the two telemetry paths have different __loaded semantics

Not introduced here, but adjacent enough to flag: in capture(), the built-in path guards with if (telemetryEnabled && builtinRef.current?.__loaded) and falls through, while the host path does if (!hostPosthog.__loaded) return;. Since the host block is last the behavior is currently identical, but that early return means any code appended after it gets skipped whenever the host client isn't loaded. Worth converting to the same fall-through shape before anything else is added to that function.

Verdict

No P0, no P1. Correct fix for a silent data-loss bug, well documented. The as unknown as is the only thing I'd change, and it's optional.

WalletConnectModule created two independent WalletConnect Cores: one
via SignClient.init() for signing, another via createAppKit()'s own
internal UniversalProvider init for the QR modal. manualWCControl only
affected close() behavior, not initialization, so both always fired.

Switch to UniversalProvider.init() (wraps a SignClient at .client, same
API) and pass that single instance into createAppKit({ universalProvider }),
so AppKit reuses it instead of spinning up its own Core.

Also pin @walletconnect/core/sign-client/universal-provider/ethereum-provider
to one version workspace-wide via pnpm overrides — the tree previously
resolved 5 different @walletconnect/core versions across @reown/appkit
and stellar-wallets-kit, each with its own module-scope singleton.
Horizon/Asset/tx-building symbols were statically imported in
StellarContextProvider, usePaymentState, PayWithStellarToken, and
(via chainAddress.ts) RozoPayButton itself, forcing the ~14M SDK to
parse/execute on every mount regardless of chain used. Converts each
to dynamic import(), deferred to actual use (wallet connect, quote,
tx submit). Adds validateAddressForChainAsync for RozoPayButton's
internal validation path; public isValidStellarAddress/
isValidSolanaAddress/validateAddressForChain stay sync for API compat.

Note: webpack still inlines stellar-sdk into the main chunk because
the SDK's index.ts barrel re-exports the sync isValidStellarAddress
from the same entry graph — these changes reduce eager execution work
but do not yet achieve real code-splitting. See conversation for the
follow-up needed (subpath export or consumer splitChunks config).
…allback

Three related races in the payId-mode source-token switch flow:

1. PayWithStellarToken/PayWithSolanaToken: the auto-transfer effect could
   fire handleTransfer twice in quick succession, and both calls passed
   the checkoutDoneRef guard before either finished (or Solana had no
   guard at all) — firing two checkout POSTs for the same payment.
   Replaced with a synchronously-claimed in-flight promise so the second
   call always awaits the first's result instead of racing it.

2. usePaymentState's payWithToken (EVM) trusted the closure-captured
   pay.order instead of a fresh store read. Combined with a memoized
   caller holding a stale reference, this let needRozoPayment evaluate
   against outdated order data intermittently.

3. needRozoPayment only compared chainId, so switching source tokens on
   the same chain (e.g. USDC Base -> USDT ETH) skipped checkout entirely
   and silently reused the stale order — worse than #2, since it never
   even created a payment. Now compares chain AND token address.

Also fixed a key collision introduced by #1's EVM counterpart: the
checkout dedup cache was keyed by payId alone, so switching source
tokens mid-flow could return a cached result for the wrong token.
Rekeyed to payId:chainId:tokenAddress.
shawnmuggle
shawnmuggle previously approved these changes Jul 26, 2026

@shawnmuggle shawnmuggle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the requested ecd3773..4e5dc1b range and checked the subsequent commits through current HEAD. No P0 found; approving per requested P0-only gate.

@shawnmuggle shawnmuggle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approval — incremental (f7211425…7cda1ff2) — no P0

Prior approval was dismissed by the new push. Scoped this pass to what landed since that SHA rather than re-reviewing the whole feat/stellar-wallet-ux diff.

What actually changed: beta → stable pin, nothing else

Six files, and every one is the release cut:

  • packages/connectkit/package.json@rozoai/intent-pay 0.1.38-beta.160.1.38
  • packages/pay-common/package.json@rozoai/intent-common 0.1.26-beta.50.1.26
  • examples/nextjs-app/package.json — both deps repinned to the stable versions
  • pnpm-lock.yaml — specifier + resolution updates matching the above
  • CHANGELOG.md (+57), bundle-analysis.html (build artifact)

Zero source changes. No .ts/.tsx touched in this range, so nothing in the signing or fund-routing path moved since the last review.

Verified against the registry

Both packages are actually published, and the lockfile isn't pointing at something that doesn't exist:

@rozoai/intent-pay    dist-tags → { latest: '0.1.38', beta: '0.1.38-beta.16' }

Integrity hashes in pnpm-lock.yaml match the published tarballs byte for byte:

  • intent-pay@0.1.38sha512-sJ6ALwejToIFOHyfXqYkOAIVvgbhk9HHqmoeZ69+BS6ChFWnYV2hCADvCv1FOjpVLTQsGzOQX6OJFtJqFKYc3w==
  • intent-common@0.1.26sha512-vnIGZK8KnFmiyACV856+Ac8yVWy3IDxKGzs1hB0p73bUOY7frFrvcndQPdqXEzEFWSVkmGdtqQvYtTj0b6bGNw==

Also worth noting latest now resolves to 0.1.38, so the pin and the published tag agree — no risk of consumers installing a version this repo never built.

On the red review check

review is failing, but not on this code:

The template is not valid. .github/workflows/ai-pr-review-v2.yml (Line: 120, Col: 20):
Error reading JToken from JsonReader. Path '', line 0, position 0.

That's the AI-review workflow failing to parse its own YAML — it dies before it ever looks at the diff. Infra bug in the review tooling, not a signal about this PR. Security scan, release, and Vercel all pass. Worth fixing separately since it will red-flag every PR in the repo until it is.

Verdict

Re-approving. The prior P1s (WalletConnect getAddress() reusing live[0], etc.) are unchanged and still non-blocking — this push didn't touch them and didn't introduce anything new.

@akbarsaputrait
akbarsaputrait merged commit 39178b2 into master Jul 26, 2026
5 of 6 checks passed
@akbarsaputrait
akbarsaputrait deleted the feat/stellar-wallet-ux branch July 26, 2026 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants