ci: Deploy script #616
ci: Deploy script #616
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
WalkthroughThe pull request introduces significant enhancements to the deployment process within the Andromeda ecosystem. Key modifications include the addition of new input parameters in the GitHub Actions workflow, the introduction of new jobs for building contracts and deployment scripts, and a restructured deployment job that relies on these new jobs. Additionally, various contract files have been updated to include new functionalities, such as handling replies from asynchronous operations, and several Cargo.toml files have been modified to include new dependencies and features. The changelog and documentation have also been updated to reflect these changes. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 47
🧹 Outside diff range and nitpick comments (50)
contracts/fungible-tokens/andromeda-cw20/src/lib.rs (1)
8-11: Consider adding documentation for the interface module.Since this module is part of the deployment infrastructure, it would be helpful to add documentation explaining its purpose and usage in the deployment process.
Add documentation like this:
#[cfg(not(target_arch = "wasm32"))] +/// Interface module providing deployment and testing utilities for the CW20 contract. +/// This module is only included in non-WASM builds and is used by the deployment scripts. mod interface; #[cfg(not(target_arch = "wasm32"))] pub use crate::interface::CW20Contract;contracts/non-fungible-tokens/andromeda-cw721/src/lib.rs (1)
9-12: LGTM! Well-structured module separation.The conditional compilation of the interface module for non-WASM targets is a good architectural decision that:
- Keeps deployment/testing code separate from the contract's WASM build
- Aligns with the PR's deployment objectives
- Follows the existing pattern used for testing modules
Consider documenting the purpose of the
interfacemodule in the codebase, explaining its role in deployment orchestration to help future maintainers understand the architecture.contracts/finance/andromeda-splitter/src/interface.rs (1)
6-6: LGTM! Standardized contract interface implementationThe
contract_interface!macro usage aligns with the framework's standardization efforts, providing a consistent way to define contract interfaces across the Andromeda ecosystem.This standardization:
- Simplifies contract deployment and management
- Reduces potential for configuration errors
- Makes the codebase more maintainable
contracts/fungible-tokens/andromeda-cw20/src/interface.rs (2)
1-2: Remove unused importADOMetadataThe
ADOMetadataimport fromandromeda_std::deployappears to be unused in this interface file.-use andromeda_std::{ado_base::MigrateMsg, contract_interface, deploy::ADOMetadata}; +use andromeda_std::{ado_base::MigrateMsg, contract_interface};
1-6: Well-structured interface design for deploymentThe interface file provides a clean separation between contract implementation and deployment concerns, which aligns well with the PR's objective of facilitating deployment across different chains. This abstraction will make it easier to manage deployments through the
cw-orchestratoras mentioned in the PR objectives.contracts/os/andromeda-adodb/src/interface.rs (1)
1-10: Good architectural direction with standardized contract interfacesThe shift to using the
contract_interface!macro and standardized contract IDs aligns well with the deployment package objectives. This approach:
- Reduces boilerplate code
- Standardizes contract interface declarations
- Makes the deployment process more maintainable
Consider documenting these architectural changes in the deployment guide to help other developers understand the new patterns.
contracts/finance/andromeda-timelock/src/interface.rs (2)
1-2: Remove unused importADOMetadataThe
ADOMetadatatype is imported but not used in this file.-use andromeda_std::{ado_base::MigrateMsg, contract_interface, deploy::ADOMetadata}; +use andromeda_std::{ado_base::MigrateMsg, contract_interface};
1-6: Consider documenting deployment requirementsSince this interface is crucial for the deployment process, consider adding documentation comments that specify:
- Required environment variables (DEPLOYMENT_CHAIN, TEST_MNEMONIC)
- Optional configuration (DEPLOYMENT_KERNEL_ADDRESS)
- Usage examples with cw-orchestrator
This would improve the developer experience when using this contract in deployments.
contracts/non-fungible-tokens/andromeda-cw721/src/interface.rs (1)
1-6: Document deployment prerequisites and interface usage.Since this interface is crucial for the new deployment infrastructure, consider adding documentation comments that:
- Reference required environment variables (DEPLOYMENT_CHAIN, TEST_MNEMONIC)
- Explain how this interface integrates with
cw-orchestrator- Provide example usage in deployment scripts
This will help maintain consistency with the deployment process described in the PR.
contracts/data-storage/andromeda-primitive/src/lib.rs (1)
11-12: LGTM! Clean architectural separation of interface code.The conditional compilation of the interface module for non-wasm32 targets is a good architectural decision, keeping deployment-specific code separate from the core contract logic.
contracts/os/andromeda-economics/src/interface.rs (1)
1-10: Good architectural shift to macro-based contract interface.The transition from trait-based to macro-based contract interface implementation is a positive change that:
- Reduces boilerplate code
- Provides a more consistent contract registration pattern
- Better supports the deployment automation goals of this PR
This approach will make it easier to maintain and extend the contract deployment system.
contracts/fungible-tokens/andromeda-cw20-staking/src/lib.rs (1)
11-12: LGTM: Clean separation of deployment interfaceThe conditional compilation of the interface module for non-wasm32 targets is a good practice, keeping deployment-specific code separate from the runtime WASM binary.
This pattern helps maintain a clean separation between runtime and deployment code, reducing the final WASM binary size.
contracts/fungible-tokens/andromeda-cw20-staking/src/interface.rs (1)
1-10: Well-structured interface supporting deployment automation.The interface implementation aligns well with the PR's deployment objectives and follows Andromeda's architectural patterns. It provides the necessary hooks for automated deployment and migration, which is crucial for the
make deployworkflow mentioned in the PR objectives.Consider documenting the following in the README or deployment guide:
- The relationship between this interface and the deployment process
- Any specific configuration needed for the CW20 staking contract during deployment
- How this contract fits into the broader Andromeda protocol deployment strategy
contracts/os/andromeda-ibc-registry/src/interface.rs (1)
10-14: LGTM! Clean macro-based interface implementationThe new
contract_interface!macro implementation provides a more maintainable and standardized way to define the contract interface. This aligns well with modern CosmWasm patterns and reduces boilerplate compared to the previous manualUploadabletrait implementation.Consider documenting the contract interface pattern in the project's developer guide to ensure consistency across other contracts in the ecosystem.
contracts/finance/andromeda-validator-staking/src/interface.rs (1)
1-10: Consider adding interface documentation.While the implementation is solid, adding documentation comments (///) for the contract interface would improve maintainability and help other developers understand the contract's purpose and usage within the Andromeda ecosystem.
Add documentation like this:
+/// Contract interface for the Validator Staking ADO. +/// This contract handles staking operations with validators in the Andromeda ecosystem. pub const CONTRACT_ID: &str = "validator-staking"; +/// Implements the standard interface for Validator Staking operations. +/// This includes execution, instantiation, querying, and migration capabilities. contract_interface!( ValidatorStakingContract, CONTRACT_ID, "andromeda_validator_staking.wasm" );scripts/attach_contract_versions.sh (1)
1-14: Consider architectural improvements for better integrationSince this script is part of the deployment process, consider these architectural improvements:
- Add logging support to track the versioning process during deployment
- Integrate with the Slack notification system (using
SLACK_WEBHOOK_URL)- Add a dry-run mode for validation without file operations
- Consider adding support for parallel processing of contracts
Would you like me to provide an implementation for any of these suggestions?
🧰 Tools
🪛 Shellcheck
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
contracts/finance/andromeda-splitter/Cargo.toml (1)
28-30: LGTM! Good dependency organization.The changes improve the package structure by:
- Moving
cw-orchto target-specific dependencies, which reduces WASM binary size- Making test-related dependencies optional, allowing for conditional compilation
Consider adding a comment above the target-specific dependencies section to explain its purpose:
+# Dependencies used only for deployment, testing, and local development [target.'cfg(not(target_arch = "wasm32"))'.dependencies] cw-orch = { workspace = true } cw-multi-test = { workspace = true, optional = true } andromeda-testing = { workspace = true, optional = true }packages/deploy/src/chains.rs (1)
1-31: Consider implementing a more flexible configuration management systemThe current implementation tightly couples chain configurations with code. For better maintainability and flexibility, consider:
- Moving chain configurations to external YAML/JSON files
- Implementing environment-specific configuration loading
- Adding support for dynamic chain discovery from a registry
- Supporting configuration overrides via environment variables
This would make it easier to manage multiple environments and update configurations without code changes.
contracts/fungible-tokens/andromeda-cw20-staking/Cargo.toml (1)
33-33: Consider marking cw-orch as optionalFor consistency with other testing dependencies like
cw-multi-testandandromeda-testing, consider markingcw-orchas optional. This would prevent unnecessary compilation in development environments where deployment functionality isn't needed.-cw-orch = { workspace = true } +cw-orch = { workspace = true, optional = true }packages/std/Cargo.toml (1)
41-41: Consider conditional compilation for cw-orchSince
cw-orchis primarily used for deployment and testing purposes, consider making it conditional on thedeployfeature or non-wasm targets to reduce the production build size.-cw-orch = { workspace = true } +cw-orch = { workspace = true, optional = true }Then update the
[features]section:-deploy = [] +deploy = ["dep:cw-orch"]packages/deploy/src/contract_interface.rs (1)
31-34: Consider caching the version string.The
version()method creates a new String on each call. For better performance, consider caching the version string.Consider using a lazy static approach:
+use once_cell::sync::Lazy; + impl<Chain> ADOMetadata for $contract_name<Chain> { + static VERSION: Lazy<String> = Lazy::new(|| env!("CARGO_PKG_VERSION").to_string()); + fn version() -> String { - let version = env!("CARGO_PKG_VERSION"); - version.to_string() + VERSION.clone() } }packages/deploy/src/contracts.rs (2)
15-16: Consider using a struct instead of tuple type for better maintainability.While the tuple type works, a struct would provide named fields and better extensibility for future additions. This would make the code more maintainable and self-documenting.
Consider refactoring to:
-type UploadFn = Box<dyn FnOnce(&DaemonBase<Wallet>) -> Result<u64, CwOrchError>>; -pub type DeployableContract = (String, String, UploadFn); +type UploadFn = Box<dyn FnOnce(&DaemonBase<Wallet>) -> Result<u64, CwOrchError>>; +pub struct DeployableContract { + pub name: String, + pub version: String, + pub upload_fn: UploadFn, +}
33-45: Document deployment order and contract dependencies.The order of contracts in the vector might be significant for deployment dependencies. Please add documentation explaining:
- Required deployment order (if any)
- Inter-contract dependencies
- Any initialization requirements
Add documentation like:
+/// Returns a vector of all deployable contracts in the required deployment order. +/// +/// # Deployment Order +/// The contracts are ordered based on their dependencies: +/// 1. Base contracts (Splitter, ValidatorStaking, etc.) +/// 2. Token contracts (CW20, CW721) +/// 3. Dependent contracts (CW20Staking, etc.) pub fn all_contracts() -> Vec<DeployableContract> { vec![ deployable!(SplitterContract),packages/deploy/src/main.rs (3)
8-10: Improve error handling for environment initializationConsider logging the result of
dotenv()initialization to help diagnose configuration issues.- dotenv().ok(); + if let Err(e) = dotenv() { + log::warn!("Failed to load .env file: {}", e); + }
12-13: Enhance error message for DEPLOYMENT_CHAINThe error message could be more helpful by including valid chain options (e.g., 'galileo-4') as mentioned in the PR objectives.
- let chain = env::var("DEPLOYMENT_CHAIN").expect("DEPLOYMENT_CHAIN must be set"); + let chain = env::var("DEPLOYMENT_CHAIN").expect("DEPLOYMENT_CHAIN must be set (e.g., 'galileo-4')");
8-64: Consider restructuring for better maintainabilityThe main function is handling multiple responsibilities. Consider:
- Extracting OS and ADODB deployment into separate functions
- Creating a dedicated error type for deployment failures
- Implementing a more structured logging strategy
Example structure:
#[derive(Debug)] enum DeploymentError { EnvError(std::env::VarError), OSDeployment(String), ADODBDeployment(String), SlackNotification(String), } fn deploy_os(chain: &str, kernel_address: Option<String>) -> Result<String, DeploymentError> { // OS deployment logic } fn deploy_adodb(chain: &str, kernel_address: String, contracts: Vec<String>) -> Result<(), DeploymentError> { // ADODB deployment logic } fn main() -> Result<(), DeploymentError> { // Simplified main function calling the above }Makefile (2)
34-37: Document the purpose of build-arm targetWhile the implementation looks correct, please add a comment explaining when to use
build-armversus the regularbuildtarget. This will help users choose the appropriate build command for their architecture.
68-72: Consider adding deployment profilesTo improve usability, consider adding deployment profiles that handle different deployment scenarios. For example:
deploy-dev: export RUST_LOG=debug deploy-dev: build version-map @echo "Deploying to dev environment..." # deployment commands deploy-prod: export RUST_LOG=info deploy-prod: build version-map attach-contract-versions @echo "Deploying to production environment..." # deployment commands with additional safety checksThis would provide better separation of concerns and make it clearer which steps are necessary for different deployment scenarios.
.github/workflows/deploy.yml (1)
14-21: Add default value for deploy_os inputConsider adding a default value for the
deploy_osinput to ensure predictable behavior when the input is not provided.deploy_os: description: "Deploy OS" required: false type: boolean + default: falsepackages/std/src/os/kernel.rs (3)
38-38: LGTM! The ExecuteFns derive macro enhances deployment capabilities.The addition of
cw_orch::ExecuteFnsderive macro aligns with the PR objectives by generating helper functions for contract execution, making it easier to manage deployments programmatically.This enhancement will enable the deployment script to interact with the kernel contract more efficiently, particularly for operations like ADO creation and channel management during the deployment process.
106-106: LGTM! QueryFns complements ExecuteFns for comprehensive contract interaction.The addition of
cw_orch::QueryFnsalongsideQueryResponsesprovides a complete set of helper functions for both querying and type safety.This enhancement enables the deployment script to efficiently query kernel state during deployment, which is crucial for verifying deployment steps and managing contract state.
122-123: LGTM! Improved naming clarity while maintaining compatibility.The rename from
TypetoAdoTypeenhances code clarity by being more specific, while the#[serde(rename = "type")]attribute maintains API compatibility.Consider adding a code comment explaining why the serialized name is kept as "type" to help future maintainers understand the reasoning behind this compatibility decision.
contracts/finance/andromeda-timelock/src/contract.rs (1)
Line range hint
1-263: Consider tracking submessage IDs for better error handlingThe contract sends submessages for fund releases but doesn't track their IDs. This makes it difficult to handle specific failure cases in the reply handler. Consider:
- Adding submessage IDs as constants
- Using these IDs when creating submessages
- Adding match arms in the reply handler for different submessage types
Example approach:
// At module level const RELEASE_FUNDS_REPLY_ID: u64 = 1; // In release functions SubMsg::reply_on_error(msg, RELEASE_FUNDS_REPLY_ID) // In reply handler match msg.id { RELEASE_FUNDS_REPLY_ID => handle_release_funds_reply(msg), _ => Err(ContractError::UnknownReplyId { id: msg.id }), }packages/std/src/os/adodb.rs (1)
Line range hint
89-106: Address TODO for denom validation before deployment.The comment "TODO: Add denom validation in future cosmwasm version" should be addressed as it affects fee validation during deployment. Proper denom validation is crucial for preventing issues with native token fees.
Would you like me to help implement denom validation or create a GitHub issue to track this?
packages/std/src/ado_contract/execute.rs (1)
Line range hint
1-147: Consider deployment-focused improvementsGiven this PR's focus on deployment, consider these enhancements:
Add deployment-specific validation in
instantiate:
- Validate
DEPLOYMENT_CHAINmatches the target chain- Verify
TEST_MNEMONICderived address matches the sender- Add deployment-specific events/attributes
Enhance
update_kernel_address:
- Add validation for
DEPLOYMENT_KERNEL_ADDRESS- Include chain-specific address validation
Would you like me to propose specific code changes for these improvements?
packages/std/src/deploy.rs (2)
5-6: Consider avoidingusestatements inside macro definitions to prevent potential scope issuesIncluding
usestatements within macro definitions can lead to unexpected behaviors due to scope pollution when the macro is expanded in different modules. To maintain clarity and avoid potential conflicts, it's better to fully qualify paths within the macro or require necessary imports in the scope where the macro is invoked.
33-33: Avoid unnecessary conversion with.to_string()If
$contract_idis already aStringor implementsInto<String>, calling.to_string()may be unnecessary. Consider returning$contract_iddirectly to prevent redundant operations.Apply this diff:
fn name() -> String { - $contract_id.to_string() + $contract_id }packages/deploy/src/adodb.rs (8)
40-41: Handle potential panic when sending Slack notificationUsing
unwrap()on thesend()method can cause a panic if the notification fails to send. It's better to handle the error gracefully.Apply this diff to handle the error:
-SlackNotification::ADOWarning(chain.chain_id.to_string(), invalid_contracts.clone()) - .send() - .unwrap(); +if let Err(e) = SlackNotification::ADOWarning(chain.chain_id.to_string(), invalid_contracts.clone()) + .send() { + log::error!("Failed to send ADODB warning notification: {:?}", e); +}
53-53: Handle potential panic when sending deployment start notificationSimilarly, using
unwrap()here can cause a panic. Handle the error to prevent unexpected crashes.Apply this diff:
-SlackNotification::ADODeploymentStarted(chain.chain_id.to_string(), valid_contracts.clone()) - .send() - .unwrap(); +if let Err(e) = SlackNotification::ADODeploymentStarted(chain.chain_id.to_string(), valid_contracts.clone()) + .send() { + log::error!("Failed to send deployment started notification: {:?}", e); +}
70-70: Handle potential panic when sending deployment completion notificationAgain, replace
unwrap()with error handling to prevent panics if the notification fails.Apply this diff:
-SlackNotification::ADODeploymentCompleted(chain.chain_id.to_string(), valid_contracts.clone()) - .send() - .unwrap(); +if let Err(e) = SlackNotification::ADODeploymentCompleted(chain.chain_id.to_string(), valid_contracts.clone()) + .send() { + log::error!("Failed to send deployment completed notification: {:?}", e); +}
35-37: Simplify collection of invalid contractsYou can simplify the code by removing unnecessary cloning and using
collectdirectly.Apply this diff to streamline the code:
-let invalid_contracts = contracts_to_deploy - .iter() - .filter(|name| !all_contracts.iter().any(|(n, _, _)| &n == name)) - .cloned() - .collect::<Vec<String>>(); +let invalid_contracts: Vec<String> = contracts_to_deploy + .iter() + .filter(|name| !all_contracts.iter().any(|(n, _, _)| n == *name)) + .map(|name| name.to_string()) + .collect();
47-49: Avoid unnecessary cloning when collecting valid contractsSimilar to the previous comment, you can simplify the collection of valid contracts.
Apply this diff:
-let valid_contracts = contracts_to_deploy - .iter() - .filter(|name| all_contracts.iter().any(|(n, _, _)| &n == name)) - .cloned() - .collect::<Vec<String>>(); +let valid_contracts: Vec<String> = contracts_to_deploy + .iter() + .filter(|name| all_contracts.iter().any(|(n, _, _)| n == *name)) + .map(|name| name.to_string()) + .collect();
58-60: Clarify the loop conditionThe loop condition can be refactored for better readability by checking if the contract should be deployed.
Apply this diff:
-if !contracts_to_deploy.is_empty() && !contracts_to_deploy.contains(&name) { +if contracts_to_deploy.is_empty() || contracts_to_deploy.contains(&name) { // Proceed with deployment +} else { + continue; }
63-65: Add error context when publishing contractsAdding context to errors can help with debugging if publishing fails.
Apply this diff to include error context:
-let code_id = upload(&daemon)?; -adodb.publish(name.clone(), code_id, version.clone(), None, None)?; +let code_id = upload(&daemon).map_err(|e| { + DeployError::from(e).context(format!("Failed to upload contract {} version {}", name, version)) +})?; +adodb.publish(name.clone(), code_id, version.clone(), None, None).map_err(|e| { + DeployError::from(e).context(format!("Failed to publish contract {} version {}", name, version)) +})?;Ensure that your
DeployErrortype supports error chaining and context.
72-72: Log the deployment summaryIt's helpful to log a summary of the deployed contracts after deployment completes.
Apply this diff:
Ok(()) +log::info!("Successfully deployed contracts: {:?}", deployed_contracts);packages/deploy/src/slack.rs (1)
26-29: Consider reusing the HTTP client for improved efficiencyCreating a new HTTP client with
Client::new()each time may be inefficient if sending multiple notifications. Consider initializing a singleClientinstance and reusing it, especially if notifications are sent in quick succession.packages/std/src/os/vfs.rs (4)
Line range hint
175-177: ImplementFromtrait forSubDirBoundfor better flexibilityYou have implemented
From<SubDirBound> for (Addr, String). Consider also implementingFrom<(Addr, String)> for SubDirBoundto allow easy construction ofSubDirBoundfrom tuples when needed.impl From<(Addr, String)> for SubDirBound { fn from(val: (Addr, String)) -> Self { SubDirBound { address: val.0, name: val.1, } } }
Line range hint
228-230: Add error context forInvalidAddressIn the
vfs_resolve_pathfunction, when querying the address, an error results in a genericContractError::InvalidAddress {}. For better debugging and user feedback, include the original error or additional context in theContractError.Consider modifying the error handling:
match addr { Ok(addr) => Ok(addr), Err(e) => Err(ContractError::InvalidAddress { error: Some(e.to_string()), }), }And update the
ContractError::InvalidAddressvariant to include an optionalerrorfield.
Line range hint
267-274: Optimize test case iteration usingIterator::anyIn your test cases, you're using a loop to check each case individually. Consider using
test_cases.iter().any(...)for a more functional approach, making the code more concise and idiomatic.Example refactor:
#[test] fn test_validate_component_name() { let all_pass = test_cases.iter().all(|test| { let res = validate_component_name(test.input.to_string()); res.is_ok() != test.should_err }); assert!(all_pass, "Not all test cases passed for validate_component_name"); }
Line range hint
353-355: Correct comment regarding untestable cases due to mock dependenciesThe comments mention that certain test cases cannot be validated correctly due to mock dependencies. Consider elaborating on this or finding alternative ways to test those cases to ensure comprehensive test coverage.
Perhaps use conditional compilation or feature flags to handle tests that require specific environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (63)
.github/workflows/deploy.yml(1 hunks)CHANGELOG.md(1 hunks)Cargo.toml(2 hunks)Makefile(2 hunks)contracts/data-storage/andromeda-counter/src/contract.rs(2 hunks)contracts/data-storage/andromeda-counter/src/interface.rs(1 hunks)contracts/data-storage/andromeda-primitive/Cargo.toml(1 hunks)contracts/data-storage/andromeda-primitive/src/contract.rs(2 hunks)contracts/data-storage/andromeda-primitive/src/interface.rs(1 hunks)contracts/data-storage/andromeda-primitive/src/lib.rs(1 hunks)contracts/finance/andromeda-splitter/Cargo.toml(1 hunks)contracts/finance/andromeda-splitter/src/interface.rs(1 hunks)contracts/finance/andromeda-timelock/Cargo.toml(1 hunks)contracts/finance/andromeda-timelock/src/contract.rs(2 hunks)contracts/finance/andromeda-timelock/src/interface.rs(1 hunks)contracts/finance/andromeda-timelock/src/lib.rs(1 hunks)contracts/finance/andromeda-validator-staking/Cargo.toml(1 hunks)contracts/finance/andromeda-validator-staking/src/interface.rs(1 hunks)contracts/finance/andromeda-validator-staking/src/lib.rs(1 hunks)contracts/finance/andromeda-vesting/Cargo.toml(1 hunks)contracts/finance/andromeda-vesting/src/contract.rs(2 hunks)contracts/finance/andromeda-vesting/src/interface.rs(1 hunks)contracts/finance/andromeda-vesting/src/lib.rs(1 hunks)contracts/fungible-tokens/andromeda-cw20-staking/Cargo.toml(1 hunks)contracts/fungible-tokens/andromeda-cw20-staking/src/contract.rs(2 hunks)contracts/fungible-tokens/andromeda-cw20-staking/src/interface.rs(1 hunks)contracts/fungible-tokens/andromeda-cw20-staking/src/lib.rs(1 hunks)contracts/fungible-tokens/andromeda-cw20/Cargo.toml(1 hunks)contracts/fungible-tokens/andromeda-cw20/src/contract.rs(2 hunks)contracts/fungible-tokens/andromeda-cw20/src/interface.rs(1 hunks)contracts/fungible-tokens/andromeda-cw20/src/lib.rs(1 hunks)contracts/non-fungible-tokens/andromeda-cw721/Cargo.toml(1 hunks)contracts/non-fungible-tokens/andromeda-cw721/src/contract.rs(2 hunks)contracts/non-fungible-tokens/andromeda-cw721/src/interface.rs(1 hunks)contracts/non-fungible-tokens/andromeda-cw721/src/lib.rs(1 hunks)contracts/os/andromeda-adodb/src/interface.rs(1 hunks)contracts/os/andromeda-economics/src/interface.rs(1 hunks)contracts/os/andromeda-ibc-registry/src/contract.rs(2 hunks)contracts/os/andromeda-ibc-registry/src/interface.rs(1 hunks)contracts/os/andromeda-kernel/src/contract.rs(1 hunks)contracts/os/andromeda-kernel/src/interface.rs(1 hunks)contracts/os/andromeda-vfs/src/interface.rs(1 hunks)packages/deploy/Cargo.toml(1 hunks)packages/deploy/src/adodb.rs(1 hunks)packages/deploy/src/chains.rs(1 hunks)packages/deploy/src/contract_interface.rs(1 hunks)packages/deploy/src/contracts.rs(1 hunks)packages/deploy/src/error.rs(1 hunks)packages/deploy/src/lib.rs(1 hunks)packages/deploy/src/main.rs(1 hunks)packages/deploy/src/os.rs(1 hunks)packages/deploy/src/slack.rs(1 hunks)packages/std/Cargo.toml(2 hunks)packages/std/src/ado_contract/execute.rs(1 hunks)packages/std/src/deploy.rs(1 hunks)packages/std/src/lib.rs(1 hunks)packages/std/src/os/adodb.rs(1 hunks)packages/std/src/os/ibc_registry.rs(1 hunks)packages/std/src/os/kernel.rs(3 hunks)packages/std/src/os/vfs.rs(1 hunks)scripts/attach_contract_versions.sh(1 hunks)scripts/build_all.sh(0 hunks)scripts/build_all_arm.sh(1 hunks)
💤 Files with no reviewable changes (1)
- scripts/build_all.sh
✅ Files skipped from review due to trivial changes (4)
- contracts/finance/andromeda-vesting/src/interface.rs
- packages/deploy/Cargo.toml
- packages/deploy/src/lib.rs
- scripts/build_all_arm.sh
🧰 Additional context used
🪛 actionlint
.github/workflows/deploy.yml
65-65: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 Shellcheck
scripts/attach_contract_versions.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🔇 Additional comments (74)
packages/deploy/src/error.rs (1)
1-2: LGTM! Imports are appropriate.
The imports are correctly chosen for implementing a custom error type.
contracts/fungible-tokens/andromeda-cw20/src/lib.rs (1)
8-9: LGTM! Good use of conditional compilation.
The conditional compilation for non-wasm32 targets is a good practice as it keeps the WASM binary lean while providing necessary tooling for deployment and testing.
contracts/finance/andromeda-vesting/src/lib.rs (1)
7-11: Well-structured conditional compilation for deployment support!
The addition of the interface module and VestingContract export with appropriate conditional compilation demonstrates good architectural design. This separation ensures that deployment-specific code doesn't bloat the WASM builds while making the contract available for deployment orchestration.
Let's verify the integration with deployment scripts:
contracts/non-fungible-tokens/andromeda-cw721/src/lib.rs (1)
8-8: LGTM!
The added empty line improves code readability by logically separating the test-related modules from the interface-related ones.
contracts/os/andromeda-vfs/src/interface.rs (2)
1-6: LGTM: Clean and focused imports
The imports are well-organized and specifically scoped to the required functionality from andromeda_std. The removal of direct contract imports in favor of the standardized interface approach is a good architectural decision.
10-10: Architectural improvement: Standardized contract interface
The shift to using contract_interface! macro is a significant improvement:
- Provides consistent interface definition across contracts
- Reduces boilerplate and potential for errors
- Better integrates with the deployment framework
However, ensure that the wasm file name "andromeda_vfs.wasm" matches your build configuration.
#!/bin/bash
# Description: Verify wasm file naming consistency in build scripts
echo "Checking Cargo.toml for package name..."
rg -l "name\s*=\s*['\"]andromeda[_-]vfs['\"]" -g "Cargo.toml"
echo "Checking build scripts for wasm file references..."
rg -l "andromeda_vfs\.wasm" -g "*.{sh,rs,toml}"contracts/finance/andromeda-splitter/src/interface.rs (1)
1-2: LGTM! Imports support deployment and migration capabilities
The addition of MigrateMsg and ADOMetadata imports from andromeda_std properly supports contract migration and metadata handling required for the deployment process.
contracts/data-storage/andromeda-counter/src/interface.rs (2)
6-6: LGTM! Contract interface declaration aligns with deployment requirements
The new contract_interface! macro provides a cleaner way to define the contract interface and explicitly specifies the WASM artifact name, which is essential for the deployment process.
#!/bin/bash
# Description: Verify the WASM filename exists in build configurations
echo "Checking for WASM filename in Cargo.toml files..."
fd Cargo.toml --type f --exec grep -l "andromeda_counter"
echo "Checking for WASM filename in deployment scripts..."
rg "andromeda_counter\.wasm" --type rust4-4: Verify the impact of CONTRACT_ID change
The CONTRACT_ID has been simplified from "counter_contract" to "counter". While this change aligns with naming conventions, we should verify that this doesn't break existing deployments or references.
✅ Verification successful
CONTRACT_ID change verified successfully
All references to counter_contract have been updated to counter, ensuring consistent updates across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for references to the old and new CONTRACT_ID
# to ensure all occurrences are updated consistently
echo "Checking for old CONTRACT_ID references..."
rg "counter_contract" --type rust
echo "Checking for new CONTRACT_ID references..."
rg "\"counter\"" --type rust
Length of output: 457
Script:
#!/bin/bash
# Description: Verify that there are no remaining references to the old CONTRACT_ID "counter_contract"
echo "Searching for any remaining references to 'counter_contract'..."
rg 'counter_contract' --type rust
Length of output: 176
contracts/fungible-tokens/andromeda-cw20/src/interface.rs (1)
4-4: LGTM! Contract ID follows convention
The contract ID is appropriately defined and follows the established naming pattern used across other Andromeda contracts.
contracts/os/andromeda-adodb/src/interface.rs (2)
1-6: LGTM! Well-structured imports
The imports are properly organized and aligned with the deployment-focused changes, making good use of the standard library.
8-8: Verify impact of CONTRACT_ID change
The CONTRACT_ID has been simplified from "adodb_contract" to "adodb". While this is cleaner, it's a breaking change that could affect existing deployments.
Let's verify the usage of this constant across the codebase:
✅ Verification successful
CONTRACT_ID change verified with no adverse impacts found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any references to the old contract ID that might need updating
# Search for the old contract ID
echo "Searching for old contract ID references..."
rg "adodb_contract" -l
# Search for current contract ID usage to ensure consistency
echo "Verifying new contract ID usage..."
rg "\"adodb\"" -l
# Look for potential contract ID usage in JSON/YAML configs
echo "Checking configuration files..."
find . -type f \( -name "*.json" -o -name "*.yaml" -o -name "*.toml" \) -exec grep -l "adodb_contract" {} \;
Length of output: 3871
contracts/finance/andromeda-timelock/src/interface.rs (2)
4-4: LGTM! Well-defined contract identifier
The constant is appropriately named and follows Rust conventions. Making it public allows for reuse in other modules.
6-6: Verify WASM artifact name and location
The contract interface references "andromeda_timelock.wasm". Let's verify this matches the build configuration.
✅ Verification successful
WASM artifact name and location verified
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify WASM artifact name in build configuration
# Check Cargo.toml for package name to confirm WASM artifact naming
rg -l "name.*=.*\"andromeda[_-]timelock\"" $(fd Cargo.toml)
# Check build scripts or makefiles for WASM artifact references
rg -l "andromeda_timelock\.wasm" $(fd -e rs -e toml -e Makefile)
Length of output: 28778
contracts/non-fungible-tokens/andromeda-cw721/src/interface.rs (3)
1-2: LGTM! Imports are well-structured and aligned with deployment requirements.
The imports correctly include all necessary types for contract execution, instantiation, querying, and migration, which are essential for the deployment infrastructure.
6-6: Verify WASM artifact naming consistency with build process.
The interface references "andromeda_cw721.wasm". We should ensure this matches the artifact name produced by the build process mentioned in the PR description (make build).
#!/bin/bash
# Description: Verify WASM file naming consistency
# Expected: Build configuration should produce "andromeda_cw721.wasm"
echo "Checking Cargo.toml for package name..."
rg -g 'Cargo.toml' 'name.*=.*"andromeda[-_]cw721"' -C 2
echo "Checking build scripts for WASM naming..."
rg -g 'Makefile' -g '*.sh' 'andromeda[-_]cw721\.wasm' -C 24-4: Verify CONTRACT_ID uniqueness across the protocol.
While the identifier seems appropriate, we should ensure it's unique across all Andromeda contracts to prevent deployment conflicts.
✅ Verification successful
Re-running CONTRACT_ID uniqueness verification.
Re-running CONTRACT_ID uniqueness verification with corrected scripts.
CONTRACT_ID "cw721" is unique and correctly identifies the NFT contract implementation.
The identifier is unique across the codebase and appropriately represents the CW721 NFT standard implementation in the Andromeda protocol.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for duplicate CONTRACT_ID definitions across the codebase
# Expected: Only one occurrence of CONTRACT_ID = "cw721"
echo "Searching for duplicate CONTRACT_ID definitions..."
rg -g '!target/' -g '*.rs' 'CONTRACT_ID.*=.*"cw721"' -C 2
echo "Searching for other uses of 'cw721' identifier..."
rg -g '!target/' 'cw721' -C 2
Length of output: 718
Script:
#!/bin/bash
# Description: Re-verify CONTRACT_ID uniqueness across the codebase
echo "Searching for duplicate CONTRACT_ID definitions..."
rg -g '!target/' -g '*.rs' 'pub\s+const\s+CONTRACT_ID\s*:\s*&str\s*=\s*"cw721";' -C 2
echo "Searching for other occurrences of 'cw721' identifier..."
rg -g '!target/' 'cw721' -C 2
Length of output: 121881
Script:
#!/bin/bash
# Description: Re-verify CONTRACT_ID uniqueness across the codebase
echo "Searching for duplicate CONTRACT_ID definitions..."
rg -g '!target/' -g '*.rs' 'pub\s+const\s+CONTRACT_ID\s*:\s*&str\s*=\s*"cw721";' -C 2
echo "Searching for other occurrences of 'cw721' identifier..."
rg -g '!target/' 'cw721' -C 2
Length of output: 121881
contracts/data-storage/andromeda-primitive/src/interface.rs (2)
1-2: LGTM! Imports are well-organized and necessary.
The imports correctly include all required types for contract messages and deployment metadata.
4-4: Verify CONTRACT_ID uniqueness across the codebase.
The constant is well-defined and follows naming conventions. However, let's ensure its uniqueness to prevent deployment conflicts.
✅ Verification successful
CONTRACT_ID is unique across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for duplicate CONTRACT_ID values across the codebase
# Expected: Only one occurrence of CONTRACT_ID = "primitive"
echo "Searching for duplicate CONTRACT_ID definitions..."
rg -l 'CONTRACT_ID.*=.*"primitive"'
echo "Searching for other uses of 'primitive' as an identifier..."
ast-grep --pattern 'const $_: &str = "primitive"'
Length of output: 395
contracts/data-storage/andromeda-primitive/src/lib.rs (1)
13-14: Verify interface usage in deployment scripts.
The public export of PrimitiveContract aligns with the deployment requirements mentioned in the PR objectives.
Let's verify how this interface is being used in the deployment scripts:
✅ Verification successful
Interface usage in deployment scripts verified successfully.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of PrimitiveContract in deployment code
# Expected: Should find references in deployment scripts showing how this contract is instantiated
# Search for PrimitiveContract usage
rg -l "PrimitiveContract"
# Look for deployment-related code using this contract
ast-grep --pattern 'PrimitiveContract::$_($$$)'
Length of output: 222
Script:
#!/bin/bash
# Description: Extract lines containing PrimitiveContract in contracts.rs with context
# Expected: Show how PrimitiveContract is instantiated or used in contracts.rs
rg -C 5 "PrimitiveContract" packages/deploy/src/contracts.rs
Length of output: 851
contracts/finance/andromeda-validator-staking/src/lib.rs (1)
10-13: LGTM! The interface module addition aligns with the deployment infrastructure.
The conditional compilation and interface module structure is well-designed for separating deployment-specific code from the core contract logic. This pattern ensures that the deployment interfaces don't bloat the final WASM binary.
Let's verify this pattern is consistently applied across other contracts:
contracts/os/andromeda-economics/src/interface.rs (2)
1-6: LGTM! Imports align with the new macro-based interface architecture.
The imports correctly include all necessary components for the new contract interface implementation, including migration support.
10-10: Verify WASM artifact naming in build system.
The contract interface macro specifies "andromeda_economics.wasm" as the output file. This needs to align with the build system configuration.
#!/bin/bash
# Description: Verify build system configuration for WASM artifact naming
# Check Cargo.toml for package name and other relevant settings
echo "Checking Cargo.toml configuration..."
fd Cargo.toml --type f --exec grep -l "andromeda[-_]economics" {} \; \
--exec cat {}
# Check build scripts or makefiles for WASM naming
echo "Checking build scripts..."
fd "Makefile|build\.rs" --type f --exec grep -l "andromeda[-_]economics" {} \; \
--exec cat {}contracts/fungible-tokens/andromeda-cw20-staking/src/lib.rs (1)
13-14: Verify interface usage in deployment scripts
The export of CW20StakingContract aligns with the PR's deployment objectives.
Let's verify how this interface is used in the deployment scripts:
packages/std/src/lib.rs (2)
14-14: LGTM: Consistent spacing maintained
The added empty line maintains good code organization by separating the conditional module declarations.
15-16: LGTM: Well-structured deployment module integration
The new deploy module is appropriately feature-gated and aligns with the PR's objective of introducing deployment capabilities. The public visibility allows other crates to utilize the deployment functionality when the feature is enabled.
Let's verify the feature declaration in Cargo.toml:
contracts/fungible-tokens/andromeda-cw20-staking/src/interface.rs (3)
1-2: LGTM! Imports are well-structured and complete.
The imports provide all necessary components for contract deployment, migration, and interaction within the Andromeda ecosystem.
6-10: Verify WASM binary name consistency with build configuration.
The interface references "andromeda_cw20_staking.wasm". Let's ensure this matches the build output name.
#!/bin/bash
# Description: Verify consistency of WASM binary naming in build configuration
# Check Cargo.toml for package name
echo "Checking package name in Cargo.toml:"
rg -g 'Cargo.toml' 'name.*=.*"andromeda[-_]cw20[-_]staking"'
# Check build scripts for WASM binary naming
echo -e "\nChecking build scripts for WASM binary naming:"
rg -g 'Makefile|*.sh' 'andromeda_cw20_staking\.wasm'4-4: Verify uniqueness of CONTRACT_ID across the Andromeda ecosystem.
While the ID format follows conventions, we should ensure it doesn't conflict with existing contracts.
contracts/os/andromeda-ibc-registry/src/interface.rs (1)
1-6: LGTM! Imports align with deployment requirements
The imports are properly scoped and include necessary components (MigrateMsg, ADOMetadata) for the deployment functionality being added.
contracts/finance/andromeda-validator-staking/src/interface.rs (3)
1-2: LGTM! Well-structured imports.
The imports are correctly scoped and include all necessary types for contract operations, migration support, and deployment integration.
6-10: Verify WASM artifact configuration and build integration.
The contract interface is well-defined, but we should verify:
- The WASM file name matches the build configuration
- The contract is properly integrated with the deployment system
#!/bin/bash
# Description: Verify contract build configuration and integration
# Check Cargo.toml for wasm build configuration
echo "Checking build configuration..."
rg "name.*=.*\"andromeda[_-]validator[_-]staking\"" -A 10 "Cargo.toml"
# Verify contract is included in deployment scripts
echo -e "\nChecking deployment integration..."
rg "validator-staking" --type yaml .github/workflows/4-4: Verify CONTRACT_ID uniqueness across the Andromeda ecosystem.
While the constant is well-defined, we should ensure this ID is unique across all Andromeda contracts to prevent potential conflicts during deployment.
✅ Verification successful
CONTRACT_ID is unique across the Andromeda ecosystem.
No duplicates found, ensuring no conflicts during deployment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for duplicate CONTRACT_ID definitions across the codebase
# Search for any other occurrences of the same CONTRACT_ID value
echo "Searching for duplicate CONTRACT_ID values..."
rg -l "CONTRACT_ID.*=.*\"validator-staking\"" --type rust
# Search for all CONTRACT_ID definitions to review naming patterns
echo -e "\nListing all CONTRACT_ID definitions for review..."
rg "pub const CONTRACT_ID.*=.*" --type rust
Length of output: 482
Script:
#!/bin/bash
# Description: Check for duplicate CONTRACT_ID definitions across the codebase
# Search for any other occurrences of the same CONTRACT_ID value
echo "Searching for duplicate CONTRACT_ID values..."
rg -l 'CONTRACT_ID\s*=\s*"validator-staking"' --type rust
# Search for all CONTRACT_ID definitions to review naming patterns
echo -e "\nListing all CONTRACT_ID definitions for review..."
rg 'pub const CONTRACT_ID\s*=\s*".*"' --type rust
Length of output: 327
contracts/finance/andromeda-timelock/Cargo.toml (1)
28-28: LGTM! Verify workspace inheritance configuration.
The addition of cw-orch as a non-wasm32 dependency is appropriate for the deployment infrastructure. The workspace inheritance ensures consistent versioning across packages.
Let's verify the workspace-level configuration:
✅ Verification successful
Workspace inheritance configuration verified.
All instances of cw-orch have consistent versioning across the workspace.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify cw-orch configuration in workspace
# Expected: cw-orch should be defined in the workspace dependencies with a specific version
# Check workspace Cargo.toml for cw-orch configuration
rg -A 5 'cw-orch.*=' Cargo.toml
# Verify consistent usage across other packages
fd Cargo.toml -x rg -l 'cw-orch.*='
Length of output: 1025
contracts/finance/andromeda-validator-staking/Cargo.toml (1)
32-32: LGTM! Verify workspace dependency.
The addition of cw-orch as a development-only dependency is appropriate for the deployment functionality. The workspace inheritance ensures version consistency across the project.
Let's verify the workspace dependency configuration:
✅ Verification successful
Approved! The cw-orch workspace dependency is correctly configured and consistent across all contracts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify cw-orch workspace dependency configuration
# Expected: Should find cw-orch in workspace Cargo.toml with a specific version
# Check workspace root Cargo.toml for cw-orch dependency
rg -A 1 'cw-orch.*=.*' ./Cargo.toml
# Verify consistent usage across other contract Cargo.toml files
fd Cargo.toml -t f | xargs rg 'cw-orch.*=.*'
Length of output: 1863
packages/deploy/src/chains.rs (1)
1-10: LGTM! Well-structured network configuration.
The network configuration follows Cosmos SDK standards with appropriate coin type (118) and address prefix format.
contracts/finance/andromeda-vesting/Cargo.toml (1)
31-31: LGTM! Addition of cw-orch follows best practices.
The conditional inclusion of cw-orch for non-wasm32 targets is correct and aligns with the PR's deployment infrastructure objectives.
Let's verify consistent cw-orch integration across other contracts:
✅ Verification successful
LGTM! Addition of cw-orch follows best practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if cw-orch is consistently added across contract Cargo.toml files
# Expected: All contract Cargo.toml files should have the same cw-orch dependency configuration
# Find all Cargo.toml files in contracts directory
fd Cargo.toml contracts/ --type f --exec rg -A 1 -B 1 "cw-orch.*workspace.*true" {}
Length of output: 1561
contracts/fungible-tokens/andromeda-cw20/Cargo.toml (1)
32-32: LGTM! The cw-orch dependency addition aligns with deployment requirements.
The addition of cw-orch as a non-wasm32 dependency is appropriate for the deployment infrastructure being implemented.
Let's verify the workspace dependency definition:
packages/std/Cargo.toml (1)
13-13: LGTM: New feature flag for deployment functionality
The addition of the deploy feature flag aligns well with the PR objectives and follows Rust's conventional approach for optional features.
contracts/non-fungible-tokens/andromeda-cw721/Cargo.toml (1)
39-39: LGTM! Verify workspace version.
The addition of cw-orch as a native-only dependency is appropriate for the deployment functionality being introduced. The workspace-level version specification ensures consistency.
Let's verify the workspace version specification:
✅ Verification successful
Workspace version specification is consistent.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if cw-orch version is properly specified in the workspace
# Expected: Find cw-orch in the root Cargo.toml workspace dependencies
# Check workspace dependencies
rg -A 5 'dependencies].*cw-orch' ./Cargo.toml
# Verify consistent usage across other contracts
fd -e toml . | xargs rg 'cw-orch.*workspace'
Length of output: 1595
contracts/data-storage/andromeda-primitive/Cargo.toml (1)
41-43: LGTM! The dependency changes align with deployment requirements.
The addition of cw-orch and making test dependencies optional are good improvements. The changes properly separate deployment tooling from the core contract code by using target-specific dependencies.
Let's verify the workspace-level dependency configuration:
contracts/os/andromeda-kernel/src/interface.rs (4)
15-29: LGTM! Comprehensive contract wrapper configuration
The contract wrapper is well-configured with all necessary handlers:
- Basic operations (execute, instantiate, query)
- Reply handling for async operations
- Migration support
- Complete IBC functionality
33-37: Verify WASM artifact path resolution
The wasm path resolution uses a workspace-relative path. Ensure the artifact naming and location are consistent with the build system.
#!/bin/bash
# Description: Verify WASM artifact consistency
# Check if the WASM file exists in expected locations
echo "Checking for WASM artifact..."
fd -t f "andromeda_kernel.wasm"
# Verify build configuration
echo "Checking build configuration..."
fd -t f "Cargo.toml" -x rg -l "andromeda[-_]kernel"1-8: Verify the impact of CONTRACT_ID change
The CONTRACT_ID has been changed from "kernel_contract" to "kernel". This change could affect existing deployments or references.
9-10: Document migration strategy and verify implementation
The contract now supports migrations through MigrateMsg. This is a significant change that requires:
- Clear documentation of the migration strategy
- Verification of the migrate handler implementation
packages/deploy/src/contract_interface.rs (1)
1-37: Verify chain-specific deployment configuration.
Given that this macro is part of the deployment package and the PR objectives mention chain-specific deployment (e.g., galileo-4), we should verify the integration with chain-specific configurations.
✅ Verification successful
Chain-specific deployment configuration verified.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify chain-specific configuration handling
# Check for chain-specific configuration files
echo "Checking for chain configuration files..."
fd -e json -e yaml -e toml . | rg -i "chain|network|deploy"
# Check for chain ID usage in deployment scripts
echo "Checking for chain ID usage in deployment scripts..."
rg -A 5 "DEPLOYMENT_CHAIN|chain_id|network_id"
Length of output: 20249
packages/deploy/src/contracts.rs (1)
1-14: LGTM! Imports are well-organized and necessary.
The imports are logically grouped and align with the deployment requirements.
Cargo.toml (2)
15-16: LGTM: Test directories properly organized
The addition of ibc-tests alongside tests-integration in the internal testing section maintains good organization of test directories.
32-34: LGTM: Deploy feature properly configured
The addition of the "deploy" feature to andromeda-std aligns with the PR's deployment functionality objectives. The feature flag allows for conditional compilation of deployment-related code, following Rust best practices.
Let's verify the feature usage across the codebase:
Makefile (1)
40-43: Clarify target dependencies and usage
Please clarify:
- Is this target a prerequisite for deployment?
- What's the relationship between this target and the existing
version-maptarget? - Should this be added as a dependency to other targets?
contracts/data-storage/andromeda-primitive/src/contract.rs (1)
3-3: LGTM: Required imports added correctly
The new imports for Reply and StdError are properly placed with other cosmwasm imports.
packages/std/src/os/ibc_registry.rs (2)
Line range hint 43-52: Verify consistent ExecuteFns implementation across contracts.
The ExecuteMsg enum correctly implements cw_orch::ExecuteFns. Let's ensure this pattern is consistently applied across other contract message types for proper deployment orchestration.
#!/bin/bash
# Check for consistent ExecuteFns implementation across contract messages
ast-grep --pattern 'enum ExecuteMsg {
$$$
}'43-43: LGTM! Verify cw-orchestrator dependency.
The addition of cw_orch::ExecuteFns derive macro aligns with the PR's deployment functionality requirements.
Let's verify the cw-orchestrator dependency configuration:
✅ Verification successful
Dependency cw_orch is correctly configured in Cargo.toml.
The addition of cw_orch::ExecuteFns derive macro aligns with the PR's deployment functionality requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if cw-orchestrator is properly configured in Cargo.toml
rg -l "cw-orchestrator.*=.*" | while read -r file; do
echo "=== $file ==="
rg "cw-orchestrator.*=.*" "$file"
done
Length of output: 353
Script:
#!/bin/bash
# Check if cw_orch is properly configured in Cargo.toml
rg -l "cw_orch.*=.*" | while read -r file; do
echo "=== $file ==="
rg "cw_orch.*=.*" "$file"
done
Length of output: 37
Script:
#!/bin/bash
# Search for dependencies starting with 'cw' in Cargo.toml
rg '^cw[_-].* = ".*"' Cargo.toml
Length of output: 186
CHANGELOG.md (1)
17-17: LGTM! The changelog entry is well-formatted and accurately reflects the PR objectives.
The new entry follows the Keep a Changelog format and properly documents the addition of deployment capabilities referenced in PR #616.
contracts/os/andromeda-kernel/src/contract.rs (2)
172-172: LGTM! Improved naming clarity.
The rename from Type to AdoType makes the query variant more specific and clearer in its purpose.
172-172: Verify the impact of the breaking change.
The rename of the query variant from Type to AdoType is a breaking change that requires verification.
Let's verify all usages of this query variant have been updated:
contracts/os/andromeda-ibc-registry/src/contract.rs (1)
17-18: LGTM!
The addition of the Reply import is appropriate for the new reply handling functionality.
contracts/finance/andromeda-timelock/src/contract.rs (2)
13-14: LGTM: Import additions for reply handling
The added imports (Reply, StdError, SubMsg) are appropriate for implementing submessage response handling.
254-263: 🛠️ Refactor suggestion
Improve reply handler implementation
The current implementation has several areas for improvement:
- The error handling could panic if
msg.resultis Ok - No handling of successful submessage responses
- Missing documentation about handled submessage types
Consider this safer implementation:
#[cfg_attr(not(feature = "library"), entry_point)]
+/// Handles responses from submessages
+/// Currently handles:
+/// - Fund release submessage responses
pub fn reply(_deps: DepsMut, _env: Env, msg: Reply) -> Result<Response, ContractError> {
- if msg.result.is_err() {
- return Err(ContractError::Std(StdError::generic_err(
- msg.result.unwrap_err(),
- )));
- }
+ match msg.result {
+ Err(err) => Err(ContractError::Std(StdError::generic_err(format!(
+ "Submessage error: {}", err
+ )))),
+ Ok(response) => {
+ // TODO: Handle successful submessage response data if needed
+ Ok(Response::new()
+ .add_attribute("action", "reply")
+ .add_attribute("status", "success"))
+ }
+ }
- Ok(Response::default())
}Let's verify which submessages this contract sends:
packages/std/src/os/adodb.rs (3)
Line range hint 186-234: LGTM! Robust version handling for deployment.
The ADOVersion implementation provides strong validation and version handling capabilities that will support the deployment process:
- Proper semantic version validation
- Support for "latest" version
- Clear separation of type and version components
- Comprehensive test coverage
18-18: LGTM! Verify cw-orch dependency for deployment automation.
The addition of #[derive(cw_orch::ExecuteFns)] aligns with the PR's objective of enhancing deployment automation using cw-orchestrator. This macro will auto-generate execution functions for contract operations, making the deployment process more streamlined.
Let's verify the cw-orch dependency and its version:
Line range hint 19-42: Ensure proper access controls for sensitive operations.
The ExecuteMsg variants handle critical operations that could impact protocol security and stability:
Publish: Controls code deployment and versioningUpdateActionFees: Affects protocol economicsUpdatePublisher: Modifies publishing permissions
Please verify that the auto-generated execution functions from cw-orch maintain proper access controls and validation checks.
Let's check for existing access control patterns:
contracts/data-storage/andromeda-counter/src/contract.rs (2)
4-4: LGTM: Import additions are appropriate
The addition of Reply and StdError types is necessary for the new reply handler implementation.
304-313: 🛠️ Refactor suggestion
Improve error handling and add documentation
The reply entry point implementation has several areas for improvement:
-
The error handling could be more robust:
- Line 308:
unwrap_err()could panic ifmsg.resultisOk - The function doesn't utilize the successful reply data
- Line 308:
-
Missing documentation about:
- Which contract operations trigger this reply handler
- What the expected reply data structure is
- How the reply affects the contract's state
Consider this safer implementation:
#[cfg_attr(not(feature = "library"), entry_point)]
+/// Handles replies from submessages
+///
+/// # Parameters
+///
+/// * `_deps` - Mutable dependencies
+/// * `_env` - Environment information
+/// * `msg` - Reply message containing the response from a submessage
pub fn reply(_deps: DepsMut, _env: Env, msg: Reply) -> Result<Response, ContractError> {
- if msg.result.is_err() {
- return Err(ContractError::Std(StdError::generic_err(
- msg.result.unwrap_err(),
- )));
- }
+ match msg.result {
+ cosmwasm_std::SubMsgResult::Ok(response) => {
+ // TODO: Handle successful reply data if needed
+ Ok(Response::default())
+ }
+ cosmwasm_std::SubMsgResult::Err(err) => {
+ Err(ContractError::Std(StdError::generic_err(err)))
+ }
+ }
- Ok(Response::default())
}Let's verify if there are any submessages that would trigger this reply handler:
Would you like me to help:
- Add unit tests for the reply handler?
- Document which operations trigger this handler?
packages/std/src/ado_contract/execute.rs (1)
Line range hint 142-147: Critical change in migration version check requires security review
The modification to allow migrations when storage_version <= version (instead of strictly <) has significant implications:
-
While this change enables re-running migrations for the same version, which can be useful for deployment scripts, it also:
- Could potentially allow unnecessary migrations that waste gas
- Might enable version downgrades if not properly guarded elsewhere
- May affect contract upgrade security
-
Please clarify:
- Why is this change necessary for the deployment system?
- Are there safeguards in place to prevent version downgrades?
- How does this interact with the new deployment scripts?
Let's verify the migration patterns in the codebase:
contracts/fungible-tokens/andromeda-cw20/src/contract.rs (1)
9-9: LGTM: Import additions are appropriate
The addition of Reply and StdError imports from cosmwasm_std aligns with the new reply handling functionality.
contracts/finance/andromeda-vesting/src/contract.rs (1)
11-12: LGTM: Import additions are appropriate
The new imports Reply and StdError are correctly added to support the reply handler implementation.
contracts/non-fungible-tokens/andromeda-cw721/src/contract.rs (1)
5-6: LGTM! Required imports added for reply handling.
The new imports Reply and StdError are correctly added to support the new reply functionality.
contracts/fungible-tokens/andromeda-cw20-staking/src/contract.rs (1)
10-11: LGTM! Required imports for reply functionality.
The added imports are necessary for the new reply entry point implementation.
packages/deploy/src/adodb.rs (1)
55-66: Verify that deployment only includes specified contracts
Ensure that only the intended contracts are deployed, especially when contracts_to_deploy is specified.
Run the following script to confirm that the correct contracts are being deployed:
packages/deploy/src/slack.rs (2)
7-32: LGTM!
The send_notification function is well-implemented and correctly handles sending Slack notifications based on the presence of the SLACK_WEBHOOK_URL environment variable.
50-79: Review Slack message formatting for better readability
The current message formatting uses code blocks and table-like structures, which may not render as expected in Slack. Consider revising the formatting to enhance readability. Utilizing Slack's Block Kit or adjusting the message format can improve the presentation of the notifications.
Since this involves visual formatting, please test the notifications to ensure they display correctly in Slack.
packages/std/src/os/vfs.rs (2)
152-152:
Incorrect use of #[schemars(regex = ...)] with constant names
The #[schemars(regex = "COMPONENT_NAME_REGEX")] attribute uses the string "COMPONENT_NAME_REGEX" as a regex pattern, but schemars expects the actual regex pattern as a string literal, not the name of a constant. This will result in the regex not functioning as intended during schema validation.
Apply this diff to fix the issue by embedding the actual regex pattern:
- #[schemars(regex = "COMPONENT_NAME_REGEX")]
+ #[schemars(regex = r"^[A-Za-z0-9.\-_]{2,80}$")]Repeat this change for all occurrences where constants are used in #[schemars(regex = ...)] attributes.
Likely invalid or redundant comment.
152-152: Ensure #[derive(cw_orch::ExecuteFns)] is compatible with the enum
The addition of #[derive(cw_orch::ExecuteFns)] to the ExecuteMsg enum aims to generate helper functions for contract execution. Ensure that all variants of ExecuteMsg are compatible with this derive macro and that it generates the expected functions without conflicts.
Run the following script to verify the generated functions:
Replace your_package_name with the actual package name. Ensure that the build succeeds and the helper functions are generated as expected.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
packages/deploy/src/adodb.rs (1)
19-22: Consider validating kernel address formatWhile
Addr::uncheckedis used for admin operations, basic format validation would improve robustness and prevent deployment issues.Consider adding basic address validation:
+ if !kernel_address.starts_with("andromeda") { + return Err(DeployError::InvalidAddress(kernel_address)); + } let kernel = KernelContract::new(daemon.clone()); kernel.set_address(&Addr::unchecked(kernel_address.clone()));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/deploy/src/adodb.rs(1 hunks)
🔇 Additional comments (5)
packages/deploy/src/adodb.rs (5)
1-15: LGTM! Well-structured imports and function signature.
The imports are comprehensive and the function signature is well-designed with clear parameters and proper error handling.
16-17: Handle potential panic from unwrap
This issue was previously identified. Using unwrap() on DaemonBuilder::new(chain.clone()).build() can cause a panic if the build fails. Consider using the ? operator to propagate errors instead.
23-27: LGTM! Proper ADODB contract setup.
The implementation correctly retrieves and sets up the ADODB contract with proper error handling.
44-50: LGTM! Efficient contract filtering implementation.
The filtering logic is well-implemented with proper use of iterators and minimal cloning.
55-71: LGTM! Solid deployment implementation.
The deployment logic is well-structured with proper tracking of deployed contracts.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (12)
packages/deploy/src/report.rs (2)
3-8: Add documentation and derive additional traits.The struct definition could benefit from the following improvements:
- Add documentation for the struct and its fields, especially clarifying what each element in the
contractstuple represents- Consider deriving additional useful traits
Apply this diff:
-#[derive(Serialize)] +#[derive(Debug, Clone, Serialize)] +/// Represents a deployment report for the Andromeda protocol pub struct DeploymentReport { + /// The ID of the chain where deployment occurred pub chain_id: String, + /// List of deployed contracts: (contract_name, contract_address, code_id) pub contracts: Vec<(String, String, u64)>, + /// Address of the kernel contract pub kernel_address: String, }
3-21: Consider adding Slack notification capability.Since the PR objectives mention Slack integration via
SLACK_WEBHOOK_URL, consider extending the reporting functionality to support Slack notifications.Consider adding a trait for report destinations:
/// Trait for different report destinations pub trait ReportDestination { fn send_report(&self, report: &DeploymentReport) -> Result<(), Box<dyn std::error::Error>>; } /// Implementation for Slack notifications pub struct SlackDestination { webhook_url: String, } impl ReportDestination for SlackDestination { fn send_report(&self, report: &DeploymentReport) -> Result<(), Box<dyn std::error::Error>> { // Implementation for sending to Slack // Use the webhook_url to post the report todo!() } } // Update DeploymentReport to use destinations impl DeploymentReport { pub fn send_to_destinations(&self, destinations: &[Box<dyn ReportDestination>]) -> Result<(), Box<dyn std::error::Error>> { for destination in destinations { destination.send_report(self)?; } Ok(()) } }packages/deploy/src/main.rs (2)
13-14: Add chain ID validationConsider validating the chain ID format to catch configuration errors early. This could prevent deployment attempts to incorrectly formatted or unsupported chains.
- let chain = env::var("DEPLOYMENT_CHAIN").expect("DEPLOYMENT_CHAIN must be set"); + let chain = env::var("DEPLOYMENT_CHAIN").expect("DEPLOYMENT_CHAIN must be set"); + if !is_valid_chain_id(&chain) { + eprintln!("Invalid or unsupported chain ID: {}", chain); + std::process::exit(1); + }
21-22: Improve environment variable parsing for DEPLOY_OSThe current parsing of DEPLOY_OS is case-sensitive after the to_lowercase() call. Consider using a more robust parsing approach.
- let should_deploy_os = env::var("DEPLOY_OS").unwrap_or_default().to_lowercase() == "true"; + let should_deploy_os = env::var("DEPLOY_OS") + .map(|v| v.trim().to_lowercase()) + .unwrap_or_default() + .parse::<bool>() + .unwrap_or(false);.github/workflows/deploy.yml (1)
85-90: Enhance deployment report handlingThe deployment report upload step could benefit from additional error handling and retention configuration.
- name: Upload Deployment Report uses: actions/upload-artifact@v3 with: name: deployment-report path: ./deployment-reports/ if-no-files-found: error + retention-days: 30 + - name: Process Deployment Report + if: always() + run: | + if [ -f "./deployment-reports/deployment.json" ]; then + echo "Deployment report generated successfully" + cat "./deployment-reports/deployment.json" + else + echo "::error::Deployment report not generated" + exit 1 + fipackages/deploy/src/adodb.rs (3)
12-16: Add function documentationAdd documentation to explain the function's purpose, parameters, return value, and possible errors. This will help users understand how to use this critical deployment functionality.
Add this documentation above the function:
+/// Deploys contracts to the specified blockchain through the Andromeda protocol. +/// +/// # Arguments +/// * `chain` - The chain identifier (e.g., "galileo-4") +/// * `kernel_address` - The address of the kernel contract +/// * `contracts` - Optional list of specific contracts to deploy. If None, deploys all contracts. +/// +/// # Returns +/// A vector of tuples containing (contract_name, version, code_id) for successfully deployed contracts. +/// +/// # Errors +/// Returns `DeployError` if: +/// * Chain initialization fails +/// * Kernel or ADODB operations fail +/// * Contract deployment fails pub fn deploy(
32-38: Log invalid contract names and improve validation performanceThe contract validation could be improved by:
- Logging invalid contract names for debugging
- Using a HashSet for better performance when checking contract existence
Consider this improvement:
+ use std::collections::HashSet; + let contracts_to_deploy = contracts.unwrap_or_default(); log::info!("Checking for invalid contracts"); + let valid_contract_names: HashSet<_> = all_contracts.iter().map(|(n, _, _)| n).collect(); let invalid_contracts = contracts_to_deploy .iter() - .filter(|name| !all_contracts.iter().any(|(n, _, _)| &n == name)) + .filter(|name| !valid_contract_names.contains(name)) .cloned() .collect::<Vec<String>>(); + + // Log invalid contracts for debugging + if !invalid_contracts.is_empty() { + log::warn!("Found invalid contracts: {:?}", invalid_contracts); + }
60-86: Enhance deployment robustness and performanceConsider the following improvements to the deployment process:
- Add retry mechanism for failed deployments
- Add progress tracking for long deployments
- Consider parallel deployment for better performance
Here's a suggested approach:
log::info!("Deploying contracts"); let mut deployed_contracts: Vec<(String, String, u64)> = vec![]; + let total_contracts = all_contracts.len(); + let mut processed = 0; for (name, version, upload) in all_contracts { + processed += 1; + log::info!("Processing contract {}/{}: {} {}", processed, total_contracts, name, version); + if !contracts_to_deploy.is_empty() && !contracts_to_deploy.contains(&name) { log::info!( "Skipping {} {} - not included in deploy list", name, version ); continue; } let versions = adodb.ado_versions(&name, None, None)?; log::info!("{} Versions: {:?}", name, versions); if versions.contains(&format!("{}@{}", name, version)) { log::info!("Skipping {} {} - already deployed", name, version); continue; } log::info!("Deploying {} {}", name, version); - let code_id = upload(&daemon)?; - let res = adodb.publish(name.clone(), code_id, version.clone(), None, None); - if let Err(e) = res { - log::error!("Error deploying {}: {}", name, e); - continue; - } + let max_retries = 3; + let mut retry_count = 0; + let result = loop { + if retry_count >= max_retries { + break Err(format!("Max retries ({}) exceeded", max_retries)); + } + match upload(&daemon) { + Ok(code_id) => { + match adodb.publish(name.clone(), code_id, version.clone(), None, None) { + Ok(_) => break Ok(code_id), + Err(e) => { + retry_count += 1; + log::warn!("Attempt {}/{}: Error publishing {}: {}", + retry_count, max_retries, name, e); + std::thread::sleep(std::time::Duration::from_secs(2)); + continue; + } + } + } + Err(e) => { + retry_count += 1; + log::warn!("Attempt {}/{}: Error uploading {}: {}", + retry_count, max_retries, name, e); + std::thread::sleep(std::time::Duration::from_secs(2)); + continue; + } + } + }; + + match result { + Ok(code_id) => deployed_contracts.push((name, version, code_id)), + Err(e) => { + log::error!("Failed to deploy {}: {}", name, e); + continue; + } + } - deployed_contracts.push((name, version, code_id)); }For parallel deployment, consider using
rayon:use rayon::prelude::*; // ... in the deployment loop let deployed_contracts: Vec<_> = all_contracts .par_iter() .filter_map(|(name, version, upload)| { // ... deployment logic ... Some((name.clone(), version.clone(), code_id)) }) .collect();packages/deploy/src/slack.rs (3)
34-42: Add documentation for enum variantsConsider adding documentation comments to describe each variant's purpose and the meaning of their associated data.
Example:
pub enum SlackNotification { + /// Notification sent when deployment starts + /// * `String` - Chain name + /// * `Option<String>` - Optional kernel address for updates DeploymentStarted(String, Option<String>), // ... document other variants similarly }
54-55: Extract timestamp formatting to a constant or configurationThe timestamp format is hardcoded in multiple places. Consider extracting it to a constant or configuration value.
+const TIMESTAMP_FORMAT: &str = "%Y-%m-%d %H:%M:%S"; + impl std::fmt::Display for SlackNotification { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { SlackNotification::DeploymentStarted(chain, kernel_address) => { - let timestamp = Local::now().format("%Y-%m-%d %H:%M:%S").to_string(); + let timestamp = Local::now().format(TIMESTAMP_FORMAT).to_string();Also applies to: 58-59
69-69: Split long line for better readabilityThe formatting logic for ADODeploymentCompleted is quite long. Consider splitting it into multiple lines or extracting the formatting logic to a separate method.
- write!(f, "✅ *ADO Library Deployment Completed*\n```\n| Chain | {} |\n| Contracts |\n| Name | Version | Code ID |\n{}\n```", chain, contracts.iter().map(|(name, version, code_id)| format!("| {:<14} | {:<12} | {:<12} |", name, version, code_id)).collect::<Vec<String>>().join("\n")) + let contract_lines = contracts + .iter() + .map(|(name, version, code_id)| { + format!("| {:<14} | {:<12} | {:<12} |", name, version, code_id) + }) + .collect::<Vec<String>>() + .join("\n"); + write!( + f, + "✅ *ADO Library Deployment Completed*\n```\n| Chain | {} |\n| Contracts |\n| Name | Version | Code ID |\n{}\n```", + chain, + contract_lines + )contracts/os/andromeda-adodb/src/contract.rs (1)
Line range hint
73-75: Consider documenting or implementing the commented-out queriesThere are commented-out query handlers for
UnpublishedCodeIdsandUnpublishedADOVersions. Since this PR introduces deployment functionality, these queries might be relevant for tracking deployment status.Would you like me to help implement these query handlers or create GitHub issues to track their implementation?
Also applies to: 77-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
.github/workflows/deploy.yml(1 hunks).gitignore(1 hunks)contracts/os/andromeda-adodb/src/contract.rs(1 hunks)packages/deploy/src/adodb.rs(1 hunks)packages/deploy/src/lib.rs(1 hunks)packages/deploy/src/main.rs(1 hunks)packages/deploy/src/report.rs(1 hunks)packages/deploy/src/slack.rs(1 hunks)packages/std/src/os/adodb.rs(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/deploy/src/lib.rs
- packages/std/src/os/adodb.rs
🧰 Additional context used
🪛 actionlint
.github/workflows/deploy.yml
65-65: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (7)
packages/deploy/src/report.rs (1)
11-20: 🛠️ Refactor suggestion
Improve robustness of file operations and add documentation.
The file writing implementation needs improvements in several areas:
- Hardcoded paths should be configurable
- Filenames should include timestamps to prevent overwrites
- Chain IDs should be sanitized when used in filenames
Apply this diff:
+use std::path::PathBuf;
+use chrono::Utc;
+
impl DeploymentReport {
+ /// Writes the deployment report to a JSON file in the specified directory.
+ /// The filename includes the chain ID and timestamp.
+ ///
+ /// # Errors
+ /// Returns an IO error if file operations fail
pub fn write_to_json(&self) -> Result<(), std::io::Error> {
- let reports_dir = "./deployment-reports";
+ let reports_dir = std::env::var("DEPLOYMENT_REPORTS_DIR")
+ .unwrap_or_else(|_| "./deployment-reports".to_string());
+ let reports_path = PathBuf::from(&reports_dir);
- if !std::path::Path::new(reports_dir).exists() {
- std::fs::create_dir_all(reports_dir)?;
+ if !reports_path.exists() {
+ std::fs::create_dir_all(&reports_path)?;
}
- let file_name = format!("./deployment-reports/{}_deployment.json", self.chain_id);
+
+ // Sanitize chain_id for use in filename
+ let safe_chain_id = self.chain_id.replace(|c: char| !c.is_ascii_alphanumeric(), "_");
+ let timestamp = Utc::now().format("%Y%m%d_%H%M%S");
+
+ let file_name = reports_path.join(
+ format!("{}_deployment_{}.json", safe_chain_id, timestamp)
+ );
+
let json_data = serde_json::to_string_pretty(&self)?;
- std::fs::write(file_name, json_data)?;
+ std::fs::write(&file_name, json_data)?;
Ok(())
}
}This implementation:
- Makes the reports directory configurable via environment variable
- Adds timestamps to prevent overwrites
- Sanitizes chain IDs for safe filename usage
- Uses proper path handling with
PathBuf - Adds documentation with error information
Let's verify the dependencies are available:
✅ Verification successful
Chrono dependency is correctly specified.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if chrono is available in Cargo.toml
rg 'chrono = ' -A 2
Length of output: 610
packages/deploy/src/main.rs (4)
1-11: LGTM! Good setup with logging and environment handling.
The initialization code follows good practices by setting up logging and environment variables.
17-19: Skip comment as it's already covered in past reviews
41-52: Skip comment as it's already covered in past reviews
54-65: Skip comment as it's already covered in past reviews
.github/workflows/deploy.yml (1)
14-21: LGTM: Well-structured input parameters
The new input parameters are well-defined and align perfectly with the PR objectives, providing flexibility for selective deployment of the OS and specific contracts.
packages/deploy/src/slack.rs (1)
61-64: ** Sanitize error messages in notifications**
The existing review comment about sanitizing error messages in notifications is still valid. Consider implementing the suggested changes to avoid potential security risks.
Also applies to: 71-73
Motivation
These changes add a new package that allows deployment of the operating system on to a given chain. Currently only
galileo-4is supported but support for other chains can be implemented easily in future.Implementation
A new
deploypackage was added. It utilisescw-orchestratorto upload and instantiate/migrate the OS and ADO contracts and complete any required setup steps. This can be run using the following:The following environment variables are required:
DEPLOYMENT_CHAIN- the chain id or name of the chainTEST_MNEMONIC- the mnemonic of the wallet you wish to useThe following environment variables are optional:
DEPLOYMENT_KERNEL_ADDRESS- the kernel address you wish to updateSLACK_WEBHOOK_URL- used for slack notificationsDEPLOY_CONTRACTS- comma separated list of contracts to deploySummary by CodeRabbit
New Features
deployfunction for managing contract deployments.replyfunction in various contracts for improved asynchronous message handling.DeploymentReportstruct for tracking deployment details.Bug Fixes
Documentation
CHANGELOG.mdto reflect the latest changes and improvements.Tests
Chores
chains.rsfile for blockchain network configurations.