From 5564884c30c977e9f01e0dd1d8bf0093dccbe874 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 25 Oct 2023 11:44:53 +0200 Subject: [PATCH 01/10] Factor out up_hash_code() --- near-plugins-derive/src/upgradable.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/near-plugins-derive/src/upgradable.rs b/near-plugins-derive/src/upgradable.rs index be573e7..eebba51 100644 --- a/near-plugins-derive/src/upgradable.rs +++ b/near-plugins-derive/src/upgradable.rs @@ -140,6 +140,13 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { fn up_set_staging_duration_unchecked(&self, staging_duration: near_sdk::Duration) { self.up_storage_write(__UpgradableStorageKey::StagingDuration, &staging_duration.try_to_vec().unwrap()); } + + /// Computes the `sha256` hash of `code` and panics if the conversion to `CryptoHash` fails. + fn up_hash_code(code: &[u8]) -> ::near_sdk::CryptoHash { + let hash = near_sdk::env::sha256(code); + std::convert::TryInto::try_into(hash) + .expect("sha256 should convert to CryptoHash") + } } #[near_bindgen] @@ -176,7 +183,7 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { fn up_staged_code_hash(&self) -> Option<::near_sdk::CryptoHash> { self.up_staged_code() - .map(|code| std::convert::TryInto::try_into(near_sdk::env::sha256(code.as_ref())).unwrap()) + .map(|code| self.up_hash_code(code.as_ref())) } #[#cratename::access_control_any(roles(#(#acl_roles_code_deployers),*))] From 021101430cbbc9bf1a580ad5646cc647b3f62e47 Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 30 Oct 2023 17:59:50 +0100 Subject: [PATCH 02/10] Add hash parameter to up_deploy_code --- near-plugins-derive/src/upgradable.rs | 17 +++++++++++++++-- near-plugins/src/upgradable.rs | 18 +++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/near-plugins-derive/src/upgradable.rs b/near-plugins-derive/src/upgradable.rs index eebba51..d7f2475 100644 --- a/near-plugins-derive/src/upgradable.rs +++ b/near-plugins-derive/src/upgradable.rs @@ -183,11 +183,11 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { fn up_staged_code_hash(&self) -> Option<::near_sdk::CryptoHash> { self.up_staged_code() - .map(|code| self.up_hash_code(code.as_ref())) + .map(|code| Self::up_hash_code(code.as_ref())) } #[#cratename::access_control_any(roles(#(#acl_roles_code_deployers),*))] - fn up_deploy_code(&mut self, function_call_args: Option<#cratename::upgradable::FunctionCallArgs>) -> near_sdk::Promise { + fn up_deploy_code(&mut self, hash: Option, function_call_args: Option<#cratename::upgradable::FunctionCallArgs>) -> near_sdk::Promise { let staging_timestamp = self.up_get_timestamp(__UpgradableStorageKey::StagingTimestamp) .unwrap_or_else(|| ::near_sdk::env::panic_str("Upgradable: staging timestamp isn't set")); @@ -202,6 +202,19 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { } let code = self.up_staged_code().unwrap_or_else(|| ::near_sdk::env::panic_str("Upgradable: No staged code")); + if let Some(hash) = hash { + let expected_hash = ::near_sdk::base64::encode(Self::up_hash_code(code.as_ref())); + if hash != expected_hash { + near_sdk::env::panic_str( + format!( + "Upgradable: Cannot deploy due to wrong hash: expected hash: {}", + expected_hash, + ) + .as_str(), + ) + } + } + let promise = near_sdk::Promise::new(near_sdk::env::current_account_id()) .deploy_contract(code); match function_call_args { diff --git a/near-plugins/src/upgradable.rs b/near-plugins/src/upgradable.rs index 356c1cd..621d3b7 100644 --- a/near-plugins/src/upgradable.rs +++ b/near-plugins/src/upgradable.rs @@ -102,6 +102,18 @@ pub trait Upgradable { /// Allows an authorized account to deploy the staged code. It panics if no code is staged. /// + /// # Verifying the hash of staged code + /// + /// Some workflows (e.g. when a DAO interacts with an `Upgradable` contract) are facilitated if + /// deployment succeeds only in case the hash of staged code corresponds to a given hash. This + /// behavior can be enabled with the `hash` parameter. In case it is `Some(h)`, the deployment + /// succeeds only if `h` equals the base64 encoded string of the staged code's `sha256` hash. In + /// particular, the encoding according to [`near_sdk::base64::encode`] is expected. Note that + /// `near_sdk` uses a rather dated version of the `base64` crate whose API differs from current + /// versions. + /// + /// Otherwise, if `hash` equals `None`, this verification step is skipped. + /// /// # Attaching a function call /// /// If `function_call_args` are provided, code is deployed in a batch promise that contains the @@ -143,7 +155,11 @@ pub trait Upgradable { /// [asynchronous design]: https://docs.near.org/concepts/basics/transactions/overview /// [state migration]: https://docs.near.org/develop/upgrade#migrating-the-state /// [storage staked]: https://docs.near.org/concepts/storage/storage-staking#btw-you-can-remove-data-to-unstake-some-tokens - fn up_deploy_code(&mut self, function_call_args: Option) -> Promise; + fn up_deploy_code( + &mut self, + hash: Option, + function_call_args: Option, + ) -> Promise; /// Initializes the duration of the delay for deploying the staged code. It defaults to zero if /// code is staged before the staging duration is initialized. Once the staging duration has From 0b2a2cb53d58518f48718f9de616e90c0e5b1113 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 31 Oct 2023 14:48:36 +0100 Subject: [PATCH 03/10] Update existing tests --- .../tests/common/upgradable_contract.rs | 2 ++ near-plugins-derive/tests/upgradable.rs | 36 ++++++++++++++----- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/near-plugins-derive/tests/common/upgradable_contract.rs b/near-plugins-derive/tests/common/upgradable_contract.rs index 16d39d8..17ea199 100644 --- a/near-plugins-derive/tests/common/upgradable_contract.rs +++ b/near-plugins-derive/tests/common/upgradable_contract.rs @@ -71,11 +71,13 @@ impl UpgradableContract { pub async fn up_deploy_code( &self, caller: &Account, + hash: Option, function_call_args: Option, ) -> workspaces::Result { caller .call(self.contract.id(), "up_deploy_code") .args_json(json!({ + "hash": hash, "function_call_args": function_call_args, })) .max_gas() diff --git a/near-plugins-derive/tests/upgradable.rs b/near-plugins-derive/tests/upgradable.rs index fa85fd9..a92ed30 100644 --- a/near-plugins-derive/tests/upgradable.rs +++ b/near-plugins-derive/tests/upgradable.rs @@ -433,12 +433,18 @@ async fn test_deploy_code_without_delay() -> anyhow::Result<()> { setup.assert_staged_code(Some(code)).await; // Deploy staged code. - let res = setup.upgradable_contract.up_deploy_code(&dao, None).await?; + let res = setup + .upgradable_contract + .up_deploy_code(&dao, None, None) + .await?; assert_success_with_unit_return(res); Ok(()) } +// TODO add test_deploy_code_with_hash_success +// TODO add test_deploy_code_with_hash_failure + /// Verifies the upgrade was successful by calling a method that's available only on the upgraded /// contract. Ensures the new contract can be deployed and state remains valid without /// explicit state migration. @@ -462,7 +468,10 @@ async fn test_deploy_code_and_call_method() -> anyhow::Result<()> { setup.assert_staged_code(Some(code)).await; // Deploy staged code. - let res = setup.upgradable_contract.up_deploy_code(&dao, None).await?; + let res = setup + .upgradable_contract + .up_deploy_code(&dao, None, None) + .await?; assert_success_with_unit_return(res); // The newly deployed contract defines the function `is_upgraded`. Calling it successfully @@ -507,7 +516,7 @@ async fn test_deploy_code_with_migration() -> anyhow::Result<()> { }; let res = setup .upgradable_contract - .up_deploy_code(&dao, Some(function_call_args)) + .up_deploy_code(&dao, None, Some(function_call_args)) .await?; assert_success_with_unit_return(res); @@ -550,7 +559,7 @@ async fn test_deploy_code_with_migration_failure_rollback() -> anyhow::Result<() }; let res = setup .upgradable_contract - .up_deploy_code(&dao, Some(function_call_args)) + .up_deploy_code(&dao, None, Some(function_call_args)) .await?; assert_failure_with(res, "Failing migration on purpose"); @@ -658,7 +667,10 @@ async fn test_deploy_code_with_delay() -> anyhow::Result<()> { fast_forward_beyond(&worker, staging_duration).await; // Deploy staged code. - let res = setup.upgradable_contract.up_deploy_code(&dao, None).await?; + let res = setup + .upgradable_contract + .up_deploy_code(&dao, None, None) + .await?; assert_success_with_unit_return(res); Ok(()) @@ -688,7 +700,10 @@ async fn test_deploy_code_with_delay_failure_too_early() -> anyhow::Result<()> { fast_forward_beyond(&worker, sdk_duration_from_secs(1)).await; // Verify trying to deploy staged code fails. - let res = setup.upgradable_contract.up_deploy_code(&dao, None).await?; + let res = setup + .upgradable_contract + .up_deploy_code(&dao, None, None) + .await?; assert_failure_with(res, ERR_MSG_DEPLOY_CODE_TOO_EARLY); // Verify `code` wasn't deployed by calling a function that is defined only in the initial @@ -717,7 +732,7 @@ async fn test_deploy_code_permission_failure() -> anyhow::Result<()> { // call this method. let res = setup .upgradable_contract - .up_deploy_code(&setup.unauth_account, None) + .up_deploy_code(&setup.unauth_account, None, None) .await?; assert_insufficient_acl_permissions( res, @@ -756,7 +771,10 @@ async fn test_deploy_code_empty_failure() -> anyhow::Result<()> { // The staging timestamp is set when staging code and removed when unstaging code. So when there // is no code staged, there is no staging timestamp. Hence the error message regarding a missing // staging timestamp is expected. - let res = setup.upgradable_contract.up_deploy_code(&dao, None).await?; + let res = setup + .upgradable_contract + .up_deploy_code(&dao, None, None) + .await?; assert_failure_with(res, ERR_MSG_NO_STAGING_TS); Ok(()) @@ -1004,7 +1022,7 @@ async fn test_acl_permission_scope() -> anyhow::Result<()> { // deploy code. let res = setup .upgradable_contract - .up_deploy_code(&setup.unauth_account, None) + .up_deploy_code(&setup.unauth_account, None, None) .await?; assert_insufficient_acl_permissions( res, From 986135c613c08cda3dcb3cc959d2d79fae2b8cce Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 1 Nov 2023 12:08:15 +0100 Subject: [PATCH 04/10] Test up_deploy_code with hash parameter --- near-plugins-derive/tests/upgradable.rs | 66 ++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/near-plugins-derive/tests/upgradable.rs b/near-plugins-derive/tests/upgradable.rs index a92ed30..c4da0f6 100644 --- a/near-plugins-derive/tests/upgradable.rs +++ b/near-plugins-derive/tests/upgradable.rs @@ -204,6 +204,13 @@ fn convert_code_to_crypto_hash(code: &[u8]) -> CryptoHash { .expect("Code should be converted to CryptoHash") } +/// Computes the hash `code` according the to requirements of the `hash` parameter of +/// `Upgradable::up_deploy_code`. +fn convert_code_to_deploy_hash(code: &[u8]) -> String { + let hash = near_sdk::env::sha256(code); + near_sdk::base64::encode(hash) +} + /// Smoke test of contract setup. #[tokio::test] async fn test_setup() -> anyhow::Result<()> { @@ -442,8 +449,63 @@ async fn test_deploy_code_without_delay() -> anyhow::Result<()> { Ok(()) } -// TODO add test_deploy_code_with_hash_success -// TODO add test_deploy_code_with_hash_failure +#[tokio::test] +async fn test_deploy_code_with_hash_success() -> anyhow::Result<()> { + let worker = workspaces::sandbox().await?; + let dao = worker.dev_create_account().await?; + let setup = Setup::new(worker.clone(), Some(dao.id().clone()), None).await?; + + // Stage some code. + let code = vec![1, 2, 3]; + let res = setup + .upgradable_contract + .up_stage_code(&dao, code.clone()) + .await?; + assert_success_with_unit_return(res); + setup.assert_staged_code(Some(code.clone())).await; + + // Deploy staged code. + let hash = convert_code_to_deploy_hash(&code); + let res = setup + .upgradable_contract + .up_deploy_code(&dao, Some(hash), None) + .await?; + assert_success_with_unit_return(res); + + Ok(()) +} + +/// Verifies failure of `up_deploy_code(Some(hash), ...)` when `hash` does not correspond to the +/// hash of staged code. +#[tokio::test] +async fn test_deploy_code_with_hash_invalid_hash() -> anyhow::Result<()> { + let worker = workspaces::sandbox().await?; + let dao = worker.dev_create_account().await?; + let setup = Setup::new(worker.clone(), Some(dao.id().clone()), None).await?; + + // Stage some code. + let code = vec![1, 2, 3]; + let res = setup + .upgradable_contract + .up_stage_code(&dao, code.clone()) + .await?; + assert_success_with_unit_return(res); + setup.assert_staged_code(Some(code.clone())).await; + + // Deployment is aborted if an invalid hash is provided. + let res = setup + .upgradable_contract + .up_deploy_code(&dao, Some("invalid_hash".to_owned()), None) + .await?; + let actual_hash = convert_code_to_deploy_hash(&code); + let expected_err = format!( + "Upgradable: Cannot deploy due to wrong hash: expected hash: {}", + actual_hash + ); + assert_failure_with(res, &expected_err); + + Ok(()) +} /// Verifies the upgrade was successful by calling a method that's available only on the upgraded /// contract. Ensures the new contract can be deployed and state remains valid without From 974c3412551c26e029c2abecedfff5c5a954121a Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 13 Nov 2023 16:47:39 +0100 Subject: [PATCH 05/10] Fix crate name --- near-plugins-derive/tests/upgradable.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/near-plugins-derive/tests/upgradable.rs b/near-plugins-derive/tests/upgradable.rs index 5489841..1966e24 100644 --- a/near-plugins-derive/tests/upgradable.rs +++ b/near-plugins-derive/tests/upgradable.rs @@ -457,7 +457,7 @@ async fn test_deploy_code_without_delay() -> anyhow::Result<()> { #[tokio::test] async fn test_deploy_code_with_hash_success() -> anyhow::Result<()> { - let worker = workspaces::sandbox().await?; + 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?; @@ -485,7 +485,7 @@ async fn test_deploy_code_with_hash_success() -> anyhow::Result<()> { /// hash of staged code. #[tokio::test] async fn test_deploy_code_with_hash_invalid_hash() -> anyhow::Result<()> { - let worker = workspaces::sandbox().await?; + 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?; From 1ba84e325a7f29f364690276543bc1ba319ce6d0 Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 13 Nov 2023 17:02:29 +0100 Subject: [PATCH 06/10] Fix versions of dependencies to downgrade in CI --- scripts/fix_dependencies.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/fix_dependencies.sh b/scripts/fix_dependencies.sh index 6130537..2b25d29 100755 --- a/scripts/fix_dependencies.sh +++ b/scripts/fix_dependencies.sh @@ -13,5 +13,5 @@ # for some other attempts and how they failed in CI). cargo update -p anstyle@1.0.4 --precise 1.0.2 cargo update -p anstyle-parse@0.2.2 --precise 0.2.1 -cargo update -p clap@4.4.7 --precise 4.3.24 -cargo update -p clap_lex@0.5.1 --precise 0.5.0 +cargo update -p clap@4.4.8 --precise 4.3.24 +cargo update -p clap_lex@0.6.0 --precise 0.5.0 From a7bb84ed41316d36ce0e87f6926917ddcc63ff75 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 14 Nov 2023 08:31:15 +0100 Subject: [PATCH 07/10] Another dependency downgrade fix --- scripts/fix_dependencies.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/fix_dependencies.sh b/scripts/fix_dependencies.sh index 2b25d29..d21b92e 100755 --- a/scripts/fix_dependencies.sh +++ b/scripts/fix_dependencies.sh @@ -14,4 +14,4 @@ cargo update -p anstyle@1.0.4 --precise 1.0.2 cargo update -p anstyle-parse@0.2.2 --precise 0.2.1 cargo update -p clap@4.4.8 --precise 4.3.24 -cargo update -p clap_lex@0.6.0 --precise 0.5.0 +cargo update -p clap_lex@0.5.1 --precise 0.5.0 From aeff8256d4e98ef402d3ff6b6dca5785636b86b0 Mon Sep 17 00:00:00 2001 From: karim-en Date: Wed, 11 Sep 2024 12:42:12 +0100 Subject: [PATCH 08/10] Oblige to pass hash --- near-plugins-derive/src/upgradable.rs | 20 +++--- .../tests/common/upgradable_contract.rs | 2 +- near-plugins-derive/tests/upgradable.rs | 68 ++++++++++++------- near-plugins/src/upgradable.rs | 5 +- 4 files changed, 54 insertions(+), 41 deletions(-) diff --git a/near-plugins-derive/src/upgradable.rs b/near-plugins-derive/src/upgradable.rs index aaf6673..c1397a6 100644 --- a/near-plugins-derive/src/upgradable.rs +++ b/near-plugins-derive/src/upgradable.rs @@ -187,7 +187,7 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { } #[#cratename::access_control_any(roles(#(#acl_roles_code_deployers),*))] - fn up_deploy_code(&mut self, hash: Option, function_call_args: Option<#cratename::upgradable::FunctionCallArgs>) -> near_sdk::Promise { + fn up_deploy_code(&mut self, hash: String, function_call_args: Option<#cratename::upgradable::FunctionCallArgs>) -> near_sdk::Promise { let staging_timestamp = self.up_get_timestamp(__UpgradableStorageKey::StagingTimestamp) .unwrap_or_else(|| ::near_sdk::env::panic_str("Upgradable: staging timestamp isn't set")); @@ -202,17 +202,15 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { } let code = self.up_staged_code().unwrap_or_else(|| ::near_sdk::env::panic_str("Upgradable: No staged code")); - if let Some(hash) = hash { - let expected_hash = ::near_sdk::base64::encode(Self::up_hash_code(code.as_ref())); - if hash != expected_hash { - near_sdk::env::panic_str( - format!( - "Upgradable: Cannot deploy due to wrong hash: expected hash: {}", - expected_hash, - ) - .as_str(), + let expected_hash = ::near_sdk::base64::encode(Self::up_hash_code(code.as_ref())); + if hash != expected_hash { + near_sdk::env::panic_str( + format!( + "Upgradable: Cannot deploy due to wrong hash: expected hash: {}", + expected_hash, ) - } + .as_str(), + ) } let promise = ::near_sdk::Promise::new(::near_sdk::env::current_account_id()) diff --git a/near-plugins-derive/tests/common/upgradable_contract.rs b/near-plugins-derive/tests/common/upgradable_contract.rs index 9fe4792..6d1e239 100644 --- a/near-plugins-derive/tests/common/upgradable_contract.rs +++ b/near-plugins-derive/tests/common/upgradable_contract.rs @@ -71,7 +71,7 @@ impl UpgradableContract { pub async fn up_deploy_code( &self, caller: &Account, - hash: Option, + hash: String, function_call_args: Option, ) -> near_workspaces::Result { caller diff --git a/near-plugins-derive/tests/upgradable.rs b/near-plugins-derive/tests/upgradable.rs index af1fbc5..6b50e98 100644 --- a/near-plugins-derive/tests/upgradable.rs +++ b/near-plugins-derive/tests/upgradable.rs @@ -109,13 +109,13 @@ impl Setup { } /// Asserts staged code equals `expected_code`. - async fn assert_staged_code(&self, expected_code: Option>) { + async fn assert_staged_code(&self, expected_code: Option<&Vec>) { let staged = self .upgradable_contract .up_staged_code(&self.unauth_account) .await .expect("Call to up_staged_code should succeed"); - assert_eq!(staged, expected_code); + assert_eq!(staged.as_ref(), expected_code); } /// Asserts the staging duration of the `Upgradable` contract equals the `expected_duration`. @@ -339,7 +339,7 @@ async fn test_staging_empty_code_clears_storage() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // Verify staging empty code removes it. let res = setup @@ -443,12 +443,12 @@ async fn test_deploy_code_without_delay() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // Deploy staged code. let res = setup .upgradable_contract - .up_deploy_code(&dao, None, None) + .up_deploy_code(&dao, convert_code_to_deploy_hash(&code), None) .await?; assert_success_with_unit_return(res); @@ -468,20 +468,20 @@ async fn test_deploy_code_with_hash_success() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code.clone())).await; + setup.assert_staged_code(Some(&code)).await; // Deploy staged code. let hash = convert_code_to_deploy_hash(&code); let res = setup .upgradable_contract - .up_deploy_code(&dao, Some(hash), None) + .up_deploy_code(&dao, hash, None) .await?; assert_success_with_unit_return(res); Ok(()) } -/// Verifies failure of `up_deploy_code(Some(hash), ...)` when `hash` does not correspond to the +/// Verifies failure of `up_deploy_code(hash, ...)` when `hash` does not correspond to the /// hash of staged code. #[tokio::test] async fn test_deploy_code_with_hash_invalid_hash() -> anyhow::Result<()> { @@ -496,12 +496,12 @@ async fn test_deploy_code_with_hash_invalid_hash() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code.clone())).await; + setup.assert_staged_code(Some(&code)).await; // Deployment is aborted if an invalid hash is provided. let res = setup .upgradable_contract - .up_deploy_code(&dao, Some("invalid_hash".to_owned()), None) + .up_deploy_code(&dao, "invalid_hash".to_owned(), None) .await?; let actual_hash = convert_code_to_deploy_hash(&code); let expected_err = format!( @@ -533,12 +533,12 @@ async fn test_deploy_code_and_call_method() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // Deploy staged code. let res = setup .upgradable_contract - .up_deploy_code(&dao, None, None) + .up_deploy_code(&dao, convert_code_to_deploy_hash(&code), None) .await?; assert_success_with_unit_return(res); @@ -573,7 +573,7 @@ async fn test_deploy_code_with_migration() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // Deploy staged code and call the new contract's `migrate` method. let function_call_args = FunctionCallArgs { @@ -584,7 +584,11 @@ async fn test_deploy_code_with_migration() -> anyhow::Result<()> { }; let res = setup .upgradable_contract - .up_deploy_code(&dao, None, Some(function_call_args)) + .up_deploy_code( + &dao, + convert_code_to_deploy_hash(&code), + Some(function_call_args), + ) .await?; assert_success_with_unit_return(res); @@ -616,7 +620,7 @@ async fn test_deploy_code_with_migration_failure_rollback() -> anyhow::Result<() .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // Deploy staged code and call the new contract's `migrate_with_failure` method. let function_call_args = FunctionCallArgs { @@ -627,7 +631,11 @@ async fn test_deploy_code_with_migration_failure_rollback() -> anyhow::Result<() }; let res = setup .upgradable_contract - .up_deploy_code(&dao, None, Some(function_call_args)) + .up_deploy_code( + &dao, + convert_code_to_deploy_hash(&code), + Some(function_call_args), + ) .await?; assert_failure_with(res, "Failing migration on purpose"); @@ -662,7 +670,7 @@ async fn test_deploy_code_in_batch_transaction_pitfall() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // 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. @@ -729,7 +737,7 @@ async fn test_deploy_code_with_delay() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // Let the staging duration pass. fast_forward_beyond(&worker, staging_duration).await; @@ -737,7 +745,7 @@ async fn test_deploy_code_with_delay() -> anyhow::Result<()> { // Deploy staged code. let res = setup .upgradable_contract - .up_deploy_code(&dao, None, None) + .up_deploy_code(&dao, convert_code_to_deploy_hash(&code), None) .await?; assert_success_with_unit_return(res); @@ -762,7 +770,7 @@ async fn test_deploy_code_with_delay_failure_too_early() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // Let some time pass but not enough. fast_forward_beyond(&worker, sdk_duration_from_secs(1)).await; @@ -770,7 +778,7 @@ async fn test_deploy_code_with_delay_failure_too_early() -> anyhow::Result<()> { // Verify trying to deploy staged code fails. let res = setup .upgradable_contract - .up_deploy_code(&dao, None, None) + .up_deploy_code(&dao, convert_code_to_deploy_hash(&code), None) .await?; assert_failure_with(res, ERR_MSG_DEPLOY_CODE_TOO_EARLY); @@ -794,13 +802,17 @@ async fn test_deploy_code_permission_failure() -> anyhow::Result<()> { .up_stage_code(&dao, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // Only the roles passed as `code_deployers` to the `Upgradable` derive macro may successfully // call this method. let res = setup .upgradable_contract - .up_deploy_code(&setup.unauth_account, None, None) + .up_deploy_code( + &setup.unauth_account, + convert_code_to_deploy_hash(&code), + None, + ) .await?; assert_insufficient_acl_permissions( res, @@ -841,7 +853,7 @@ async fn test_deploy_code_empty_failure() -> anyhow::Result<()> { // staging timestamp is expected. let res = setup .upgradable_contract - .up_deploy_code(&dao, None, None) + .up_deploy_code(&dao, "".to_owned(), None) .await?; assert_failure_with(res, ERR_MSG_NO_STAGING_TS); @@ -1083,14 +1095,18 @@ async fn test_acl_permission_scope() -> anyhow::Result<()> { .up_stage_code(&code_stager, code.clone()) .await?; assert_success_with_unit_return(res); - setup.assert_staged_code(Some(code)).await; + setup.assert_staged_code(Some(&code)).await; // Verify `code_stager` is not authorized to deploy staged code. Only grantees of at least one // of the roles passed as `code_deployers` to the `Upgradable` derive macro are authorized to // deploy code. let res = setup .upgradable_contract - .up_deploy_code(&setup.unauth_account, None, None) + .up_deploy_code( + &setup.unauth_account, + convert_code_to_deploy_hash(&code), + None, + ) .await?; assert_insufficient_acl_permissions( res, diff --git a/near-plugins/src/upgradable.rs b/near-plugins/src/upgradable.rs index 59aa0eb..a32b020 100644 --- a/near-plugins/src/upgradable.rs +++ b/near-plugins/src/upgradable.rs @@ -106,13 +106,12 @@ pub trait Upgradable { /// /// Some workflows (e.g. when a DAO interacts with an `Upgradable` contract) are facilitated if /// deployment succeeds only in case the hash of staged code corresponds to a given hash. This - /// behavior can be enabled with the `hash` parameter. In case it is `Some(h)`, the deployment + /// behavior can be enabled with the `hash` parameter. In case it is `h`, the deployment /// succeeds only if `h` equals the base64 encoded string of the staged code's `sha256` hash. In /// particular, the encoding according to [`near_sdk::base64::encode`] is expected. Note that /// `near_sdk` uses a rather dated version of the `base64` crate whose API differs from current /// versions. /// - /// Otherwise, if `hash` equals `None`, this verification step is skipped. /// /// # Attaching a function call /// @@ -157,7 +156,7 @@ pub trait Upgradable { /// [storage staked]: https://docs.near.org/concepts/storage/storage-staking#btw-you-can-remove-data-to-unstake-some-tokens fn up_deploy_code( &mut self, - hash: Option, + hash: String, function_call_args: Option, ) -> Promise; From ae5c02592a574a6cae1c1baf331bfb9b1e6279e0 Mon Sep 17 00:00:00 2001 From: karim-en Date: Wed, 11 Sep 2024 15:11:19 +0100 Subject: [PATCH 09/10] Fix tesrt & clippy --- near-plugins-derive/tests/upgradable.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/near-plugins-derive/tests/upgradable.rs b/near-plugins-derive/tests/upgradable.rs index 6b50e98..e5882b4 100644 --- a/near-plugins-derive/tests/upgradable.rs +++ b/near-plugins-derive/tests/upgradable.rs @@ -213,8 +213,9 @@ fn convert_code_to_crypto_hash(code: &[u8]) -> CryptoHash { /// Computes the hash `code` according the to requirements of the `hash` parameter of /// `Upgradable::up_deploy_code`. fn convert_code_to_deploy_hash(code: &[u8]) -> String { + use near_sdk::base64::Engine; let hash = near_sdk::env::sha256(code); - near_sdk::base64::encode(hash) + near_sdk::base64::prelude::BASE64_STANDARD.encode(&hash) } /// Smoke test of contract setup. @@ -675,16 +676,18 @@ 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!({ "function_call_args": FunctionCallArgs { + .args_json(json!({ + "hash": convert_code_to_deploy_hash(&code), + "function_call_args": FunctionCallArgs { function_name: "migrate_with_failure".to_string(), arguments: Vec::new(), amount: NearToken::from_yoctonear(0), gas: Gas::from_tgas(2), } })) - .gas(Gas::from_tgas(201)); + .gas(Gas::from_tgas(220)); let fn_call_remove_code = near_workspaces::operations::Function::new("up_stage_code") .args_borsh(Vec::::new()) - .gas(Gas::from_tgas(90)); + .gas(Gas::from_tgas(80)); let res = dao .batch(setup.contract.id()) From ce96fd0aeab598545cd4d2c47d6c0b2d6d22211d Mon Sep 17 00:00:00 2001 From: karim-en Date: Wed, 11 Sep 2024 15:13:39 +0100 Subject: [PATCH 10/10] Fix clippy --- near-plugins-derive/tests/upgradable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/near-plugins-derive/tests/upgradable.rs b/near-plugins-derive/tests/upgradable.rs index e5882b4..6c2fde3 100644 --- a/near-plugins-derive/tests/upgradable.rs +++ b/near-plugins-derive/tests/upgradable.rs @@ -215,7 +215,7 @@ fn convert_code_to_crypto_hash(code: &[u8]) -> CryptoHash { fn convert_code_to_deploy_hash(code: &[u8]) -> String { use near_sdk::base64::Engine; let hash = near_sdk::env::sha256(code); - near_sdk::base64::prelude::BASE64_STANDARD.encode(&hash) + near_sdk::base64::prelude::BASE64_STANDARD.encode(hash) } /// Smoke test of contract setup.