-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(widget): Chain & token update #3331
Conversation
WalkthroughThe changes in this pull request involve the addition of new constants and modifications to existing ones within the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3331 +/- ##
====================================================
+ Coverage 33.25951% 90.44834% +57.18883%
====================================================
Files 543 54 -489
Lines 34769 1026 -33743
Branches 82 82
====================================================
- Hits 11564 928 -10636
+ Misses 22182 95 -22087
+ Partials 1023 3 -1020
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
packages/widget/src/index.tsx (1)
Line range hint
90-114
: Review token export organizationConsider reorganizing token exports to maintain a consistent pattern:
- Group by token type (native, wrapped, stable)
- Maintain alphabetical ordering within groups
Consider applying this structure:
// Native Tokens export const ETH = BRIDGEABLE.ETH export const POL = BRIDGEABLE.POL // ... other native tokens // Wrapped Tokens export const WETH = BRIDGEABLE.WETH export const WPOL = BRIDGEABLE.WPOL // ... other wrapped tokens // Stable Tokens export const USDC = BRIDGEABLE.USDC export const USDT = BRIDGEABLE.USDT // ... other stable tokens // Other Tokens export const WLD = BRIDGEABLE.WLD // ... other tokens
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- packages/widget/src/constants/bridgeable.ts (6 hunks)
- packages/widget/src/constants/chains.ts (2 hunks)
- packages/widget/src/index.tsx (2 hunks)
🔇 Additional comments (6)
packages/widget/src/index.tsx (2)
111-111
: Verify WLD (Worldcoin) token integrationThe new WLD token export needs verification of its complete implementation.
#!/bin/bash # Description: Verify WLD token implementation and chain support # Test: Check WLD token definition rg -A 10 "WLD\s*=\s*{" packages/widget/src/constants/bridgeable.ts # Test: Check WORLDCHAIN configuration rg -A 5 "WORLDCHAIN" packages/widget/src/constants/chains.ts
114-114
: Breaking Change: WMATIC renamed to WPOLThe renaming of WMATIC to WPOL is a breaking change that requires careful consideration.
This change could break existing implementations. Consider:
- Adding a deprecation notice for WMATIC
- Maintaining backward compatibility temporarily
- Documenting the migration path
#!/bin/bash # Description: Check for existing WMATIC usage that needs updating # Test: Look for WMATIC references in the codebase rg "WMATIC" --type tspackages/widget/src/constants/chains.ts (1)
511-511
: LGTM!The integration of WORLDCHAIN into the CHAINS object follows the established pattern and maintains proper ordering.
packages/widget/src/constants/bridgeable.ts (3)
810-810
: LGTM: ETH token WORLDCHAIN support added correctly.The addition of WORLDCHAIN support follows the established pattern of using ZeroAddress for native tokens and 18 decimals.
Also applies to: 823-823
999-999
: LGTM: USDCe token WORLDCHAIN support added correctly.The addition of WORLDCHAIN support uses the correct decimals (6) for USDC and follows the established pattern.
Also applies to: 1006-1006
1433-1452
: LGTM: WLD token added with proper configuration.The Worldcoin (WLD) token is correctly implemented with:
- Support for ETHEREUM, OPTIMISM, and WORLDCHAIN
- Consistent use of 18 decimals
- Proper priority ranking
@@ -87,6 +87,7 @@ export const ONEETH = BRIDGEABLE.ONEETH | |||
export const ONEUSDC = BRIDGEABLE.ONEUSDC | |||
export const ONEUSDT = BRIDGEABLE.ONEUSDT | |||
export const PEPE = BRIDGEABLE.PEPE | |||
export const POL = BRIDGEABLE.POL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
POL token export requires comprehensive updates across the codebase
The verification reveals several issues that need to be addressed:
- The POL token is not properly defined in
bridgeable.ts
despite being exported inindex.tsx
- While there are references to POL and WPOL symbols in
bridgeMap.ts
, the corresponding token definitions are missing - Multiple packages still reference MATIC/WMATIC tokens that need to be updated:
- packages/widget/src/constants/bridgeable.ts
- packages/synapse-interface/constants/tokens/bridgeable.ts
- packages/synapse-constants/src/constants/tokens/bridgeable.ts
- packages/rest-api/src/constants/bridgeable.ts
The export of POL token cannot be done in isolation without first implementing the proper token definitions and updating all related MATIC references across the codebase.
🔗 Analysis chain
Verify POL token implementation
The addition of POL token export needs verification to ensure all necessary chain configurations are in place.
Let me gather more information about the POL token implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify POL token implementation and its chain configurations
# Test: Check if POL is properly defined in bridgeable constants
rg -A 5 "POL\s*=\s*{" packages/widget/src/constants/bridgeable.ts
# Test: Check for any existing MATIC references that might need updates
rg "MATIC" --type ts
Length of output: 8595
Script:
#!/bin/bash
# Check if POL token is properly defined in bridgeable constants
rg -A 5 "POL\s*=\s*" packages/widget/src/constants/bridgeable.ts
# Check for POL references in bridge maps and routes
rg "POL" packages/widget/src/constants/bridgeMap.ts
# Check for POL chain configuration
rg -A 5 "POL" packages/widget/src/constants/chains.ts
Length of output: 446
export const WORLDCHAIN: Chain = { | ||
id: 480, | ||
name: 'World Chain', | ||
rpcUrls: { | ||
primary: 'https://worldchain-mainnet.g.alchemy.com/public', | ||
fallback: 'https://worldchain-mainnet.g.alchemy.com/public', | ||
}, | ||
explorerUrl: 'https://worldchain-mainnet.explorer.alchemy.com', | ||
explorerName: 'World Chain Explorer', | ||
blockTime: 3000, | ||
imgUrl: | ||
'https://synapse-interface-worldchain.sanguine-fe.pages.dev/_next/static/media/worldchain.62d1dfd2.svg', | ||
networkName: 'World Chain', | ||
networkUrl: 'https://worldchain-mainnet.g.alchemy.com/public', | ||
nativeCurrency: { | ||
name: 'Ethereum', | ||
symbol: 'ETH', | ||
decimals: 18, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Update RPC fallback URL and verify image URL configuration
The image URL is accessible and properly configured with appropriate cache headers and immutable content settings, indicating it's a stable resource. However, the RPC configuration still needs attention:
- The primary and fallback RPC URLs are identical (
https://worldchain-mainnet.g.alchemy.com/public
), which doesn't provide true failover capability. - Consider using a different RPC provider for the fallback URL to ensure service continuity during provider-specific outages.
🔗 Analysis chain
Configure different primary and fallback RPC URLs for redundancy.
The current configuration uses identical URLs for both primary and fallback RPC endpoints, which defeats the purpose of having a fallback. Consider using a different provider for the fallback URL to ensure reliability during outages.
Also verify if the image URL is pointing to a permanent location, as it appears to reference a specific deployment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the image URL is accessible
curl -I "https://synapse-interface-worldchain.sanguine-fe.pages.dev/_next/static/media/worldchain.62d1dfd2.svg"
Length of output: 1389
export const POL: BridgeableToken = { | ||
addresses: { | ||
[CHAINS.POLYGON.id]: ZeroAddress, | ||
}, | ||
decimals: { | ||
[CHAINS.POLYGON.id]: 18, | ||
}, | ||
symbol: 'MATIC', | ||
name: 'MATIC', | ||
swapableType: 'MATIC', | ||
color: 'blue', | ||
priorityRank: 300, | ||
routeSymbol: 'POL', | ||
imgUrl: | ||
'https://45a97b3d.sanguine-fe.pages.dev/_next/static/media/matic.237cd2b6.svg', | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
MATIC to POL renaming requires additional updates
The renaming from MATIC to POL is not consistently applied across the codebase. While the route symbols have been updated in some files, there are still several references to MATIC that need attention:
- The token symbol and name are still set as "MATIC" in the POL token definition
- Multiple chain-specific references like 'MATIC-137' and 'MATIC-53935' remain unchanged
- No documentation of this rename was found in changelog or markdown files
- Other packages (synapse-interface, synapse-constants, rest-api) have inconsistent implementations with both old and new route symbols
Please ensure:
- All token references are consistently using either MATIC or POL naming
- The change is properly documented in the changelog
- The naming convention is aligned across all packages
🔗 Analysis chain
Verify the impact of MATIC to POL renaming.
While the implementation looks correct, please ensure:
- The renaming from MATIC to POL is documented
- All references to these route symbols in the codebase are updated
Also applies to: 1246-1246
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for MATIC/POL route symbol references
echo "Searching for MATIC route symbol references..."
rg "routeSymbol.*MATIC"
echo "Searching for POL route symbol references..."
rg "routeSymbol.*POL"
echo "Searching for WMATIC route symbol references..."
rg "routeSymbol.*WMATIC"
Length of output: 1289
Script:
#!/bin/bash
# Let's check for any documentation about this change
echo "Checking for documentation mentioning MATIC to POL rename..."
rg -i "matic.*pol|polygon.*pol" -g "*.md" -g "CHANGELOG*"
# Let's also check for any other potential MATIC references that might need updating
echo "Checking for other MATIC references..."
rg "MATIC" -g "*.ts" -g "*.tsx" --no-filename | sort | uniq
Length of output: 2482
Bundle ReportChanges will decrease total bundle size by 3.91MB (-10.96%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
Summary by CodeRabbit
New Features
Bug Fixes