Skip to content

fix: block token contract addresses in recipient field#1078

Open
ted-palmer wants to merge 4 commits into
mainfrom
ted/int2-899-block-token-contracts-in-recipient-field
Open

fix: block token contract addresses in recipient field#1078
ted-palmer wants to merge 4 commits into
mainfrom
ted/int2-899-block-token-contracts-in-recipient-field

Conversation

@ted-palmer

Copy link
Copy Markdown
Collaborator

No description provided.

@ted-palmer
ted-palmer requested a review from pedromcunha July 14, 2026 15:14
@vercel

vercel Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

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

Project Deployment Actions Updated (UTC)
relay-sdk-demo Ready Ready Preview, Comment Jul 14, 2026 6:40pm

Request Review

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

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


📝 Docs Drift Check — No docs impact

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

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

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


📝 Docs Drift Check — No docs impact

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

Comment thread packages/ui/src/hooks/useKnownTokenContract.ts
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

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


Code Review — PR #1078

  • Gather context (diff, commits, review feedback)
  • Read changed files in full
  • Verify correctness of the new hook + wiring
  • Post review

Code Review — 2 note(s) found

Clean, well-structured change. The layered approach is sound: a synchronous check against the selected destination token, then a local scan of the chain-config currencies (currency/erc20Currencies/featuredTokens/solverCurrencies), and only then a fallback to the currencies API — which directly addresses @pedromcunha's request to check chain tokens before making a request. The API query is correctly keyed on address (no stale results), gated by isValidAddress + enabled, and retry: false ensures the check fails open on error without hanging isChecking. The modal folds isCheckingTokenContract into isLoading to disable Save while the lookup is pending, and recent-address pills are now routed back through input validation instead of confirming directly. Adding response.isFetching to the useTokenList memo deps is harmless.

The two items below are low-severity defense-in-depth gaps, not blockers.

  1. NOTE Execute-time guard only covers the async match
    packages/ui/src/components/widgets/SwapWidgetRenderer.tsx:926 — the last-line-of-defense throw guards on recipientMatchesKnownToken but not on recipientIsDestinationToken. If a recipient equal to the selected destination token's contract reached this path, the throw wouldn't fire; it relies solely on isValidToAddress gating the CTA upstream. For symmetry with isValidToAddress/toAddressWithFallback (both check both flags), consider if (recipientIsDestinationToken || recipientMatchesKnownToken).

  2. NOTE CTA not gated on the async loading state
    packages/ui/src/components/widgets/SwapWidgetRenderer.tsx:433-446 — unlike CustomAddressModal (which folds isChecking into isLoading), the renderer ignores the returned isChecking. While the lookup for a freshly-changed customToAddress is in flight, recipientMatchesKnownToken is false, so isValidToAddress is momentarily true and the execute guard wouldn't throw. In practice customToAddress is only set through the modal (which already blocks contracts), so this is a narrow race — but if customToAddress could ever be seeded from props/deeplink, the block would be bypassed until the lookup resolves. useKnownTokenContract already returns isChecking; gating on it here closes the gap.

The async check intentionally fails open on API error (documented in useKnownTokenContract.ts), so blocking of arbitrary non-selected token contracts is best-effort by design; the synchronous selected-token check remains authoritative. No issues found with security, error handling, the changeset, the new event constant, or the toToken prop wiring in WidgetContainer/OnrampWidget.
ted/int2-899-block-token-contracts-in-recipient-field

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

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


📝 Docs Drift Check — No docs impact

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

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