From f792c0b5179d5aa6cb51f4b85a688a8c68950e14 Mon Sep 17 00:00:00 2001 From: Marwen Abid Date: Mon, 13 Apr 2026 11:58:58 -0700 Subject: [PATCH] EN-33 Add Ability to Edit Owner, edit Pauser and Upgrade Card Contract --- contracts/factory/Makefile | 2 +- contracts/factory/src/events.rs | 36 +++ contracts/factory/src/lib.rs | 147 ++++++++++- contracts/factory/src/test.rs | 446 +++++++++++++++++++++++++++++++- contracts/issuer/src/lib.rs | 21 +- 5 files changed, 637 insertions(+), 15 deletions(-) diff --git a/contracts/factory/Makefile b/contracts/factory/Makefile index b971934..2892c30 100644 --- a/contracts/factory/Makefile +++ b/contracts/factory/Makefile @@ -7,7 +7,7 @@ test: build build: stellar contract build - @ls -l target/wasm32v1-none/release/*.wasm + @ls -l ../target/wasm32v1-none/release/factory.wasm fmt: cargo fmt --all diff --git a/contracts/factory/src/events.rs b/contracts/factory/src/events.rs index f913668..1c9402d 100644 --- a/contracts/factory/src/events.rs +++ b/contracts/factory/src/events.rs @@ -87,3 +87,39 @@ pub struct UserVelocityUpdated { /// Max spend per transaction. pub per_transaction_spend_limit: i128, } + +#[contractevent] +#[derive(Clone)] +pub struct OwnerUpdated { + /// Previous owner address. + pub old_owner: Address, + /// New owner address. + pub new_owner: Address, +} + +#[contractevent] +#[derive(Clone)] +pub struct PauserUpdated { + /// Previous pauser address. + pub old_pauser: Address, + /// New pauser address. + pub new_pauser: Address, +} + +#[contractevent] +#[derive(Clone)] +pub struct ContractUpgraded { + /// New WASM hash applied to the contract. + pub new_wasm_hash: BytesN<32>, +} + +#[contractevent] +#[derive(Clone)] +pub struct IssuerUpgraded { + /// Issuer identifier. + pub issuer_id: BytesN<32>, + /// Token address associated with the issuer. + pub token: Address, + /// New WASM hash applied to the issuer contract. + pub new_wasm_hash: BytesN<32>, +} diff --git a/contracts/factory/src/lib.rs b/contracts/factory/src/lib.rs index c852d02..0adcdb4 100644 --- a/contracts/factory/src/lib.rs +++ b/contracts/factory/src/lib.rs @@ -17,8 +17,8 @@ mod test; mod velocity; use crate::events::{ - DebitorUpdated, DestinationUpdated, IssuerCreated, ManagedUpdated, TransferExecuted, - UserVelocityUpdated, + ContractUpgraded, DebitorUpdated, DestinationUpdated, IssuerCreated, IssuerUpgraded, + ManagedUpdated, OwnerUpdated, PauserUpdated, TransferExecuted, UserVelocityUpdated, }; use pausable::{require_not_paused, Pausable}; use soroban_sdk::{ @@ -29,7 +29,7 @@ use storage::{ authorized_debitor, is_allowed_destination, issuer_address, issuer_manager, issuer_wasm_hash, owner, pauser, remove_allowed_destination, remove_authorized_debitor, set_allowed_destination, set_authorized_debitor, set_issuer_address, set_issuer_manager, set_issuer_wasm_hash, - set_owner, set_paused, set_pauser, user_velocity, + set_paused, user_velocity, }; use velocity::{ set_user_velocity_limits, validate_and_update_user_velocity, validate_velocity_config, @@ -47,6 +47,7 @@ pub trait IssuerContract { destination: Address, amount: i128, ); + fn upgrade(env: Env, new_wasm_hash: BytesN<32>); } #[contracterror] @@ -72,6 +73,8 @@ pub enum FactoryError { IssuerManagerNotFound = 8, /// Issuer for the `(issuer_id, token)` pair already exists. IssuerAlreadyExists = 9, + /// Caller is not authorized for this operation. + NotAuthorized = 10, /// Operation requires the contract to be unpaused. EnforcedPause = 1000, /// Operation requires the contract to be paused. @@ -110,8 +113,8 @@ impl Factory { /// # Authorization /// No runtime authorization check. This entrypoint is only callable at contract initialization. pub fn __constructor(env: Env, owner: Address, pauser: Address, issuer_wasm_hash: BytesN<32>) { - set_owner(&env, &owner); - set_pauser(&env, &pauser); + storage::set_owner(&env, &owner); + storage::set_pauser(&env, &pauser); set_paused(&env, false); set_issuer_wasm_hash(&env, &issuer_wasm_hash); } @@ -457,6 +460,140 @@ impl Factory { } .publish(&env); } + + /// Returns the current owner address. + /// + /// # Arguments + /// * `env` - Contract environment. + /// + /// # Authorization + /// No authorization required. + /// + /// # Returns + /// The current owner address. + pub fn get_owner(env: Env) -> Address { + owner(&env) + } + + /// Returns the current pauser address. + /// + /// # Arguments + /// * `env` - Contract environment. + /// + /// # Authorization + /// No authorization required. + /// + /// # Returns + /// The current pauser address. + pub fn get_pauser(env: Env) -> Address { + pauser(&env) + } + + /// Transfers ownership to a new address. + /// + /// # Arguments + /// * `env` - Contract environment. + /// * `new_owner` - Address to receive ownership. + /// + /// # Authorization + /// Requires authorization from the current owner. Not gated by pause state. + pub fn set_owner(env: Env, new_owner: Address) { + let current_owner = owner(&env); + current_owner.require_auth(); + + storage::set_owner(&env, &new_owner); + + OwnerUpdated { + old_owner: current_owner, + new_owner, + } + .publish(&env); + } + + /// Transfers the pauser role to a new address. + /// + /// When called by the owner, the pause guard is skipped so the owner can + /// recover from a compromised pauser key that has frozen the contract. + /// When called by the current pauser, the contract must not be paused. + /// + /// # Arguments + /// * `env` - Contract environment. + /// * `caller` - Address invoking the operation (must be the current owner or pauser). + /// * `new_pauser` - Address to receive the pauser role. + /// + /// # Authorization + /// Requires authorization from `caller`, who must be the current owner or pauser. + pub fn set_pauser(env: Env, caller: Address, new_pauser: Address) { + let current_owner = owner(&env); + let current_pauser = pauser(&env); + + if caller == current_owner { + caller.require_auth(); + } else if caller == current_pauser { + require_not_paused(&env); + caller.require_auth(); + } else { + panic_with_error!(&env, FactoryError::NotAuthorized); + } + + storage::set_pauser(&env, &new_pauser); + + PauserUpdated { + old_pauser: current_pauser, + new_pauser, + } + .publish(&env); + } + + /// Replaces this contract's WASM bytecode in-place, preserving its address and storage. + /// + /// # Arguments + /// * `env` - Contract environment. + /// * `new_wasm_hash` - Hash of the uploaded WASM to install. + /// + /// # Authorization + /// Requires authorization from the current owner. Not gated by pause state. + pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) { + owner(&env).require_auth(); + + env.deployer() + .update_current_contract_wasm(new_wasm_hash.clone()); + + ContractUpgraded { new_wasm_hash }.publish(&env); + } + + /// Upgrades an issuer contract's WASM bytecode in-place. + /// + /// # Arguments + /// * `env` - Contract environment. + /// * `issuer_id` - Issuer identifier. + /// * `token` - Token address associated with the issuer. + /// * `new_wasm_hash` - Hash of the uploaded WASM to install on the issuer. + /// + /// # Authorization + /// Requires authorization from the current owner. Not gated by pause state. + pub fn upgrade_issuer( + env: Env, + issuer_id: BytesN<32>, + token: Address, + new_wasm_hash: BytesN<32>, + ) { + owner(&env).require_auth(); + + let Some(issuer_contract_address) = issuer_address(&env, &issuer_id, &token) else { + panic_with_error!(&env, FactoryError::IssuerNotFound); + }; + + let issuer = IssuerClient::new(&env, &issuer_contract_address); + issuer.upgrade(&new_wasm_hash); + + IssuerUpgraded { + issuer_id, + token, + new_wasm_hash, + } + .publish(&env); + } } #[contractimpl] diff --git a/contracts/factory/src/test.rs b/contracts/factory/src/test.rs index e502c1e..676ce95 100644 --- a/contracts/factory/src/test.rs +++ b/contracts/factory/src/test.rs @@ -4,7 +4,7 @@ extern crate std; use super::{ - events::{ManagedUpdated, UserVelocityUpdated}, + events::{ManagedUpdated, OwnerUpdated, PauserUpdated, UserVelocityUpdated}, storage::set_user_velocity, Factory, FactoryClient, FactoryError, UserVelocity, }; @@ -480,13 +480,12 @@ fn create_issuer_fails_for_duplicate_issuer_token_pair() { let setup = TestContext::for_flow(true, true); let factory = FactoryClient::new(&setup.env, &setup.factory_address); - let result = factory - .try_create_issuer( - &setup.issuer_id, - &setup.token_address, - &setup.manager, - &setup.destination, - ); + let result = factory.try_create_issuer( + &setup.issuer_id, + &setup.token_address, + &setup.manager, + &setup.destination, + ); assert_eq!(result, Err(Ok(FactoryError::IssuerAlreadyExists.into()))); } @@ -1014,6 +1013,437 @@ fn velocity_overflow_maps_to_period_limit_error() { ); } +// --- Request 1 & 2: get_owner / get_pauser query functions --- + +#[test] +fn get_owner_returns_configured_owner() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + + assert_eq!(factory.get_owner(), setup.owner); +} + +#[test] +fn get_pauser_returns_configured_pauser() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + + assert_eq!(factory.get_pauser(), setup.pauser); +} + +// --- Request 3: set_owner (owner transfer) --- + +#[test] +fn set_owner_transfers_ownership() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_owner = Address::generate(&setup.env); + + factory.set_owner(&new_owner); + assert_eq!(factory.get_owner(), new_owner); +} + +#[test] +fn set_owner_emits_owner_updated_event() { + let setup = TestContext::for_auth(); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_owner = Address::generate(&setup.env); + + factory + .mock_auths(&[MockAuth { + address: &setup.owner, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "set_owner", + args: (&new_owner,).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .set_owner(&new_owner); + + let expected_event = OwnerUpdated { + old_owner: setup.owner.clone(), + new_owner: new_owner.clone(), + }; + let events = setup + .env + .events() + .all() + .filter_by_contract(&setup.factory_address); + assert!(events + .events() + .contains(&expected_event.to_xdr(&setup.env, &setup.factory_address))); +} + +#[test] +fn set_owner_auth_is_enforced() { + let setup = TestContext::for_auth(); + let attacker = Address::generate(&setup.env); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + + let unauthorized = factory + .mock_auths(&[MockAuth { + address: &attacker, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "set_owner", + args: (&attacker,).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .try_set_owner(&attacker); + assert!(unauthorized.is_err()); + assert_eq!(factory.get_owner(), setup.owner); +} + +#[test] +fn set_owner_works_when_paused() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_owner = Address::generate(&setup.env); + + factory.pause(); + assert!(factory.paused()); + + factory.set_owner(&new_owner); + assert_eq!(factory.get_owner(), new_owner); +} + +#[test] +fn new_owner_can_exercise_owner_functions_after_transfer() { + let setup = TestContext::for_auth(); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_owner = Address::generate(&setup.env); + let destination = Address::generate(&setup.env); + + factory + .mock_auths(&[MockAuth { + address: &setup.owner, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "set_owner", + args: (&new_owner,).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .set_owner(&new_owner); + + // New owner can update destinations. + factory + .mock_auths(&[MockAuth { + address: &new_owner, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "update_issuer_destination", + args: (&setup.issuer_id, &destination, &true).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .update_issuer_destination(&setup.issuer_id, &destination, &true); + + // Old owner is rejected. + let old_owner_rejected = factory + .mock_auths(&[MockAuth { + address: &setup.owner, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "update_issuer_destination", + args: (&setup.issuer_id, &destination, &false).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .try_update_issuer_destination(&setup.issuer_id, &destination, &false); + assert!(old_owner_rejected.is_err()); +} + +// --- Request 4: set_pauser (pauser transfer with dual-auth) --- + +#[test] +fn set_pauser_by_owner_transfers_pauser_role() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_pauser = Address::generate(&setup.env); + + factory.set_pauser(&setup.owner, &new_pauser); + assert_eq!(factory.get_pauser(), new_pauser); +} + +#[test] +fn set_pauser_by_pauser_transfers_pauser_role() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_pauser = Address::generate(&setup.env); + + factory.set_pauser(&setup.pauser, &new_pauser); + assert_eq!(factory.get_pauser(), new_pauser); +} + +#[test] +fn set_pauser_emits_pauser_updated_event() { + let setup = TestContext::for_auth(); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_pauser = Address::generate(&setup.env); + + factory + .mock_auths(&[MockAuth { + address: &setup.owner, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "set_pauser", + args: (&setup.owner, &new_pauser).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .set_pauser(&setup.owner, &new_pauser); + + let expected_event = PauserUpdated { + old_pauser: setup.pauser.clone(), + new_pauser: new_pauser.clone(), + }; + let events = setup + .env + .events() + .all() + .filter_by_contract(&setup.factory_address); + assert!(events + .events() + .contains(&expected_event.to_xdr(&setup.env, &setup.factory_address))); +} + +#[test] +fn set_pauser_rejects_unauthorized_caller() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let attacker = Address::generate(&setup.env); + + let result = factory.try_set_pauser(&attacker, &attacker); + assert_eq!(result, Err(Ok(FactoryError::NotAuthorized.into()))); + assert_eq!(factory.get_pauser(), setup.pauser); +} + +#[test] +fn set_pauser_by_owner_works_when_paused() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_pauser = Address::generate(&setup.env); + + factory.pause(); + assert!(factory.paused()); + + // Owner can rotate pauser even while paused — critical recovery path. + factory.set_pauser(&setup.owner, &new_pauser); + assert_eq!(factory.get_pauser(), new_pauser); +} + +#[test] +fn set_pauser_by_pauser_blocked_when_paused() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_pauser = Address::generate(&setup.env); + + factory.pause(); + assert!(factory.paused()); + + // Pauser cannot rotate themselves while paused. + let result = factory.try_set_pauser(&setup.pauser, &new_pauser); + assert_eq!(result, Err(Ok(FactoryError::EnforcedPause.into()))); + assert_eq!(factory.get_pauser(), setup.pauser); +} + +#[test] +fn owner_recovers_from_compromised_pauser() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let recovery_pauser = Address::generate(&setup.env); + + // Compromised pauser freezes the contract. + factory.pause(); + assert!(factory.paused()); + + // Owner rotates pauser even though contract is paused. + factory.set_pauser(&setup.owner, &recovery_pauser); + assert_eq!(factory.get_pauser(), recovery_pauser); + + // Old (compromised) pauser can no longer unpause. + // Note: in for_flow mode, mock_all_auths is on, so we verify via get_pauser + // that the role has changed. The auth test below covers explicit auth checks. +} + +#[test] +fn set_pauser_auth_is_enforced_for_owner_path() { + let setup = TestContext::for_auth(); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let attacker = Address::generate(&setup.env); + let new_pauser = Address::generate(&setup.env); + + // Attacker claims to be owner but cannot provide owner auth. + let unauthorized = factory + .mock_auths(&[MockAuth { + address: &attacker, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "set_pauser", + args: (&setup.owner, &new_pauser).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .try_set_pauser(&setup.owner, &new_pauser); + assert!(unauthorized.is_err()); + assert_eq!(factory.get_pauser(), setup.pauser); +} + +#[test] +fn set_pauser_auth_is_enforced_for_pauser_path() { + let setup = TestContext::for_auth(); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let attacker = Address::generate(&setup.env); + let new_pauser = Address::generate(&setup.env); + + // Attacker claims to be pauser but cannot provide pauser auth. + let unauthorized = factory + .mock_auths(&[MockAuth { + address: &attacker, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "set_pauser", + args: (&setup.pauser, &new_pauser).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .try_set_pauser(&setup.pauser, &new_pauser); + assert!(unauthorized.is_err()); + assert_eq!(factory.get_pauser(), setup.pauser); +} + +#[test] +fn new_pauser_can_pause_and_unpause_after_rotation() { + let setup = TestContext::for_auth(); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let new_pauser = Address::generate(&setup.env); + + // Owner rotates pauser. + factory + .mock_auths(&[MockAuth { + address: &setup.owner, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "set_pauser", + args: (&setup.owner, &new_pauser).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .set_pauser(&setup.owner, &new_pauser); + + // New pauser can pause. + factory + .mock_auths(&[MockAuth { + address: &new_pauser, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "pause", + args: ().into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .pause(); + assert!(factory.paused()); + + // Old pauser is rejected. + let old_pauser_rejected = factory + .mock_auths(&[MockAuth { + address: &setup.pauser, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "unpause", + args: ().into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .try_unpause(); + assert!(old_pauser_rejected.is_err()); + + // New pauser can unpause. + factory + .mock_auths(&[MockAuth { + address: &new_pauser, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "unpause", + args: ().into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .unpause(); + assert!(!factory.paused()); +} + +// --- Request 5: upgrade (contract upgradeability) --- + +#[test] +fn upgrade_requires_owner_auth() { + let setup = TestContext::for_auth(); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let attacker = Address::generate(&setup.env); + let fake_hash = rand_bytes(&setup.env); + + let unauthorized = factory + .mock_auths(&[MockAuth { + address: &attacker, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "upgrade", + args: (&fake_hash,).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .try_upgrade(&fake_hash); + assert!(unauthorized.is_err()); +} + +#[test] +fn upgrade_issuer_requires_owner_auth() { + let setup = TestContext::for_auth(); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let attacker = Address::generate(&setup.env); + let fake_hash = rand_bytes(&setup.env); + + let unauthorized = factory + .mock_auths(&[MockAuth { + address: &attacker, + invoke: &MockAuthInvoke { + contract: &setup.factory_address, + fn_name: "upgrade_issuer", + args: (&setup.issuer_id, &setup.token_address, &fake_hash).into_val(&setup.env), + sub_invokes: &[], + }, + }]) + .try_upgrade_issuer(&setup.issuer_id, &setup.token_address, &fake_hash); + assert!(unauthorized.is_err()); +} + +#[test] +fn upgrade_issuer_fails_for_unknown_issuer() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + let unknown_issuer = rand_bytes(&setup.env); + let fake_hash = rand_bytes(&setup.env); + + let result = factory.try_upgrade_issuer(&unknown_issuer, &setup.token_address, &fake_hash); + assert_eq!(result, Err(Ok(FactoryError::IssuerNotFound.into()))); +} + +#[test] +fn upgrade_issuer_succeeds_with_valid_wasm() { + let setup = TestContext::for_flow(true, true); + let factory = FactoryClient::new(&setup.env, &setup.factory_address); + + // Re-upload the same issuer WASM to get a valid hash. + let new_issuer_hash = upload_issuer_wasm(&setup.env); + + // upgrade_issuer should succeed — factory authorizes issuer.upgrade(). + factory.upgrade_issuer(&setup.issuer_id, &setup.token_address, &new_issuer_hash); +} + proptest! { #![proptest_config(proptest_config())] diff --git a/contracts/issuer/src/lib.rs b/contracts/issuer/src/lib.rs index 8ce7556..290b4cb 100644 --- a/contracts/issuer/src/lib.rs +++ b/contracts/issuer/src/lib.rs @@ -12,7 +12,7 @@ #[cfg(test)] mod test; -use soroban_sdk::{contract, contractimpl, contracttype, token, Address, Env}; +use soroban_sdk::{contract, contractimpl, contracttype, token, Address, BytesN, Env}; #[contract] pub struct Issuer; @@ -71,4 +71,23 @@ impl Issuer { &amount, ); } + + /// Replaces this contract's WASM bytecode in-place. + /// + /// # Arguments + /// * `env` - Contract environment. + /// * `new_wasm_hash` - Hash of the uploaded WASM to install. + /// + /// # Authorization + /// Requires authorization from the stored factory address. + pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) { + let factory: Address = env + .storage() + .instance() + .get(&DataKey::Factory) + .expect("factory not set"); + factory.require_auth(); + + env.deployer().update_current_contract_wasm(new_wasm_hash); + } }