chore: refactor ChainAddresses and add new network addresses
#84
Conversation
Simplify the `ChainAddresses` struct by making mandatory fields non-optional and adding support for new networks like Worldchain and Astrochain Sepolia. This change ensures consistency in the data structure and simplifies usage across the codebase.
WalkthroughThe changes introduce several modifications across multiple files, primarily focusing on the Changes
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
src/chains.rs (1)
40-40: LGTM: SUPPORTED_CHAINS updated correctlyThe
SUPPORTED_CHAINSconstant has been correctly updated to include the new chain IDs, and the array size has been adjusted from 21 to 24. This change is consistent with the enum modifications and aligns with the PR objectives.For improved maintainability, consider using a macro or const function to automatically determine the array size based on the number of enum variants. This would eliminate the need to manually update the array size when adding or removing chain IDs in the future.
Also applies to: 62-64
src/entities/weth9.rs (2)
42-54: LGTM! Consider grouping chain IDs for better readability.The additions of new chain IDs and their corresponding WETH tokens are consistent with the existing pattern and align with the PR objectives.
For improved readability, consider grouping the chain IDs by network or purpose. For example:
let tokens = FxHashMap::from_iter(vec![ // Ethereum and test networks (1, Self::on_chain(1).unwrap()), (3, Self::on_chain(3).unwrap()), (4, Self::on_chain(4).unwrap()), (5, Self::on_chain(5).unwrap()), (42, Self::on_chain(42).unwrap()), // Layer 2 and sidechains (10, Self::on_chain(10).unwrap()), (42161, Self::on_chain(42161).unwrap()), (421611, Self::on_chain(421611).unwrap()), (421614, Self::on_chain(421614).unwrap()), (8453, Self::on_chain(8453).unwrap()), (324, Self::on_chain(324).unwrap()), // Other networks (56, Self::on_chain(56).unwrap()), (137, Self::on_chain(137).unwrap()), (43114, Self::on_chain(43114).unwrap()), // Newly added networks (11155420, Self::on_chain(11155420).unwrap()), (7777777, Self::on_chain(7777777).unwrap()), (81457, Self::on_chain(81457).unwrap()), (480, Self::on_chain(480).unwrap()), (1301, Self::on_chain(1301).unwrap()), ]);This grouping can make it easier to maintain and understand the different networks supported.
175-188: LGTM! New chain IDs 7777777 and 81457 added correctly. Consider adding comments for clarity.The additions of chain IDs 7777777 (Zora mainnet) and 81457 (Blast Sepolia testnet) are implemented correctly and consistently with other entries. The token addresses, names, and decimals are appropriate for WETH.
For improved clarity, consider adding comments to identify these networks:
// Zora mainnet 7777777 => Some(token!( 7777777, "4200000000000000000000000000000000000006", 18, "WETH", "Wrapped Ether" )), // Blast Sepolia testnet 81457 => Some(token!( 81457, "4300000000000000000000000000000000000004", 18, "WETH", "Wrapped Ether" )),This will help other developers quickly identify which network each chain ID corresponds to.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- src/addresses.rs (6 hunks)
- src/chains.rs (2 hunks)
- src/entities/weth9.rs (4 hunks)
- src/prelude.rs (1 hunks)
🧰 Additional context used
🔇 Additional comments (15)
src/prelude.rs (1)
2-2: LGTM: Addition of addresses module export.The addition of
addresses::*to the public exports aligns with the PR objectives of refactoringChainAddresses. This change makes the addresses module publicly accessible, which should simplify its usage throughout the codebase.However, be cautious with wildcard imports (
*) as they can potentially introduce naming conflicts and make it harder to track which items are being used. Consider explicitly listing the items you want to export if there aren't too many.Let's verify the existence and contents of the
addressesmodule:✅ Verification successful
LGTM: Addition of
addressesmodule export.The inclusion of
addresses::*in the public exports aligns with the PR's objective of refactoringChainAddresses, making theaddressesmodule accessible across the codebase.However, using wildcard imports (
*) can lead to potential naming conflicts and reduce code clarity. If feasible, consider explicitly specifying the exported items to enhance maintainability.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and contents of the addresses module # Test 1: Check if the addresses module file exists fd --type f --glob '*addresses*.rs' src # Test 2: If it exists, display its contents fd --type f --glob '*addresses*.rs' src --exec cat {}Length of output: 26856
src/chains.rs (2)
31-33: LGTM: New chain IDs added correctlyThe new variants ZKSYNC, WORLDCHAIN, and ASTROCHAIN_SEPOLIA have been added to the
ChainIdenum with their respective chain IDs. This aligns with the PR objective of adding support for new networks.
Line range hint
1-64: Verify PR completeness and ZKSYNC additionThe changes in this file align with the PR objective of adding support for new networks, specifically Worldchain and Astrochain Sepolia. However, there are two points to consider:
The PR objectives mention refactoring
ChainAddresses, but there's no evidence of this refactoring in thechains.rsfile. Please ensure that this refactoring is present in other files of this PR or clarify if it's no longer part of the scope.The ZKSYNC chain has been added, which wasn't explicitly mentioned in the PR objectives. While this addition seems beneficial, it would be good to update the PR description to include this change for clarity.
Could you please clarify these points and update the PR description if necessary?
src/entities/weth9.rs (3)
119-125: LGTM! New chain ID 11155420 added correctly.The addition of chain ID 11155420 (OP Sepolia) is implemented correctly and consistently with other entries. The token address, name, and decimals are appropriate for WETH.
140-146: LGTM! New chain ID 421614 added correctly.The addition of chain ID 421614 (Arbitrum Sepolia) is implemented correctly and consistently with other entries. The token address, name, and decimals are appropriate for WETH.
Line range hint
1-231: Overall, the changes look good and align with the PR objectives.The modifications to the
WETH9struct successfully add support for new networks, including Worldchain and Astrochain Sepolia, as well as other relevant networks. The implementations are consistent with existing patterns and follow best practices.A few suggestions have been made to improve readability and clarity:
- Grouping chain IDs in the
newmethod for better organization.- Adding comments to identify networks in the
on_chainmethod.- Verifying the correctness of WETH addresses for new networks.
These minor improvements will enhance the maintainability of the code without affecting its functionality.
src/addresses.rs (9)
12-12: Update to nonfungible_position_manager_address field is acceptableChanging
nonfungible_position_manager_addressfrom anOption<Address>toAddressmakes it a mandatory field, simplifying the struct and ensuring consistency across usage.
15-16: Addition of mixed_route_quoter_v2_address fieldAdding
mixed_route_quoter_v2_addressenhances support for version 2 of the Mixed Route Quoter, allowing for improved functionality.
18-19: Introduction of v4_pool_manager_address and v4_position_manager_address fieldsIncluding
v4_pool_manager_addressandv4_position_manager_addressprepares the struct for upcoming version 4 features, promoting future scalability.
94-95: Added V2_FACTORY_ADDRESS entries for new networksThe inclusion of
(ChainId::WORLDCHAIN as u64, V2_FACTORY_ADDRESS)and(ChainId::ASTROCHAIN_SEPOLIA as u64, V2_FACTORY_ADDRESS)ensures that these new networks are supported in the V2 Factory Addresses map.
135-142: Added V2_ROUTER_ADDRESSES entries for new networksAdding router addresses for
WORLDCHAINandASTROCHAIN_SEPOLIAextends compatibility with these networks, facilitating seamless operations on them.
170-175: Verify addresses for MAINNET and GOERLIPlease ensure that the
mixed_route_quoter_v1_addressvalues forMAINNET_ADDRESSESandGOERLI_ADDRESSESare correct and correspond to the deployed contracts on those networks.
494-500: Update NONFUNGIBLE_POSITION_MANAGER_ADDRESSES mappingWith
nonfungible_position_manager_addressnow being mandatory, ensure that all entries inNONFUNGIBLE_POSITION_MANAGER_ADDRESSESare present and correctly mapped for each supported chain.
355-365:⚠️ Potential issueConfirm correctness of WORLDCHAIN_ADDRESSES
The addresses provided for
WORLDCHAIN_ADDRESSESneed to be verified to ensure they match the actual deployed contract addresses on the WORLDCHAIN network.
366-374:⚠️ Potential issueConfirm correctness of ASTROCHAIN_SEPOLIA_ADDRESSES
Similarly, please verify that the addresses listed under
ASTROCHAIN_SEPOLIA_ADDRESSEScorrespond to the correct contracts on the ASTROCHAIN Sepolia network.
They are useless AI comments which aren't even correct. What's "The address for the Uniswap V2 Factory Address claim contract"? The code is self-explanatory. |
Simplify the
ChainAddressesstruct by making mandatory fields non-optional and adding support for new networks like Worldchain and Astrochain Sepolia. This change ensures consistency in the data structure and simplifies usage across the codebase.Summary by CodeRabbit
New Features
Bug Fixes
Documentation