Avax deployers #112
Avax deployers #112
Conversation
WalkthroughThis pull request introduces two new GitHub Actions workflow files for deploying Yearn infrastructure on the Avalanche network. The Changes
Sequence DiagramsequenceDiagram
participant User
participant GitHub Actions
participant Foundry
participant Avalanche Network
User->>GitHub Actions: Trigger workflow_dispatch
GitHub Actions->>GitHub Actions: Validate input parameters
GitHub Actions->>Foundry: Install toolchain
Foundry->>Foundry: Build project
Foundry->>Avalanche Network: Broadcast deployment transaction
Avalanche Network-->>Foundry: Deployment confirmation
Possibly related PRs
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #112 +/- ##
=======================================
Coverage 89.23% 89.23%
=======================================
Files 5 5
Lines 604 604
Branches 90 90
=======================================
Hits 539 539
Misses 54 54
Partials 11 11 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
.github/workflows/deploy-avax-vault.yaml (3)
46-48: Update environment URL to point to network explorerThe current URL points to static documentation. Consider updating it to point to the Avalanche C-Chain explorer where the deployed vault can be viewed, using the deployed contract address as a variable.
environment: name: avalanche - url: https://term-finance.github.io/yearn-v3-term-vault/ + url: https://snowtrace.io/address/${{ env.DEPLOYED_VAULT_ADDRESS }}🧰 Tools
🪛 yamllint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
42-42: Fix YAML formatting issuesRemove trailing spaces and add newline at end of file.
- + environment: + environment: - +Also applies to: 46-46, 79-79
🧰 Tools
🪛 yamllint (1.35.1)
[error] 42-42: trailing spaces
(trailing-spaces)
44-77: Add security hardening measuresConsider implementing these security improvements:
- Add
permissionsfield to limit GitHub token scope- Implement environment protection rules for the
avalancheenvironment- Add timeouts to prevent hanging deployments
Add these configurations:
permissions: contents: read concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true timeout-minutes: 10🧰 Tools
🪛 actionlint (1.7.4)
58-58: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
[warning] 60-60: too many spaces after colon
(colons)
.github/workflows/deploy-avax-strategy.yaml (2)
61-61: Consider making gas price configurable.The hard-coded gas price of 500000000000 might not be optimal for all market conditions on Avalanche. Consider making this configurable through workflow inputs.
- - run: forge script script/Strategy.s.sol:DeployStrategy --rpc-url $RPC_URL --broadcast --gas-price 500000000000 --verify --verbosity 4 + - run: forge script script/Strategy.s.sol:DeployStrategy --rpc-url $RPC_URL --broadcast --gas-price ${{ github.event.inputs.gasPrice || '500000000000' }} --verify --verbosity 4🧰 Tools
🪛 actionlint (1.7.4)
61-61: shellcheck reported issue in this script: SC2086:info:1:61: Double quote to prevent globbing and word splitting
(shellcheck)
1-85: Fix YAML formatting issues.There are several formatting issues in the file:
- Trailing spaces on lines 45, 49, and 85
- Missing newline at end of file
- Extra spaces after colons
🧰 Tools
🪛 actionlint (1.7.4)
61-61: shellcheck reported issue in this script: SC2086:info:1:61: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[warning] 63-63: too many spaces after colon
(colons)
[error] 85-85: no new line character at the end of file
(new-line-at-end-of-file)
[error] 85-85: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/deploy-avax-strategy.yaml(1 hunks).github/workflows/deploy-avax-vault.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/deploy-avax-strategy.yaml
61-61: shellcheck reported issue in this script: SC2086:info:1:61: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/deploy-avax-vault.yaml
58-58: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/deploy-avax-strategy.yaml
[error] 45-45: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[warning] 63-63: too many spaces after colon
(colons)
[error] 85-85: no new line character at the end of file
(new-line-at-end-of-file)
[error] 85-85: trailing spaces
(trailing-spaces)
.github/workflows/deploy-avax-vault.yaml
[error] 42-42: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[warning] 60-60: too many spaces after colon
(colons)
[error] 79-79: no new line character at the end of file
(new-line-at-end-of-file)
[error] 79-79: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/deploy-avax-strategy.yaml (2)
1-3: LGTM! Well-structured workflow configuration.
The workflow is appropriately named and configured for manual triggering, which is suitable for deployment operations.
4-44: Verify input parameter validation in deployment script.
While the workflow parameters are well-documented, there are potential concerns:
- Default '0x' for addresses could lead to deployment failures if not properly validated
- Decimal values (e.g., discountRateMarkup, requiredReserveRatio) need proper validation
Let's verify the validation logic in the deployment script:
✅ Verification successful
Input parameters are properly validated in the deployment script
The deployment script includes comprehensive validation:
- Address validation:
- Proper length and hex format checking in
parseAddress - Underlying vault asset verification in
checkUnderlyingVaultAsset
- Proper length and hex format checking in
- Numeric validation:
- Proper digit validation in
parseUint - Array parsing validation for collateral tokens and ratios
- Proper digit validation in
- Environment variables are used instead of workflow defaults, ensuring proper validation at runtime
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for input parameter validation in the Strategy.s.sol script
ast-grep --pattern 'function run() $_ {
$$$
require($_);
$$$
}'
Length of output: 71
Script:
#!/bin/bash
# Let's first find the deployment script
fd "Strategy.s.sol|deploy.*strategy.*\.s\.sol" --type f
# Then let's look for any validation logic in deployment-related files
rg -i "require|assert|revert" -A 3 -B 3 --type sol
# Also search for parameter handling
rg -i "constructor|function.*deploy" -A 5 -B 2 --type sol
Length of output: 248
Script:
#!/bin/bash
# Found the script file, let's examine its contents
cat script/Strategy.s.sol
# Also look for any validation patterns without file type restriction
rg -i "require|assert|revert" script/Strategy.s.sol
# And check parameter handling
rg -i "constructor|function.*deploy" script/Strategy.s.sol
Length of output: 10196
| env: | ||
| RPC_URL: ${{ secrets.RPC_URL }} | ||
| PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }} | ||
| ETHERSCAN_API_KEY: ${{ secrets.ETHERSCAN_API_KEY }} | ||
| ASSET_ADDRESS: ${{ github.event.inputs.asset }} | ||
| VAULT_FACTORY: ${{ vars.VAULT_FACTORY }} | ||
| ACCOUNTANT_FACTORY: ${{ vars.ACCOUNTANT_FACTORY }} | ||
| VAULT_NAME: ${{ github.event.inputs.vaultName }} | ||
| VAULT_SYMBOL: ${{ github.event.inputs.vaultSymbol }} | ||
| FEE_RECIPIENT: ${{ vars.FEE_RECIPIENT }} | ||
| DEPOSIT_LIMIT: ${{ github.event.inputs.depositLimit }} | ||
| VAULT_GOVERNANCE_FACTORY: ${{ vars.VAULT_GOVERNANCE_FACTORY }} | ||
| KEEPER_ADDRESS: ${{ vars.KEEPER }} | ||
| STRATEGY_ADDER: ${{ vars.STRATEGY_ADDER }} | ||
| PROFIT_MAX_UNLOCK_TIME: ${{ github.event.inputs.profitMaxUnlockTime }} | ||
| DEFAULT_PERFORMANCE: ${{ github.event.inputs.defaultPerformance }} | ||
| DEFAULT_MAX_FEE: ${{ github.event.inputs.defaultMaxFee }} | ||
| DEFAULT_MAX_GAIN: ${{ github.event.inputs.defaultMaxGain }} | ||
| DEFAULT_MAX_LOSS: ${{ github.event.inputs.defaultMaxLoss }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add environment variable verification and post-deployment checks
- Add verification step for required environment variables
- Capture and verify the deployed vault address
- Add post-deployment verification steps
Add these steps after the deployment:
- name: Verify environment variables
run: |
required_vars=("RPC_URL" "PRIVATE_KEY" "ETHERSCAN_API_KEY" "VAULT_FACTORY" "ACCOUNTANT_FACTORY")
for var in "${required_vars[@]}"; do
if [ -z "${!var}" ]; then
echo "Error: Required environment variable $var is not set"
exit 1
fi
done
- name: Verify deployment
run: |
# Add verification of the deployed vault
# Example: Check if the vault contract is verified on Snowtrace
# Check if the vault parameters match the inputs🧰 Tools
🪛 yamllint (1.35.1)
[warning] 60-60: too many spaces after colon
(colons)
| - run: forge install | ||
| - run: forge build | ||
| - run: forge tree | ||
| - run: forge script script/VaultManagement.s.sol:SetupVaultManagement --rpc-url $RPC_URL --broadcast --verify --verbosity 4 |
There was a problem hiding this comment.
Fix script command quoting
The forge script command should have quotes around the script path and arguments to prevent issues with special characters.
- - run: forge script script/VaultManagement.s.sol:SetupVaultManagement --rpc-url $RPC_URL --broadcast --verify --verbosity 4
+ - run: forge script "script/VaultManagement.s.sol:SetupVaultManagement" --rpc-url "$RPC_URL" --broadcast --verify --verbosity 4📝 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.
| - run: forge script script/VaultManagement.s.sol:SetupVaultManagement --rpc-url $RPC_URL --broadcast --verify --verbosity 4 | |
| - run: forge script "script/VaultManagement.s.sol:SetupVaultManagement" --rpc-url "$RPC_URL" --broadcast --verify --verbosity 4 |
🧰 Tools
🪛 actionlint (1.7.4)
58-58: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting
(shellcheck)
| asset: | ||
| description: 'Asset address' | ||
| required: true | ||
| default: '0x' | ||
| vaultName: | ||
| description: 'Yearn vault name' | ||
| required: true | ||
| default: '0x' | ||
| vaultSymbol: | ||
| description: 'Yearn vault symbol' | ||
| required: true | ||
| default: '0x' | ||
| profitMaxUnlockTime: | ||
| description: 'Profit max unlock time' | ||
| required: true | ||
| default: '0' | ||
| depositLimit: | ||
| description: 'Deposit limit' | ||
| required: false | ||
| default: '0' | ||
| defaultPerformance: | ||
| description: 'Default performance fee' | ||
| required: false | ||
| default: '0' | ||
| defaultMaxFee: | ||
| description: 'Default max fee' | ||
| required: false | ||
| default: '0' | ||
| defaultMaxGain: | ||
| description: 'Default max gain' | ||
| required: false | ||
| default: '0' | ||
| defaultMaxLoss: | ||
| description: 'Default max loss' | ||
| required: false | ||
| default: '0' |
There was a problem hiding this comment.
Add input validation and improve default values
Several improvements needed for the workflow inputs:
- The default value '0x' for addresses is invalid. Consider removing default values for required addresses.
- Add input validation for Ethereum addresses using a pattern constraint.
- Document the expected units for numerical inputs (e.g., profitMaxUnlockTime in seconds? depositLimit in wei?).
Example fix for the asset input:
asset:
description: 'Asset address'
required: true
- default: '0x'
+ pattern: '^0x[a-fA-F0-9]{40}$'Committable suggestion skipped: line range outside the PR's diff.
| ETHERSCAN_API_KEY: ${{ secrets.ETHERSCAN_API_KEY }} | ||
| ASSET_ADDRESS: ${{ github.event.inputs.asset }} | ||
| YEARN_VAULT_ADDRESS: ${{ vars.YEARN_VAULT_ADDRESS }} | ||
| IS_TEST: true |
There was a problem hiding this comment.
Remove or justify IS_TEST=true in production deployment.
Setting IS_TEST=true in a production deployment workflow is concerning. This might bypass important safety checks.
Please either:
- Remove this environment variable for production deployments
- Document why it's necessary to have it set to true
| - 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/Strategy.s.sol:DeployStrategy --rpc-url $RPC_URL --broadcast --gas-price 500000000000 --verify --verbosity 4 | ||
| env: | ||
| RPC_URL: ${{ secrets.RPC_URL }} | ||
| PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }} | ||
| GOVERNOR_DEPLOYER_KEY: ${{ secrets.GOVERNANCE_DEPLOYER_KEY }} | ||
| ETHERSCAN_API_KEY: ${{ secrets.ETHERSCAN_API_KEY }} | ||
| ASSET_ADDRESS: ${{ github.event.inputs.asset }} | ||
| YEARN_VAULT_ADDRESS: ${{ vars.YEARN_VAULT_ADDRESS }} | ||
| IS_TEST: true | ||
| STRATEGY_NAME: ${{ github.event.inputs.strategyName }} | ||
| TERM_CONTROLLER_ADDRESS: ${{ vars.TERM_CONTROLLER_ADDRESS }} | ||
| DISCOUNT_RATE_ADAPTER_ADDRESS: ${{ vars.DISCOUNT_RATE_ADAPTER_ADDRESS }} | ||
| DISCOUNT_RATE_MARKUP: ${{ github.event.inputs.discountRateMarkup }} | ||
| TIME_TO_MATURITY_THRESHOLD: ${{ github.event.inputs.timeToMaturityThreshold }} | ||
| REPOTOKEN_CONCENTRATION_LIMIT: ${{ github.event.inputs.repoTokenConcentrationLimit }} | ||
| ADMIN_ADDRESS: ${{ vars.ADMIN_ADDRESS }} | ||
| DEVOPS_ADDRESS: ${{ vars.DEVOPS_ADDRESS }} | ||
| KEEPER_ADDRESS: ${{ vars.KEEPER }} | ||
| GOVERNOR_ROLE_ADDRESS: ${{ vars.GOVERNANCE_FACTORY }} | ||
| STRATEGY_MANAGEMENT_ADDRESS: ${{ github.event.inputs.strategyManagementAddress }} | ||
| NEW_REQUIRED_RESERVE_RATIO: ${{ github.event.inputs.requiredReserveRatio }} | ||
| COLLATERAL_TOKEN_ADDRESSES: ${{ github.event.inputs.collateralTokens }} | ||
| MIN_COLLATERAL_RATIOS: ${{ github.event.inputs.minCollateralRatios }} | ||
| PROFIT_MAX_UNLOCK_TIME: ${{ github.event.inputs.profitMaxUnlock }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add post-deployment verification steps.
The workflow would benefit from additional steps to verify the deployment success:
- Verify the deployed contract is operational
- Check if the strategy is properly initialized
- Validate key parameters
Would you like me to provide an example of post-deployment verification steps?
🧰 Tools
🪛 actionlint (1.7.4)
61-61: shellcheck reported issue in this script: SC2086:info:1:61: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
[warning] 63-63: too many spaces after colon
(colons)
| deploy: | ||
| runs-on: ubuntu-latest | ||
| environment: | ||
| name: avalanche | ||
| url: https://term-finance.github.io/yearn-v3-term-vault/ |
There was a problem hiding this comment.
💡 Codebase verification
Documentation URL is not accessible
The environment URL https://term-finance.github.io/yearn-v3-term-vault/ returns a 404 error, indicating that the GitHub Pages documentation site is either not published or incorrectly configured. Please ensure that:
- The GitHub Pages site is properly set up for the repository
- The documentation is published to the correct branch
- The URL path is accurate
🔗 Analysis chain
Verify the documentation URL accessibility.
The environment URL points to a GitHub Pages documentation site. Please ensure this documentation is publicly accessible and contains relevant deployment information.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the documentation URL is accessible
curl -I https://term-finance.github.io/yearn-v3-term-vault/
Length of output: 1063
🧰 Tools
🪛 yamllint (1.35.1)
[error] 49-49: trailing spaces
(trailing-spaces)
| - run: forge install | ||
| - run: forge build | ||
| - run: forge tree | ||
| - run: forge script script/Strategy.s.sol:DeployStrategy --rpc-url $RPC_URL --broadcast --gas-price 500000000000 --verify --verbosity 4 |
There was a problem hiding this comment.
Fix shell script quoting for security.
The forge script command should have its arguments properly quoted to prevent word splitting and potential injection issues.
- - run: forge script script/Strategy.s.sol:DeployStrategy --rpc-url $RPC_URL --broadcast --gas-price 500000000000 --verify --verbosity 4
+ - run: forge script "script/Strategy.s.sol:DeployStrategy" --rpc-url "$RPC_URL" --broadcast --gas-price "500000000000" --verify --verbosity 4📝 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.
| - run: forge script script/Strategy.s.sol:DeployStrategy --rpc-url $RPC_URL --broadcast --gas-price 500000000000 --verify --verbosity 4 | |
| - run: forge script "script/Strategy.s.sol:DeployStrategy" --rpc-url "$RPC_URL" --broadcast --gas-price "500000000000" --verify --verbosity 4 |
🧰 Tools
🪛 actionlint (1.7.4)
61-61: shellcheck reported issue in this script: SC2086:info:1:61: Double quote to prevent globbing and word splitting
(shellcheck)
Summary by CodeRabbit