diff --git a/docs/migrations/pausable-separate-roles.md b/docs/migrations/pausable-separate-roles.md index b5315dd..194d0ad 100644 --- a/docs/migrations/pausable-separate-roles.md +++ b/docs/migrations/pausable-separate-roles.md @@ -2,6 +2,16 @@ This guide explains how to migrate your code to use the new `pause_roles` and `unpause_roles` attributes instead of the consolidated `manager_roles` attribute in the Pausable plugin. +## Important Warning About Role Enums + +When using `#[derive(AccessControlRole)]`, **never remove existing variants or add new variants in the middle of the enum**. The order of variants is critical because: + +1. Each variant is mapped to specific bit positions in the permissions bitflags +2. Removing or reordering variants will cause existing permissions to be mapped incorrectly +3. This can result in accounts unintentionally gaining or losing access to features + +**Always add new variants at the end of the enum to preserve existing permission mappings.** + ## Changes Required ### Before @@ -38,19 +48,18 @@ With this change, you can: ## Step-by-Step Migration -1. **Update your Role enum** to include separate roles for pausing and unpausing, if desired: +1. **Update your Role enum** by adding new roles at the end to preserve existing mappings: ```rust #[derive(AccessControlRole, Deserialize, Serialize, Copy, Clone)] #[serde(crate = "near_sdk::serde")] pub enum Role { - // Previous role that could both pause and unpause - // PauseManager, + // Existing roles (DO NOT change order) + PauseManager, // Will now be used for pause permissions only + // Other existing roles... - // New separate roles - PauseManager, // Can only pause features - UnpauseManager, // Can only unpause features - // Other roles... + // Add new roles at the end only + UnpauseManager, // New role for unpause permissions } ``` @@ -67,6 +76,15 @@ With this change, you can: )] ``` + To maintain backward compatibility where existing PauseManager accounts can still do both operations: + + ```rust + #[pausable( + pause_roles(Role::PauseManager), + unpause_roles(Role::PauseManager, Role::UnpauseManager) + )] + ``` + 3. **Update contract initialization** to grant the appropriate roles: ```rust @@ -89,9 +107,24 @@ With this change, you can: } ``` -4. **Update tests** to test both pause and unpause permissions separately. +4. **Update or add a migration function** if you're upgrading an existing contract: + + ```rust + #[private] + pub fn migrate_pause_unpause_roles(&mut self) { + // Grant UnpauseManager role to existing PauseManager accounts + // This maintains the same capabilities they had before + + let pause_managers = self.acl_get_grantees("PauseManager", 0, 100); + for account_id in pause_managers { + self.acl_grant_role(Role::UnpauseManager.into(), account_id); + } + } + ``` + +5. **Update tests** to test both pause and unpause permissions separately. -## Example +## Complete Example Here's a complete example of a contract using the new separated roles: diff --git a/near-plugins-derive/tests/contracts/pausable_new/Cargo.toml b/near-plugins-derive/tests/contracts/pausable_new/Cargo.toml new file mode 100644 index 0000000..45baf15 --- /dev/null +++ b/near-plugins-derive/tests/contracts/pausable_new/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "pausable_new" +version = "0.0.0" +edition = "2021" + +[lib] +crate-type = ["cdylib", "rlib"] + +[dependencies] +near-plugins = { path = "../../../../near-plugins" } +near-sdk = "5.2" + +[profile.release] +codegen-units = 1 +opt-level = "z" +lto = true +debug = false +panic = "abort" +overflow-checks = true + +[workspace] diff --git a/near-plugins-derive/tests/contracts/pausable_new/Makefile b/near-plugins-derive/tests/contracts/pausable_new/Makefile new file mode 100644 index 0000000..aa2cf20 --- /dev/null +++ b/near-plugins-derive/tests/contracts/pausable_new/Makefile @@ -0,0 +1,8 @@ +build: + cargo build --target wasm32-unknown-unknown --release + +# Helpful for debugging. Requires `cargo-expand`. +expand: + cargo expand > expanded.rs + +.PHONY: build expand diff --git a/near-plugins-derive/tests/contracts/pausable_new/rust-toolchain b/near-plugins-derive/tests/contracts/pausable_new/rust-toolchain new file mode 100644 index 0000000..d6d7b98 --- /dev/null +++ b/near-plugins-derive/tests/contracts/pausable_new/rust-toolchain @@ -0,0 +1,4 @@ +[toolchain] +channel = "1.84.0" +components = ["clippy", "rustfmt"] +targets = [ "wasm32-unknown-unknown" ] diff --git a/near-plugins-derive/tests/contracts/pausable_new/src/lib.rs b/near-plugins-derive/tests/contracts/pausable_new/src/lib.rs new file mode 100644 index 0000000..93d1cc4 --- /dev/null +++ b/near-plugins-derive/tests/contracts/pausable_new/src/lib.rs @@ -0,0 +1,105 @@ +use near_plugins::{ + access_control, if_paused, pause, AccessControlRole, AccessControllable, Pausable, +}; +use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize}; +use near_sdk::serde::{Deserialize, Serialize}; +use near_sdk::{env, near, AccountId, PanicOnDefault}; + +/// Define roles for access control of `Pausable` features. +/// IMPORTANT: Keep the same order of existing variants to preserve permission mappings. +#[derive(AccessControlRole, Deserialize, Serialize, Copy, Clone)] +#[serde(crate = "near_sdk::serde")] +pub enum Role { + /// Now only used for pausing features + PauseManager, + /// Existing roles kept in the same order + Unrestricted4Increaser, + /// Existing roles kept in the same order + Unrestricted4Decreaser, + /// Existing roles kept in the same order + Unrestricted4Modifier, + /// Add new roles at the end + UnpauseManager, +} + +#[access_control(role_type(Role))] +#[near(contract_state)] +#[derive(Pausable, PanicOnDefault)] +#[pausable(pause_roles(Role::PauseManager), unpause_roles(Role::UnpauseManager))] +pub struct Counter { + counter: u64, +} + +#[near] +impl Counter { + /// Constructor initializes the counter to 0 and sets up ACL. + #[init] + pub fn new(pause_manager: AccountId, unpause_manager: AccountId) -> Self { + let mut contract = Self { counter: 0 }; + + // Make the contract itself super admin + near_sdk::require!( + contract.acl_init_super_admin(env::current_account_id()), + "Failed to initialize super admin", + ); + + // Grant roles to the provided accounts + let result = contract.acl_grant_role(Role::PauseManager.into(), pause_manager); + near_sdk::require!(Some(true) == result, "Failed to grant pause role"); + + let result = contract.acl_grant_role(Role::UnpauseManager.into(), unpause_manager); + near_sdk::require!(Some(true) == result, "Failed to grant unpause role"); + + contract + } + + /// Returns the current counter value + #[pause] + pub fn get_counter(&self) -> u64 { + self.counter + } + + /// Increments the counter - can be paused + #[pause] + pub fn increment(&mut self) { + self.counter += 1; + } + + /// Similar to `#[pause]` but use an explicit name for the feature. + #[pause(name = "Increase by two")] + pub fn increase_2(&mut self) { + self.counter += 2; + } + + /// Similar to `#[pause]` but roles passed as argument may still successfully call this method + /// even when the corresponding feature is paused. + #[pause(except(roles(Role::Unrestricted4Increaser, Role::Unrestricted4Modifier)))] + pub fn increase_4(&mut self) { + self.counter += 4; + } + + /// This method can only be called when "increment" is paused. + #[if_paused(name = "increment")] + pub fn decrease_1(&mut self) { + self.counter -= 1; + } + + /// For verifying that an account has a specific role + pub fn has_role(&self, role: String, account_id: AccountId) -> bool { + self.acl_has_role(role, account_id) + } + + /// Migration function to maintain backward compatibility + /// Grants UnpauseManager role to existing PauseManager accounts + #[private] + pub fn migrate_pause_unpause_roles(&mut self) { + // Get all accounts with PauseManager role + let pause_managers = self.acl_get_grantees("PauseManager".to_string(), 0, 100); + + // Grant UnpauseManager role to all existing PauseManager accounts + for account_id in pause_managers { + let result = self.acl_grant_role(Role::UnpauseManager.into(), account_id); + near_sdk::require!(result.is_some(), "Failed to grant UnpauseManager role"); + } + } +} diff --git a/near-plugins-derive/tests/contracts/pausable_old/Cargo.toml b/near-plugins-derive/tests/contracts/pausable_old/Cargo.toml new file mode 100644 index 0000000..a041eb4 --- /dev/null +++ b/near-plugins-derive/tests/contracts/pausable_old/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "pausable_old" +version = "0.0.0" +edition = "2021" + +[lib] +crate-type = ["cdylib", "rlib"] + +[dependencies] +near-plugins = { path = "../../../../near-plugins" } +near-sdk = "5.2" + +[profile.release] +codegen-units = 1 +opt-level = "z" +lto = true +debug = false +panic = "abort" +overflow-checks = true + +[workspace] diff --git a/near-plugins-derive/tests/contracts/pausable_old/Makefile b/near-plugins-derive/tests/contracts/pausable_old/Makefile new file mode 100644 index 0000000..aa2cf20 --- /dev/null +++ b/near-plugins-derive/tests/contracts/pausable_old/Makefile @@ -0,0 +1,8 @@ +build: + cargo build --target wasm32-unknown-unknown --release + +# Helpful for debugging. Requires `cargo-expand`. +expand: + cargo expand > expanded.rs + +.PHONY: build expand diff --git a/near-plugins-derive/tests/contracts/pausable_old/rust-toolchain b/near-plugins-derive/tests/contracts/pausable_old/rust-toolchain new file mode 100644 index 0000000..d6d7b98 --- /dev/null +++ b/near-plugins-derive/tests/contracts/pausable_old/rust-toolchain @@ -0,0 +1,4 @@ +[toolchain] +channel = "1.84.0" +components = ["clippy", "rustfmt"] +targets = [ "wasm32-unknown-unknown" ] diff --git a/near-plugins-derive/tests/contracts/pausable_old/src/lib.rs b/near-plugins-derive/tests/contracts/pausable_old/src/lib.rs new file mode 100644 index 0000000..5fcf34d --- /dev/null +++ b/near-plugins-derive/tests/contracts/pausable_old/src/lib.rs @@ -0,0 +1,85 @@ +use near_plugins::{ + access_control, if_paused, pause, AccessControlRole, AccessControllable, Pausable, +}; +use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize}; +use near_sdk::serde::{Deserialize, Serialize}; +use near_sdk::{env, near, AccountId, PanicOnDefault}; + +/// Define roles for access control of `Pausable` features. +#[derive(AccessControlRole, Deserialize, Serialize, Copy, Clone)] +#[serde(crate = "near_sdk::serde")] +pub enum Role { + /// The pause manager in the old style can both pause and unpause + PauseManager, + /// For testing except functionality + Unrestricted4Increaser, + /// For testing except functionality + Unrestricted4Decreaser, + /// For testing except functionality + Unrestricted4Modifier, +} + +#[access_control(role_type(Role))] +#[near(contract_state)] +#[derive(Pausable, PanicOnDefault)] +#[pausable(pause_roles(Role::PauseManager), unpause_roles(Role::PauseManager))] +pub struct Counter { + counter: u64, +} + +#[near] +impl Counter { + /// Constructor initializes the counter to 0 and sets up ACL. + #[init] + pub fn new(pause_manager: AccountId) -> Self { + let mut contract = Self { counter: 0 }; + + // Make the contract itself super admin + near_sdk::require!( + contract.acl_init_super_admin(env::current_account_id()), + "Failed to initialize super admin", + ); + + // Grant role to the provided account + let result = contract.acl_grant_role(Role::PauseManager.into(), pause_manager); + near_sdk::require!(Some(true) == result, "Failed to grant pause role"); + + contract + } + + /// Returns the current counter value + #[pause] + pub fn get_counter(&self) -> u64 { + self.counter + } + + /// Increments the counter - can be paused + #[pause] + pub fn increment(&mut self) { + self.counter += 1; + } + + /// Similar to `#[pause]` but use an explicit name for the feature. + #[pause(name = "Increase by two")] + pub fn increase_2(&mut self) { + self.counter += 2; + } + + /// Similar to `#[pause]` but roles passed as argument may still successfully call this method + /// even when the corresponding feature is paused. + #[pause(except(roles(Role::Unrestricted4Increaser, Role::Unrestricted4Modifier)))] + pub fn increase_4(&mut self) { + self.counter += 4; + } + + /// This method can only be called when "increment" is paused. + #[if_paused(name = "increment")] + pub fn decrease_1(&mut self) { + self.counter -= 1; + } + + /// For verifying that an account has a specific role + pub fn has_role(&self, role: String, account_id: AccountId) -> bool { + self.acl_has_role(role, account_id) + } +} diff --git a/near-plugins-derive/tests/pausable_migration.rs b/near-plugins-derive/tests/pausable_migration.rs new file mode 100644 index 0000000..3bf368e --- /dev/null +++ b/near-plugins-derive/tests/pausable_migration.rs @@ -0,0 +1,253 @@ +pub mod common; + +use anyhow::Result; +use common::utils::{ + assert_insufficient_acl_permissions, assert_success_with, assert_success_with_unit_return, +}; +use near_sdk::serde_json::json; +use near_workspaces::network::Sandbox; +use near_workspaces::{Account, Contract, Worker}; +use std::path::Path; + +// Paths to the contract directories +const OLD_CONTRACT_PATH: &str = "./tests/contracts/pausable_old"; +const NEW_CONTRACT_PATH: &str = "./tests/contracts/pausable_new"; + +/// Test struct to manage resources and helper methods +struct MigrationTest { + worker: Worker, + contract: Contract, + pause_manager: Account, + unpause_manager: Account, +} + +impl MigrationTest { + /// Deploy the old contract and create test accounts + async fn new() -> Result { + let worker = near_workspaces::sandbox().await?; + + // Compile and deploy the old style contract + let wasm = + common::repo::compile_project(Path::new(OLD_CONTRACT_PATH), "pausable_old").await?; + let contract = worker.dev_deploy(&wasm).await?; + + // Create accounts for testing + let pause_manager = worker.dev_create_account().await?; + let unpause_manager = worker.dev_create_account().await?; + + // Initialize the old contract with just the pause_manager + contract + .call("new") + .args_json(json!({ + "pause_manager": pause_manager.id(), + })) + .max_gas() + .transact() + .await? + .into_result()?; + + Ok(Self { + worker, + contract, + pause_manager, + unpause_manager, + }) + } + + /// Verify the manager has the expected role + async fn verify_role(&self, account: &Account, role: &str, expected: bool) -> Result<()> { + let has_role: bool = self + .contract + .call("has_role") + .args_json(json!({ + "role": role, + "account_id": account.id(), + })) + .view() + .await? + .json()?; + + assert_eq!(has_role, expected, "Role verification failed for {}", role); + Ok(()) + } + + /// Pause a feature + async fn pause_feature( + &self, + account: &Account, + feature: &str, + ) -> Result { + let result = account + .call(self.contract.id(), "pa_pause_feature") + .args_json(json!({ "key": feature })) + .max_gas() + .transact() + .await?; + + Ok(result) + } + + /// Unpause a feature + async fn unpause_feature( + &self, + account: &Account, + feature: &str, + ) -> Result { + let result = account + .call(self.contract.id(), "pa_unpause_feature") + .args_json(json!({ "key": feature })) + .max_gas() + .transact() + .await?; + + Ok(result) + } + + /// Check if a feature is paused + async fn is_paused(&self, account: &Account, feature: &str) -> Result { + let result = account + .call(self.contract.id(), "pa_is_paused") + .args_json(json!({ "key": feature })) + .view() + .await?; + + Ok(result.json()?) + } + + /// Deploy the new contract code and migrate + async fn upgrade_contract(&self) -> Result<()> { + // Compile the new style contract + let wasm = + common::repo::compile_project(Path::new(NEW_CONTRACT_PATH), "pausable_new").await?; + + // Deploy the new contract code + let _ = self.contract.as_account().deploy(&wasm).await?; + + // Call the migration function to maintain backward compatibility + let res = self + .contract + .call("migrate_pause_unpause_roles") + .max_gas() + .transact() + .await?; + + assert_success_with_unit_return(res); + + // Grant UnpauseManager role to the unpause_manager account + let res = self + .contract + .as_account() + .call(self.contract.id(), "acl_grant_role") + .args_json(json!({ + "role": "UnpauseManager", + "account_id": self.unpause_manager.id(), + })) + .max_gas() + .transact() + .await?; + + assert_success_with(res, true); + + Ok(()) + } +} + +/// Test the migration from old-style pausable to new-style pausable +#[tokio::test] +async fn test_pausable_migration() -> Result<()> { + // Setup the test with old contract + let test = MigrationTest::new().await?; + + // Verify initial roles + test.verify_role(&test.pause_manager, "PauseManager", true) + .await?; + + // Test that pause_manager can both pause and unpause features in the old contract + let res = test.pause_feature(&test.pause_manager, "increment").await?; + assert_success_with(res, true); + + let res = test + .unpause_feature(&test.pause_manager, "increment") + .await?; + assert_success_with(res, true); + + // Upgrade to the new contract + test.upgrade_contract().await?; + + // After migration, pause_manager should have both roles + test.verify_role(&test.pause_manager, "PauseManager", true) + .await?; + test.verify_role(&test.pause_manager, "UnpauseManager", true) + .await?; + test.verify_role(&test.unpause_manager, "UnpauseManager", true) + .await?; + test.verify_role(&test.unpause_manager, "PauseManager", false) + .await?; + + // Test that pause_manager can still pause features + let res = test.pause_feature(&test.pause_manager, "increment").await?; + assert_success_with(res, true); + + // Test that pause_manager can still unpause features (due to migration granting both roles) + let res = test + .unpause_feature(&test.pause_manager, "increment") + .await?; + assert_success_with(res, true); + + // Pause the feature again before testing unpause_manager + let res = test.pause_feature(&test.pause_manager, "increment").await?; + assert_success_with(res, true); + + // Verify the feature is actually paused + let is_paused = test.is_paused(&test.pause_manager, "increment").await?; + assert!( + is_paused, + "Feature should be paused before testing unpause_manager" + ); + + // Test that unpause_manager can unpause features but not pause them + let res = test + .pause_feature(&test.unpause_manager, "increment") + .await?; + assert_insufficient_acl_permissions(res, "pa_pause_feature", vec!["PauseManager".to_string()]); + + let res = test + .unpause_feature(&test.unpause_manager, "increment") + .await?; + assert_success_with(res, true); + + // Verify the feature was successfully unpaused + let is_paused = test.is_paused(&test.pause_manager, "increment").await?; + assert!( + !is_paused, + "Feature should be unpaused after unpause_manager action" + ); + + // Add a second account with only PauseManager role + let pause_only = test.worker.dev_create_account().await?; + let res = test + .contract + .as_account() + .call(test.contract.id(), "acl_grant_role") + .args_json(json!({ + "role": "PauseManager", + "account_id": pause_only.id(), + })) + .max_gas() + .transact() + .await?; + assert_success_with(res, true); + + // Verify the new account can pause but not unpause + let res = test.pause_feature(&pause_only, "increment").await?; + assert_success_with(res, true); + + let res = test.unpause_feature(&pause_only, "increment").await?; + assert_insufficient_acl_permissions( + res, + "pa_unpause_feature", + vec!["UnpauseManager".to_string()], + ); + + Ok(()) +}