Skip to content

Commit

Permalink
Oblige to pass hash
Browse files Browse the repository at this point in the history
  • Loading branch information
@karim-en
karim-en committed Sep 11, 2024
1 parent deaaab1 commit aeff825
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 41 deletions.
20 changes: 9 additions & 11 deletions near-plugins-derive/src/upgradable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>, 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"));

Expand All @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion near-plugins-derive/tests/common/upgradable_contract.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl UpgradableContract {
pub async fn up_deploy_code(
&self,
caller: &Account,
hash: Option<String>,
hash: String,
function_call_args: Option<FunctionCallArgs>,
) -> near_workspaces::Result<ExecutionFinalResult> {
caller
Expand Down
68 changes: 42 additions & 26 deletions near-plugins-derive/tests/upgradable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,13 @@ impl Setup {
}

/// Asserts staged code equals `expected_code`.
async fn assert_staged_code(&self, expected_code: Option<Vec<u8>>) {
async fn assert_staged_code(&self, expected_code: Option<&Vec<u8>>) {
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`.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -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<()> {
Expand All @@ -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!(
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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 {
Expand All @@ -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);

Expand Down Expand Up @@ -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 {
Expand All @@ -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");

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -729,15 +737,15 @@ 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;

// 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);

Expand All @@ -762,15 +770,15 @@ 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;

// 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);

Expand All @@ -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,
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions near-plugins/src/upgradable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -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<String>,
hash: String,
function_call_args: Option<FunctionCallArgs>,
) -> Promise;

Expand Down

0 comments on commit aeff825

Please sign in to comment.