-
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
feature(synapse-interface): Chain and bridgeMap updates #3318
Conversation
WalkthroughThe changes in this pull request involve updates to various components and constants within the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
|
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: 5
🧹 Outside diff range and nitpick comments (6)
packages/synapse-interface/constants/chains/extraWagmiChains.ts (1)
79-84
: Add etherscan property to blockExplorers for consistencyFor consistency with other chain configurations, consider adding an
etherscan
property to theblockExplorers
object, even if it points to the same explorer as thedefault
.Example addition:
blockExplorers: { default: { name: 'World Chain Explorer', url: 'https://worldchain-mainnet.explorer.alchemy.com', }, + etherscan: { + name: 'World Chain Explorer', + url: 'https://worldchain-mainnet.explorer.alchemy.com', + }, },packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsx (1)
62-65
: LGTM! Consider extracting date strings as constants.The changes to the
AnnouncementBanner
props accurately reflect the new World Chain support. The updatedbannerId
,bannerContent
, and date range are appropriate for the promotional period.Consider extracting the date strings as constants at the top of the file or in a separate constants file. This would make future updates easier and reduce the risk of typos. For example:
const WORLD_CHAIN_PROMO_START = '2024-10-09T18:45:09+00:00'; const WORLD_CHAIN_PROMO_END = '2024-11-15T18:45:09+00:00'; // Then use these constants in the component startDate={new Date(WORLD_CHAIN_PROMO_START)} endDate={new Date(WORLD_CHAIN_PROMO_END)}packages/synapse-interface/scripts/generateMaps.js (3)
45-47
: LGTM. Consider adding a comment for the new chain ID.The addition of chain ID 480 to
allowedChainIdsForRfq
is correct and aligns with the introduction of the "World Chain" mentioned in the PR objectives.Consider adding a comment next to the new chain ID for better clarity:
const allowedChainIdsForRfq = [ - 1, 10, 56, 480, 8453, 42161, 59144, 81457, 534352, + 1, 10, 56, 480, // World Chain + 8453, 42161, 59144, 81457, 534352, ]
470-474
: LGTM. Consider using a case-insensitive comparison for robustness.The special handling for 'USDC.e' in the
getRFQSymbol
function is a good addition. It ensures consistent naming for RFQ symbols, treating USDC.e as USDC.For improved robustness, consider using a case-insensitive comparison:
const getRFQSymbol = (symbol) => { - if (symbol === 'USDC.e') { + if (symbol.toLowerCase() === 'usdc.e') { return 'RFQ.USDC' } else { return `RFQ.${symbol}` } }This change would handle potential variations like 'USDC.E' or 'usdc.e' consistently.
379-392
: LGTM. Consider applying the USDC.e handling to thedestination
array as well.The refactoring in the
printMaps
function improves code readability and maintains consistency with thegetRFQSymbol
function changes. The special handling for 'RFQ.USDC.e' in theorigin
array is a good addition.For consistency, consider applying the same USDC.e handling to the
destination
array:const destination = await getDestinationBridgeSymbols(chainId, token) + .map((t) => (t === 'RFQ.USDC.e' ? 'RFQ.USDC' : t))
This ensures that 'USDC.e' is handled consistently in both
origin
anddestination
arrays.packages/synapse-interface/constants/bridgeMap.ts (1)
Line range hint
1-1337
: Summary of changes: New network and token updatesThis update to the BRIDGE_MAP includes several significant changes:
- Addition of WLD token to Ethereum mainnet and Optimism.
- Rebranding of MATIC to POL on Polygon network.
- Minor capitalization update for WMetis on Metis Andromeda.
- Addition of a new network (ID: 480) for World Chain.
These changes expand the supported networks and tokens, and update some existing token representations. While the changes appear to be implemented correctly, they may have broader implications on the system.
Consider the following to ensure smooth integration of these changes:
- Update any network-specific logic to handle the new World Chain (ID: 480).
- Review and update any UI components that display token symbols to reflect the MATIC to POL change.
- Ensure that token selection and bridging logic can handle the new WLD token across supported networks.
- Update any documentation or user guides to reflect these new tokens and network additions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/synapse-interface/assets/chains/worldchain.svg
is excluded by!**/*.svg
packages/synapse-interface/assets/icons/wld.svg
is excluded by!**/*.svg
📒 Files selected for processing (9)
- packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsx (1 hunks)
- packages/synapse-interface/constants/bridgeMap.ts (6 hunks)
- packages/synapse-interface/constants/chains/extraWagmiChains.ts (1 hunks)
- packages/synapse-interface/constants/chains/index.tsx (1 hunks)
- packages/synapse-interface/constants/chains/master.tsx (3 hunks)
- packages/synapse-interface/constants/chains/supportedChains.ts (1 hunks)
- packages/synapse-interface/constants/tokens/bridgeable.ts (6 hunks)
- packages/synapse-interface/scripts/data/providers.json (1 hunks)
- packages/synapse-interface/scripts/generateMaps.js (3 hunks)
🧰 Additional context used
🔇 Additional comments (18)
packages/synapse-interface/constants/chains/supportedChains.ts (2)
24-24
: LGTM: Import statement updated correctly.The import statement has been appropriately updated to include
worldchain
from the extra wagmi chains file. This change aligns with the addition ofworldchain
to thesupportedChains
array.
29-30
: LGTM:supportedChains
array updated withworldchain
.The
worldchain
has been successfully added to thesupportedChains
array, which is consistent with the import statement changes and aligns with the PR objectives.Note that the position of
optimism
in the array has changed. While this doesn't affect functionality, it may impact the order of chains in the UI.To ensure the changes don't have unintended consequences, please verify:
- The order of chains in the UI is as expected.
- Any logic depending on the order of chains in this array is still correct.
You can use the following script to check for any order-dependent logic:
✅ Verification successful
No order-dependent usages of
supportedChains
found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of supportedChains that might depend on order # Test: Look for array index access or slicing of supportedChains rg -p 'supportedChains\[' packages/ rg -p 'supportedChains\.slice' packages/ # Test: Check for any sorting or filtering of supportedChains rg -p 'supportedChains.*sort' packages/ rg -p 'supportedChains.*filter' packages/Length of output: 166
Script:
#!/bin/bash # Description: Enhanced search for order-dependent usage of supportedChains # Search for additional array methods that might affect order rg 'supportedChains\.(map|reduce|forEach|find|every|some)' src/ # Search for usage of supportedChains in UI component rendering rg 'render.*supportedChains' src/ rg 'return\s+\[.*supportedChains.*\]' src/ # Search for any custom sorting or ordering functions applied to supportedChains rg 'supportedChains\s*=\s*[^;]*sort' src/ rg 'supportedChains\s*=\s*[^;]*orderBy' src/Length of output: 462
Script:
#!/bin/bash # Description: Enhanced search for order-dependent usage of supportedChains within the correct directory # Search for additional array methods that might affect order rg 'supportedChains\.(map|reduce|forEach|find|every|some)' packages/ # Search for usage of supportedChains in UI component rendering rg 'render.*supportedChains' packages/ rg 'return\s+\[.*supportedChains.*\]' packages/ # Search for any custom sorting or ordering functions applied to supportedChains rg 'supportedChains\s*=\s*[^;]*sort' packages/ rg 'supportedChains\s*=\s*[^;]*orderBy' packages/Length of output: 394
packages/synapse-interface/constants/chains/index.tsx (2)
Line range hint
1-62
: Approve changes in exports and function usage.The updates to
CHAIN_IDS
andCHAINS_BY_ID
exports, usinggetids()
andgetChainsByID()
respectively, introduce more dynamic generation of chain-related data. This approach enhances maintainability and allows for automatic updates when new chains are added.Let's verify the implementation of the new functions:
#!/bin/bash # Description: Verify the implementation of getids and getChainsByID functions. # Test 1: Check the implementation of getids function echo "Checking getids function:" rg -n 'const getids = \(\) => \{[\s\S]*?\}' --type ts --type tsx # Test 2: Check the implementation of getChainsByID function echo "Checking getChainsByID function:" rg -n 'const getChainsByID = \(\): ChainsByChainID => \{[\s\S]*?\}' --type ts --type tsx # Test 3: Verify that CHAIN_IDS and CHAINS_BY_ID are exported correctly echo "Verifying exports:" rg -n 'export const CHAIN_IDS = getids\(\)' --type ts --type tsx rg -n 'export const CHAINS_BY_ID = getChainsByID\(\)' --type ts --type tsx
45-45
: LGTM: WORLDCHAIN added correctly.The addition of WORLDCHAIN with ID 480 to the ChainId object is consistent with the existing structure.
Let's verify the uniqueness of this chain ID across the codebase:
packages/synapse-interface/constants/chains/extraWagmiChains.ts (1)
71-75
: Verify the native currency for World ChainThe native currency is currently set to Ether (ETH) with the symbol "ETH". This is unusual for a chain named "World Chain". Please verify if this is correct or if it should be a different currency specific to the World Chain.
packages/synapse-interface/constants/chains/master.tsx (4)
23-23
: LGTM: New import for World Chain image.The import statement for
worldchainImg
is consistent with the existing import pattern for chain images.
165-165
: Please clarify the rationale for changing OPTIMISM's priorityRank.The
priorityRank
for OPTIMISM has been increased from 80 to 91. Could you explain the reasoning behind this specific value and how it relates to other chains' priorities?
591-591
: LGTM: Updated LINEA's 'isNew' status.Changing
isNew
tofalse
for LINEA is appropriate if it's no longer considered a new addition to the system.
594-618
: LGTM: Addition of WORLDCHAIN, but please clarify priorityRank.The addition of WORLDCHAIN is well-structured and consistent with other chain definitions. However, could you explain the rationale behind setting its
priorityRank
to 99, which is higher than most other chains?Also, ensure that the following are correct:
- Chain ID: 480
- RPC URLs and explorer URL (they use alchemy.com)
- Layer 2 classification
packages/synapse-interface/scripts/generateMaps.js (1)
Line range hint
1-474
: Overall, the changes look good and align with the PR objectives.The modifications effectively introduce support for the new "World Chain" (chain ID 480) and improve the handling of USDC.e tokens. The changes are well-integrated with the existing code and maintain consistency across the file. The refactoring in the
printMaps
function enhances code readability.A few minor suggestions have been made to further improve clarity and consistency:
- Adding a comment for the new chain ID in
allowedChainIdsForRfq
.- Using case-insensitive comparison in the
getRFQSymbol
function.- Applying the USDC.e handling to the
destination
array in theprintMaps
function.These suggestions are optional and do not affect the overall functionality of the changes.
packages/synapse-interface/constants/tokens/bridgeable.ts (4)
45-45
: Import statement added for new token logo.The addition of the
wldLogo
import is consistent with the introduction of the new WLD token.
715-715
: ETH token definition updated for WORLDCHAIN.The addition of the WORLDCHAIN address (using
zeroAddress
) for ETH is correct and consistent with supporting the new chain.
864-864
: USDCe token definition updated for WORLDCHAIN.The addition of the WORLDCHAIN address for USDCe is correct and consistent with supporting the new chain.
1252-1271
: New WLD (Worldcoin) token added.The addition of the WLD token with addresses for ETH, OPTIMISM, and WORLDCHAIN is consistent with the project's objectives. The token properties are correctly defined, following the established pattern.
packages/synapse-interface/constants/bridgeMap.ts (4)
31-37
: LGTM: New WLD token added correctlyThe addition of the WLD token for network '1' (Ethereum mainnet) is implemented correctly. The token details, including the address, decimals, and RFQ usage, are consistent with the structure used for other tokens in the file.
465-471
: LGTM: WLD token added consistently for OptimismThe WLD token has been added to network '10' (Optimism) with consistent implementation compared to the Ethereum mainnet. The token details, including the network-specific address, are correctly specified.
889-911
: New network '480' (World Chain) added: Verify network IDA new network with ID '480', presumably for the World Chain, has been added to the BRIDGE_MAP. The structure and included tokens (WLD, USDC.e, ETH) are consistent with other network entries.
To ensure the correctness of the new network ID, please run the following verification:
#!/bin/bash # Description: Verify the World Chain network ID and check for any references echo "Searching for references to World Chain or network ID 480:" rg --type-add 'code:*.{ts,tsx,js,jsx}' -t code '(World.*Chain|480)' --glob '!**/constants/bridgeMap.ts' echo "Verify that the network ID 480 is correctly associated with World Chain in other parts of the codebase"
936-938
: Minor capitalization update for WMetis: Check for consistencyThe symbol for the wrapped Metis token has been updated from 'WMETIS' to 'WMetis'. This change in capitalization is likely for branding consistency.
To ensure consistency across the codebase, please run the following verification:
✅ Verification successful
Consistent Capitalization of WMetis Confirmed
All references to the wrapped Metis token have been updated to 'WMetis' consistently across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistency in Metis token naming echo "Searching for 'METIS' references in the codebase:" rg --type-add 'code:*.{ts,tsx,js,jsx}' -t code 'METIS' --glob '!**/constants/bridgeMap.ts' echo "Searching for 'Metis' references in the codebase:" rg --type-add 'code:*.{ts,tsx,js,jsx}' -t code 'Metis' --glob '!**/constants/bridgeMap.ts' echo "Verify that all references to the Metis token use consistent capitalization"Length of output: 19969
Script:
#!/bin/bash # Description: Check for consistency in Metis token naming echo "Searching for 'WMETIS' references in the codebase:" rg --type-add 'code:*.{ts,tsx,js,jsx}' 'WMETIS' --glob '!**/constants/bridgeMap.ts' echo "Searching for 'WMetis' references in the codebase:" rg --type-add 'code:*.{ts,tsx,js,jsx}' 'WMetis' --glob '!**/constants/bridgeMap.ts' echo "Verify that all references to the Metis token use consistent capitalization"Length of output: 682
packages/synapse-interface/constants/chains/extraWagmiChains.ts
Outdated
Show resolved
Hide resolved
packages/synapse-interface/constants/chains/extraWagmiChains.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3318 +/- ##
===================================================
- Coverage 31.92469% 26.98386% -4.94083%
===================================================
Files 238 178 -60
Lines 14553 11833 -2720
Branches 356 82 -274
===================================================
- Hits 4646 3193 -1453
+ Misses 9614 8357 -1257
+ Partials 293 283 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Bundle ReportChanges will decrease total bundle size by 3.9MB (-10.93%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
Deploying sanguine-fe with Cloudflare Pages
|
Summary by CodeRabbit
New Features
Bug Fixes
Chores