cancel admin rework

Author: ozgunozerk

examples/nft-access-control/src/test.rs

@@ -214,7 +214,7 @@ fn admin_transfer_works() {
     e.mock_all_auths();
 
     // Owner (current admin) initiates the transfer
-    client.transfer_admin_role(&owner, &new_admin);
+    client.transfer_admin_role(&owner, &new_admin, &1000);
 
     // New admin accepts
     client.accept_admin_transfer(&new_admin);
@@ -234,10 +234,10 @@ fn admin_transfer_cancelled() {
 
     e.mock_all_auths();
 
-    client.transfer_admin_role(&owner, &new_admin);
+    client.transfer_admin_role(&owner, &new_admin, &1000);
 
     // Now cancel
-    client.cancel_admin_transfer(&owner);
+    client.transfer_admin_role(&owner, &new_admin, &0);
 
     // New admin tries to accept—should panic
     client.accept_admin_transfer(&new_admin);
@@ -254,7 +254,7 @@ fn non_admin_cannot_initiate_transfer() {
 
     e.mock_all_auths();
 
-    client.transfer_admin_role(&intruder, &new_admin);
+    client.transfer_admin_role(&intruder, &new_admin, &1000);
 }
 
 #[test]
@@ -268,7 +268,7 @@ fn non_recipient_cannot_accept_transfer() {
 
     e.mock_all_auths();
 
-    client.transfer_admin_role(&owner, &new_admin);
+    client.transfer_admin_role(&owner, &new_admin, &1000);
 
     // Imposter tries to accept
     client.accept_admin_transfer(&imposter);

examples/nft-access-control/test_snapshots/test/non_recipient_cannot_accept_transfer.1.json

@@ -54,7 +54,7 @@
             },
             "ext": "v0"
           },
-          34560
+          1000
         ]
       ],
       [

packages/contract-utils/access-control/src/access_control.rs

@@ -144,41 +144,27 @@ pub trait AccessControl {
     /// * `e` - Access to Soroban environment.
     /// * `caller` - The address of the caller, must be the admin.
     /// * `new_admin` - The account to transfer the admin privileges to.
+    /// * `live_until_ledger` - The ledger number at which the pending transfer
+    ///   expires. If `live_until_ledger` is `0`, the pending transfer is cancelled.
+    ///   `live_until_ledger` argument is implicitly bounded by the maximum allowed
+    ///   TTL extension for a temporary storage entry and specifying a higher value
+    ///   will cause the code to panic.
     ///
     /// # Errors
     ///
     /// * `AccessControlError::Unauthorized` - If the `caller` is not the admin.
+    /// * `AccessControlError::NoPendingAdminTransfer` - If tried to cancel the
+    ///   pending admin transfer when there is no pending admin transfer.
     ///
     /// # Events
     ///
     /// * topics - `["admin_transfer_started", current_admin: Address]`
-    /// * data - `[new_admin: Address]`
+    /// * data - `[new_admin: Address, live_until_ledger: u32]`
     ///
     /// # Security Warning
     ///
     /// **IMPORTANT**: You MUST implement proper authorization in your contract.
-    fn transfer_admin_role(e: &Env, caller: Address, new_admin: Address);
-
-    /// Cancels a pending admin role transfer.
-    ///
-    /// # Arguments
-    ///
-    /// * `e` - Access to Soroban environment.
-    /// * `caller` - The address of the caller, must be the admin.
-    ///
-    /// # Errors
-    ///
-    /// * `AccessControlError::Unauthorized` - If the `caller` is not the admin.
-    ///
-    /// # Events
-    ///
-    /// * topics - `["admin_transfer_cancelled", admin: Address]`
-    /// * data - `[]` (empty data)
-    ///
-    /// # Security Warning
-    ///
-    /// **IMPORTANT**: You MUST implement proper authorization in your contract.
-    fn cancel_admin_transfer(e: &Env, caller: Address);
+    fn transfer_admin_role(e: &Env, caller: Address, new_admin: Address, live_until_ledger: u32);
 
     /// Completes the 2-step admin transfer.
     ///
@@ -233,6 +219,7 @@ pub enum AccessControlError {
     AccountNotFound = 122,
     NoPendingAdminTransfer = 123,
     OutOfBounds = 124,
+    InvalidLiveUntilLedger = 125,
 }
 
 // ################## EVENTS ##################
@@ -304,30 +291,21 @@ pub fn emit_role_admin_changed(
 /// * `e` - Access to Soroban environment.
 /// * `current_admin` - The current admin initiating the transfer.
 /// * `new_admin` - The proposed new admin.
+/// * `live_until_ledger` - The ledger number at which the pending transfer will
+///   expire. If this value is `0`, it means the pending transfer is cancelled.
 ///
 /// # Events
 ///
-/// * topics - `["admin_transfer_started", current_admin: Address]`
-/// * data - `[new_admin: Address]`
-pub fn emit_admin_transfer_started(e: &Env, current_admin: &Address, new_admin: &Address) {
-    let topics = (Symbol::new(e, "admin_transfer_started"), current_admin);
-    e.events().publish(topics, new_admin);
-}
-
-/// Emits an event when an admin transfer is cancelled.
-///
-/// # Arguments
-///
-/// * `e` - Access to Soroban environment.
-/// * `admin` - The admin who cancelled the transfer.
-///
-/// # Events
-///
-/// * topics - `["admin_transfer_cancelled", admin: Address]`
-/// * data - `[]` (empty data)
-pub fn emit_admin_transfer_cancelled(e: &Env, admin: &Address) {
-    let topics = (Symbol::new(e, "admin_transfer_cancelled"), admin);
-    e.events().publish(topics, ());
+/// * topics - `["admin_transfer", current_admin: Address]`
+/// * data - `[new_admin: Address, live_until_ledger: u32]`
+pub fn emit_admin_transfer(
+    e: &Env,
+    current_admin: &Address,
+    new_admin: &Address,
+    live_until_ledger: u32,
+) {
+    let topics = (Symbol::new(e, "admin_transfer"), current_admin);
+    e.events().publish(topics, (new_admin, live_until_ledger));
 }
 
 /// Emits an event when an admin transfer is completed.

packages/contract-utils/access-control/src/lib.rs

@@ -5,25 +5,27 @@
 //!
 //! # Usage
 //!
-//! There is a single overarching admin, and the admin has enough privileges to call
-//! any function given in the [`AccessControl`] trait.
+//! There is a single overarching admin, and the admin has enough privileges to
+//! call any function given in the [`AccessControl`] trait.
 //!
-//! This `admin` must be set in the constructor of the contract. Else, none of the methods
-//! exposed by this module will work. You can follow the `nft-access-control` example.
+//! This `admin` must be set in the constructor of the contract. Else, none of
+//! the methods exposed by this module will work. You can follow the
+//! `nft-access-control` example.
 //!
-//! Each role can have an `admin role` specified for it. For example, if you create 2 roles:
+//! Each role can have an `admin role` specified for it. For example, if you
+//! create 2 roles:
 //! - minter
 //! - mint_admins
 //!
-//! You can assign the role `mint_admins` as the admin role of the `minter` role group.
-//! And this will allow accounts with `mint_admins` role, to grant and revoke the roles of
-//! `minter` roles.
+//! You can assign the role `mint_admins` as the admin role of the `minter` role
+//! group. And this will allow accounts with `mint_admins` role, to grant and
+//! revoke the roles of `minter` roles.
 //!
-//! One can create as many roles as they want, and create a chain of command structure if
-//! they want to with this approach.
+//! One can create as many roles as they want, and create a chain of command
+//! structure if they want to with this approach.
 //!
-//! If you need even more granular control over which roles can do what, you can introduce
-//! your own business logic, and annotate it with our macro:
+//! If you need even more granular control over which roles can do what, you can
+//! introduce your own business logic, and annotate it with our macro:
 //!
 //! ```rust
 //! #[has_role(caller, "minter_admin")]
@@ -39,15 +41,13 @@ mod storage;
 
 pub use crate::{
     access_control::{
-        emit_admin_transfer_cancelled, emit_admin_transfer_completed, emit_admin_transfer_started,
-        emit_role_admin_changed, emit_role_granted, emit_role_revoked, AccessControl,
-        AccessControlError,
+        emit_admin_transfer, emit_admin_transfer_completed, emit_role_admin_changed,
+        emit_role_granted, emit_role_revoked, AccessControl, AccessControlError,
     },
     storage::{
-        accept_admin_transfer, add_to_role_enumeration, cancel_admin_transfer, ensure_role,
-        get_admin, get_role_admin, get_role_member, get_role_member_count, grant_role, has_role,
-        remove_from_role_enumeration, renounce_role, revoke_role, set_role_admin,
-        transfer_admin_role, AccessControlStorageKey,
+        accept_admin_transfer, add_to_role_enumeration, ensure_role, get_admin, get_role_admin,
+        get_role_member, get_role_member_count, grant_role, has_role, remove_from_role_enumeration,
+        renounce_role, revoke_role, set_role_admin, transfer_admin_role, AccessControlStorageKey,
     },
 };
 

packages/contract-utils/access-control/src/storage.rs

@@ -1,11 +1,9 @@
 use soroban_sdk::{contracttype, panic_with_error, Address, Env, Symbol};
-use stellar_constants::{
-    ADMIN_TRANSFER_THRESHOLD, ADMIN_TRANSFER_TTL, ROLE_EXTEND_AMOUNT, ROLE_TTL_THRESHOLD,
-};
+use stellar_constants::{ROLE_EXTEND_AMOUNT, ROLE_TTL_THRESHOLD};
 
 use crate::{
-    emit_admin_transfer_cancelled, emit_admin_transfer_completed, emit_admin_transfer_started,
-    emit_role_admin_changed, emit_role_granted, emit_role_revoked, AccessControlError,
+    emit_admin_transfer, emit_admin_transfer_completed, emit_role_admin_changed, emit_role_granted,
+    emit_role_revoked, AccessControlError,
 };
 
 #[contracttype]
@@ -223,56 +221,54 @@ pub fn renounce_role(e: &Env, caller: &Address, role: &Symbol) {
 /// * `e` - Access to Soroban environment.
 /// * `caller` - The address of the caller, must be the admin.
 /// * `new_admin` - The account to transfer the admin privileges to.
+/// * `live_until_ledger` - The ledger number at which the pending transfer
+///   expires. If `live_until_ledger` is `0`, the pending transfer is cancelled.
+///   `live_until_ledger` argument is implicitly bounded by the maximum allowed
+///   TTL extension for a temporary storage entry and specifying a higher value
+///   will cause the code to panic.
 ///
 /// # Errors
 ///
 /// * `AccessControlError::Unauthorized` - If the `caller` is not the admin.
+/// * `AccessControlError::NoPendingAdminTransfer` - If tried to cancel the
+///   pending admin transfer when there is no pending admin transfer.
 ///
 /// # Events
 ///
 /// * topics - `["admin_transfer_started", current_admin: Address]`
-/// * data - `[new_admin: Address]`
-pub fn transfer_admin_role(e: &Env, caller: &Address, new_admin: &Address) {
+/// * data - `[new_admin: Address, live_until_ledger: u32]`
+pub fn transfer_admin_role(e: &Env, caller: &Address, new_admin: &Address, live_until_ledger: u32) {
     if caller != &get_admin(e) {
         panic_with_error!(e, AccessControlError::Unauthorized);
     }
 
-    // Store the new admin address in temporary storage
-    e.storage().temporary().set(&AccessControlStorageKey::PendingAdmin, new_admin);
-    e.storage().temporary().extend_ttl(
-        &AccessControlStorageKey::PendingAdmin,
-        ADMIN_TRANSFER_THRESHOLD,
-        ADMIN_TRANSFER_TTL,
-    );
-
-    emit_admin_transfer_started(e, caller, new_admin);
-}
+    let key = AccessControlStorageKey::PendingAdmin;
 
-/// Cancels a pending admin role transfer if it is not accepted yet.
-/// This can only be called by the current admin.
-///
-/// # Arguments
-///
-/// * `e` - Access to Soroban environment.
-/// * `caller` - The address of the caller, must be the admin.
-///
-/// # Errors
-///
-/// * `AccessControlError::Unauthorized` - If the `caller` is not the admin.
-///
-/// # Events
-///
-/// * topics - `["admin_transfer_cancelled", admin: Address]`
-/// * data - `[]` (empty data)
-pub fn cancel_admin_transfer(e: &Env, caller: &Address) {
-    if caller != &get_admin(e) {
-        panic_with_error!(e, AccessControlError::Unauthorized);
+    if live_until_ledger == 0 {
+        let Some(pending_new_admin) = e.storage().temporary().get::<_, Address>(&key) else {
+            panic_with_error!(e, AccessControlError::NoPendingAdminTransfer)
+        };
+        e.storage().temporary().remove(&key);
+
+        emit_admin_transfer(e, caller, &pending_new_admin, live_until_ledger);
+        return;
     }
 
-    e.storage().temporary().remove(&AccessControlStorageKey::PendingAdmin);
+    let current_ledger = e.ledger().sequence();
+
+    if live_until_ledger < current_ledger {
+        panic_with_error!(e, AccessControlError::InvalidLiveUntilLedger);
+    }
+
+    let live_for = live_until_ledger - current_ledger;
+
+    // Store the new admin address in temporary storage
+    e.storage().temporary().set(&key, new_admin);
+    e.storage().temporary().extend_ttl(&key, live_for, live_for);
 
-    emit_admin_transfer_cancelled(e, caller);
+    emit_admin_transfer(e, caller, new_admin, live_until_ledger);
 }
+// TODO: test for live_until_ledger = 0 when there is no pending admin
 
 /// Completes the 2-step admin transfer.
 ///