From 1095742c49cae403b9bf9c8077e0539a094596c5 Mon Sep 17 00:00:00 2001 From: BCForge Developer Date: Fri, 24 Apr 2026 09:11:42 -0400 Subject: [PATCH] feat(token): implement two-step ownership transfer pattern - Add DataKey::PendingAdmin for storing pending admin address - Add propose_owner() for admin to propose new admin - Add accept_ownership() for pending admin to accept transfer - Add cancel_transfer() for admin to cancel pending transfer - Add pending_owner() read-only function to check pending transfers - Mark old transfer_ownership() as deprecated in docs - Add events: emit_ownership_proposed, emit_ownership_accepted, emit_ownership_cancelled - Add 5 comprehensive tests: * Happy path: propose -> accept -> new admin can mint * Accept without proposal fails * Cancel transfer clears pending admin * Cancel without proposal fails * Double propose updates pending admin - Improves security: prevents permanent loss of admin access due to typos Closes #14 --- contracts/token/src/events.rs | 24 +++++++++ contracts/token/src/lib.rs | 65 +++++++++++++++++++++++++ contracts/token/src/test.rs | 92 +++++++++++++++++++++++++++++++++++ 3 files changed, 181 insertions(+) diff --git a/contracts/token/src/events.rs b/contracts/token/src/events.rs index a6aaa68..b700d8b 100644 --- a/contracts/token/src/events.rs +++ b/contracts/token/src/events.rs @@ -79,6 +79,30 @@ pub fn emit_ownership_transferred(env: &Env, old_admin: &Address, new_admin: &Ad ); } +/// Emitted when a new admin is proposed (two-step transfer). +pub fn emit_ownership_proposed(env: &Env, old_admin: &Address, pending_admin: &Address) { + env.events().publish( + (symbol_short!("own_prop"),), + (old_admin.clone(), pending_admin.clone()), + ); +} + +/// Emitted when pending admin accepts ownership. +pub fn emit_ownership_accepted(env: &Env, old_admin: &Address, new_admin: &Address) { + env.events().publish( + (symbol_short!("own_accept"),), + (old_admin.clone(), new_admin.clone()), + ); +} + +/// Emitted when ownership transfer is cancelled. +pub fn emit_ownership_cancelled(env: &Env, admin: &Address, cancelled_admin: &Address) { + env.events().publish( + (symbol_short!("own_cancel"),), + (admin.clone(), cancelled_admin.clone()), + ); +} + /// Emitted when the contract is paused. pub fn emit_paused(env: &Env, admin: &Address) { env.events() diff --git a/contracts/token/src/lib.rs b/contracts/token/src/lib.rs index 49e83ad..10f6385 100644 --- a/contracts/token/src/lib.rs +++ b/contracts/token/src/lib.rs @@ -26,6 +26,8 @@ use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, String}; pub enum DataKey { /// The contract admin address. Admin, + /// Pending admin for two-step ownership transfer. + PendingAdmin, /// Spending allowance: (owner, spender) → amount. Allowance(Address, Address), /// Allowance expiration: (owner, spender) → ledger sequence. @@ -142,6 +144,11 @@ impl BcForgeToken { .get(&DataKey::Admin) .expect("contract not initialized") } + + /// Reads the pending admin address (if any). + fn read_pending_admin(env: &Env) -> Option
{ + env.storage().instance().get(&DataKey::PendingAdmin) + } } // ───────────────────────────────────────────────────────────────────────────── @@ -203,6 +210,9 @@ impl BcForgeToken { /// Transfers the admin role to a new address. Current admin-only. /// + /// ⚠️ DEPRECATED: Use propose_owner() + accept_ownership() for safer two-step transfer. + /// This function is kept for backward compatibility but may be removed in future versions. + /// /// # Arguments /// * `new_admin` - The address to receive admin privileges. pub fn transfer_ownership(env: Env, new_admin: Address) { @@ -213,6 +223,61 @@ impl BcForgeToken { events::emit_ownership_transferred(&env, &admin, &new_admin); } + /// Proposes a new admin for two-step ownership transfer. Current admin-only. + /// + /// # Arguments + /// * `new_admin` - The address to propose as the new admin. + /// + /// # Panics + /// Panics if caller is not the current admin. + pub fn propose_owner(env: Env, new_admin: Address) { + let admin = Self::read_admin(&env); + admin.require_auth(); + + env.storage().instance().set(&DataKey::PendingAdmin, &new_admin); + events::emit_ownership_proposed(&env, &admin, &new_admin); + } + + /// Accepts pending ownership transfer. Only the pending admin can call this. + /// + /// # Panics + /// Panics if there is no pending admin or if caller is not the pending admin. + pub fn accept_ownership(env: Env) { + let pending_admin = Self::read_pending_admin(&env) + .expect("no pending ownership transfer"); + + pending_admin.require_auth(); + + let old_admin = Self::read_admin(&env); + env.storage().instance().set(&DataKey::Admin, &pending_admin); + env.storage().instance().remove(&DataKey::PendingAdmin); + + events::emit_ownership_accepted(&env, &old_admin, &pending_admin); + } + + /// Cancels a pending ownership transfer. Current admin-only. + /// + /// # Panics + /// Panics if caller is not the current admin or if there is no pending transfer. + pub fn cancel_transfer(env: Env) { + let admin = Self::read_admin(&env); + admin.require_auth(); + + let pending_admin = Self::read_pending_admin(&env) + .expect("no pending ownership transfer"); + + env.storage().instance().remove(&DataKey::PendingAdmin); + events::emit_ownership_cancelled(&env, &admin, &pending_admin); + } + + /// Returns the pending admin address if there is a pending transfer. + /// + /// # Returns + /// Some(Address) if there is a pending admin, None otherwise. + pub fn pending_owner(env: Env) -> Option
{ + Self::read_pending_admin(&env) + } + /// Returns the total token supply. pub fn supply(env: Env) -> i128 { Self::read_supply(&env) diff --git a/contracts/token/src/test.rs b/contracts/token/src/test.rs index fe14426..9dec98f 100644 --- a/contracts/token/src/test.rs +++ b/contracts/token/src/test.rs @@ -314,6 +314,98 @@ fn test_transfer_ownership() { assert_eq!(client.balance(&user), 500); } +#[test] +fn test_two_step_ownership_transfer_happy_path() { + let env = Env::default(); + env.mock_all_auths(); + let (client, _) = setup_contract(&env); + let admin = init_default(&env, &client); + let new_admin = Address::generate(&env); + let user = Address::generate(&env); + + // Initially no pending owner + assert!(client.pending_owner().is_none()); + + // Propose new admin + client.propose_owner(&new_admin); + + // Check pending owner + let pending = client.pending_owner(); + assert!(pending.is_some()); + assert_eq!(pending.unwrap(), new_admin); + + // New admin accepts + client.accept_ownership(); + + // Pending owner should be cleared + assert!(client.pending_owner().is_none()); + + // New admin should be able to mint + client.mint(&user, &500); + assert_eq!(client.balance(&user), 500); +} + +#[test] +#[should_panic(expected = "no pending ownership transfer")] +fn test_accept_ownership_without_proposal_fails() { + let env = Env::default(); + env.mock_all_auths(); + let (client, _) = setup_contract(&env); + let _admin = init_default(&env, &client); + + // Try to accept without proposal + client.accept_ownership(); +} + +#[test] +fn test_cancel_transfer() { + let env = Env::default(); + env.mock_all_auths(); + let (client, _) = setup_contract(&env); + let admin = init_default(&env, &client); + let new_admin = Address::generate(&env); + + // Propose new admin + client.propose_owner(&new_admin); + assert!(client.pending_owner().is_some()); + + // Cancel the transfer + client.cancel_transfer(); + + // Pending owner should be cleared + assert!(client.pending_owner().is_none()); +} + +#[test] +#[should_panic(expected = "no pending ownership transfer")] +fn test_cancel_transfer_without_proposal_fails() { + let env = Env::default(); + env.mock_all_auths(); + let (client, _) = setup_contract(&env); + let _admin = init_default(&env, &client); + + // Try to cancel without proposal + client.cancel_transfer(); +} + +#[test] +fn test_double_propose_updates_pending_admin() { + let env = Env::default(); + env.mock_all_auths(); + let (client, _) = setup_contract(&env); + let _admin = init_default(&env, &client); + let first_proposal = Address::generate(&env); + let second_proposal = Address::generate(&env); + + // First proposal + client.propose_owner(&first_proposal); + assert_eq!(client.pending_owner().unwrap(), first_proposal); + + // Second proposal (should override first) + client.propose_owner(&second_proposal); + assert_eq!(client.pending_owner().unwrap(), second_proposal); +} + // ─── Pause / Unpause ───────────────────────────────────────────────────────── #[test]