From b24c7b9108d7e620adcc3f6a8fc89121b8a6477c Mon Sep 17 00:00:00 2001 From: r-near <163825889+r-near@users.noreply.github.com> Date: Mon, 24 Feb 2025 12:57:44 -0800 Subject: [PATCH 1/4] fix: change code hash encoding from base64 to bs58 --- near-plugins-derive/src/upgradable.rs | 6 +++--- .../tests/common/upgradable_contract.rs | 8 ++------ near-plugins-derive/tests/upgradable.rs | 14 ++++++-------- near-plugins/src/upgradable.rs | 8 +++----- 4 files changed, 14 insertions(+), 22 deletions(-) diff --git a/near-plugins-derive/src/upgradable.rs b/near-plugins-derive/src/upgradable.rs index b46bd65..a2bcece 100644 --- a/near-plugins-derive/src/upgradable.rs +++ b/near-plugins-derive/src/upgradable.rs @@ -181,9 +181,9 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { ::near_sdk::env::storage_read(self.up_storage_key(__UpgradableStorageKey::Code).as_ref()) } - fn up_staged_code_hash(&self) -> Option<::near_sdk::CryptoHash> { + fn up_staged_code_hash(&self) -> Option { self.up_staged_code() - .map(|code| Self::up_hash_code(code.as_ref())) + .map(|code| ::near_sdk::bs58::encode(Self::up_hash_code(code.as_ref())).into_string()) } #[#cratename::access_control_any(roles(#(#acl_roles_code_deployers),*))] @@ -202,7 +202,7 @@ 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")); - let expected_hash = ::near_sdk::base64::encode(Self::up_hash_code(code.as_ref())); + let expected_hash = ::near_sdk::bs58::encode(Self::up_hash_code(code.as_ref())).into_string(); if hash != expected_hash { near_sdk::env::panic_str( format!( diff --git a/near-plugins-derive/tests/common/upgradable_contract.rs b/near-plugins-derive/tests/common/upgradable_contract.rs index 6d1e239..543e32e 100644 --- a/near-plugins-derive/tests/common/upgradable_contract.rs +++ b/near-plugins-derive/tests/common/upgradable_contract.rs @@ -1,7 +1,6 @@ use near_plugins::upgradable::{FunctionCallArgs, UpgradableDurationStatus}; use near_sdk::serde_json::json; -use near_sdk::CryptoHash; use near_sdk::Duration; use near_workspaces::result::ExecutionFinalResult; use near_workspaces::{Account, Contract}; @@ -54,16 +53,13 @@ impl UpgradableContract { Ok(res.borsh::>>()?) } - pub async fn up_staged_code_hash( - &self, - caller: &Account, - ) -> anyhow::Result> { + pub async fn up_staged_code_hash(&self, caller: &Account) -> anyhow::Result> { let res = caller .call(self.contract.id(), "up_staged_code_hash") .max_gas() .transact() .await?; - Ok(res.json::>()?) + Ok(res.json::>()?) } /// The `Promise` returned by trait method `up_deploy_code` is resolved in the `near_workspaces` diff --git a/near-plugins-derive/tests/upgradable.rs b/near-plugins-derive/tests/upgradable.rs index 34da63d..3a12bf4 100644 --- a/near-plugins-derive/tests/upgradable.rs +++ b/near-plugins-derive/tests/upgradable.rs @@ -12,7 +12,7 @@ use common::utils::{ }; use near_plugins::upgradable::FunctionCallArgs; use near_sdk::serde_json::json; -use near_sdk::{CryptoHash, Duration, Gas, NearToken, Timestamp}; +use near_sdk::{Duration, Gas, NearToken, Timestamp}; use near_workspaces::network::Sandbox; use near_workspaces::result::ExecutionFinalResult; use near_workspaces::{Account, AccountId, Contract, Worker}; @@ -203,19 +203,17 @@ impl Setup { } } -/// Panics if the conversion fails. -fn convert_code_to_crypto_hash(code: &[u8]) -> CryptoHash { - near_sdk::env::sha256(code) - .try_into() - .expect("Code should be converted to CryptoHash") +/// Converts code to a base58-encoded CryptoHash string. +fn convert_code_to_crypto_hash(code: &[u8]) -> String { + let hash = near_sdk::env::sha256(code); + near_sdk::bs58::encode(hash).into_string() } /// 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::prelude::BASE64_STANDARD.encode(hash) + near_sdk::bs58::encode(hash).into_string() } /// Smoke test of contract setup. diff --git a/near-plugins/src/upgradable.rs b/near-plugins/src/upgradable.rs index 2cca948..632f29b 100644 --- a/near-plugins/src/upgradable.rs +++ b/near-plugins/src/upgradable.rs @@ -97,7 +97,7 @@ pub trait Upgradable { fn up_staged_code(&self) -> Option>; /// Returns the hash of the staged code - fn up_staged_code_hash(&self) -> Option; + fn up_staged_code_hash(&self) -> Option; /// Allows an authorized account to deploy the staged code. It panics if no code is staged. /// @@ -106,10 +106,8 @@ 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 `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. + /// succeeds only if `h` equals the bs58 encoded string of the staged code's `sha256` hash. In + /// particular, the encoding according to [`near_sdk::bs58::encode`] is expected. /// /// /// # Attaching a function call From 0fec87d1153b87f674eb9a4f5d71eeefd132d5c6 Mon Sep 17 00:00:00 2001 From: r-near <163825889+r-near@users.noreply.github.com> Date: Mon, 24 Feb 2025 19:26:07 -0800 Subject: [PATCH 2/4] feat: support passing in code directly --- near-plugins-derive/src/upgradable.rs | 30 ++++++++++++++----- .../tests/common/upgradable_contract.rs | 2 +- near-plugins-derive/tests/upgradable.rs | 2 +- near-plugins/src/upgradable.rs | 2 +- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/near-plugins-derive/src/upgradable.rs b/near-plugins-derive/src/upgradable.rs index a2bcece..79ddb49 100644 --- a/near-plugins-derive/src/upgradable.rs +++ b/near-plugins-derive/src/upgradable.rs @@ -165,14 +165,28 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { } #[#cratename::access_control_any(roles(#(#acl_roles_code_stagers),*))] - fn up_stage_code(&mut self, #[serializer(borsh)] code: Vec) { - if code.is_empty() { - ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::Code).as_ref()); - ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::StagingTimestamp).as_ref()); - } else { - let timestamp = ::near_sdk::env::block_timestamp() + self.up_get_duration(__UpgradableStorageKey::StagingDuration).unwrap_or(0); - self.up_storage_write(__UpgradableStorageKey::Code, &code); - self.up_set_timestamp(__UpgradableStorageKey::StagingTimestamp, timestamp); + fn up_stage_code(&mut self) { + match ::near_sdk::env::input() { + None => { + ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::Code).as_ref()); + ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::StagingTimestamp).as_ref()); + }, + Some(code) if code.is_empty() => { + ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::Code).as_ref()); + ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::StagingTimestamp).as_ref()); + }, + Some(code) => { + // Calculate timestamp more safely with checked_add to prevent overflow + let duration = self.up_get_duration(__UpgradableStorageKey::StagingDuration).unwrap_or(0); + let timestamp = match ::near_sdk::env::block_timestamp().checked_add(duration) { + Some(ts) => ts, + None => ::near_sdk::env::panic_str("Timestamp calculation overflow"), + }; + + // Store the new code and timestamp + self.up_storage_write(__UpgradableStorageKey::Code, &code); + self.up_set_timestamp(__UpgradableStorageKey::StagingTimestamp, timestamp); + } } } diff --git a/near-plugins-derive/tests/common/upgradable_contract.rs b/near-plugins-derive/tests/common/upgradable_contract.rs index 543e32e..e18ae65 100644 --- a/near-plugins-derive/tests/common/upgradable_contract.rs +++ b/near-plugins-derive/tests/common/upgradable_contract.rs @@ -38,7 +38,7 @@ impl UpgradableContract { ) -> near_workspaces::Result { caller .call(self.contract.id(), "up_stage_code") - .args_borsh(code) + .args(code) .max_gas() .transact() .await diff --git a/near-plugins-derive/tests/upgradable.rs b/near-plugins-derive/tests/upgradable.rs index 3a12bf4..a8b9936 100644 --- a/near-plugins-derive/tests/upgradable.rs +++ b/near-plugins-derive/tests/upgradable.rs @@ -720,7 +720,7 @@ async fn test_deploy_code_in_batch_transaction_pitfall() -> anyhow::Result<()> { } })) .gas(Gas::from_tgas(220)); let fn_call_remove_code = near_workspaces::operations::Function::new("up_stage_code") - .args_borsh(Vec::::new()) + .args(Vec::::new()) .gas(Gas::from_tgas(80)); let res = dao diff --git a/near-plugins/src/upgradable.rs b/near-plugins/src/upgradable.rs index 632f29b..e03a807 100644 --- a/near-plugins/src/upgradable.rs +++ b/near-plugins/src/upgradable.rs @@ -91,7 +91,7 @@ pub trait Upgradable { /// specified via the `code_stagers` field of the `Upgradable` macro's `access_control_roles` /// 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_stage_code(&mut self, code: Vec); + fn up_stage_code(&mut self); /// Returns the staged code. fn up_staged_code(&self) -> Option>; From ad27f9b9211d204b058f1feb20b706ec1f450d5c Mon Sep 17 00:00:00 2001 From: r-near <163825889+r-near@users.noreply.github.com> Date: Wed, 26 Feb 2025 11:15:29 -0800 Subject: [PATCH 3/4] fix: use unwrap_or_default --- near-plugins-derive/src/upgradable.rs | 38 +++++++++++---------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/near-plugins-derive/src/upgradable.rs b/near-plugins-derive/src/upgradable.rs index 79ddb49..5257c41 100644 --- a/near-plugins-derive/src/upgradable.rs +++ b/near-plugins-derive/src/upgradable.rs @@ -166,30 +166,24 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { #[#cratename::access_control_any(roles(#(#acl_roles_code_stagers),*))] fn up_stage_code(&mut self) { - match ::near_sdk::env::input() { - None => { - ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::Code).as_ref()); - ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::StagingTimestamp).as_ref()); - }, - Some(code) if code.is_empty() => { - ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::Code).as_ref()); - ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::StagingTimestamp).as_ref()); - }, - Some(code) => { - // Calculate timestamp more safely with checked_add to prevent overflow - let duration = self.up_get_duration(__UpgradableStorageKey::StagingDuration).unwrap_or(0); - let timestamp = match ::near_sdk::env::block_timestamp().checked_add(duration) { - Some(ts) => ts, - None => ::near_sdk::env::panic_str("Timestamp calculation overflow"), - }; - - // Store the new code and timestamp - self.up_storage_write(__UpgradableStorageKey::Code, &code); - self.up_set_timestamp(__UpgradableStorageKey::StagingTimestamp, timestamp); - } + let code = ::near_sdk::env::input().unwrap_or_default(); + + if code.is_empty() { + ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::Code).as_ref()); + ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::StagingTimestamp).as_ref()); + } else { + // Calculate timestamp more safely with checked_add to prevent overflow + let duration = self.up_get_duration(__UpgradableStorageKey::StagingDuration).unwrap_or(0); + let timestamp = match ::near_sdk::env::block_timestamp().checked_add(duration) { + Some(ts) => ts, + None => ::near_sdk::env::panic_str("Timestamp calculation overflow"), + }; + + // Store the new code and timestamp + self.up_storage_write(__UpgradableStorageKey::Code, &code); + self.up_set_timestamp(__UpgradableStorageKey::StagingTimestamp, timestamp); } } - #[result_serializer(borsh)] fn up_staged_code(&self) -> Option> { ::near_sdk::env::storage_read(self.up_storage_key(__UpgradableStorageKey::Code).as_ref()) From 2c63a252018780f11483c0ada555ff82096d77ef Mon Sep 17 00:00:00 2001 From: r-near <163825889+r-near@users.noreply.github.com> Date: Wed, 26 Feb 2025 11:24:03 -0800 Subject: [PATCH 4/4] fix: use saturating_add instead --- near-plugins-derive/src/upgradable.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/near-plugins-derive/src/upgradable.rs b/near-plugins-derive/src/upgradable.rs index 5257c41..86cc0a1 100644 --- a/near-plugins-derive/src/upgradable.rs +++ b/near-plugins-derive/src/upgradable.rs @@ -172,12 +172,9 @@ pub fn derive_upgradable(input: TokenStream) -> TokenStream { ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::Code).as_ref()); ::near_sdk::env::storage_remove(self.up_storage_key(__UpgradableStorageKey::StagingTimestamp).as_ref()); } else { - // Calculate timestamp more safely with checked_add to prevent overflow + // Use saturating_add to prevent overflow let duration = self.up_get_duration(__UpgradableStorageKey::StagingDuration).unwrap_or(0); - let timestamp = match ::near_sdk::env::block_timestamp().checked_add(duration) { - Some(ts) => ts, - None => ::near_sdk::env::panic_str("Timestamp calculation overflow"), - }; + let timestamp = ::near_sdk::env::block_timestamp().saturating_add(duration); // Store the new code and timestamp self.up_storage_write(__UpgradableStorageKey::Code, &code);