feat(rest-api): Chain and token updates #3319
Conversation
WalkthroughThe changes in this pull request involve updates to token definitions and constants across several files in the REST API package. New entries for the token 'WLD' have been added to 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
|
Deploying sanguine-fe with
|
| Latest commit: |
ac66470
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://38708c33.sanguine-fe.pages.dev |
| Branch Preview URL: | https://rest-api-worldchain.sanguine-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
packages/rest-api/src/tests/tokenListRoute.test.ts (1)
17-17: LGTM. Consider adding specific tests for new tokens.The update to the expected length of keys is consistent with the PR objectives and the AI-generated summary, which mentions the addition of new token entries.
To ensure comprehensive test coverage, consider adding specific tests for the newly added tokens. This could include:
- Verifying the presence of the new 'WLD' token in the response.
- Checking the correct addresses for 'WLD' across different networks.
- Ensuring that the existing token checks (like for 'ETH' and 'SYN') are still valid and haven't been affected by the changes.
packages/rest-api/src/constants/bridgeable.ts (1)
Issues Found: Remaining "WMATIC" References
Residual "WMATIC" references detected in the following files, indicating incomplete renaming to "WPOL":
packages/rest-api/src/constants/bridgeable.tspackages/synapse-constants/src/constants/bridgeMap.tspackages/synapse-constants/src/constants/tokens/bridgeable.tspackages/synapse-interface/__tests__/__fixtures__/mockRoutes.tspackages/synapse-interface/__tests__/getFromTokens.test.tspackages/synapse-interface/__tests__/getToTokens.test.tspackages/synapse-interface/constants/tokens/bridgeable.tspackages/widget/src/constants/bridgeMap.tspackages/widget/src/constants/bridgeable.tspackages/widget/src/index.tsxpackages/widget/src/types/index.d.tsPlease update these references to ensure consistency and prevent potential issues.
🔗 Analysis chain
Line range hint
1236-1250: LGTM: WMATIC renamed to WPOLThe WMATIC token has been consistently renamed to WPOL, with the
routeSymbolupdated accordingly. This change aligns with the earlier MATIC to POL modification.To ensure consistency, please run the following script to check for any remaining references to WMATIC:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash echo "Searching for remaining WMATIC references:" grep -R --exclude-dir={node_modules,build,dist} "WMATIC" . echo "Searching for new WPOL references:" grep -R --exclude-dir={node_modules,build,dist} "WPOL" .Length of output: 31206
Script:
#!/bin/bash echo "Searching for remaining WMATIC references:" grep -R --exclude-dir=node_modules --exclude-dir=build --exclude-dir=dist "WMATIC" . echo "Searching for new WPOL references:" grep -R --exclude-dir=node_modules --exclude-dir=build --exclude-dir=dist "WPOL" .Length of output: 2428
packages/rest-api/src/constants/bridgeMap.ts (2)
31-37: LGTM. Consider adding a comment for the new WLD token.The addition of the WLD token for network '1' looks good. It follows the established pattern for other tokens in this file.
Consider adding a brief comment above this entry to explain what WLD represents (e.g., "// Worldcoin token") for better code readability and maintenance.
Line range hint
934-975: LGTM. Consider updating Metis entries for consistency.The addition of Metis tokens (WMetis and Metis) for network '1088' looks good and follows the established pattern for wrapped and unwrapped token pairs.
For consistency with other token pairs in this file, consider updating the
originanddestinationarrays for both tokens to include both versions. For example:- origin: ['Metis'], - destination: ['Metis'], + origin: ['Metis', 'WMetis'], + destination: ['Metis', 'WMetis'],This change would make these entries consistent with how other wrapped/unwrapped token pairs are handled in the file.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- packages/rest-api/src/constants/bridgeMap.ts (9 hunks)
- packages/rest-api/src/constants/bridgeable.ts (6 hunks)
- packages/rest-api/src/constants/chains.ts (2 hunks)
- packages/rest-api/src/tests/destinationTokensRoute.test.ts (1 hunks)
- packages/rest-api/src/tests/indexRoute.test.ts (1 hunks)
- packages/rest-api/src/tests/tokenListRoute.test.ts (1 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/rest-api/src/tests/indexRoute.test.ts (4)
Learnt from: abtestingalpha PR: synapsecns/sanguine#3126 File: packages/rest-api/src/tests/indexRoute.test.ts:21-31 Timestamp: 2024-10-08T15:41:49.333Z Learning: In this project, tests intentionally use hardcoded values for counts to capture changes in the number of available chains and tokens.Learnt from: abtestingalpha PR: synapsecns/sanguine#3126 File: packages/rest-api/src/tests/indexRoute.test.ts:21-31 Timestamp: 2024-09-16T17:19:34.740Z Learning: In this project, tests intentionally use hardcoded values for counts to capture changes in the number of available chains and tokens.Learnt from: abtestingalpha PR: synapsecns/sanguine#3126 File: packages/rest-api/src/tests/indexRoute.test.ts:21-31 Timestamp: 2024-10-08T15:41:48.274Z Learning: In this project, tests intentionally use hardcoded values for counts to capture changes in the number of available chains and tokens.Learnt from: abtestingalpha PR: synapsecns/sanguine#3126 File: packages/rest-api/src/tests/indexRoute.test.ts:21-31 Timestamp: 2024-10-08T15:41:49.333Z Learning: In this project, tests intentionally use hardcoded values for counts to capture changes in the number of available chains and tokens.
🔇 Additional comments (13)
packages/rest-api/src/tests/destinationTokensRoute.test.ts (1)
64-64: LGTM. Consider adding a comment and reviewing other test cases.The update to expect 6 destination tokens instead of 5 aligns with the PR objectives and the changes mentioned in the AI-generated summary. This likely reflects the addition of a new token (possibly WLD) as a valid destination.
Consider adding a comment explaining the reason for this increase, e.g.:
// Expecting 6 tokens after the addition of WLD as a valid destination expect(response.body.length).toBe(6)Please verify if similar updates are needed in other test cases within this file or related test files. You can use the following script to check for other occurrences:
✅ Verification successful
Verified: No additional test cases require updates.
The only test case affected by the change has been reviewed, and no other similar assertions were found needing modification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other test cases that might need updating # Test: Search for similar assertions in test files rg -A 5 'expect\(response\.body\.length\)\.toBe\(' packages/rest-api/src/tests/Length of output: 646
packages/rest-api/src/constants/chains.ts (2)
513-513: LGTM: World Chain added to CHAINS objectThe addition of
WORLDCHAINto theCHAINSobject is correct and consistent with other chain entries.To ensure proper integration, run the following script:
✅ Verification successful
LGTM:
WORLDCHAINproperly integrated intoCHAINS,CHAINS_ARRAY, andCHAINS_BY_IDThe addition of
WORLDCHAINis correctly integrated into theCHAINSobject and is properly reflected in bothCHAINS_ARRAYandCHAINS_BY_ID.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify WORLDCHAIN integration in CHAINS, CHAINS_ARRAY, and CHAINS_BY_ID # Test: Check if WORLDCHAIN is properly exported in the CHAINS object grep -n "WORLDCHAIN," packages/rest-api/src/constants/chains.ts # Test: Verify CHAINS_ARRAY includes WORLDCHAIN tail -n 10 packages/rest-api/src/constants/chains.ts | grep "CHAINS_ARRAY" # Test: Confirm CHAINS_BY_ID is updated with WORLDCHAIN tail -n 10 packages/rest-api/src/constants/chains.ts | grep "CHAINS_BY_ID"Length of output: 331
469-488: Verify World Chain details and consider separate fallback RPCThe
WORLDCHAINconstant looks good overall, but there are a couple of points to consider:
- The primary and fallback RPC URLs are identical. It's generally recommended to have a different fallback URL to improve reliability.
- Please verify that all the details (explorer URL, block time, etc.) are correct for the World Chain mainnet.
To ensure the accuracy of the World Chain details, you can run the following script:
packages/rest-api/src/constants/bridgeable.ts (4)
810-810: LGTM: ETH token definition updated for WORLDCHAINThe ETH token definition has been correctly updated to include WORLDCHAIN, using the standard NativeTokenAddress and 18 decimals.
Also applies to: 823-823
1000-1000: LGTM: USDCe token definition updated for WORLDCHAINThe USDCe token definition has been correctly updated to include WORLDCHAIN, using a specific address and the standard 6 decimals for USDC tokens.
Also applies to: 1007-1008
1414-1433: LGTM: WLD token added with WORLDCHAIN supportThe new WLD (Worldcoin) token has been added with appropriate definitions for ETHEREUM, OPTIMISM, and WORLDCHAIN. The structure is consistent with other token definitions.
Please confirm that the priority rank of 106 is intentional, as this places WLD quite high in the token list. You may want to run the following script to compare with other token priorities:
#!/bin/bash echo "Listing all token priority ranks:" grep -R --exclude-dir={node_modules,build,dist} "priorityRank:" . | sort -n -k2
1219-1234: Verify implications of MATIC to POL token changeThe MATIC token definition has been replaced with a new POL token definition. While it retains the MATIC symbol and properties, this change could have significant implications:
- Existing integrations might need updates to use the new token name.
- User-facing documentation may need to be updated to reflect this change.
- Ensure that this change is consistent with any broader rebranding or restructuring efforts.
Please run the following script to check for any remaining references to the old MATIC token:
packages/rest-api/src/constants/bridgeMap.ts (6)
465-471: LGTM. WLD token addition for network '10' is consistent.The addition of the WLD token for network '10' is consistent with the previous WLD token entry. It maintains the same structure and properties, which is good for consistency across networks.
1233-1239: LGTM. Clarify naming convention for Metis token.The addition of the Metis token for network '8217' looks good and follows the established structure.
Could you please clarify why the origin and destination for Metis are set to ['Metis'] instead of ['RFQ.Metis']? This differs from the naming convention used for the WLD token. Is this intentional, or should it be updated for consistency?
Line range hint
1-2095: Overall LGTM. Please address the following points:
- Verify the addition of network '480' and ensure all necessary tokens are included.
- Clarify the naming convention for Metis tokens (using 'Metis' vs 'RFQ.Metis').
- Confirm the symbol changes from 'WMATIC' to 'WPOL' and 'MATIC' to 'POL' are intentional and consistent throughout the codebase.
- Consider updating the Metis token entries for network '1088' to include both 'Metis' and 'WMetis' in their origin and destination arrays for consistency.
- Add comments for new token entries (e.g., WLD) to improve code readability and maintenance.
These changes significantly expand the supported tokens and networks in the BRIDGE_MAP. Please ensure that these updates are well-documented and communicated to the team, as they may have implications for other parts of the system that interact with these tokens and networks.
687-687: Verify the symbol change from 'MATIC' to 'POL'.The symbol change from 'MATIC' to 'POL' for the token at address '0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE' on network '137' has been noted. This change appears to be related to the previous change from 'WMATIC' to 'WPOL'.
Please confirm that this change is intentional and consistent with the previous change and other parts of the codebase. Run the following script to check for any other occurrences of 'MATIC' that might need to be updated:
#!/bin/bash # Description: Check for occurrences of 'MATIC' in the codebase echo "Searching for 'MATIC' in the codebase:" rg --type-add 'code:*.{js,ts,jsx,tsx}' -t code 'MATIC' echo "Checking for any TODO comments related to 'MATIC' or 'POL':" rg --type-add 'code:*.{js,ts,jsx,tsx}' -t code 'TODO.*(MATIC|POL)'Additionally, please ensure that these changes are documented and communicated to the team, as they may have implications for other parts of the system that interact with these tokens.
889-911: LGTM. Verify the addition of new network '480'.The addition of the WLD token for network '480' is consistent with previous WLD token entries. However, this is part of a new network being added to the BRIDGE_MAP.
Please confirm that network '480' is intended to be added and that all necessary tokens for this network have been included. Run the following script to check for any references to this network in other parts of the codebase:
✅ Verification successful
Verified the addition of network '480'. No issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for references to network '480' in the codebase echo "Searching for references to network '480':" rg --type-add 'code:*.{js,ts,jsx,tsx}' -t code '(480|0x2cFc85d8E48F8EAB294be644d9E25C3030863003)' echo "Checking for any TODO comments related to network '480':" rg --type-add 'code:*.{js,ts,jsx,tsx}' -t code 'TODO.*480'Length of output: 20160
608-608: Verify the symbol change from 'WMATIC' to 'WPOL'.The symbol change from 'WMATIC' to 'WPOL' for the token at address '0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270' on network '137' has been noted.
Please confirm that this change is intentional and consistent with other parts of the codebase. Run the following script to check for any other occurrences of 'WMATIC' that might need to be updated:
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3319 +/- ##
===================================================
+ Coverage 26.98386% 31.73912% +4.75526%
===================================================
Files 178 427 +249
Lines 11833 28457 +16624
Branches 82 82
===================================================
+ Hits 3193 9032 +5839
- Misses 8357 18581 +10224
- Partials 283 844 +561
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
Bug Fixes
Tests