Skip to content

Prevent base wallet sends to robinhood that trap funds#1072

Merged
pedromcunha merged 2 commits into
mainfrom
pedro/int2-881-prevent-base-wallet-sends-to-robinhood-that-trap-funds
Jul 10, 2026
Merged

Prevent base wallet sends to robinhood that trap funds#1072
pedromcunha merged 2 commits into
mainfrom
pedro/int2-881-prevent-base-wallet-sends-to-robinhood-that-trap-funds

Conversation

@pedromcunha

Copy link
Copy Markdown
Member

No description provided.

@pedromcunha
pedromcunha requested a review from ted-palmer July 10, 2026 16:46
@vercel

vercel Bot commented Jul 10, 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 10, 2026 4:50pm

Request Review

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @pedromcunha's task in 1m 20s —— View job


Code Review

  • Gathered context (diff, commits, changed files)
  • Analyzed usage of modified constants (useIsWalletCompatible, steps.ts)
  • Reviewed for correctness, security, and repo guidelines

Code Review — 1 note

The change is small and correct. It adds chain 4663 (Robinhood) to WalletChainIncompatible under a new coinbase key, and normalizes both the coinbasewalletevm and baseevm connectors to coinbase. In useIsWalletCompatible.ts:53-56, any wallet resolving to coinbase on chain 4663 returns false, blocking both send and receive — matching the changeset intent. The new normalization keys follow the existing lowercase {name}{vm} pattern (phantomevm, portoevm, trusttron), and since there's no coinbase entry in WalletChainRestricted, no unintended restriction is introduced.

  1. NOTE Architecture: shared normalization map affects display name
    packages/ui/src/constants/walletCompatibility.ts:29NormalizedWalletName is consumed by both the compatibility check and getWalletDisplayName in utils/steps.ts:29-33. Mapping baseevm → coinbase means a Base-wallet user will now see "Confirm in Coinbase" during transaction steps rather than "Base". That's fine for the compatibility gate (both should be blocked on 4663), but it's a side effect on the display name that may be unintended. If the display distinction matters, the incompatibility check could key off a separate map rather than reusing the display-name normalization. Worth confirming this is acceptable.

Nothing blocking. Otherwise the change is clean and consistent with existing conventions.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @pedromcunha's task in 52s —— View job


📝 Docs Drift Check — No docs impact

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

@pedromcunha
pedromcunha merged commit a29e409 into main Jul 10, 2026
6 checks passed
@pedromcunha
pedromcunha deleted the pedro/int2-881-prevent-base-wallet-sends-to-robinhood-that-trap-funds branch July 10, 2026 17:05
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