fix/PRO-3159/polygon-gas-token-updates#273
Conversation
WalkthroughThis pull request updates token handling in two modules. In the token atlas module, a new constant Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Definitions (1)src/services/tokensData.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying x with
|
| Latest commit: |
6467eaf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://69658624.x-e62.pages.dev |
| Branch Preview URL: | https://fix-pro-3159-polygon-gas-tok.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (fix/PRO-3159/polygon-gas-token-updates)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/apps/token-atlas/index.tsx (2)
81-86: Consider adding a comment explaining the special handling for wrapped/native tokensThe new
isWrappedOrNativeTokenconstant adds special handling for wrapped tokens like POL, WETH, and WXDAI. While this addresses the PR objective for managing the POL token differently, adding a comment explaining the rationale would improve maintainability.Also, the current approach using exact string matching might be brittle if token metadata changes (e.g., capitalization, formatting).
+// Identifies tokens that need special handling in queries: +// - Native tokens (like MATIC on Polygon) +// - Wrapped tokens that represent native tokens of other chains const isWrappedOrNativeToken = isNativeToken || (selectedToken.name === 'POL' && selectedToken.symbol === 'POL') || (selectedToken.name === 'Wrapped Ether' && selectedToken.symbol === 'WETH') || (selectedToken.name === 'Wrapped XDAI' && selectedToken.symbol === 'WXDAI');
88-96: Document the purpose of symbol transformation for wrapped tokensThe
getSymbolfunction transforms wrapped token symbols to their unwrapped equivalents, which aligns with the PR objective to update the management of wrapped tokens. However, there's no transformation for POL even though it's included inisWrappedOrNativeToken.Consider documenting why this transformation is necessary for the token price graph and info queries.
+// Transforms wrapped token symbols to their native equivalents for API queries +// This ensures consistent price data retrieval for both wrapped and unwrapped tokens const getSymbol = (symbol: string) => { if (isWrappedOrNativeToken && symbol === 'WETH') { return 'ETH'; } if (isWrappedOrNativeToken && symbol === 'WXDAI') { return 'XDAI'; } return symbol; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/token-atlas/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unit-tests
- GitHub Check: Cloudflare Pages
- GitHub Check: build
🔇 Additional comments (1)
src/apps/token-atlas/index.tsx (1)
104-108:Details
❓ Verification inconclusive
Verify the API behavior when omitting id and asset parameters
For wrapped or native tokens, both
idandassetparameters are set toundefined, which means the API queries will rely solely on the transformedsymbol. Does the API handle this case correctly for all tokens, especially POL which has no symbol transformation?Consider adding a comment explaining why this approach is correct for these tokens or verify that relying only on the symbol provides accurate results.
Also applies to: 116-120
🏁 Script executed:
#!/bin/bash # Check whether the API properly handles queries with only symbol parameter # Search for the token API schema or implementation to verify expected behavior echo "Searching for token API implementation..." rg -A 10 "useGetTokenInfoQuery|useGetTokenGraphQuery" --glob "**/*.{ts,tsx}" --exclude "index.tsx"Length of output: 603
Below is a new shell script to re-run the search with the correct exclusion syntax. This will help us locate the token API implementation (e.g., usage of useGetTokenInfoQuery or useGetTokenGraphQuery) in the codebase:
Once this command produces output (or if manual verification is required), please verify that the API indeed handles queries where both the
idandassetparameters are omitted—relying solely on the transformedsymbol—in all cases (especially tokens like POL that might not undergo any symbol transformation).Based on this verification, update the code by either adding a clarifying inline comment that documents why it’s safe to rely on
symbolonly for wrapped/native tokens or by adjusting the API call if needed.Action Required: Please verify that the API correctly processes token queries with only the
symbolparameter (withidandassetomitted). Also, consider adding an inline comment here (insrc/apps/token-atlas/index.tsxat lines 104–108 and 116–120) explaining this fallback logic for wrapped/native tokens.
🏁 Script executed:
#!/bin/bash # Corrected command: using -g to exclude index.tsx properly rg -A 10 "useGetTokenInfoQuery|useGetTokenGraphQuery" -g '!index.tsx'Length of output: 569
Action Required: Verify API fallback behavior for wrapped/native tokens
The verification script confirmed that the token API endpoints (i.e.
useGetTokenInfoQueryanduseGetTokenGraphQuery) are defined insrc/apps/token-atlas/api/token.ts. However, it's still unclear from the implementation whether the API correctly handles cases when bothidandassetparameters are omitted—relying solely on the transformedsymbol—especially for tokens like POL that lack symbol transformation.
- Please manually verify that the API processes queries based only on the
symbolwhen the token is wrapped or native.- Consider adding inline comments in
src/apps/token-atlas/index.tsx(lines 104–108 and 116–120) to document why it's safe to rely onsymbolonly for these tokens, assuming manual verification confirms correct API behavior.
github-actions
bot
temporarily deployed
to
Preview (fix/PRO-3159/polygon-gas-token-updates)
Co-authored-by: I Am Kio <kieran@digitalindustria.com>
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes