vault deploy jobs #107
vault deploy jobs #107
Conversation
WalkthroughA new GitHub Actions workflow Changes
Possibly related PRs
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: 6
🧹 Outside diff range and nitpick comments (7)
script/VaultGovernanceDeploy.s.sol (1)
5-12: Add NatSpec documentation and consider emitting events.The interface would benefit from:
- NatSpec documentation explaining the purpose of each parameter
- Consider adding events to track safe deployments
Here's a suggested improvement:
interface TermVaultGovernanceFactory { + /// @notice Deploys a new governance safe + /// @param proposer Address authorized to propose governance actions + /// @param vault Address of the vault being governed + /// @param accountant Address of the accountant contract + /// @param governor Address with governor privileges + /// @dev Emits SafeDeployed event upon successful deployment + event SafeDeployed( + address indexed proposer, + address indexed vault, + address indexed governor, + address accountant + ); function deploySafe( address proposer, address vault, address accountant, address governor ) external; }.github/workflows/deploy-sepolia-vault-governance.yaml (2)
29-31: Consider using a more relevant environment URLThe current URL points to documentation. Consider using a more relevant URL such as the Sepolia block explorer URL for the deployed contracts or the vault dashboard.
🧰 Tools
🪛 yamllint (1.29.0-1)
[error] 29-29: trailing spaces
(trailing-spaces)
52-54: Fix file formattingRemove extra blank lines and ensure there's exactly one newline at the end of the file.
🧰 Tools
🪛 yamllint (1.29.0-1)
[warning] 53-53: too many blank lines
(3 > 2) (empty-lines)
[error] 54-54: no new line character at the end of file
(new-line-at-end-of-file)
[error] 54-54: trailing spaces
(trailing-spaces)
.github/workflows/deploy-sepolia-vault.yaml (1)
Line range hint
1-77: Consider documenting the new vault deployment architectureThese workflow changes represent a significant shift in the vault deployment architecture, splitting governance deployment into a separate workflow. Consider:
Adding workflow documentation explaining:
- The relationship between vault and governance deployments
- The order of operations
- Required prerequisites
Creating a deployment guide covering:
- When to use each workflow
- How to configure the new parameters
- Common deployment scenarios
Would you like me to help draft this documentation?
script/DeployGovernance.s.sol (3)
Line range hint
18-107: Consider optimizing the address parsing logicThe address parsing implementation is correct but could be more gas-efficient. Consider using assembly for the hex string to address conversion.
Here's a more efficient implementation:
function parseAddress(string memory _str) internal pure returns (address) { bytes memory tmp = bytes(_str); require(tmp.length == 42, "Invalid address length"); - uint160 addr = 0; - for (uint256 i = 2; i < 42; i++) { - uint160 b = uint160(uint8(tmp[i])); - if (b >= 48 && b <= 57) { - addr = addr * 16 + (b - 48); - } else if (b >= 65 && b <= 70) { - addr = addr * 16 + (b - 55); - } else if (b >= 97 && b <= 102) { - addr = addr * 16 + (b - 87); - } else { - revert("Invalid address character"); - } - } - return address(addr); + bytes memory addr = new bytes(20); + assembly { + let result := 0 + for { let i := 2 } lt(i, 42) { i := add(i, 1) } { + let char := byte(0, mload(add(add(tmp, 0x20), i))) + switch and(char, 0xf0) + case 0x30 { result := add(mul(result, 16), sub(char, 0x30)) } + case 0x40 { result := add(mul(result, 16), sub(char, 0x37)) } + case 0x60 { result := add(mul(result, 16), sub(char, 0x57)) } + default { revert(0, 0) } + } + mstore(add(addr, 0x20), result) + } + return address(uint160(uint256(keccak256(addr)))); }
Line range hint
112-118: Add input validation for environment variablesThe current implementation assumes all environment variables are present and valid. Consider adding proper validation and error handling.
Here's a suggested implementation:
- TermStrategyGovernanceFactory factory = TermStrategyGovernanceFactory(vm.envAddress("GOVERNANCE_FACTORY")); - address proposer = vm.envAddress("PROPOSER"); - address strategy = vm.envAddress("STRATEGY"); - address governor = vm.envAddress("GOVERNOR"); - address[] memory vaultGovernors = stringToAddressArray(vm.envString("VAULT_GOVERNORS")); - - factory.deploySafe(proposer, strategy, governor, vaultGovernors); + address factoryAddr = vm.envAddress("GOVERNANCE_FACTORY"); + require(factoryAddr != address(0), "Invalid factory address"); + TermStrategyGovernanceFactory factory = TermStrategyGovernanceFactory(factoryAddr); + + address proposer = vm.envAddress("PROPOSER"); + require(proposer != address(0), "Invalid proposer address"); + + address strategy = vm.envAddress("STRATEGY"); + require(strategy != address(0), "Invalid strategy address"); + + address governor = vm.envAddress("GOVERNOR"); + require(governor != address(0), "Invalid governor address"); + + string memory vaultGovernorsStr = vm.envString("VAULT_GOVERNORS"); + require(bytes(vaultGovernorsStr).length > 0, "Empty vault governors list"); + address[] memory vaultGovernors = stringToAddressArray(vaultGovernorsStr); + require(vaultGovernors.length > 0, "No vault governors provided"); + + try factory.deploySafe(proposer, strategy, governor, vaultGovernors) { + console.log("Successfully deployed governance safe"); + } catch Error(string memory reason) { + revert(string.concat("Failed to deploy governance safe: ", reason)); + } catch { + revert("Failed to deploy governance safe: unknown error"); + }
Inconsistent governance factory interfaces detected
The codebase currently has two different governance factory interfaces:
TermVaultGovernanceFactoryinscript/VaultGovernanceDeploy.s.solTermStrategyGovernanceFactoryinscript/DeployGovernance.s.solThis suggests that
VaultGovernanceDeploy.s.solis an old deployment script that should be removed or updated to use the newTermStrategyGovernanceFactoryinterface to maintain consistency.🔗 Analysis chain
Line range hint
6-12: Verify the interface rename impact across the codebaseThe interface rename from
TermVaultGovernanceFactorytoTermStrategyGovernanceFactorysuggests a broader architectural change. Let's ensure this change is consistently applied.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the old interface name rg "TermVaultGovernanceFactory" # Search for implementations of the new interface ast-grep --pattern 'contract $_ is TermStrategyGovernanceFactory'Length of output: 318
Script:
#!/bin/bash # Let's check if VaultGovernanceDeploy.s.sol is still in use or if it's an old file git status script/VaultGovernanceDeploy.s.sol # Let's also check for any implementations or references to TermStrategyGovernanceFactory rg "TermStrategyGovernanceFactory" -A 2 # Let's verify if there are any other governance factory related files fd ".*GovernanceFactory.*" --type fLength of output: 688
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
.github/workflows/deploy-sepolia-vault-governance.yaml(1 hunks).github/workflows/deploy-sepolia-vault.yaml(2 hunks)script/DeployGovernance.s.sol(2 hunks)script/VaultGovernanceDeploy.s.sol(1 hunks)script/VaultManagement.s.sol(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.3)
.github/workflows/deploy-sepolia-vault-governance.yaml
41-41: shellcheck reported issue in this script: SC2086:info:1:81: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.29.0-1)
.github/workflows/deploy-sepolia-vault-governance.yaml
[error] 25-25: trailing spaces
(trailing-spaces)
[error] 29-29: trailing spaces
(trailing-spaces)
[warning] 43-43: too many spaces after colon
(colons)
[warning] 53-53: too many blank lines
(3 > 2) (empty-lines)
[error] 54-54: no new line character at the end of file
(new-line-at-end-of-file)
[error] 54-54: trailing spaces
(trailing-spaces)
🔇 Additional comments (11)
script/VaultManagement.s.sol (7)
17-17: Usage of 'deployer' address derived from private key
Defining deployer as vm.addr(deployerPK); correctly derives the deployer's address from the private key, ensuring that transactions are signed by the appropriate account.
28-28: Ensure the 'VAULT_GOVERNANCE_FACTORY' environment variable is set
The script retrieves vaultGovernanceFactory from the environment variable VAULT_GOVERNANCE_FACTORY. Please verify that this environment variable is correctly set in your deployment environment to prevent any runtime errors.
31-31: Update in 'deploy_new_vault' function parameters
The deploy_new_vault function now uses deployer instead of roleManager:
-address vaultAddress = vaultFactory.deploy_new_vault(asset, name, symbol, roleManager, profitMaxUnlockTime);
+address vaultAddress = vaultFactory.deploy_new_vault(asset, name, symbol, deployer, profitMaxUnlockTime);Ensure that the deployer address has the necessary permissions and is intended to be the role manager for the new vault.
37-37: Creation of Accountant without parameters
The call to accountantFactory.newAccountant(); creates a new Accountant without any constructor parameters. Confirm that the Accountant contract does not require any initialization parameters and that this aligns with the updated contract design.
42-43: Refactoring with modular internal functions
Introducing _setVaultParams and _setAccountantParams improves code organization and enhances readability by encapsulating parameter configurations.
48-70: Review of '_setVaultParams' function implementation
-
Hardcoded role identifier
112vault.set_role(keeper, 112);
Consider defining the role identifier
112as a constant or an enumerated type to improve code clarity and maintainability. This practice reduces magic numbers in the code and makes it easier to manage role identifiers. -
Verification of 'KEEPER_ADDRESS' environment variable
The
keeperaddress is retrieved from:address keeper = vm.envAddress("KEEPER_ADDRESS");
Ensure that the
KEEPER_ADDRESSenvironment variable is correctly set to prevent any deployment issues. -
Role manager transfer to 'vaultGovernanceFactory'
The vault's role manager is transferred to
vaultGovernanceFactory:vault.transfer_role_manager(vaultGovernanceFactory); vault.accept_role_manager();
Verify that
vaultGovernanceFactoryis the intended recipient of the role manager privileges and that it has the necessary implementation to manage these roles securely.
72-92:
Review of '_setAccountantParams' function implementation
-
Casting environment variables to
uint16Environment variables are being cast from
uint256touint16:uint16 defaultManagement = uint16(vm.envOr("DEFAULT_MANAGEMENT", uint256(0)));
Ensure that the values of these environment variables do not exceed the maximum value of
uint16(65,535). If they do, it will result in integer overflow and loss of data. -
Verification of fee parameters and recipient
The
Accountantis updated with default fee configurations and a new fee recipient:accountant.updateDefaultConfig(defaultManagement, defaultPerformance, defaultRefund, defaultMaxFee, defaultMaxGain, defaultMaxLoss); accountant.setFeeRecipient(newFeeRecipient);
Please verify:
- The fee parameters align with the intended financial model.
- The
newFeeRecipientaddress is correct and secure. - The
vaultGovernanceFactoryaddress has the appropriate privileges for future fee management.
script/VaultGovernanceDeploy.s.sol (2)
1-3: LGTM! License and version specifications are appropriate.
The MIT license and Solidity version ^0.8.18 are suitable choices for this contract.
14-33: Verify integration with GitHub Actions workflow.
The script appears to be used by the new deploy-sepolia-vault-governance.yaml workflow. Let's verify the parameter alignment.
✅ Verification successful
Environment variables in deployment script align with workflow inputs
The deployment script's environment variables correctly align with the GitHub Actions workflow:
- All required variables (
PROPOSER,VAULT,ACCOUNTANT,GOVERNOR,GOVERNANCE_FACTORY) are properly mapped in the workflow's env section - The workflow inputs match the script's requirements with appropriate descriptions and validation
- Additional necessary variables (
RPC_URL,PRIVATE_KEY) are securely handled via repository secrets
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the environment variables used in the script match the GitHub Actions workflow inputs
# Check workflow file for corresponding environment variables
rg -A 2 'env:.*PRIVATE_KEY|env:.*GOVERNANCE_FACTORY|env:.*PROPOSER|env:.*VAULT|env:.*ACCOUNTANT|env:.*GOVERNOR' .github/workflows/deploy-sepolia-vault-governance.yaml
# Check if there are any other deployment scripts using these environment variables
rg -t sol 'vm\.envAddress\("(GOVERNANCE_FACTORY|PROPOSER|VAULT|ACCOUNTANT|GOVERNOR)"\)' script/
Length of output: 294
Script:
#!/bin/bash
# First, let's check if the workflow file exists and see its content
fd "deploy-sepolia-vault-governance.yaml" .github/workflows/ --exec cat {}
# Then check for any Solidity files in the script directory
fd -e sol . script/ --exec echo "File: {}"
# Finally, let's check for any environment variable usage in GitHub workflows
rg "env\." .github/workflows/
Length of output: 2320
.github/workflows/deploy-sepolia-vault.yaml (2)
70-77: Verify critical environment variables and breaking changes
The environment variable changes introduce new dependencies and remove existing ones:
-
New variables:
VAULT_GOVERNANCE_FACTORYandKEEPERare critical addresses- Ensure these are configured in repository variables
-
Breaking changes:
- Removal of
ROLE_MANAGER,ROLE_NUM, andFEE_MANAGERvariables - Verify if existing deployments or scripts depend on these
- Removal of
Let's verify the configuration and usage:
✅ Verification successful
Environment variable changes are safe and aligned with governance updates
The verification shows that:
-
The new variables
VAULT_GOVERNANCE_FACTORYandKEEPERare properly configured:- Used consistently in
deploy-sepolia-vault.yaml KEEPERaligns with keeper-related workflows (actions-sepolia-vault-keeper.yml,actions-sepolia-strategy-set-keeper.yml)
- Used consistently in
-
Removed variables (
ROLE_MANAGER,ROLE_NUM,FEE_MANAGER):- No references found in any current workflow files
- Changes align with the new governance deployment workflow (
deploy-sepolia-vault-governance.yaml) - Removal appears intentional as part of governance system refactoring
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if VAULT_GOVERNANCE_FACTORY and KEEPER are used consistently
echo "Checking governance factory usage..."
rg "VAULT_GOVERNANCE_FACTORY" .github/workflows/
echo "Checking keeper usage..."
rg "KEEPER" .github/workflows/
echo "Checking for potential breaking changes..."
# Look for removed variables in other workflows or scripts
rg "ROLE_MANAGER|ROLE_NUM|FEE_MANAGER" .
Length of output: 786
Script:
#!/bin/bash
# Let's check the git history for these removed variables to understand their previous usage
echo "Checking git history for removed variables..."
git log -p -- .github/workflows/deploy-sepolia-vault.yaml | grep -A 5 -B 5 "ROLE_MANAGER\|ROLE_NUM\|FEE_MANAGER"
# Let's also check if there are any other workflow files that might be related
echo "Checking related workflow files..."
fd "deploy.*vault.*ya?ml" .github/workflows/
# Let's examine the content of related workflow files
echo "Examining related workflow files..."
fd "deploy.*vault.*ya?ml" .github/workflows/ -x cat {}
Length of output: 7677
21-38: Consider enhancing input parameter validation and documentation
The new optional parameters need more comprehensive documentation and validation:
-
Default values:
depositLimit: '0'implies unlimited deposits - is this intentional?- Zero defaults for fee/gain/loss limits might be too restrictive
-
Documentation improvements needed:
- Specify units (e.g., basis points, wei)
- Document acceptable ranges
- Clarify implications of default values
Let's verify the parameter usage in the deployment script:
| // Set up the RPC URL (optional if you're using the default foundry config) | ||
| string memory rpcUrl = vm.envString("RPC_URL"); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused RPC URL configuration.
The RPC URL is read from environment variables but never used. This could mislead users into thinking it affects the deployment.
- // Set up the RPC URL (optional if you're using the default foundry config)
- string memory rpcUrl = vm.envString("RPC_URL");
-📝 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.
| // Set up the RPC URL (optional if you're using the default foundry config) | |
| string memory rpcUrl = vm.envString("RPC_URL"); |
| factory.deploySafe(proposer, vault, accountant, governor); | ||
|
|
||
| vm.stopBroadcast(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add deployment success verification.
The script should verify that the safe deployment was successful.
factory.deploySafe(proposer, vault, accountant, governor);
+
+ // Verify deployment success by checking the deployed safe address
+ // Note: This requires adding a view function to the factory interface
+ address deployedSafe = factory.getLastDeployedSafe();
+ require(deployedSafe != address(0), "Safe deployment failed");
+ console.log("Safe deployed at:", deployedSafe);
vm.stopBroadcast();Committable suggestion skipped: line range outside the PR's diff.
| TermVaultGovernanceFactory factory = TermVaultGovernanceFactory(vm.envAddress("GOVERNANCE_FACTORY")); | ||
| address proposer = vm.envAddress("PROPOSER"); | ||
| address vault = vm.envAddress("VAULT"); | ||
| address accountant = vm.envAddress("ACCOUNTANT"); | ||
| address governor = vm.envAddress("GOVERNOR"); | ||
|
|
||
| factory.deploySafe(proposer, vault, accountant, governor); |
There was a problem hiding this comment.
Add input validation for critical addresses.
The script should validate that all addresses are non-zero and that required environment variables exist.
+ function _validateAddress(address addr, string memory name) internal pure {
+ require(addr != address(0), string.concat(name, " address cannot be zero"));
+ }
+
TermVaultGovernanceFactory factory = TermVaultGovernanceFactory(vm.envAddress("GOVERNANCE_FACTORY"));
+ _validateAddress(address(factory), "Factory");
+
address proposer = vm.envAddress("PROPOSER");
+ _validateAddress(proposer, "Proposer");
+
address vault = vm.envAddress("VAULT");
+ _validateAddress(vault, "Vault");
+
address accountant = vm.envAddress("ACCOUNTANT");
+ _validateAddress(accountant, "Accountant");
+
address governor = vm.envAddress("GOVERNOR");
+ _validateAddress(governor, "Governor");📝 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.
| TermVaultGovernanceFactory factory = TermVaultGovernanceFactory(vm.envAddress("GOVERNANCE_FACTORY")); | |
| address proposer = vm.envAddress("PROPOSER"); | |
| address vault = vm.envAddress("VAULT"); | |
| address accountant = vm.envAddress("ACCOUNTANT"); | |
| address governor = vm.envAddress("GOVERNOR"); | |
| factory.deploySafe(proposer, vault, accountant, governor); | |
| function _validateAddress(address addr, string memory name) internal pure { | |
| require(addr != address(0), string.concat(name, " address cannot be zero")); | |
| } | |
| TermVaultGovernanceFactory factory = TermVaultGovernanceFactory(vm.envAddress("GOVERNANCE_FACTORY")); | |
| _validateAddress(address(factory), "Factory"); | |
| address proposer = vm.envAddress("PROPOSER"); | |
| _validateAddress(proposer, "Proposer"); | |
| address vault = vm.envAddress("VAULT"); | |
| _validateAddress(vault, "Vault"); | |
| address accountant = vm.envAddress("ACCOUNTANT"); | |
| _validateAddress(accountant, "Accountant"); | |
| address governor = vm.envAddress("GOVERNOR"); | |
| _validateAddress(governor, "Governor"); | |
| factory.deploySafe(proposer, vault, accountant, governor); |
| env: | ||
| RPC_URL: ${{ secrets.RPC_URL }} | ||
| PRIVATE_KEY: ${{ secrets.GOVERNANCE_DEPLOYER_KEY }} | ||
| ETHERSCAN_API_KEY: ${{ secrets.ETHERSCAN_API_KEY }} | ||
| PROPOSER: ${{ github.event.inputs.proposer }} | ||
| VAULT: ${{ github.event.inputs.vault }} | ||
| ACCOUNTANT: ${{ github.event.inputs.accountant }} | ||
| GOVERNOR: ${{ github.event.inputs.governorRoleAddress }} | ||
| GOVERNANCE_FACTORY: ${{ vars.GOVERNANCE_FACTORY }} | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add environment variable verification
Consider adding a step to verify that all required environment variables are set before proceeding with deployment.
Add this step before the forge script:
- name: Verify environment variables
run: |
required_vars=("RPC_URL" "PRIVATE_KEY" "ETHERSCAN_API_KEY" "GOVERNANCE_FACTORY")
for var in "${required_vars[@]}"; do
if [ -z "${!var}" ]; then
echo "Error: Required environment variable $var is not set"
exit 1
fi
done🧰 Tools
🪛 yamllint (1.29.0-1)
[warning] 43-43: too many spaces after colon
(colons)
| proposer: | ||
| description: 'Proposer address' | ||
| required: true | ||
| default: '0x' | ||
| vault: | ||
| description: 'Vault Address' | ||
| required: true | ||
| default: '0x' | ||
| accountant: | ||
| description: 'Accountant Address' | ||
| required: true | ||
| default: '0x' | ||
| governorRoleAddress: | ||
| description: 'Governor role address' | ||
| required: true | ||
| default: '0x' | ||
| governorVaults: | ||
| description: 'Governor vaults as comma separated' | ||
| required: false | ||
| default: '' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve input parameter validation and documentation
The input parameters need better validation and documentation:
- The default value '0x' is not a valid Ethereum address
- Consider adding a pattern for validating Ethereum addresses
- Specify whether addresses should be checksummed or lowercase
Apply this diff to improve the input configuration:
proposer:
description: 'Proposer address'
required: true
- default: '0x'
+ default: '0x0000000000000000000000000000000000000000'
+ pattern: '^0x[a-fA-F0-9]{40}$'
vault:
description: 'Vault Address'
required: true
- default: '0x'
+ default: '0x0000000000000000000000000000000000000000'
+ pattern: '^0x[a-fA-F0-9]{40}$'
accountant:
description: 'Accountant Address'
required: true
- default: '0x'
+ default: '0x0000000000000000000000000000000000000000'
+ pattern: '^0x[a-fA-F0-9]{40}$'
governorRoleAddress:
description: 'Governor role address'
required: true
- default: '0x'
+ default: '0x0000000000000000000000000000000000000000'
+ pattern: '^0x[a-fA-F0-9]{40}$'📝 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.
| proposer: | |
| description: 'Proposer address' | |
| required: true | |
| default: '0x' | |
| vault: | |
| description: 'Vault Address' | |
| required: true | |
| default: '0x' | |
| accountant: | |
| description: 'Accountant Address' | |
| required: true | |
| default: '0x' | |
| governorRoleAddress: | |
| description: 'Governor role address' | |
| required: true | |
| default: '0x' | |
| governorVaults: | |
| description: 'Governor vaults as comma separated' | |
| required: false | |
| default: '' | |
| proposer: | |
| description: 'Proposer address' | |
| required: true | |
| default: '0x0000000000000000000000000000000000000000' | |
| pattern: '^0x[a-fA-F0-9]{40}$' | |
| vault: | |
| description: 'Vault Address' | |
| required: true | |
| default: '0x0000000000000000000000000000000000000000' | |
| pattern: '^0x[a-fA-F0-9]{40}$' | |
| accountant: | |
| description: 'Accountant Address' | |
| required: true | |
| default: '0x0000000000000000000000000000000000000000' | |
| pattern: '^0x[a-fA-F0-9]{40}$' | |
| governorRoleAddress: | |
| description: 'Governor role address' | |
| required: true | |
| default: '0x0000000000000000000000000000000000000000' | |
| pattern: '^0x[a-fA-F0-9]{40}$' | |
| governorVaults: | |
| description: 'Governor vaults as comma separated' | |
| required: false | |
| default: '' |
| - run: forge install | ||
| - run: forge build | ||
| - run: forge tree | ||
| - run: forge script script/VaultGovernanceDeploy.s.sol:VaultGovernanceDeploy --rpc-url $RPC_URL --broadcast --gas-price 500000000000 --verify --verbosity 4 |
There was a problem hiding this comment.
Fix script arguments and consider dynamic gas pricing
- Add quotes around script arguments to prevent word splitting
- Consider using dynamic gas pricing instead of hardcoded value
Apply this diff:
- - run: forge script script/VaultGovernanceDeploy.s.sol:VaultGovernanceDeploy --rpc-url $RPC_URL --broadcast --gas-price 500000000000 --verify --verbosity 4
+ - run: forge script "script/VaultGovernanceDeploy.s.sol:VaultGovernanceDeploy" --rpc-url "$RPC_URL" --broadcast --verify --verbosity 4Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.3)
41-41: shellcheck reported issue in this script: SC2086:info:1:81: Double quote to prevent globbing and word splitting
(shellcheck)
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes