Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Change hash encoding, support passing in code without borsh #152

Merged
merged 4 commits into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions near-plugins-derive/src/upgradable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,25 +165,30 @@ 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<u8>) {
fn up_stage_code(&mut self) {
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 {
let timestamp = ::near_sdk::env::block_timestamp() + self.up_get_duration(__UpgradableStorageKey::StagingDuration).unwrap_or(0);
// Use saturating_add to prevent overflow
let duration = self.up_get_duration(__UpgradableStorageKey::StagingDuration).unwrap_or(0);
let timestamp = ::near_sdk::env::block_timestamp().saturating_add(duration);

// 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<Vec<u8>> {
::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<String> {
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),*))]
Expand All @@ -202,7 +207,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!(
Expand Down
10 changes: 3 additions & 7 deletions near-plugins-derive/tests/common/upgradable_contract.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -39,7 +38,7 @@ impl UpgradableContract {
) -> near_workspaces::Result<ExecutionFinalResult> {
caller
.call(self.contract.id(), "up_stage_code")
.args_borsh(code)
.args(code)
.max_gas()
.transact()
.await
Expand All @@ -54,16 +53,13 @@ impl UpgradableContract {
Ok(res.borsh::<Option<Vec<u8>>>()?)
}

pub async fn up_staged_code_hash(
&self,
caller: &Account,
) -> anyhow::Result<Option<CryptoHash>> {
pub async fn up_staged_code_hash(&self, caller: &Account) -> anyhow::Result<Option<String>> {
let res = caller
.call(self.contract.id(), "up_staged_code_hash")
.max_gas()
.transact()
.await?;
Ok(res.json::<Option<CryptoHash>>()?)
Ok(res.json::<Option<String>>()?)
}

/// The `Promise` returned by trait method `up_deploy_code` is resolved in the `near_workspaces`
Expand Down
16 changes: 7 additions & 9 deletions near-plugins-derive/tests/upgradable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not crypto_hash; right now, we are returning CryptoHash. It's not very important, but maybe we could rename it to convert_code_to_bs58_encoded_hash or something like that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @r-near

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.
Expand Down Expand Up @@ -722,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::<u8>::new())
.args(Vec::<u8>::new())
.gas(Gas::from_tgas(80));

let res = dao
Expand Down
10 changes: 4 additions & 6 deletions near-plugins/src/upgradable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ 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<u8>);
fn up_stage_code(&mut self);

/// Returns the staged code.
fn up_staged_code(&self) -> Option<Vec<u8>>;

/// Returns the hash of the staged code
fn up_staged_code_hash(&self) -> Option<CryptoHash>;
fn up_staged_code_hash(&self) -> Option<String>;

/// Allows an authorized account to deploy the staged code. It panics if no code is staged.
///
Expand All @@ -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
Expand Down
Loading