feat: route custom token metadata imports through proxy#12040
feat: route custom token metadata imports through proxy#12040kaladinlight merged 12 commits intodevelopfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigrates custom token metadata retrieval from the Alchemy SDK to a proxy API: removes Alchemy env vars, SDK, CSP entries, and related modules; adds Changes
Sequence DiagramsequenceDiagram
actor User
participant SearchUI as GlobalSearch / UI
participant Hook as useGetCustomTokensQuery
participant Proxy as Proxy API
participant Store as Asset Store
User->>SearchUI: Enter token address + chain
SearchUI->>Hook: Request token metadata (address, chainId)
Hook->>Proxy: HTTP GET /custom-token?address=...&chainId=...
Proxy-->>Hook: Return MinimalAsset[]
Hook-->>SearchUI: Return MinimalAsset[]
SearchUI->>Store: selectAssets() / assetsById
Store-->>SearchUI: Existing assets map
SearchUI->>Store: Upsert new assets (makeAsset / upsert)
Store-->>SearchUI: Confirm upsert
SearchUI-->>User: Render results (or Spinner / SearchEmpty)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
NeOMakinG
left a comment
There was a problem hiding this comment.
✅ LGTM — Clean refactoring
Changes:
- Proxy routing — Custom token metadata lookups now go through
api.proxy.shapeshift.cominstead of direct Alchemy calls - Removed committed API keys —
VITE_ALCHEMY_API_KEY,VITE_ALCHEMY_POLYGON_URL,VITE_ALCHEMY_SOLANA_BASE_URLremoved from.env - Centralized chain IDs — New
CUSTOM_TOKEN_IMPORT_SUPPORTED_CHAIN_IDSconstant replaces scattered Alchemy SDK references - CSP updates — Added proxy URL, removed direct Alchemy URLs
Benefits:
- No more client-side API keys
- Centralized monitoring and rate limiting via proxy
- Cleaner code with single source of truth for supported chains
CI passes ✅
🤖 Reviewed by Claude Code
🤖 QABot Test Report✅ 1/1 tests passed
Verified: Custom token metadata now routes through proxy API, committed API keys removed. 📊 Full Report: https://qabot-kappa.vercel.app/runs/27ac5bb7-4fe0-4104-96be-c8a4055b6c6d 🤖 Automated QA by Claude Code |
…metadata-proxy-v2 # Conflicts: # headers/csps/chains/ethereum.ts
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/TradeAssetSearch/hooks/useGetCustomTokensQuery.tsx (1)
86-91: UnconventionalskipTokenusage pattern.Returning
skipTokenfrom within the query function is not the standard react-query pattern. The idiomatic approach isqueryFn: condition ? skipToken : () => fetchData(). However, since thecombinefunction (line 105) filters outskipTokenvalues, this works as a sentinel value approach.If this pattern causes any issues (e.g., react-query warnings or unexpected query states), consider refactoring to pass
skipTokendirectly toqueryFnwhen the address is invalid.💡 Optional: Idiomatic skipToken pattern
+ const isValidAddress = isValidEvmAddress || isValidSolanaAddress + const customTokenQueries = useQueries({ queries: chainIds.map(chainId => ({ queryKey: getCustomTokenQueryKey(contractAddress, chainId), - queryFn: getQueryFn(chainId), + queryFn: isValidAddress ? () => getCustomToken(chainId) : skipToken, enabled: customTokenImportEnabled, staleTime: Infinity, })), combine: queries => mergeQueryOutputs(queries, results => results.filter(isMinimalAsset)), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TradeAssetSearch/hooks/useGetCustomTokensQuery.tsx` around lines 86 - 91, The current getQueryFn returns the sentinel skipToken from inside the returned function which is non-idiomatic and can trigger react-query warnings; change getQueryFn so it returns skipToken directly as the queryFn when the address is invalid instead of returning skipToken from the inner function — i.e., for a given chainId, return skipToken if (!isValidEvmAddress && !isValidSolanaAddress) else return a function that calls getCustomToken(chainId); keep references to getCustomToken, isValidEvmAddress, isValidSolanaAddress and ensure the combine call (which filters skipToken) continues to work with this refactor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Layout/Header/GlobalSearch/AssetSearchResults.tsx`:
- Around line 22-31: The no-results empty state (SearchEmpty) is currently gated
by !isSearching and can be skipped when the parent keeps isSearching true for
active queries; update AssetSearchResults so that when noResults && searchQuery
is true it returns <SearchEmpty searchQuery={searchQuery} /> regardless of
isSearching (e.g., check/render SearchEmpty before the spinner or change the
spinner condition), leaving the Spinner shown only for active searches that are
not already known to have no results.
---
Nitpick comments:
In `@src/components/TradeAssetSearch/hooks/useGetCustomTokensQuery.tsx`:
- Around line 86-91: The current getQueryFn returns the sentinel skipToken from
inside the returned function which is non-idiomatic and can trigger react-query
warnings; change getQueryFn so it returns skipToken directly as the queryFn when
the address is invalid instead of returning skipToken from the inner function —
i.e., for a given chainId, return skipToken if (!isValidEvmAddress &&
!isValidSolanaAddress) else return a function that calls
getCustomToken(chainId); keep references to getCustomToken, isValidEvmAddress,
isValidSolanaAddress and ensure the combine call (which filters skipToken)
continues to work with this refactor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9484e74-15f4-42be-9668-5b41dd8eafba
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
.env.env.developmentheaders/csps/alchemy.tsheaders/csps/chains/ethereum.tsheaders/csps/index.tspackage.jsonsrc/components/Layout/Header/GlobalSearch/AssetSearchResults.tsxsrc/components/Layout/Header/GlobalSearch/GlobalSearchModal.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/components/TradeAssetSearch/hooks/useGetCustomTokensQuery.tsxsrc/config.tssrc/index.tsxsrc/lib/alchemySdkInstance.tssrc/lib/assetSearch/deduplicateAssets.tssrc/lib/customTokenImportSupportedChainIds.tssrc/state/slices/common-selectors.tssrc/utils/sentry/httpclient.tssrc/vite-env.d.ts
💤 Files with no reviewable changes (5)
- headers/csps/chains/ethereum.ts
- src/lib/alchemySdkInstance.ts
- package.json
- headers/csps/index.ts
- headers/csps/alchemy.ts
src/components/Layout/Header/GlobalSearch/AssetSearchResults.tsx
Outdated
Show resolved
Hide resolved
…com/shapeshift/web into feat/custom-token-metadata-proxy-v2
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Layout/Header/GlobalSearch/GlobalSearchModal.tsx`:
- Around line 71-73: The forEach callback in GlobalSearchModal.tsx implicitly
returns the result of dispatch (Biome flags this); change the callback to a
block-bodied function so it returns void — e.g. replace the arrow shorthand with
a block that calls
dispatch(assetsSlice.actions.upsertAsset(makeAsset(assetsById, token))); (or use
a for...of loop over customTokens.filter(...)) so the callback does not return
the dispatch value. Ensure you keep the same identifiers: customTokens,
assetsById, dispatch, assetsSlice.actions.upsertAsset, and makeAsset.
- Around line 61-64: The destructure of useGetCustomTokensQuery in
GlobalSearchModal drops proxy failures and lets the UI fall through to
SearchEmpty; update GlobalSearchModal to read the hook's error and isError
(e.g., const { data: customTokens, isLoading: isLoadingCustomTokens, error,
isError } = useGetCustomTokensQuery(...)) and when isError or error indicates a
proxy/5xx failure call the useErrorToast hook (useErrorToast(...)) to surface a
translated error message and log details, then prevent showing SearchEmpty as a
normal “no match” result (e.g., show error state or early return) so proxy
lookup failures are routed through the normal toast/logging path rather than
silently appearing as no results.
- Around line 66-74: The render flashes SearchEmpty because customTokens are
upserted asynchronously and isLoadingCustomTokens can flip false before the
store-backed results contain the new assetIds; modify the logic that clears the
loading gate (or the component that chooses to render SearchEmpty) to wait until
the upserted assetIds are present in results or alternatively render
customTokens directly when results are missing. Concretely, in the effect around
customTokens (and the similar block at 184-188) ensure you compute the
missingIds by comparing customTokens' assetId to selectAssets(store.getState())
and either block clearing isLoadingCustomTokens until all missingIds are present
in results (or add a boolean derived from results.includes(assetId)), or change
the render branch that uses isLoadingCustomTokens/results to fall back to
rendering customTokens until the store results include those assetIds (use
symbols: customTokens, selectAssets, assetsSlice.actions.upsertAsset, makeAsset,
isLoadingCustomTokens, results, SearchEmpty).
In `@src/lib/assetSearch/deduplicateAssets.test.ts`:
- Around line 90-93: The tests use real-looking address assetIds (axlusdcFamily,
axlusdcPrimary, AXLUSDC_OPTIMISM) but only exercise symbol/name search paths;
add assertions that explicitly perform an address-based search (using the
address-like assetId string such as axlusdcFamily or axlusdcPrimary.assetId) and
assert deduplicateAssets (or the test helper invoking the address search branch)
returns the expected deduplicated result for the address path; update the
existing test blocks that reference the AXLUSDC fixtures (the sections around
the axlusdcFamily / axlusdcPrimary fixtures and the later AXLUSDC_OPTIMISM-based
assertions) to include these address-search assertions so the new
contract-address deduplication branch is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 865e68ea-536c-4ff8-8467-43944c0e7665
📒 Files selected for processing (3)
src/components/Layout/Header/GlobalSearch/AssetSearchResults.tsxsrc/components/Layout/Header/GlobalSearch/GlobalSearchModal.tsxsrc/lib/assetSearch/deduplicateAssets.test.ts
src/components/Layout/Header/GlobalSearch/GlobalSearchModal.tsx
Outdated
Show resolved
Hide resolved
src/components/Layout/Header/GlobalSearch/GlobalSearchModal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Layout/Header/GlobalSearch/GlobalSearchModal.tsx`:
- Line 87: Remove the stray debugging console.log in the GlobalSearchModal
component: delete the console.log({ searchAssets, customAssets, results }) call
so the modal no longer logs on every keystroke; locate this call inside the
GlobalSearchModal (component/function) render/effect where searchAssets,
customAssets, and results are referenced and simply remove the line or replace
it with a non-logging alternative (e.g., a conditional debug-only logger behind
a dev flag) if needed for local debugging.
In `@src/components/TradeAssetSearch/components/SearchTermAssetList.tsx`:
- Around line 109-117: customAssets currently skips the same
filtering/eligibility path used by assetsForChain and only checks
!assetsById[token.assetId], which can surface assets the caller filtered out or
drop valid proxy hits; change the logic in SearchTermAssetList so you first
materialize proxy-backed assets from customTokens (using the same
makeAsset/proxy resolution path), run those proxy assets through the exact
eligibility filters used by assetsForChain (the same predicates that produce the
list passed to existingAssetIds), and only after that dedupe against
existingAssetIds/assetsById before returning customAssets so proxy results
respect the same chain/list filters and are correctly de-duplicated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 593aff5f-23c4-43fe-9d61-e0ebcb405067
📒 Files selected for processing (4)
src/components/Layout/Header/GlobalSearch/GlobalSearchModal.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/lib/assetSearch/deduplicateAssets.test.tssrc/lib/assetSearch/deduplicateAssets.ts
src/components/Layout/Header/GlobalSearch/GlobalSearchModal.tsx
Outdated
Show resolved
Hide resolved
…com/shapeshift/web into feat/custom-token-metadata-proxy-v2
unchainedfriend to be merged for the proxy to deploy: shapeshift/unchained#1265Description
Route custom token metadata import lookups through
api.proxy.shapeshift.cominstead of direct browser Alchemy/Metaplex calls. The primary motivation is that our Alchemy key is being abused since it's checked into the repo. This PR puts it behind a proxy so the key isn't exposed client-side. The key will be rotated once this PR merges.VITE_ALCHEMY_POLYGON_URLand related config/type referencesIssue (if applicable)
N/A
Risk
Low-medium. This changes how custom token metadata is fetched (proxy vs direct Alchemy calls), but does not affect any on-chain transactions, wallet interactions, or core state management. If the proxy is unavailable, custom token imports would fail to resolve metadata.
No protocols, transaction types, wallets, or contract interactions are affected. This only impacts the custom token import metadata lookup flow.
Testing
Engineering
Operations
Screenshots (if applicable)
N/A
Summary by CodeRabbit
New Features
Improvements
Chores