Skip to content

fix: support Robinhood Lighter as a destination and block lvm chains as origin - #1151

Merged
ted-palmer merged 2 commits into
mainfrom
ted/int2-1712-fix-robinhood-lighter-source-account-on-the-frontend
Oct 6, 2026
Merged

ted-palmer merged 2 commits into
mainfrom
ted/int2-1712-fix-robinhood-lighter-source-account-on-the-frontend

Conversation

@ted-palmer

Copy link
Copy Markdown
Collaborator

No description provided.

@vercel

vercel Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

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

Project Deployment Actions Updated
relay-sdk-demo Ready Ready Preview Oct 6, 2026 5:16pm UTC

Request Review

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @ted-palmer's task in 1m 45s —— View job


Code Review — 2 note(s)

Reviewed 10 files, ~106/-21 lines. The change is well-scoped: blocks lvm chains as an origin (isOriginChainSupported, applied in TokenSelector, PaymentMethod, and the swap-direction button), adds per-chain Lighter placeholder addresses, and keys Lighter account resolution/caching by the chain's own API URL. The per-apiUrl query key + cache-scan scoping correctly fixes the cross-chain account-index collision. A couple of things worth a look:

  1. NOTE — Correctness: silent fallback to mainnet API for a Lighter chain
    packages/ui/src/hooks/useLighterAccount.ts:54 — const apiUrl = chain?.httpRpcUrl ?? DEFAULT_LIGHTER_API_URL. If a lvm chain is passed via chainId but its httpRpcUrl is missing (or the chain isn't found in relayClient.chains), resolution silently falls back to the mainnet Lighter API. Since account indexes are per-chain, that reintroduces exactly the wrong-chain-resolution bug this PR fixes — and account indexes feed the recipient address, so a wrong resolution is a correctness/safety concern, not just a UX one. AGENTS.md also calls out avoiding fallback defaults on critical values (addresses). Consider leaving the query disabled (or logging) when a Lighter chainId resolves to no httpRpcUrl, rather than defaulting. Fix this →

  2. NOTE — httpRpcUrl semantics
    packages/ui/src/hooks/useLighterAccount.ts:77 — new URL('/api/v1/account', apiUrl) assumes httpRpcUrl is the Lighter REST API root. Everywhere else in the repo httpRpcUrl is used as an EVM/RPC transport URL (configureViemChain, viemWallet, etc.), so this overloads its meaning. Also note the leading-slash path means any path component in httpRpcUrl would be discarded (https://host/rpc → https://host/api/v1/account). This is fine if the backend configures Lighter chains' httpRpcUrl as a bare host pointing at the Lighter API — just worth confirming that's guaranteed.

Everything else looks good: the isDeadAddress additions (462196 / 33627) are the designated per-chain placeholder indexes; clearing customToAddress when switching between two different Lighter chains in handleSetToToken is correct; and the swap-direction disabled guard correctly prevents flipping a Lighter destination into the origin slot.
• ted/int2-1712-fix-robinhood-lighter-source-account-on-the-frontend

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @ted-palmer's task in 1m 13s —— View job


📝 Docs Drift Check — No docs impact

This PR does not touch surfaces that require a relayprotocol/relay-docs update.

Scope reviewed: the changed SDK exports (isDeadAddress/getDeadAddress) and internal UI hook (useLighterAccount) are not documented reference surfaces, and the widget's destination-only enforcement is consistent with the existing adapters page, which already frames Lighter as a deposit destination. No documented props, hooks, actions, or adapter signatures changed.

@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds Lighter blockchain as a swap destination and blocks it as origin.

The PR appears safe to merge; the previous recipient-reset issue is addressed.

Summary

The PR supports Lighter destinations with chain-specific placeholder addresses, account lookups, and caches. It also blocks Lighter origins.

  • Moves recipient clearing into SwapWidgetRenderer, covering controlled destination changes.
  • Addresses the previous finding. No new actionable issues were found in the changes since that review.

Reviews (2) · Last reviewed commit: "feedback"

Comment thread packages/ui/src/components/widgets/SwapWidget/index.tsx Outdated
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @ted-palmer's task in 1m 36s —— View job


Code Review

Reviewed 10 files, +116/−19. The change is well-scoped: isOriginChainSupported centralizes origin-chain gating (blocking lvm chains and preserving the existing deposit-address rule), applied consistently in TokenSelector, PaymentMethod, and the swap-direction button; Lighter account resolution/caching is correctly keyed per apiUrl, fixing the cross-chain account-index collision.

One note worth considering, plus confirmation that the earlier flagged concerns are handled.

1. [NOTE] Correctness: silent fallback to mainnet Lighter API
packages/ui/src/hooks/useLighterAccount.ts:54 — const apiUrl = chain?.httpRpcUrl ?? DEFAULT_LIGHTER_API_URL. If a lvm chain is passed via chainId but the chain isn't found in relayClient.chains or has no httpRpcUrl, resolution silently falls back to the mainnet Lighter API. Because account indexes are per-chain and the resolved index feeds the recipient address, a wrong-chain resolution is a correctness/safety concern, not just UX — and it reintroduces exactly the collision this PR fixes. AGENTS.md calls out avoiding fallback defaults on critical values (addresses) — better to leave the query disabled (or log) when a Lighter chainId resolves to no httpRpcUrl. It's config-dependent (mainnet Lighter happens to share the default URL, so only non-mainnet Lighter chains with a missing httpRpcUrl would misresolve), so this is a hardening note rather than a confirmed bug. Fix this →

On the earlier greptile P1 ("old Lighter recipient survives") — looks resolved. The concern was that a controlled toToken change from one Lighter chain to another bypasses the reset. In the current branch the reset doesn't live in handleSetToToken (which never touches customToAddress) — it's the useEffect in SwapWidgetRenderer.tsx:678-691, keyed on [isLighterChain, toChain?.id]. That fires whenever the destination chain id changes regardless of whether the change came from a user tap or a controlled prop, clearing a numeric Lighter recipient on the switch. So the controlled path is covered.

Spot-checks that look correct:

  • isOriginChainSupported preserves the prior deposit-address exception and the swap-direction disabled guard correctly blocks flipping a Lighter destination into the origin slot (undefined toChain → enabled, as before).
  • Per-apiUrl query key + the scoped cache scan (['useLighterAccount', apiUrl]) and bidirectional write both correctly namespace by chain.
  • isDeadAddress additions (462196 / 33627) are the designated per-chain placeholder indexes; getDeadAddress routes Robinhood Lighter by chain id.

Nothing blocking from me — the fallback is the only item worth a second look.
• ted/int2-1712-fix-robinhood-lighter-source-account-on-the-frontend

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @ted-palmer's task in 41s —— View job


📝 Docs Drift Check — No docs impact

This PR does not touch surfaces that require a relayprotocol/relay-docs update.

@ted-palmer
ted-palmer merged commit 48d611d into main Oct 6, 2026
9 checks passed
@ted-palmer
ted-palmer deleted the ted/int2-1712-fix-robinhood-lighter-source-account-on-the-frontend branch October 6, 2026 18:37

This branch was successfully deployed

1 active deployment
Preview — 06732d9b Deployed Oct 6, 2026 by vercel[bot]
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