feat: deploy script validate & build steps #736
Conversation
WalkthroughThis pull request introduces significant changes to the deployment and build processes across multiple files. The modifications focus on streamlining the GitHub Actions workflow, enhancing contract building and validation scripts, and updating dependency management. Key changes include removing ARM-specific build scripts, adding new validation and build functions in the deployment package, and updating the CI/CD configuration to simplify artifact handling and deployment steps. Changes
Possibly related PRs
Suggested labels
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
scripts/build.sh (1)
Line range hint
1-115: Add input validation and error handling.The script handles paths and executes commands without comprehensive validation or error handling.
Consider these improvements:
#!/bin/bash +set -euo pipefail + +# Validate input arguments +if [ $# -eq 0 ]; then + echo "Error: No targets specified" + echo "Usage: $0 [all|contract_name|category_name]..." + exit 1 +fi + +# Sanitize and validate paths +ARTIFACTS_DIR="./artifacts" +IBC_TESTS_DIR="./ibc-tests/artifacts" +E2E_ARTIFACTS_DIR="./packages/andromeda-testing-e2e/artifacts" + +for dir in "$ARTIFACTS_DIR" "$IBC_TESTS_DIR" "$E2E_ARTIFACTS_DIR"; do + if ! mkdir -p "$dir"; then + echo "Error: Failed to create directory: $dir" + exit 1 + fi +doneAlso, add error handling for critical operations:
build_contract () { local CONTRACT_PATH=$1; + if [[ ! -d "$CONTRACT_PATH" ]]; then + echo "Error: Invalid contract path: $CONTRACT_PATH" + exit 1 + fi local CONTRACT=`basename $CONTRACT_PATH`; echo "Building contract $CONTRACT..."
🧹 Nitpick comments (19)
scripts/build_schema.sh (3)
28-31: Add atomic directory creationThe directory creation check and mkdir operation should be atomic to prevent race conditions.
Apply this diff to make the directory creation atomic:
-if [ ! -d "./schemas" ]; then - mkdir schemas; -fi; +mkdir -p schemas
33-64: Add logging and error reporting improvementsThe script would benefit from better error reporting and logging to aid in debugging deployment issues.
Consider adding:
- Timestamp to log messages
- Success/failure status for each contract
- Summary at the end of execution
Example enhancement:
+log() { + echo "[$(date '+%Y-%m-%d %H:%M:%S')] $1" +} + +PROCESSED_COUNT=0 +FAILED_COUNT=0 + if [ $# -eq 0 ]; then - echo "No contracts specified. Processing all contracts..." + log "No contracts specified. Processing all contracts..."
Line range hint
6-12: Consider integrating with ADODB version checkingThe
get_version_filenamefunction could be enhanced to integrate with ADODB version checking mentioned in the PR objectives. This would ensure consistency between built schemas and deployed contracts.Consider:
- Adding version verification against ADODB
- Skipping schema generation for up-to-date contracts
- Adding a flag to force rebuild regardless of version
scripts/build.sh (1)
Line range hint
1-115: Enhance logging and progress tracking.The script could benefit from improved logging to help diagnose build issues and track progress.
Add structured logging:
+# Add logging functions +log_info() { + echo "[INFO] $*" +} + +log_error() { + echo "[ERROR] $*" >&2 +} + +log_success() { + echo "[SUCCESS] $*" +} + build_contract () { local CONTRACT_PATH=$1; local CONTRACT=`basename $CONTRACT_PATH`; - echo "Building contract $CONTRACT..." + log_info "Building contract $CONTRACT..." if ! cargo wasm -p $CONTRACT -q; then + log_error "Failed to build contract: $CONTRACT" exit 1 fi + log_success "Successfully built contract: $CONTRACT"packages/deploy/src/build.rs (3)
26-39: Avoid panicking on script error
Panicking upon non-zero exit codes in build_schemas might halt the entire deployment process without cleanup or retries. Consider replacing panic with graceful error handling to allow for partial recoveries or more descriptive error messages.
41-54: Unify repetitive build logic
build_contracts and build_schemas follow a very similar pattern. Consider abstracting common logic (running a script, reading output, checking status) into a utility function to reduce code duplication and potential maintenance overhead.
56-67: Capture build logs
Redirecting stdout and stderr to null in build_all_contracts obscures logs that might be useful for debugging build failures. Consider capturing logs or allowing optional verbosity to aid in troubleshooting.packages/deploy/src/validate.rs (5)
30-45: Handle connection and query failures gracefully
get_adodb_contract panics if chain config or ADODB address is invalid. If partial or optional usage of ADODB is ever needed, consider returning a Result to handle errors more gracefully.
47-63: Return Result for chain validation
validate_chain_configs immediately panics on missing or invalid configuration. If there's a scenario where the caller might gracefully recover or choose a fallback chain, consider returning a Result.
64-85: Consolidate DEPLOY_OS and kernel address logic
validate_kernel_address requires that either DEPLOYMENT_KERNEL_ADDRESS is set or DEPLOY_OS is true. If future deployments might require an alternative scenario (e.g., ephemeral kernel for testing), factor in additional validations or optional states.
87-132: Leverage existing contract registry checks
filter_invalid_contracts calls get_contract for each name to verify existence. If performance becomes a concern, consider an approach that caches or indexes contract data, especially if you have many contracts.
236-252: Double-check filtering order
validate_deployment_contracts filters invalid contracts first, then filters out already-deployed contracts. If order matters (e.g., removing duplicates or normalizing names), ensure the correct sequence is used so no contracts are accidentally missed or reintroduced.packages/deploy/src/main.rs (4)
17-18: Build invocation
build::run() is logically placed after validation. Ensure that partial validation failures do not erroneously continue the build step.
47-47: Avoid forcing kernel address
Line 47 enforces that the kernel address must be set after possible OS deployment. This is correct for current logic but might limit future possibilities of ephemeral kernels or alternative flows.
63-63: Error messaging on ADODB deployment failure
Providing the chain and kernel_address in the SlackNotification is helpful. Consider adding the full error details or a direct link to logs for quicker resolution.
78-78: Comprehensive deployment report
Including kernel_address in the final DeploymentReport clarifies the deployed environment. Consider also recording the OS version if relevant to debugging or post-deployment validations.packages/deploy/src/contracts.rs (1)
100-105: LGTM! Consider performance optimizations for contract lookup.The implementation correctly combines and searches through all contracts. Consider these improvements:
- Cache the combined list to avoid rebuilding it on each call
- Add case-insensitive comparison for better UX
+use std::sync::OnceLock; + +static CONTRACTS: OnceLock<Vec<DeployableContract>> = OnceLock::new(); + pub fn get_contract(contract_name: String) -> Option<DeployableContract> { - all_contracts() - .into_iter() - .chain(os_contracts()) - .find(|(name, _, _)| *name == contract_name) + let contracts = CONTRACTS.get_or_init(|| { + all_contracts() + .into_iter() + .chain(os_contracts()) + .collect() + }); + contracts + .iter() + .find(|(name, _, _)| name.eq_ignore_ascii_case(&contract_name)) + .cloned() }.github/workflows/deploy.yml (1)
Line range hint
121-190: Add timeout for schema parser trigger.The schema parser job should include a timeout to prevent hanging deployments.
trigger-schema-parser: + timeout-minutes: 15 needs: [build_and_deploy] runs-on: ubuntu-latestCHANGELOG.md (1)
20-20: Consider expanding the changelog entry with more specific details.While the entry follows the correct format, consider expanding it to better reflect the significant changes described in the PR objectives. Suggested expansion:
-feat: deploy script validate & build steps [(#736)](https://github.com/andromedaprotocol/andromeda-core/pull/736) +feat: deploy script validate & build steps [(#736)](https://github.com/andromedaprotocol/andromeda-core/pull/736) + - Added validation step for chain configuration and inputs + - Optimized build process to compile only updated contracts + - Added support for conditional OS contract buildsThis would provide more visibility into the key improvements made to the deployment process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.github/workflows/deploy.yml(4 hunks)CHANGELOG.md(1 hunks)Makefile(0 hunks)packages/deploy/Cargo.toml(3 hunks)packages/deploy/src/build.rs(1 hunks)packages/deploy/src/chains/mod.rs(1 hunks)packages/deploy/src/contracts.rs(1 hunks)packages/deploy/src/lib.rs(1 hunks)packages/deploy/src/main.rs(5 hunks)packages/deploy/src/validate.rs(1 hunks)packages/std/Cargo.toml(0 hunks)scripts/build.sh(4 hunks)scripts/build_all.sh(1 hunks)scripts/build_all_arm.sh(0 hunks)scripts/build_schema.sh(2 hunks)
💤 Files with no reviewable changes (3)
- Makefile
- packages/std/Cargo.toml
- scripts/build_all_arm.sh
🔇 Additional comments (10)
scripts/build_all.sh (1)
4-9: LGTM! Verify the optimizer versions.
The architecture detection and image selection logic is well-implemented. However, let's verify the security status of the optimizer images.
✅ Verification successful
Version 0.16.1 is the latest available for both architectures
The version check confirms that 0.16.1 is the latest release for both cosmwasm/optimizer and cosmwasm/optimizer-arm64 images, indicating proper version selection. While we couldn't verify security advisories in the sandbox environment, the use of the latest stable version follows best practices.
- Both architectures (arm64 and amd64) are aligned at version 0.16.1
- Version history shows consistent maintenance and release patterns
- The version selection in the build script matches the latest available release
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for latest versions and security advisories for the CosmWasm optimizer images
# Check latest tags for both images
echo "Checking latest optimizer tags..."
curl -s https://registry.hub.docker.com/v2/repositories/cosmwasm/optimizer/tags | jq -r '.results[].name' | head -n 5
curl -s https://registry.hub.docker.com/v2/repositories/cosmwasm/optimizer-arm64/tags | jq -r '.results[].name' | head -n 5
# Check for any security advisories
echo "Checking for security advisories..."
docker scout cves cosmwasm/optimizer:0.16.1
docker scout cves cosmwasm/optimizer-arm64:0.16.1
Length of output: 781
scripts/build.sh (1)
Line range hint 1-115: Verify the build configuration aligns with deployment requirements.
Given the PR objectives to optimize the deployment process, verify that the build script supports the new deployment workflow requirements.
Let's check the repository for deployment configuration and contract definitions:
✅ Verification successful
The build script aligns well with deployment requirements
The build script effectively supports the deployment workflow by:
- Providing flexible build options for single contracts, categories, or all contracts
- Generating optimized WASM artifacts in standardized locations (
artifacts/,ibc-tests/artifacts/,packages/andromeda-testing-e2e/artifacts/) - Supporting the deployment package (
packages/deploy) which handles contract deployment across different chains - Including proper optimization flags and build configurations aligned with production requirements
The script is part of a well-structured deployment pipeline where:
- Individual contracts can be built and optimized using
wasm-opt - Artifacts are placed in consistent locations used by testing and deployment tools
- The build process integrates with the deployment package that handles multi-chain deployments
- Build configurations are properly set for production releases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for deployment configuration files
echo "Checking deployment configuration files..."
fd -t f "deploy.*\.(json|yaml|toml)"
# Check for contract definitions that might need special build handling
echo "Checking for contract definitions..."
fd -t f "Contract\.toml$"
# Check for any OS-specific contracts that need special handling
echo "Checking for OS-specific contracts..."
rg -l "DEPLOY_OS|os_contract" contracts/
Length of output: 372
Script:
#!/bin/bash
# Let's check the repository structure and build requirements
# Check repository structure for contract organization
echo "Contract directory structure:"
tree contracts/ -L 2 -d
# Check for any build-related configuration files
echo -e "\nBuild configuration files:"
fd -t f "(Cargo|Makefile|\.cargo)"
# Check for any deployment or testing scripts
echo -e "\nDeployment and testing scripts:"
fd -t f ".*\.(sh|js|ts)" -E "node_modules" scripts/ tests/
# Check Cargo.toml for build dependencies and features
echo -e "\nCargo configuration:"
cat Cargo.toml
# Look for documentation about deployment
echo -e "\nDeployment documentation:"
fd "README" -t f | xargs grep -l "deploy" || true
Length of output: 7337
Script:
#!/bin/bash
# Let's check the deployment documentation and related configurations
# Check README files for deployment instructions
echo "Deployment documentation content:"
cat README.md
echo -e "\nKernel deployment docs:"
cat contracts/os/andromeda-kernel/README.md
# Check deploy package configuration
echo -e "\nDeploy package configuration:"
cat packages/deploy/Cargo.toml
# Check if there are any deployment-specific scripts
echo -e "\nChecking for deployment scripts:"
rg -l "deploy|upload" scripts/
Length of output: 25851
packages/deploy/src/build.rs (1)
3-24: Clarify handling of empty contract lists
If neither DEPLOY_CONTRACTS is set nor DEPLOY_OS is true, the function returns an empty list of contracts. This might cause confusion in downstream logic if an empty list is an unintentional scenario. Consider logging a warning or adding documentation to clarify this behavior.
packages/deploy/src/validate.rs (1)
133-234: Ensure versioning logic remains accurate
filter_deployed_contracts checks deployed versions by string-formatting “name@version”. If the version format changes, old versions might not be recognized. Keep this consistent across all contract definitions and consider a stricter pattern check.
✅ Verification successful
Let me gather information about the version formatting across the codebase.
Based on the search results, I can now provide a final response as I have found sufficient evidence about the version formatting across the codebase:
Version format is consistently implemented and validated
The version format name@version is consistently implemented and validated throughout the codebase:
- In
packages/std/src/os/adodb.rs, there's a dedicatedADOVersiontype that handles version formatting and validation - The validation is enforced through the
validate()method which checks the format - The format is consistently used in both the ADODB contract and the deployment process
- Test cases in
contracts/os/andromeda-adodb/src/tests.rsverify the version format handling - The version parsing is properly handled using the
semvercrate for semantic versioning validation
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for version string formatting patterns
rg -A 2 "name@version|@version|version@" --type rust
# Look for version-related functions and patterns in the codebase
ast-grep --pattern 'fn $name($$$) {
$$$
format!("{}@{}", $_, $_)
$$$
}'
# Search for other version string manipulations
rg "\.version|version\." --type rust -A 2
Length of output: 30611
packages/deploy/src/lib.rs (1)
2-10: Modules introduced
You have added pub mod build; and pub mod validate;. This modularization is well-structured, separating build logic from validation concerns. Be sure to document these modules thoroughly in the crate-level docs for discoverability and clarity.
packages/deploy/src/chains/mod.rs (1)
23-23: Enhance readability of the panic message
Swapping unwrap() for unwrap_or_else provides a clearer error message when the chain is not found. This is beneficial for debugging. If you foresee scenarios where missing chains may be recoverable, consider returning a Result instead of panicking.
packages/deploy/src/main.rs (1)
1-4: Import statements
Imports for build and validate clearly indicate the new deployment workflow. No issues found.
packages/deploy/Cargo.toml (2)
3-3: LGTM! Version bump and new dependencies align with deployment script enhancements.
The version bump and new dependencies support the added validation and build steps:
env_loggerandlogfor structured loggingdotenvfor environment configurationreqwestfor HTTP requests (ADODB validation)thiserrorfor error handlingserde_jsonfor JSON processingchronofor timestamp handling
Also applies to: 17-23
79-89: LGTM! Release profile settings are optimized for production.
The release profile settings are well-configured for optimal production builds:
- Maximum optimization level (3)
- Link-time optimization enabled
- Single codegen unit for better optimization
- Panic set to abort for smaller binary size
- Overflow checks enabled for safety
.github/workflows/deploy.yml (1)
23-58: LGTM! Commented jobs align with deployment optimization goals.
The removal of separate build jobs streamlines the deployment process, aligning with the PR's objective to optimize deployment by building only necessary contracts.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/deploy/README.md (4)
3-8: Add more detail about the validation stepThe verification step (step 1) should be expanded to include the specific validation checks mentioned in the PR objectives:
- Configuration existence for specified chain
- "Deploy OS" and "Kernel Address" validation
- Contract name verification
- Version comparison against ADODB
14-18: Clarify Docker's role in the build processSince Docker is used for the wasm-optimizer, it would be helpful to explain this requirement and specify if different Docker configurations are needed for full vs. selective builds.
20-26: Add version specification details for DEPLOY_CONTRACTSSince the deployment process now includes version checking against ADODB, consider adding information about how to specify versions in the
DEPLOY_CONTRACTSvariable, or if this is handled automatically.
42-45: Expand the notes section with validation detailsConsider adding notes about:
- The validation process and potential failure scenarios
- How to interpret and resolve validation errors
- The behavior when contracts fail version validation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/deploy/README.md(1 hunks)
🔇 Additional comments (1)
packages/deploy/README.md (1)
1-45: Verify documentation against implementation
Let's verify that the documentation accurately reflects the implementation:
✅ Verification successful
Let me gather more specific information about the implementation details to verify against the documentation.
Based on the results from the shell scripts, I can now generate the final response as we have sufficient evidence to verify the documentation against the implementation.
Documentation accurately reflects implementation
The implementation in the codebase fully supports all documented features:
- Environment variables (
DEPLOYMENT_CHAIN,DEPLOYMENT_KERNEL_ADDRESS,DEPLOY_OS,DEPLOY_CONTRACTS) are properly handled inpackages/deploy/src/main.rsandpackages/deploy/src/validate.rs - The documented steps (verification, building, OS deployment, ADO deployment) are implemented in corresponding modules
- Version comparison for updating ADOs is implemented as mentioned in the documentation notes
- The conditional logic for kernel address requirement matches the documentation (not required if
DEPLOY_OS=true)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all documented features are implemented
# Check for validation function
echo "Checking for validation implementation..."
rg -l "validate|verification" --type rust
# Check for build script implementations
echo "Checking for build implementations..."
rg -l "wasm-optimizer|build.*script" --type rust
# Check for version comparison logic
echo "Checking for version comparison implementation..."
rg -l "version.*check|ADODB" --type rust
Length of output: 5276
Script:
#!/bin/bash
# Check deployment chain implementation
echo "Checking deployment chain implementation..."
rg -A 5 "DEPLOYMENT_CHAIN" packages/deploy/src/
# Check kernel address handling
echo "Checking kernel address handling..."
rg -A 5 "DEPLOYMENT_KERNEL_ADDRESS" packages/deploy/src/
# Check OS deployment flag handling
echo "Checking OS deployment flag..."
rg -A 5 "DEPLOY_OS" packages/deploy/src/
# Check contract deployment list handling
echo "Checking contract deployment list..."
rg -A 5 "DEPLOY_CONTRACTS" packages/deploy/src/
# Check version comparison for existing OS
echo "Checking version comparison logic..."
rg -A 5 "version.*compare|update.*version" packages/deploy/src/
Length of output: 14612
Motivation
These changes add two extra steps to the deployment script:
These steps were added as the current deploy script builds all contracts, despite only needing a subset of them.
Implementation
For the validate step the following process occurs:
Once the above step is complete the environment variables should be configured correctly for the build step. At this point the build step uses wasm-optimizer if all contracts are being deployed, otherwise it uses our custom build script to only build the required contracts. If
DEPLOY_OSis set totruethen it will also build theosset of contracts.Testing
Tested using Github Actions
Version Changes
andromeda-deploy:0.0.1->0.0.2Future work
Validate all contracts are created as
DeployableContracts.Summary by CodeRabbit
New Features
Bug Fixes
Chores
Refactor