-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add all CCTP supported chains to domain map #2399
Conversation
WalkthroughThe update introduces support for additional blockchain networks by including new chain IDs for the Avalanche, Base, and Polygon platforms. This enhancement is applied to both mainnet and testnet environments, ensuring broader compatibility and integration capabilities. The modifications are neatly encapsulated in the domain mapping configurations, facilitating seamless interactions across diverse blockchain landscapes. 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 (
|
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: 1
Review Status
Configuration used: .coderabbit.yaml
Files selected for processing (1)
- services/cctp-relayer/relayer/chains.go (1 hunks)
Additional Context Used
Additional comments not posted (5)
services/cctp-relayer/relayer/chains.go (5)
9-13
: The addition of new chain IDs for Avalanche (43114
), Base (8453
), and Polygon (137
) networks in the mainnet configurations is correct and aligns with the PR objectives. Ensure these IDs are accurate and up-to-date with the respective blockchain documentation.
17-21
: The addition of new chain IDs for Avalanche Fuji (43113
), Base Sepolia (84532
), and Polygon Amoy (80002
) networks in the testnet configurations is correct and aligns with the PR objectives. Ensure these IDs are accurate and up-to-date with the respective blockchain documentation.
26-30
: The addition of new mappings in themainnetDomainMap
for Avalanche, Base, and Polygon networks is correct. However, ensure that the domain numbers (e.g.,1
,6
,7
) are correctly assigned and do not conflict with existing mappings or future expansions.
36-40
: The addition of new mappings in thetestnetDomainMap
for Avalanche Fuji, Base Sepolia, and Polygon Amoy networks is correct. Similar to the mainnet mappings, ensure that the domain numbers (e.g.,1
,6
,7
) are correctly assigned and do not conflict with existing mappings or future expansions.
6-43
: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [47-107]
Overall, the functions related to domain and chain ID mappings (
IsTestnetChainID
,CircleDomainToChainID
,ChainIDToCircleDomain
,initReverseMaps
) are implemented correctly and follow logical patterns consistent with the objectives of the PR. However, ensure that the initialization oftestnetChainIDMap
andmainnetChainIDMap
ininitReverseMaps
is thread-safe if concurrent access is expected. Consider using a sync.Once or similar mechanism to guarantee that the initialization is performed exactly once in a thread-safe manner.
|
||
// Mainnet chain IDs. | ||
const ethereumChainID = 1 | ||
const avalancheChainID = 43114 | ||
const optimismChainID = 10 | ||
const arbitrumChainID = 42161 | ||
const baseChainID = 8453 | ||
const polygonChainID = 137 | ||
|
||
// Testnet chain IDs. | ||
const sepoliaChainID = 11155111 | ||
const avalancheFujiChainID = 43113 | ||
const opSepoliaChainID = 11155420 | ||
const arbitrumSepoliaChainID = 421614 | ||
const baseSepoliaChainID = 84532 | ||
const polygonAmoyChainID = 80002 | ||
|
||
// see https://developers.circle.com/stablecoins/docs/evm-smart-contracts#mainnet-contract-addresses | ||
var mainnetDomainMap = map[uint32]uint32{ | ||
0: ethereumChainID, | ||
1: avalancheChainID, | ||
2: optimismChainID, | ||
3: arbitrumChainID, | ||
6: baseChainID, | ||
7: polygonChainID, | ||
} | ||
|
||
// see https://developers.circle.com/stablecoins/docs/evm-smart-contracts#testnet-contract-addresses | ||
var testnetDomainMap = map[uint32]uint32{ | ||
0: sepoliaChainID, | ||
1: avalancheFujiChainID, | ||
2: opSepoliaChainID, | ||
3: arbitrumSepoliaChainID, | ||
6: baseSepoliaChainID, | ||
7: polygonAmoyChainID, | ||
} | ||
|
||
var testnetChainIDMap map[uint32]uint32 // Maps chainID to domain for testnet |
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.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [5-5]
The TODO comment suggests moving this file to a common location. Consider addressing this as part of a broader refactoring effort to improve code organization and maintainability. It might be beneficial to create a dedicated package for chain-related constants and utilities if there are multiple relayers or services that could benefit from shared access to this information.
Would you like me to open a GitHub issue to track this potential refactoring task?
Deploying sanguine-fe with Cloudflare Pages
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2399 +/- ##
===================================================
- Coverage 46.89062% 46.75013% -0.14049%
===================================================
Files 383 395 +12
Lines 29202 30432 +1230
Branches 83 83
===================================================
+ Hits 13693 14227 +534
- Misses 14049 14678 +629
- Partials 1460 1527 +67
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