From 6149e0378fe46c7f740153cc0274b6da1f194112 Mon Sep 17 00:00:00 2001 From: Karim Date: Thu, 28 Nov 2024 19:20:46 +0000 Subject: [PATCH] feat: make the upgrade process safer (#147) * feat: make the upgrade process safer * apply fmt * fix tests * update rust version * fix clippy --------- Co-authored-by: Olga Kunyavskaya --- .github/workflows/test.yml | 2 +- Cargo.toml | 2 +- near-plugins-derive/src/upgradable.rs | 4 +- .../access_controllable/rust-toolchain | 2 +- .../tests/contracts/ownable/rust-toolchain | 2 +- .../tests/contracts/pausable/rust-toolchain | 2 +- .../tests/contracts/upgradable/rust-toolchain | 2 +- .../contracts/upgradable_2/rust-toolchain | 2 +- .../upgradable_state_migration/rust-toolchain | 2 +- near-plugins-derive/tests/upgradable.rs | 48 ++++++++++++++++--- near-plugins/src/access_controllable.rs | 4 +- near-plugins/src/upgradable.rs | 3 ++ 12 files changed, 58 insertions(+), 17 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2956f3b..0da8c40 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -8,7 +8,7 @@ on: env: RUST_BACKTRACE: full - MSRV: 1.79.0 + MSRV: 1.80.0 jobs: tests: diff --git a/Cargo.toml b/Cargo.toml index 588f7ac..88a4562 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,7 @@ authors = ["Aurora Labs "] # An update of the MSRV requires updating: # - `rust-toolchain` files in `near-plugins-derive/tests/contracts/**` # - the toolchain installed in CI via the `toolchain` parameter of `actions-rs/toolchain@v1` -rust-version = "1.79.0" +rust-version = "1.80.0" description = "Ergonomic plugin system to extend NEAR contracts." license = "CC0-1.0" readme = "README.md" diff --git a/near-plugins-derive/src/upgradable.rs b/near-plugins-derive/src/upgradable.rs index c1397a6..b46bd65 100644 --- a/near-plugins-derive/src/upgradable.rs +++ b/near-plugins-derive/src/upgradable.rs @@ -216,7 +216,7 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { let promise = ::near_sdk::Promise::new(::near_sdk::env::current_account_id()) .deploy_contract(code); match function_call_args { - None => promise, + None => promise.function_call("up_verify_state".to_owned(), vec![], near_sdk::NearToken::from_yoctonear(0), near_sdk::Gas::from_tgas(2)), Some(args) => { // Execute the `DeployContract` and `FunctionCall` actions in a batch // transaction to make a failure of the function call roll back the code @@ -226,6 +226,8 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { } } + fn up_verify_state(&self) {} + #[#cratename::access_control_any(roles(#(#acl_roles_duration_initializers),*))] fn up_init_staging_duration(&mut self, staging_duration: ::near_sdk::Duration) { ::near_sdk::require!(self.up_get_duration(__UpgradableStorageKey::StagingDuration).is_none(), "Upgradable: staging duration was already initialized"); diff --git a/near-plugins-derive/tests/contracts/access_controllable/rust-toolchain b/near-plugins-derive/tests/contracts/access_controllable/rust-toolchain index ad58735..14f0527 100644 --- a/near-plugins-derive/tests/contracts/access_controllable/rust-toolchain +++ b/near-plugins-derive/tests/contracts/access_controllable/rust-toolchain @@ -1,4 +1,4 @@ [toolchain] -channel = "1.79.0" +channel = "1.80.0" components = ["clippy", "rustfmt"] targets = [ "wasm32-unknown-unknown" ] diff --git a/near-plugins-derive/tests/contracts/ownable/rust-toolchain b/near-plugins-derive/tests/contracts/ownable/rust-toolchain index ad58735..14f0527 100644 --- a/near-plugins-derive/tests/contracts/ownable/rust-toolchain +++ b/near-plugins-derive/tests/contracts/ownable/rust-toolchain @@ -1,4 +1,4 @@ [toolchain] -channel = "1.79.0" +channel = "1.80.0" components = ["clippy", "rustfmt"] targets = [ "wasm32-unknown-unknown" ] diff --git a/near-plugins-derive/tests/contracts/pausable/rust-toolchain b/near-plugins-derive/tests/contracts/pausable/rust-toolchain index ad58735..14f0527 100644 --- a/near-plugins-derive/tests/contracts/pausable/rust-toolchain +++ b/near-plugins-derive/tests/contracts/pausable/rust-toolchain @@ -1,4 +1,4 @@ [toolchain] -channel = "1.79.0" +channel = "1.80.0" components = ["clippy", "rustfmt"] targets = [ "wasm32-unknown-unknown" ] diff --git a/near-plugins-derive/tests/contracts/upgradable/rust-toolchain b/near-plugins-derive/tests/contracts/upgradable/rust-toolchain index ad58735..14f0527 100644 --- a/near-plugins-derive/tests/contracts/upgradable/rust-toolchain +++ b/near-plugins-derive/tests/contracts/upgradable/rust-toolchain @@ -1,4 +1,4 @@ [toolchain] -channel = "1.79.0" +channel = "1.80.0" components = ["clippy", "rustfmt"] targets = [ "wasm32-unknown-unknown" ] diff --git a/near-plugins-derive/tests/contracts/upgradable_2/rust-toolchain b/near-plugins-derive/tests/contracts/upgradable_2/rust-toolchain index ad58735..14f0527 100644 --- a/near-plugins-derive/tests/contracts/upgradable_2/rust-toolchain +++ b/near-plugins-derive/tests/contracts/upgradable_2/rust-toolchain @@ -1,4 +1,4 @@ [toolchain] -channel = "1.79.0" +channel = "1.80.0" components = ["clippy", "rustfmt"] targets = [ "wasm32-unknown-unknown" ] diff --git a/near-plugins-derive/tests/contracts/upgradable_state_migration/rust-toolchain b/near-plugins-derive/tests/contracts/upgradable_state_migration/rust-toolchain index ad58735..14f0527 100644 --- a/near-plugins-derive/tests/contracts/upgradable_state_migration/rust-toolchain +++ b/near-plugins-derive/tests/contracts/upgradable_state_migration/rust-toolchain @@ -1,4 +1,4 @@ [toolchain] -channel = "1.79.0" +channel = "1.80.0" components = ["clippy", "rustfmt"] targets = [ "wasm32-unknown-unknown" ] diff --git a/near-plugins-derive/tests/upgradable.rs b/near-plugins-derive/tests/upgradable.rs index 6c2fde3..34da63d 100644 --- a/near-plugins-derive/tests/upgradable.rs +++ b/near-plugins-derive/tests/upgradable.rs @@ -438,7 +438,7 @@ async fn test_deploy_code_without_delay() -> anyhow::Result<()> { let setup = Setup::new(worker.clone(), Some(dao.id().clone()), None).await?; // Stage some code. - let code = vec![1, 2, 3]; + let code = common::repo::compile_project(Path::new(PROJECT_PATH), "upgradable").await?; let res = setup .upgradable_contract .up_stage_code(&dao, code.clone()) @@ -463,7 +463,7 @@ async fn test_deploy_code_with_hash_success() -> anyhow::Result<()> { let setup = Setup::new(worker.clone(), Some(dao.id().clone()), None).await?; // Stage some code. - let code = vec![1, 2, 3]; + let code = common::repo::compile_project(Path::new(PROJECT_PATH), "upgradable").await?; let res = setup .upgradable_contract .up_stage_code(&dao, code.clone()) @@ -491,7 +491,7 @@ async fn test_deploy_code_with_hash_invalid_hash() -> anyhow::Result<()> { let setup = Setup::new(worker.clone(), Some(dao.id().clone()), None).await?; // Stage some code. - let code = vec![1, 2, 3]; + let code = common::repo::compile_project(Path::new(PROJECT_PATH), "upgradable").await?; let res = setup .upgradable_contract .up_stage_code(&dao, code.clone()) @@ -647,6 +647,42 @@ async fn test_deploy_code_with_migration_failure_rollback() -> anyhow::Result<() Ok(()) } +/// Deploys a new version of the contract with missed migration +/// Verifies the failure rolls back the deployment, i.e. the initial +/// code remains active. +#[tokio::test] +async fn test_deploy_code_with_missed_migration() -> anyhow::Result<()> { + let worker = near_workspaces::sandbox().await?; + let dao = worker.dev_create_account().await?; + let setup = Setup::new(worker.clone(), Some(dao.id().clone()), None).await?; + + // Compile the other version of the contract and stage its code. + let code = common::repo::compile_project( + Path::new(PROJECT_PATH_STATE_MIGRATION), + "upgradable_state_migration", + ) + .await?; + let res = setup + .upgradable_contract + .up_stage_code(&dao, code.clone()) + .await?; + assert_success_with_unit_return(res); + setup.assert_staged_code(Some(&code)).await; + + // Deploy staged code + let res = setup + .upgradable_contract + .up_deploy_code(&dao, convert_code_to_deploy_hash(&code), None) + .await?; + assert_failure_with(res, "Cannot deserialize the contract state"); + + // Verify `code` wasn't deployed by calling a function that is defined only in the initial + // contract but not in the contract corresponding to the `code`. + setup.assert_is_set_up(&setup.unauth_account).await; + + Ok(()) +} + /// Deploys staged code in a batch transaction with two function call actions: /// /// 1. `up_deploy_code` with a function call to a migration method that fails @@ -676,7 +712,7 @@ async fn test_deploy_code_in_batch_transaction_pitfall() -> anyhow::Result<()> { // Construct the function call actions to be executed in a batch transaction. // Note that we are attaching a call to `migrate_with_failure`, which will fail. let fn_call_deploy = near_workspaces::operations::Function::new("up_deploy_code") - .args_json(json!({ + .args_json(json!({ "hash": convert_code_to_deploy_hash(&code), "function_call_args": FunctionCallArgs { function_name: "migrate_with_failure".to_string(), @@ -734,7 +770,7 @@ async fn test_deploy_code_with_delay() -> anyhow::Result<()> { .await?; // Stage some code. - let code = vec![1, 2, 3]; + let code = common::repo::compile_project(Path::new(PROJECT_PATH), "upgradable").await?; let res = setup .upgradable_contract .up_stage_code(&dao, code.clone()) @@ -767,7 +803,7 @@ async fn test_deploy_code_with_delay_failure_too_early() -> anyhow::Result<()> { .await?; // Stage some code. - let code = vec![1, 2, 3]; + let code = common::repo::compile_project(Path::new(PROJECT_PATH), "upgradable").await?; let res = setup .upgradable_contract .up_stage_code(&dao, code.clone()) diff --git a/near-plugins/src/access_controllable.rs b/near-plugins/src/access_controllable.rs index 99ff67c..74a6d84 100644 --- a/near-plugins/src/access_controllable.rs +++ b/near-plugins/src/access_controllable.rs @@ -192,7 +192,7 @@ pub trait AccessControllable { /// } /// } /// ``` - /// + /// /// ```json /// { /// "standard":"AccessControllable", @@ -356,7 +356,7 @@ pub trait AccessControllable { /// /// * Get roles with [`Self::acl_get_roles`]. /// * Get (a subset) of permissioned accounts with [`Self::acl_get_super_admins`], - /// [`Self::acl_get_admins`], or [`Self::acl_get_grantees`]. + /// [`Self::acl_get_admins`], or [`Self::acl_get_grantees`]. /// /// [gas limit]: https://github.com/near/nearcore/pull/4381 fn acl_get_permissioned_accounts(&self) -> PermissionedAccounts; diff --git a/near-plugins/src/upgradable.rs b/near-plugins/src/upgradable.rs index b67fa5e..2cca948 100644 --- a/near-plugins/src/upgradable.rs +++ b/near-plugins/src/upgradable.rs @@ -194,6 +194,9 @@ pub trait Upgradable { /// attribute. The example contract (accessible via the `README`) shows how access control roles /// can be defined and passed on to the `Upgradable` macro. fn up_apply_update_staging_duration(&mut self); + + /// Panic if state is not valid. + fn up_verify_state(&self); } #[near(serializers = [json])]