vault deploy jobs #107
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| name: "[sepolia-deploy] deploy governance for vault" | ||
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| 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: '' | ||
|
|
||
| jobs: | ||
| deploy: | ||
| runs-on: ubuntu-latest | ||
| environment: | ||
| name: sepolia | ||
| url: https://term-finance.github.io/yearn-v3-term-vault/ | ||
| steps: | ||
| - uses: actions/checkout@master | ||
| with: | ||
| fetch-depth: 0 | ||
| submodules: recursive | ||
| - uses: foundry-rs/foundry-toolchain@v1 | ||
| - 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
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 4
🧰 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) |
||
| 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 }} | ||
|
|
||
|
Comment on lines
+42
to
+51
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) |
||
|
|
||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,34 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // SPDX-License-Identifier: MIT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pragma solidity ^0.8.18; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import "forge-std/Script.sol"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| interface TermVaultGovernanceFactory { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function deploySafe( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| address proposer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| address vault, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| address accountant, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| address governor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) external; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| contract VaultGovernanceDeploy is Script { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function run() external { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uint256 deployerPK = vm.envUint("PRIVATE_KEY"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Set up the RPC URL (optional if you're using the default foundry config) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string memory rpcUrl = vm.envString("RPC_URL"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+20
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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vm.startBroadcast(deployerPK); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+29
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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vm.stopBroadcast(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
+31
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();
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🛠️ Refactor suggestion
Improve input parameter validation and documentation
The input parameters need better validation and documentation:
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