-
Notifications
You must be signed in to change notification settings - Fork 59
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
chore: add data for Base mainnet go live (part one) #372
Conversation
📝 WalkthroughWalkthroughThe pull request introduces several updates to the Hardhat configuration and related files to support the "base_mainnet" network. Key changes include the addition of a custom chain configuration in Changes
Possibly related PRs
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
|
This has been deployed, and produced the following output:
|
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: 0
🧹 Outside diff range and nitpick comments (4)
v1/hardhat.config.ts (2)
50-57
: LGTM! Consider adding an API key for Base mainnet.The addition of the Base mainnet configuration to the
customChains
array is correct and aligns with the PR objectives. The chainId, network name, and URLs are accurate for Base mainnet.Consider adding an API key for Base mainnet in the
apiKey
object above, similar to other networks. This would ensure that the verification process works smoothly for Base mainnet as well. You can add it like this:apiKey: { // ... existing keys base_mainnet: process.env.BASESCAN_API_KEY || "", }Don't forget to add the corresponding environment variable in your
.env
file and deployment process.
50-57
: Summary: Base mainnet support successfully addedThe changes in this file successfully add support for Base mainnet to the Hardhat configuration. This addition enhances the protocol's flexibility by allowing it to interact with and verify contracts on the Base mainnet.
As the project continues to expand support for various networks, consider implementing a more scalable approach to manage network configurations. This could involve:
- Creating a separate configuration file for network details.
- Implementing a function to dynamically load network configurations.
- Using environment variables or a configuration management system for easier deployment across different environments.
These steps would make it easier to add or modify network support in the future without cluttering the main Hardhat configuration file.
v1/lib/address.tools.ts (1)
78-78
: LGTM: Addition of Base mainnet to zetaProtocolMainNetworks arrayThe addition of "base_mainnet" to the zetaProtocolMainNetworks array is consistent with the type declaration change and ensures that all ZetaProtocolMainNetwork values are included.
Consider ordering the array elements alphabetically for better maintainability and consistency. Here's a suggested revision:
export const zetaProtocolMainNetworks: ZetaProtocolMainNetwork[] = [ "base_mainnet", "bsc_mainnet", "btc_mainnet", "eth_mainnet", "polygon_mainnet", "zeta_mainnet", ];This alphabetical ordering would make it easier to locate specific networks and maintain the list as it grows.
v1/package.json (1)
92-92
: Formatting change approved. Consider project-wide consistency.The addition of a trailing comma in the version field is a minor syntactical change that improves consistency with modern JSON styling practices.
To ensure consistency across the project, consider running a JSON formatter on all configuration files. You can use a tool like Prettier with the following command:
npx prettier --write "**/*.json"
This will help maintain a consistent style across all JSON files in the project.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
v1/data/addresses.json
is excluded by!v1/data/**
v1/data/addresses.mainnet.json
is excluded by!v1/data/**
📒 Files selected for processing (4)
- v1/hardhat.config.ts (1 hunks)
- v1/lib/address.tools.ts (1 hunks)
- v1/lib/contracts.constants.ts (1 hunks)
- v1/package.json (2 hunks)
🔇 Additional comments (5)
v1/lib/contracts.constants.ts (1)
15-20
: Verify the salt values forbase_mainnet
The addition of
base_mainnet
toSALT_NUMBERS
aligns with the PR objective of adding support for Base mainnet. However, there are a few points to consider:
- All values for
base_mainnet
are set to "0". Is this intentional for the initial setup?- The comment above
SALT_NUMBERS
mentions that values should be calculated using a get-salt script. Has this script been run forbase_mainnet
?- Other mainnet entries (e.g.,
eth_mainnet
,bsc_mainnet
) have non-zero values. Shouldbase_mainnet
follow a similar pattern?If setting all values to "0" is intentional, consider adding a comment explaining why. Otherwise, please verify if the get-salt script should be run to generate appropriate values.
To check if there's any existing usage of non-zero values for Base mainnet, you can run:
v1/lib/address.tools.ts (2)
Line range hint
1-108
: Summary: Changes successfully implement Base mainnet supportThe changes in this file successfully add support for the Base mainnet to the Zeta Protocol. The implementation is consistent with the existing code structure and achieves the PR objectives.
Key points:
- ZetaProtocolMainNetwork type now includes "base_mainnet".
- zetaProtocolMainNetworks array has been updated accordingly.
These changes enhance the protocol's flexibility and network support as intended. The code is well-structured and maintains consistency with the existing implementation.
70-70
: LGTM: Addition of Base mainnet to ZetaProtocolMainNetwork typeThe addition of "base_mainnet" to the ZetaProtocolMainNetwork type is consistent with the PR objective and follows the existing structure. This change enhances the protocol's flexibility by supporting the Base mainnet.
To ensure consistency across the codebase, please run the following script:
This script will help identify any areas that might need updates due to this change.
✅ Verification successful
Verified: Addition of Base mainnet to ZetaProtocolMainNetwork type
The addition of
"base_mainnet"
to theZetaProtocolMainNetwork
type has been successfully verified. All usages across the codebase are consistent and align with the PR objectives, ensuring enhanced support for the Base mainnet without introducing any conflicts.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of ZetaProtocolMainNetwork type and "base_mainnet" string # Test 1: Search for ZetaProtocolMainNetwork type usage echo "Searching for ZetaProtocolMainNetwork type usage:" rg --type typescript "ZetaProtocolMainNetwork" # Test 2: Search for "base_mainnet" string usage echo "Searching for \"base_mainnet\" string usage:" rg --type typescript "base_mainnet"Length of output: 356
Script:
#!/bin/bash # Description: Verify the usage of ZetaProtocolMainNetwork type and "base_mainnet" string in TypeScript files # Test 1: Search for ZetaProtocolMainNetwork type usage in .ts and .tsx files echo "Searching for ZetaProtocolMainNetwork type usage in TypeScript files:" rg "ZetaProtocolMainNetwork" -g "*.ts" -g "*.tsx" # Test 2: Search for "base_mainnet" string usage in .ts and .tsx files echo "Searching for \"base_mainnet\" string usage in TypeScript files:" rg "base_mainnet" -g "*.ts" -g "*.tsx"Length of output: 943
v1/package.json (2)
93-95
:⚠️ Potential issueConsider moving chokidar to devDependencies.
The addition of chokidar, a file watching library, as a production dependency is unusual. Typically, file watching is a development-time concern rather than a runtime requirement for a smart contract project.
Please clarify the intended use of chokidar in production. If it's only needed for development tasks, consider moving it to the
devDependencies
section:- "dependencies": { - "chokidar": "^4.0.1" - }"devDependencies": { + "chokidar": "^4.0.1", // ... other dev dependencies }
To verify the usage of chokidar in the project, run the following script:
#!/bin/bash # Description: Check for chokidar usage in the project echo "Searching for chokidar usage in the project:" rg --type js --type ts 'require\(["\']chokidar["\']|from\s+["\']chokidar["\']'If chokidar is only used in development scripts or tools, it should be moved to
devDependencies
.
50-50
: Hardhat version update looks good, but verify compatibility.The update from Hardhat 2.17.2 to 2.22.12 is a significant version jump. While this update is likely to bring improvements and bug fixes, it's important to ensure it doesn't introduce any breaking changes to your build process or scripts.
Please run the following script to check for any potential issues:
Description
This PR contains the settings and files needed to deploy a v1 contract stack for the first time on an EVM chain.
New Features
Summary by CodeRabbit
New Features
Chores