-
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
Adding JEWEL support for Metis and ETH #2310
Conversation
WalkthroughThe update brings enhancements across various packages, focusing on Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Deploying sanguine-fe with Cloudflare Pages
|
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.
Review Status
Actionable comments generated: 2
Configuration used: .coderabbit.yaml
Files selected for processing (12)
- packages/synapse-constants/README.md (2 hunks)
- packages/synapse-constants/constants/bridgeMap.ts (2 hunks)
- packages/synapse-constants/constants/tokens/bridgeMap.ts (2 hunks)
- packages/synapse-constants/constants/tokens/bridgeable.ts (1 hunks)
- packages/synapse-constants/data/providers.json (1 hunks)
- packages/synapse-constants/package.json (3 hunks)
- packages/synapse-constants/scripts/generateMaps.js (1 hunks)
- packages/synapse-interface/constants/bridgeMap.ts (2 hunks)
- packages/synapse-interface/constants/tokens/bridgeable.ts (1 hunks)
- packages/widget/package.json (2 hunks)
- packages/widget/src/constants/bridgeMap.ts (2 hunks)
- packages/widget/src/constants/bridgeable.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/widget/package.json
Additional comments: 16
packages/synapse-constants/data/providers.json (1)
- 6-17: The updates to the provider URLs for IDs 137, 250, 81457, and 42161 are noted. Ensure that these new URLs point to trusted and reliable providers to maintain the security and integrity of blockchain interactions.
packages/synapse-constants/package.json (2)
- 3-3: The version update from "1.3.6" to "1.3.61" is appropriate for the changes made in this PR. It indicates a minor, non-breaking enhancement.
- 22-23: The addition of the
maps:generate
script is a valuable enhancement for automating the generation of TypeScript files for bridge maps. Ensure to test this script in various development environments to confirm its reliability and effectiveness.packages/synapse-interface/constants/tokens/bridgeable.ts (1)
- 786-787: The addition of JEWEL token addresses for the METIS and ETH chains aligns with the PR objectives to support JEWEL tokens on these platforms. This change is crucial for enabling JEWEL token swappability and use across different parts of the system. The implementation appears correct and follows the established pattern for token configuration within this file.
packages/synapse-constants/constants/tokens/bridgeable.ts (1)
- 877-878: The addition of METIS and ETH chain support for the JEWEL token aligns with the PR objectives to integrate support for JEWEL tokens on these platforms. This change enables JEWEL tokens to be recognized and swapped on the METIS and ETH chains, enhancing the project's compatibility with newer blockchain assets.
packages/widget/src/constants/bridgeable.ts (2)
- 907-908: The addition of the METIS chain address for the JEWEL token is correctly implemented. This aligns with the PR's objective to support JEWEL tokens on the METIS chain.
- 915-916: The addition of the ETHEREUM chain address for the JEWEL token is correctly implemented. This aligns with the PR's objective to support JEWEL tokens on the ETHEREUM chain.
packages/synapse-constants/constants/bridgeMap.ts (2)
- 24-30: The addition of the 'JEWEL' token entry for chain ID '1' is correctly implemented with appropriate token properties.
- 805-811: The addition of the 'JEWEL' token entry for chain ID '1088' is correctly implemented, maintaining consistency with other entries.
packages/synapse-constants/constants/tokens/bridgeMap.ts (3)
- 805-811: The addition of JEWEL tokens for chain ID 1088 looks correct and aligns with the PR's objectives. Please ensure that the chain ID is accurate and consider if the empty
swappable
array has the intended behavior for these tokens.- 30-30: The addition of JEWEL tokens for chain ID 8217 also appears correct. Similar to the previous comment, please verify the accuracy of the chain ID and the implications of the empty
swappable
array.- 30-30: The updates to nUSD tokens across various chain IDs seem to be correctly implemented, enhancing the token's support and swappability. Please ensure consistency across chain IDs and verify the correctness of the swappable configurations.
packages/synapse-interface/constants/bridgeMap.ts (2)
- 24-30: The addition of the 'JEWEL' token mapping for the first network looks correct and follows the established pattern for token entries in the
BRIDGE_MAP
. Good job maintaining consistency with the structure and fields.- 805-811: The addition of the 'JEWEL' token mapping for the second network is also correctly implemented, following the established conventions within the
BRIDGE_MAP
. Consistency in the structure and fields is well maintained.packages/widget/src/constants/bridgeMap.ts (2)
- 805-811: The addition of the JEWEL token under the '1088' network is correctly implemented with appropriate properties. This aligns with the PR's objectives to support JEWEL tokens on various platforms.
- 21-33: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [8217-8217]
The addition of the JEWEL token under the '8217' network is correctly implemented. It's important to ensure that the network ID '8217' corresponds to the intended blockchain platform as per the PR's objectives.
prettyPrintTS(bridgeMap, 'BRIDGE_MAP', './constants/tokens/bridgeMap.ts') | ||
prettyPrintTS(bridgeMap, 'BRIDGE_MAP', './constants/bridgeMap.ts') |
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.
The calls to prettyPrintTS
for generating TypeScript files for bridgeMap
are a good addition for maintaining the bridge maps. However, it appears that prettyPrintTS
is called twice with the same arguments, which seems redundant. If the intention was to generate TypeScript files in two different locations, please ensure the file paths in the arguments are distinct. Otherwise, consider removing the redundant call.
- prettyPrintTS(bridgeMap, 'BRIDGE_MAP', './constants/bridgeMap.ts')
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
prettyPrintTS(bridgeMap, 'BRIDGE_MAP', './constants/tokens/bridgeMap.ts') | |
prettyPrintTS(bridgeMap, 'BRIDGE_MAP', './constants/bridgeMap.ts') | |
prettyPrintTS(bridgeMap, 'BRIDGE_MAP', './constants/tokens/bridgeMap.ts') |
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.
LGTM with a minor nit
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.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (2)
- packages/synapse-constants/package.json (2 hunks)
- packages/widget/package.json (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/synapse-constants/package.json
- packages/widget/package.json
This PR adds the new JEWEL tokens on ETH and Mainnet to the following packages:
Summary by CodeRabbit
METIS
andETH
chains for theJEWEL
token.synapse-constants
package documentation with new usage instructions and script details.53f2e97068c92e337ced2b549a62663600fc7f25: synapse-interface preview link
b19b31be6996876cbfbaec9a1c96238924b3a9c8: synapse-interface preview link