Deploy/sonic #211
Deploy/sonic #211
Conversation
WalkthroughThis pull request introduces a new network configuration for the "sonic" blockchain across multiple Hardhat configuration files. It updates the Etherscan settings to include a sonic-specific API key and custom chain definition (chainId 146 with dedicated API and browser URLs). Legacy deployment and state configuration files for Arbitrum, Ethereum, and Ganache have been removed, while new files for sonic deployments—along with a TimelockWhitelist entry—are added. Additionally, timelock tasks have been refactored to use a new signer utility with updated task signatures, and new dependencies and minor build script adjustments have been introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant T as Timelock Task (taskWithSigner)
participant G as getSigner
participant W as Wallet
U->>T: Initiate timelock task
T->>G: Request signer using provided SignerArgs
alt Private Key Provided
G->>W: Create wallet from private key
else Keystore Provided
G->>W: Decrypt keystore & create wallet
else
G->>U: Prompt for private key
U->>W: Supply private key, create wallet
end
W-->>T: Return signer instance
T->>W: Sign and execute transaction
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (10)
🪧 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 comments (1)
packages/timelock/tasks/timelock.ts (1)
303-303: 🛠️ Refactor suggestionAdd validation for target address
The target address is set to an empty string which will cause the transaction to fail when executed. Add validation to prevent executing with an empty target.
- const target = ''; // target address pool + const target = ''; // target address pool + if (!target) { + throw new Error('Target address must be configured before running this task'); + }
🧹 Nitpick comments (4)
scripts/build_script.sh (1)
49-53: Consider adding a comment explaining why contest-winner-nft build is disabledThe build steps for the contest-winner-nft package have been commented out. Is this intended to be a temporary or permanent change? Consider adding a brief comment explaining the reason for disabling this part of the build script to improve maintenance clarity.
packages/timelock/tasks/utils.ts (2)
71-105: Added getSigner function for secure credential handlingThis comprehensive function manages signer creation from different credential sources with appropriate warnings about security practices. It includes interactive prompt support when credentials aren't provided directly.
However, there's potential for code duplication. The
readSensitiveDatafunction defined insidegetSignerappears to duplicate functionality frompackages/cli-common/src/sensitive.ts.Consider importing the
readSensitiveDatafunction from the cli-common package instead of redefining it:- const readSensitiveData = async (label: string): Promise<string> => { - const response = await prompts({ - type: 'invisible', - name: 'result', - message: label, - }); - - return response.result as string; - }; + import { readSensitiveData } from '@marginly/cli-common/src/sensitive';
82-84: Consider enhancing private key securityWhile the code warns about using plain text private keys, it still accepts them. For production deployments, this could pose a security risk.
Consider adding options for more secure key handling, such as environment variable support or integration with hardware wallets for production deployments.
packages/timelock/tasks/timelock.ts (1)
294-294: Consistent variable declaration styleThis line uses
constfor signer declaration while other similar lines uselet. Consider using consistent variable declaration styles across tasks.- const signer = await getSigner(taskArgs, provider); + let signer = await getSigner(taskArgs, provider);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (23)
packages/contracts/hardhat.config.ts(3 hunks)packages/deploy/src/data/deploy/arbitrum-v1.5-pendle/config.json(0 hunks)packages/deploy/src/data/deploy/arbitrum-v1.5-pendle/deployment.json(0 hunks)packages/deploy/src/data/deploy/arbitrum-v1.5-pendle/states/2024-04-23.json(0 hunks)packages/deploy/src/data/deploy/arbitrum-v1.5-pendle/states/2024-05-07.json(0 hunks)packages/deploy/src/data/deploy/ethereum-v1.5-pendle/config.json(0 hunks)packages/deploy/src/data/deploy/ethereum-v1.5-pendle/deployment.json(0 hunks)packages/deploy/src/data/deploy/ethereum-v1.5-pendle/states/2024-05-08.json(0 hunks)packages/deploy/src/data/deploy/ganache/config.json(0 hunks)packages/deploy/src/data/deploy/ganache/deployment.json(0 hunks)packages/deploy/src/data/deploy/ganache/states/2023-05-10.json(0 hunks)packages/deploy/src/data/deploy/sonic-v1.5/config.json(1 hunks)packages/deploy/src/data/deploy/sonic-v1.5/deployment.json(1 hunks)packages/deploy/src/data/deploy/sonic-v1.5/states/2025-04-14.json(1 hunks)packages/periphery/hardhat-configs/hardhat.config.ts(2 hunks)packages/router/hardhat.config.ts(3 hunks)packages/timelock/deployment/sonic/deployment-TimelockWhitelist-2025-04-14.json(1 hunks)packages/timelock/hardhat.common.ts(1 hunks)packages/timelock/hardhat.config.ts(1 hunks)packages/timelock/package.json(1 hunks)packages/timelock/tasks/timelock.ts(7 hunks)packages/timelock/tasks/utils.ts(2 hunks)scripts/build_script.sh(1 hunks)
💤 Files with no reviewable changes (10)
- packages/deploy/src/data/deploy/arbitrum-v1.5-pendle/deployment.json
- packages/deploy/src/data/deploy/ganache/config.json
- packages/deploy/src/data/deploy/ethereum-v1.5-pendle/states/2024-05-08.json
- packages/deploy/src/data/deploy/ethereum-v1.5-pendle/deployment.json
- packages/deploy/src/data/deploy/arbitrum-v1.5-pendle/config.json
- packages/deploy/src/data/deploy/ganache/deployment.json
- packages/deploy/src/data/deploy/arbitrum-v1.5-pendle/states/2024-04-23.json
- packages/deploy/src/data/deploy/ethereum-v1.5-pendle/config.json
- packages/deploy/src/data/deploy/ganache/states/2023-05-10.json
- packages/deploy/src/data/deploy/arbitrum-v1.5-pendle/states/2024-05-07.json
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/timelock/tasks/utils.ts (1)
packages/cli-common/src/sensitive.ts (1)
readSensitiveData(3-11)
packages/timelock/tasks/timelock.ts (1)
packages/timelock/tasks/utils.ts (3)
taskWithSigner(53-58)SignerArgs(41-45)getSigner(71-105)
🔇 Additional comments (23)
packages/timelock/hardhat.config.ts (1)
18-20: Network configuration added for Sonic blockchain.The addition of the Sonic network configuration aligns with the PR objective of supporting a new blockchain. The RPC URL is correctly set to the Sonic Labs endpoint.
packages/timelock/package.json (1)
25-26: Dependencies added for improved signer utility.The addition of
promptsand its type definitions is appropriate for enhancing the user interface for signer-related tasks. The version pinning is also properly done to ensure consistency.packages/deploy/src/data/deploy/sonic-v1.5/deployment.json (1)
1-11: New deployment configuration for Sonic network looks good.The deployment configuration correctly defines the Marginly pool and Aave keeper addresses for the Sonic network deployment. The structure follows the project's conventions for deployment configurations.
packages/deploy/src/data/deploy/sonic-v1.5/states/2025-04-14.json (1)
1-32: Deployment state snapshot for Sonic network.The state snapshot comprehensively captures all the necessary contract addresses and transaction hashes for the Sonic network deployment.
I notice that the date in the filename is "2025-04-14" which is in the future. Please verify if this is intentional or if the date should be updated to reflect the actual deployment date.
packages/timelock/deployment/sonic/deployment-TimelockWhitelist-2025-04-14.json (1)
1-7: Deployment record looks goodThe deployment record for the TimelockControllerWhitelist contract on the sonic blockchain follows the expected JSON format with all required fields (address, transaction hash, and block number).
packages/router/hardhat.config.ts (3)
36-38: Network configuration for sonic looks goodThe sonic network configuration is correctly added with the appropriate RPC URL.
47-47: API key configuration looks goodThe etherscan API key for sonic is properly configured to use the SONIC_API_KEY environment variable.
66-73: Custom chain configuration for sonic looks goodThe sonic custom chain is properly configured with the correct chainId and URLs for the API and browser interfaces.
packages/periphery/hardhat-configs/hardhat.config.ts (3)
41-43: Network configuration for sonic looks goodThe sonic network configuration is correctly added with the appropriate RPC URL, consistent with other configuration files in the project.
50-50: API key configuration looks goodThe etherscan API key for sonic is properly configured to use the SONIC_API_KEY environment variable.
61-68: Custom chain configuration for sonic looks goodThe sonic custom chain is properly configured with the correct chainId and URLs for the API and browser interfaces, maintaining consistency with other configuration files.
packages/timelock/hardhat.common.ts (1)
24-38: Hardhat configuration updated to support Sonic network verificationThe change restructures the Etherscan configuration to support verification on the Sonic blockchain. The API key is now organized by network, and a custom chain definition for Sonic (chainId 146) has been added with the appropriate API and browser URLs.
packages/contracts/hardhat.config.ts (3)
50-52: Added Sonic network configurationNetwork configuration for Sonic blockchain has been added with the appropriate RPC URL.
64-64: Added Sonic Etherscan API key configurationThe Etherscan API key configuration now includes a reference to
process.env.SONIC_API_KEYfor the Sonic network.
99-106: Added Sonic custom chain definition for verificationCustom chain configuration for the Sonic network has been added with chainId 146 and appropriate API and browser URLs for contract verification.
packages/deploy/src/data/deploy/sonic-v1.5/config.json (2)
52-61: Verify pool parameters for Sonic deploymentThe pool configuration uses high leverage (20x) and specific fee parameters. Ensure these values are appropriate for the Sonic network's liquidity and volatility characteristics.
1-84:Details
✅ Verification successful
New deployment configuration for Sonic blockchain
This file provides a complete deployment configuration for the Sonic blockchain, including:
- System context with Sonic's RPC URL
- Chain ID verification (146)
- Token definitions for "ws" and "usdc.e"
- Price oracle configuration using Uniswap V3
- Marginly factory settings with specific contract addresses
- Marginly pool configuration for the ws-usdc.e pair
- Adapter configuration for Uniswap V3
- Marginly keeper configuration
However, I recommend verifying that the hardcoded addresses in this configuration are correct for the Sonic network environment:
🏁 Script executed:
#!/bin/bash # Check if the token addresses exist on Sonic network echo "Checking if token addresses exist on Sonic network..." curl -s -X POST https://rpc.soniclabs.com \ -H "Content-Type: application/json" \ -d '{ "jsonrpc":"2.0", "method":"eth_getCode", "params":["0x039e2fb66102314ce7b64ce5ce3e5183bc94ad38", "latest"], "id":1 }' | jq '.result' echo "Checking if factory address exists on Sonic network..." curl -s -X POST https://rpc.soniclabs.com \ -H "Content-Type: application/json" \ -d '{ "jsonrpc":"2.0", "method":"eth_getCode", "params":["0xcb2436774C3e191c85056d248EF4260ce5f27A9D", "latest"], "id":1 }' | jq '.result'Length of output: 55057
Deployment Configuration Verified for Sonic Network
The RPC checks confirm that the “ws” token address (0x039e2fb66102314ce7b64ce5ce3e5183bc94ad38) and the factory address (0xcb2436774C3e191c85056d248EF4260ce5f27A9D) both return substantial contract bytecode from the Sonic network—indicating that these contracts are indeed deployed as expected. The configuration file at
packages/deploy/src/data/deploy/sonic-v1.5/config.jsonis complete and correctly sets up the network parameters and contract addresses.• The “ws” token address shows valid deployed code.
• The factory address for the Uniswap V3 oracle is similarly validated.If necessary, please consider verifying the remaining addresses (e.g., for the “usdc.e” token) with a similar process.
packages/timelock/tasks/utils.ts (2)
41-45: Added SignerArgs interface for credential managementNew interface to standardize how signing credentials are handled across tasks.
53-58: Added taskWithSigner utility functionThis helper function creates tasks with standardized options for providing signer credentials (private key or keystore file).
packages/timelock/tasks/timelock.ts (4)
14-14: Good refactoring to improve security and reusabilityThe change to import dedicated utilities for handling signers improves the codebase's security posture by abstracting authentication methods and encouraging safer practices than raw private keys.
21-25: Improved security through better authentication optionsThe task definition now offers multiple authentication methods (private key, keystore file, or interactive mode) instead of just exposing private keys directly. This is a significant security improvement.
41-44: Well-documented whitelist entriesThe whitelist entries have been updated with appropriate comments explaining the purpose of each address/method pair, which improves maintainability.
22-83:Details
❓ Verification inconclusive
Verify compatibility with existing workflow
The task parameter refactoring from direct private key to multiple authentication methods might break existing CI/CD workflows or scripts that used the old parameter style.
🏁 Script executed:
#!/bin/bash # Check if there are any existing scripts or documentation that use the old parameter style rg "npx hardhat --network .* --config hardhat\.config\.ts deploy-timelock-wl --signer" --type md --type shLength of output: 108
Please Verify CI/CD Workflow Compatibility with Refactored Signer Parameters
- A grep search for legacy task invocations using the old
--signerparameter did not return any results in our repository.- However, the refactoring from a direct private key approach to a multi-method authentication (via
SignerArgs) may still impact external CI/CD scripts or documentation not stored in this codebase.- Action: Manually verify your deployment scripts and documentation (if maintained externally) to ensure that they use the updated authentication method. If any legacy usage is found, please update those workflows accordingly.
| const timelockAddress = ''; | ||
|
|
||
| const timelock = TimelockController__factory.connect(timelockAddress, signer) as any as TimelockController; | ||
| const delay = 259_200; // 3 * 24 * 60 * 60 | ||
| const timelock = TimelockController__factory.connect(timelockAddress, signer) as any as TimelockController; | ||
| const delay = 259_200; // 3 * 24 * 60 * 60 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for hardcoded addresses
The timelockAddress is set to an empty string and should be populated before running this task. Consider adding validation to check for empty addresses before attempting to connect.
- const timelockAddress = '';
+ const timelockAddress = '';
+ if (!timelockAddress) {
+ throw new Error('timelockAddress must be configured before running this task');
+ }📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const timelockAddress = ''; | |
| const timelock = TimelockController__factory.connect(timelockAddress, signer) as any as TimelockController; | |
| const delay = 259_200; // 3 * 24 * 60 * 60 | |
| const timelock = TimelockController__factory.connect(timelockAddress, signer) as any as TimelockController; | |
| const delay = 259_200; // 3 * 24 * 60 * 60 | |
| const timelockAddress = ''; | |
| if (!timelockAddress) { | |
| throw new Error('timelockAddress must be configured before running this task'); | |
| } | |
| const timelock = TimelockController__factory.connect(timelockAddress, signer) as any as TimelockController; | |
| const delay = 259_200; // 3 * 24 * 60 * 60 |
| const ownableContractAddress = ''; | ||
| const ownableContract = Ownable2Step__factory.connect(ownableContractAddress, signer); | ||
| const timelockAddress = ''; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for hardcoded addresses
Both ownableContractAddress and timelockAddress are set to empty strings and should be populated before running this task. Consider adding validation to prevent runtime errors.
- const ownableContractAddress = '';
- const ownableContract = Ownable2Step__factory.connect(ownableContractAddress, signer);
- const timelockAddress = '';
+ const ownableContractAddress = '';
+ const timelockAddress = '';
+ if (!ownableContractAddress || !timelockAddress) {
+ throw new Error('ownableContractAddress and timelockAddress must be configured before running this task');
+ }
+ const ownableContract = Ownable2Step__factory.connect(ownableContractAddress, signer);📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const ownableContractAddress = ''; | |
| const ownableContract = Ownable2Step__factory.connect(ownableContractAddress, signer); | |
| const timelockAddress = ''; | |
| const ownableContractAddress = ''; | |
| const timelockAddress = ''; | |
| if (!ownableContractAddress || !timelockAddress) { | |
| throw new Error('ownableContractAddress and timelockAddress must be configured before running this task'); | |
| } | |
| const ownableContract = Ownable2Step__factory.connect(ownableContractAddress, signer); |
| //npx hardhat --network holesky --config hardhat.config.ts timelock-grant-role --keystore <private-key> | ||
| taskWithSigner('timelock-grant-role').setAction(async (taskArgs: SignerArgs, hre: HardhatRuntimeEnvironment) => { | ||
| const provider = hre.ethers.provider; | ||
|
|
||
| let signer = new hre.ethers.Wallet(taskArgs.signer, provider); | ||
| let signer = await getSigner(taskArgs, provider); | ||
|
|
||
| const cancellers = []; | ||
| const timelockAddress = ''; | ||
| const cancellers = []; | ||
| const timelockAddress = ''; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Template task needs configuration before use
This task contains empty arrays and placeholder addresses that need to be configured before use.
🏁 Script executed:
#!/bin/bash
# Check if this task is used anywhere in the codebase
rg "npx hardhat --network .* --config hardhat\.config\.ts timelock-grant-role" --type mdLength of output: 90
Attention: Configure Template Task Before Use
The timelock-grant-role task defined in packages/timelock/tasks/timelock.ts still contains placeholder configurations that must be updated before deployment. In particular:
- The empty array (
const cancellers = []) and the placeholder string for the timelock address (const timelockAddress = '') should be replaced with valid values. - Confirm that this task is either fully configured for production or clearly marked as a template to avoid accidental usage.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/unit-tests-uniswap.yml (1)
20-23: Update the Node.js setup action version.
Actionlint indicates that usingactions/setup-node@v3may be too old for current GitHub Actions environments. Please consider upgrading to a later version (for example,actions/setup-node@v4if available) to ensure improved compatibility and reliability.🧰 Tools
🪛 actionlint (1.7.4)
21-21: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/unit-tests-contracts.yml (1)
16-20: Upgrade the Node.js setup action version for contracts tests.
The workflow currently usesactions/setup-node@v3with Node.js version 21.1.0. Based on static analysis hints, it is recommended to update the action to a newer version (e.g.,actions/setup-node@v4) to address potential compatibility issues.🧰 Tools
🪛 actionlint (1.7.4)
16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
18-18: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/integration-tests.yml (1)
51-54: Modernize the Node.js setup action.
While updating the Node.js version to 21.1.0 is aligned with other changes, theactions/setup-node@v3action is flagged as outdated. An upgrade to a more recent version (such as@v4) is advisable to meet current GitHub Actions standards.🧰 Tools
🪛 actionlint (1.7.4)
52-52: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/unit-tests-router.yml (1)
16-20: Update the Node.js setup action for router tests.
Similar to the other workflow files, this configuration usesactions/setup-node@v3, which static analysis suggests is too old. Please consider updating to a newer version (e.g.,actions/setup-node@v4) to maintain consistency and ensure compatibility.🧰 Tools
🪛 actionlint (1.7.4)
16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
18-18: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/integration-tests.yml(1 hunks).github/workflows/unit-tests-contracts.yml(1 hunks).github/workflows/unit-tests-router.yml(1 hunks).github/workflows/unit-tests-uniswap.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/unit-tests-contracts.yml
18-18: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/unit-tests-uniswap.yml
21-21: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/integration-tests.yml
52-52: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/unit-tests-router.yml
18-18: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Summary by CodeRabbit
New Features
Enhancements
Chores