From 46e7ea9e98ea9fdf730607aae52d722c8a29e600 Mon Sep 17 00:00:00 2001 From: merge-test Date: Thu, 23 Jul 2026 17:39:27 +0100 Subject: [PATCH] feat: timelock + multi-sig admin flow for upgrade/freeze/extend (#92) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single-key admin control over upgrade(), freeze(), unfreeze(), and extend_deadline() was a single point of compromise: Soroban upgrades are immediate, so one stolen creator key could instantly replace the contract WASM. This adds the propose → approve → execute governance flow modelled on Compound/OpenZeppelin's TimelockController: - AdminAction { kind, payload, execute_after, voters } (#[contracttype]) with ActionKind::{Upgrade, Freeze, Unfreeze, ExtendDeadline}. - propose_admin_action / approve_admin_action / execute_admin_action entrypoints in a new campaign/src/admin.rs. Proposing counts as the first approval; executed actions are deleted before their effect is applied, so they can never replay. - Quorum: 2 approvals with a multi-sig signer set, 1 with the legacy single signer. execute_after is enforced at execution (timelock). - set_admin_signers rotates the signer set and requires auth from every current signer — rotating a multi-sig is itself multi-sig. - Backwards compatible: without a stored signer set the admin set defaults to [creator] (1-of-1) and every direct entrypoint keeps working exactly as before; once ≥2 signers are configured, direct calls panic MultisigActive and the flow is the only path. - New error codes 94–102 (91–93 left reserved for the in-flight receipt work in #158); DataKey::{AdminAction(u64), AdminActionCount, AdminSigners} appended; bump_all_persistent covers the new keys. 16 new tests cover the acceptance matrix: cannot execute before execute_after, cannot execute with <2 approvals under multi-sig, replay protection, payload validation, signer-set validation, and 1-of-1 backwards compatibility (direct calls + the flow itself). Closes #92 --- campaign/src/admin.rs | 255 +++++++++++++ campaign/src/contract.rs | 19 +- campaign/src/event.rs | 31 ++ campaign/src/lib.rs | 115 +++++- campaign/src/storage.rs | 65 +++- campaign/src/test/admin_action_tests.rs | 479 ++++++++++++++++++++++++ campaign/src/types.rs | 81 +++- 7 files changed, 1020 insertions(+), 25 deletions(-) create mode 100644 campaign/src/admin.rs create mode 100644 campaign/src/test/admin_action_tests.rs diff --git a/campaign/src/admin.rs b/campaign/src/admin.rs new file mode 100644 index 00000000..b36bb875 --- /dev/null +++ b/campaign/src/admin.rs @@ -0,0 +1,255 @@ +//! Issue #92 – Timelock + multi-sig admin governance. +//! +//! Privileged operations (`upgrade`, `freeze`, `unfreeze`, `extend_deadline`) +//! previously required only `creator.require_auth()` — a single point of +//! compromise: one stolen key could instantly replace the contract WASM. +//! +//! This module introduces the propose → approve → execute flow modelled on +//! Compound / OpenZeppelin's `TimelockController`: +//! +//! - Any admin signer may **propose** an [`AdminAction`] with an +//! `execute_after` timestamp (the timelock, giving the community time to +//! detect a malicious upgrade before it lands on-chain). Proposing counts +//! as the proposer's approval. +//! - Other signers **approve** the action by id. +//! - Once the quorum is met **and** `execute_after` has passed, any signer +//! may **execute**. The action is deleted before its effect is applied, so +//! it can never be replayed. +//! +//! Quorum: `min(2, signer_count)`. With a multi-sig signer set (≥2 signers) +//! two approvals are required and the **direct** admin entrypoints are +//! disabled ([`Error::MultisigActive`]). With a single signer — the +//! backwards-compatible default of `[creator]` — the flow degrades to a +//! 1-of-1 quorum and the existing direct entrypoints keep working unchanged. + +use soroban_sdk::{Address, Bytes, Env, Map, Vec}; + +use crate::event; +use crate::storage::{ + get_admin_action, get_admin_signers_storage, get_campaign, increment_admin_action_count, + is_frozen, remove_admin_action, set_admin_action, set_admin_signers_storage, set_frozen, +}; +use crate::types::{ActionKind, AdminAction, Error}; + +/// The current admin signer set: the stored set if configured, otherwise the +/// backwards-compatible default `[creator]`. +/// +/// # Panics +/// - `Error::NotInitialized` if the campaign is not initialized +pub fn get_admin_signers(env: &Env) -> Vec
{ + if let Some(signers) = get_admin_signers_storage(env) { + return signers; + } + let campaign = get_campaign(env).unwrap_or_else(|| env.panic_with_error(Error::NotInitialized)); + let mut signers = Vec::new(env); + signers.push_back(campaign.creator); + signers +} + +/// Approvals required to execute an action: 2 with a multi-sig set, 1 with +/// the single-signer (legacy) configuration. +fn required_approvals(signer_count: u32) -> u32 { + if signer_count >= 2 { + 2 + } else { + 1 + } +} + +/// Panic with `Error::NotAdminSigner` unless `who` is in the signer set. +fn assert_signer(env: &Env, signers: &Vec
, who: &Address) { + if !signers.contains(who) { + env.panic_with_error(Error::NotAdminSigner); + } +} + +/// Guard for the legacy direct entrypoints (`upgrade`, `freeze`, `unfreeze`, +/// `extend_deadline`): allowed only while a single signer is configured +/// (1-of-1 quorum). Returns that signer so the caller can `require_auth()` it. +/// +/// # Panics +/// - `Error::MultisigActive` if ≥2 signers are configured +pub fn require_direct_admin(env: &Env) -> Address { + let signers = get_admin_signers(env); + if signers.len() >= 2 { + env.panic_with_error(Error::MultisigActive); + } + signers.get(0).unwrap() +} + +/// Validate that `payload` matches the encoding its `kind` demands. +fn validate_payload(env: &Env, kind: &ActionKind, payload: &Bytes) { + let valid = match kind { + ActionKind::Upgrade => payload.len() == 32, + ActionKind::Freeze | ActionKind::Unfreeze => payload.is_empty(), + ActionKind::ExtendDeadline => payload.len() == 8, + }; + if !valid { + env.panic_with_error(Error::InvalidActionPayload); + } +} + +/// Propose an admin action. The proposer must be an admin signer and is +/// recorded as the first approval. Returns the new action's id. +/// +/// # Panics +/// - `Error::NotAdminSigner` if `proposer` is not in the signer set +/// - `Error::InvalidActionPayload` if the payload doesn't match the kind +/// - `Error::InvalidExecuteAfter` if `execute_after` is in the past +pub fn propose_admin_action( + env: &Env, + proposer: Address, + kind: ActionKind, + payload: Bytes, + execute_after: u64, +) -> u64 { + proposer.require_auth(); + + let signers = get_admin_signers(env); + assert_signer(env, &signers, &proposer); + validate_payload(env, &kind, &payload); + + if execute_after < env.ledger().timestamp() { + env.panic_with_error(Error::InvalidExecuteAfter); + } + + let mut voters: Map = Map::new(env); + voters.set(proposer.clone(), true); + + let action = AdminAction { + kind, + payload, + execute_after, + voters, + }; + + let action_id = increment_admin_action_count(env); + set_admin_action(env, action_id, &action); + + event::admin_action_proposed(env, action_id, &proposer, execute_after); + action_id +} + +/// Approve a pending admin action. Each signer may approve once. +/// +/// # Panics +/// - `Error::NotAdminSigner` if `approver` is not in the signer set +/// - `Error::ActionNotFound` if no action exists under `action_id` +/// - `Error::AlreadyApproved` if this signer already approved +pub fn approve_admin_action(env: &Env, approver: Address, action_id: u64) { + approver.require_auth(); + + let signers = get_admin_signers(env); + assert_signer(env, &signers, &approver); + + let mut action = get_admin_action(env, action_id) + .unwrap_or_else(|| env.panic_with_error(Error::ActionNotFound)); + + if action.voters.contains_key(approver.clone()) { + env.panic_with_error(Error::AlreadyApproved); + } + + action.voters.set(approver.clone(), true); + set_admin_action(env, action_id, &action); + + event::admin_action_approved(env, action_id, &approver, action.voters.len()); +} + +/// Execute an approved admin action once its timelock has elapsed. +/// +/// The action is **deleted before** its effect is applied, so execution can +/// never replay and a re-entrant call finds no action. +/// +/// # Panics +/// - `Error::NotAdminSigner` if `executor` is not in the signer set +/// - `Error::ActionNotFound` if no action exists under `action_id` +/// - `Error::TimelockNotElapsed` if `execute_after` has not passed +/// - `Error::InsufficientApprovals` if approvals < quorum +/// - `Error::ContractFrozen` for an `Upgrade` or `ExtendDeadline` action +/// while the contract is frozen (matching the direct entrypoints) +pub fn execute_admin_action(env: &Env, executor: Address, action_id: u64) { + executor.require_auth(); + + let signers = get_admin_signers(env); + assert_signer(env, &signers, &executor); + + let action = get_admin_action(env, action_id) + .unwrap_or_else(|| env.panic_with_error(Error::ActionNotFound)); + + if env.ledger().timestamp() < action.execute_after { + env.panic_with_error(Error::TimelockNotElapsed); + } + + if action.voters.len() < required_approvals(signers.len()) { + env.panic_with_error(Error::InsufficientApprovals); + } + + // Delete before applying: no replay, and a re-entrant call finds nothing. + remove_admin_action(env, action_id); + + let timestamp = env.ledger().timestamp(); + match action.kind { + ActionKind::Upgrade => { + // Freeze invariant matches the direct upgrade() entrypoint. + if is_frozen(env) { + env.panic_with_error(Error::ContractFrozen); + } + let mut hash = [0u8; 32]; + action.payload.copy_into_slice(&mut hash); + let wasm_hash = soroban_sdk::BytesN::from_array(env, &hash); + env.deployer() + .update_current_contract_wasm(wasm_hash.clone()); + event::contract_upgraded(env, &executor, wasm_hash, timestamp); + } + ActionKind::Freeze => { + set_frozen(env, true); + event::contract_frozen(env, &executor, timestamp); + } + ActionKind::Unfreeze => { + set_frozen(env, false); + event::contract_unfrozen(env, &executor, timestamp); + } + ActionKind::ExtendDeadline => { + let mut buf = [0u8; 8]; + action.payload.copy_into_slice(&mut buf); + let new_end_time = u64::from_be_bytes(buf); + // Shares the direct entrypoint's validation (status, bounds, + // freeze check) without re-authing the creator. + crate::contract::apply_extend_deadline(env, new_end_time, &executor); + } + } + + event::admin_action_executed(env, action_id, &executor); +} + +/// Replace the admin signer set. +/// +/// Requires authorization from **every current signer** — rotating a +/// multi-sig set is itself a multi-sig operation, so a single compromised +/// key can neither expand nor collapse the quorum. With the default +/// single-signer set this degrades to the familiar creator-only auth. +/// +/// # Panics +/// - `Error::InvalidSigners` if `new_signers` is empty or has duplicates +pub fn set_admin_signers(env: &Env, new_signers: Vec
) { + if new_signers.is_empty() { + env.panic_with_error(Error::InvalidSigners); + } + // Reject duplicates: each signer must count once toward the quorum. + for i in 0..new_signers.len() { + let a = new_signers.get(i).unwrap(); + for j in (i + 1)..new_signers.len() { + if a == new_signers.get(j).unwrap() { + env.panic_with_error(Error::InvalidSigners); + } + } + } + + let current = get_admin_signers(env); + for signer in current.iter() { + signer.require_auth(); + } + + set_admin_signers_storage(env, &new_signers); + event::admin_signers_updated(env, new_signers.len()); +} diff --git a/campaign/src/contract.rs b/campaign/src/contract.rs index cb7e195a..b6fc7f57 100644 --- a/campaign/src/contract.rs +++ b/campaign/src/contract.rs @@ -8,7 +8,7 @@ use crate::storage::{get_campaign, is_frozen, set_campaign}; use crate::types::{CampaignStatus, Error}; use crate::validation::validate_campaign_transition; use crate::MAX_DEADLINE_GAP_SECONDS; -use soroban_sdk::{panic_with_error, Env}; +use soroban_sdk::{panic_with_error, Address, Env}; /// Issue #212 – End the campaign early (before deadline). /// @@ -88,11 +88,22 @@ pub fn cancel_campaign(env: &Env) { /// - `Error::InvalidEndTime` if `new_end_time` is more than ten years out /// - `Error::InvalidCampaignTransition` if campaign is not Active or GoalReached pub fn extend_deadline(env: &Env, new_end_time: u64) { + // Issue #92 – the direct call is the legacy 1-of-1 path: it auths the + // sole admin signer and is rejected once a multi-sig set is configured. + let admin = crate::admin::require_direct_admin(env); + admin.require_auth(); + + apply_extend_deadline(env, new_end_time, &admin); +} + +/// Issue #92 – Validation + mutation of `extend_deadline`, shared by the +/// direct entrypoint (above, after auth) and the timelock + multi-sig +/// `execute_admin_action` path (which has already established authority and +/// must not re-auth the creator). +pub(crate) fn apply_extend_deadline(env: &Env, new_end_time: u64, actor: &Address) { let mut campaign = get_campaign(env).unwrap_or_else(|| panic_with_error!(env, Error::NotInitialized)); - campaign.creator.require_auth(); - // Freeze invariant: all write operations are rejected while frozen (see freeze()). if is_frozen(env) { panic_with_error!(env, Error::ContractFrozen); @@ -113,7 +124,7 @@ pub fn extend_deadline(env: &Env, new_end_time: u64) { campaign.end_time = new_end_time; set_campaign(env, &campaign); - event::deadline_extended(env, &campaign.creator, old_deadline, new_end_time); + event::deadline_extended(env, actor, old_deadline, new_end_time); } /// Issue #235 — Get campaign status with computed fields. diff --git a/campaign/src/event.rs b/campaign/src/event.rs index b33b88d4..2902b02f 100644 --- a/campaign/src/event.rs +++ b/campaign/src/event.rs @@ -112,3 +112,34 @@ pub fn asset_unblocked(env: &Env, admin: &Address, asset: &Address, timestamp: u env.events() .publish(("campaign", "asset_unblocked"), (admin, asset, timestamp)); } + +// ─── Issue #92 – timelock + multi-sig admin governance ─────────────────────── + +/// Issue #92 – Emitted when an admin action is proposed. The `execute_after` +/// timestamp is the community's window to detect a malicious proposal. +pub fn admin_action_proposed(env: &Env, action_id: u64, proposer: &Address, execute_after: u64) { + env.events().publish( + ("campaign", "admin_action_proposed"), + (action_id, proposer, execute_after), + ); +} + +/// Issue #92 – Emitted when a signer approves a pending admin action. +pub fn admin_action_approved(env: &Env, action_id: u64, approver: &Address, approvals: u32) { + env.events().publish( + ("campaign", "admin_action_approved"), + (action_id, approver, approvals), + ); +} + +/// Issue #92 – Emitted when an admin action is executed. +pub fn admin_action_executed(env: &Env, action_id: u64, executor: &Address) { + env.events() + .publish(("campaign", "admin_action_executed"), (action_id, executor)); +} + +/// Issue #92 – Emitted when the admin signer set is replaced. +pub fn admin_signers_updated(env: &Env, signer_count: u32) { + env.events() + .publish(("campaign", "admin_signers_updated"), signer_count); +} diff --git a/campaign/src/lib.rs b/campaign/src/lib.rs index 396593be..6156c9e8 100644 --- a/campaign/src/lib.rs +++ b/campaign/src/lib.rs @@ -25,6 +25,7 @@ // the warning keeps CI clean without changing the published event topics. #![allow(deprecated)] +pub mod admin; pub mod asset_auth; pub mod backend; pub mod contract; @@ -39,7 +40,7 @@ pub mod types; pub mod validation; pub mod views; -use soroban_sdk::{contract, contractimpl, Address, BytesN, Env, Vec}; +use soroban_sdk::{contract, contractimpl, Address, Bytes, BytesN, Env, Vec}; use storage::{ acquire_lock, block_asset, bump_all_persistent, get_cached_report_storage, get_campaign, get_donor, get_donor_asset_donation, get_milestone, increment_donor_asset_donation, @@ -51,9 +52,9 @@ use storage::{ }; use types::{ - AssetInfo, CampaignData, CampaignInitializedEvent, CampaignReport, CampaignStatus, - CampaignStatusResponse, DashboardMetrics, DonorRecord, Error, MilestoneData, PlatformSummary, - StellarAsset, + ActionKind, AdminAction, AssetInfo, CampaignData, CampaignInitializedEvent, CampaignReport, + CampaignStatus, CampaignStatusResponse, DashboardMetrics, DonorRecord, Error, MilestoneData, + PlatformSummary, StellarAsset, }; use reports::{ @@ -651,10 +652,10 @@ impl CampaignContract { /// - `Error::NotInitialized` if campaign not yet initialized /// - `Error::ContractFrozen` if the contract is currently frozen pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) { - let campaign = - get_campaign(&env).unwrap_or_else(|| panic_with_error(&env, Error::NotInitialized)); - - campaign.creator.require_auth(); + // Issue #92 – direct call = legacy 1-of-1 path; auths the sole admin + // signer and panics `MultisigActive` once a multi-sig set exists. + let admin = admin::require_direct_admin(&env); + admin.require_auth(); // Freeze check — consistent with donate(), claim_refund(), and release_milestone() if is_frozen(&env) { @@ -666,7 +667,7 @@ impl CampaignContract { .update_current_contract_wasm(new_wasm_hash.clone()); let timestamp = env.ledger().timestamp(); - event::contract_upgraded(&env, &campaign.creator, new_wasm_hash, timestamp); + event::contract_upgraded(&env, &admin, new_wasm_hash, timestamp); } /// Issue #246 – Freeze the contract, blocking all mutating operations. @@ -678,15 +679,14 @@ impl CampaignContract { /// - `Error::Unauthorized` if not called by the creator /// - `Error::NotInitialized` if campaign not yet initialized pub fn freeze(env: Env) { - let campaign = - get_campaign(&env).unwrap_or_else(|| panic_with_error(&env, Error::NotInitialized)); - - campaign.creator.require_auth(); + // Issue #92 – direct call = legacy 1-of-1 path (see upgrade()). + let admin = admin::require_direct_admin(&env); + admin.require_auth(); set_frozen(&env, true); let timestamp = env.ledger().timestamp(); - event::contract_frozen(&env, &campaign.creator, timestamp); + event::contract_frozen(&env, &admin, timestamp); } /// Issue #120 – Public TTL maintenance entrypoint. @@ -714,15 +714,91 @@ impl CampaignContract { /// - `Error::Unauthorized` if not called by the creator /// - `Error::NotInitialized` if campaign not yet initialized pub fn unfreeze(env: Env) { - let campaign = - get_campaign(&env).unwrap_or_else(|| panic_with_error(&env, Error::NotInitialized)); - - campaign.creator.require_auth(); + // Issue #92 – direct call = legacy 1-of-1 path (see upgrade()). + let admin = admin::require_direct_admin(&env); + admin.require_auth(); set_frozen(&env, false); let timestamp = env.ledger().timestamp(); - event::contract_unfrozen(&env, &campaign.creator, timestamp); + event::contract_unfrozen(&env, &admin, timestamp); + } + + // ─── Issue #92 – timelock + multi-sig admin governance ────────────────── + + /// Issue #92 – Propose an admin action (upgrade / freeze / unfreeze / + /// extend-deadline) for the timelock + multi-sig flow. The proposer must + /// be an admin signer; proposing counts as the first approval. Returns + /// the action id. + /// + /// # Panics + /// - `Error::NotAdminSigner` if `proposer` is not an admin signer + /// - `Error::InvalidActionPayload` if the payload doesn't match the kind + /// - `Error::InvalidExecuteAfter` if `execute_after` is in the past + pub fn propose_admin_action( + env: Env, + proposer: Address, + kind: ActionKind, + payload: Bytes, + execute_after: u64, + ) -> u64 { + admin::propose_admin_action(&env, proposer, kind, payload, execute_after) + } + + /// Issue #92 – Approve a pending admin action. Each signer approves once. + /// + /// # Panics + /// - `Error::NotAdminSigner` if `approver` is not an admin signer + /// - `Error::ActionNotFound` if the action doesn't exist + /// - `Error::AlreadyApproved` if this signer already approved + pub fn approve_admin_action(env: Env, approver: Address, action_id: u64) { + admin::approve_admin_action(&env, approver, action_id) + } + + /// Issue #92 – Execute an approved admin action once the timelock has + /// elapsed and the quorum (2 approvals with a multi-sig set, 1 with the + /// legacy single signer) is met. The action is deleted before its effect + /// applies, so it can never replay. + /// + /// # Panics + /// - `Error::NotAdminSigner` if `executor` is not an admin signer + /// - `Error::ActionNotFound` if the action doesn't exist + /// - `Error::TimelockNotElapsed` if `execute_after` hasn't passed + /// - `Error::InsufficientApprovals` if approvals < quorum + pub fn execute_admin_action(env: Env, executor: Address, action_id: u64) { + admin::execute_admin_action(&env, executor, action_id) + } + + /// Issue #92 – Replace the admin signer set. Requires auth from every + /// current signer, so rotating a multi-sig is itself multi-sig; with the + /// default `[creator]` set this is the familiar creator-only auth. + /// + /// # Panics + /// - `Error::InvalidSigners` if the new set is empty or has duplicates + pub fn set_admin_signers(env: Env, new_signers: Vec
) { + admin::set_admin_signers(&env, new_signers) + } + + /// Issue #92 – Current admin signer set (stored set, or `[creator]`). + /// No auth required (read-only view). + pub fn get_admin_signers(env: Env) -> Vec
{ + admin::get_admin_signers(&env) + } + + /// Issue #92 – Fetch a pending admin action by id. + /// No auth required (read-only view). + /// + /// # Panics + /// - `Error::ActionNotFound` if the action doesn't exist (or was executed) + pub fn get_admin_action(env: Env, action_id: u64) -> AdminAction { + storage::get_admin_action(&env, action_id) + .unwrap_or_else(|| panic_with_error(&env, Error::ActionNotFound)) + } + + /// Issue #92 – Number of admin actions ever proposed. + /// No auth required (read-only view). + pub fn get_admin_action_count(env: Env) -> u64 { + storage::get_admin_action_count(&env) } /// Issue #175 – assert the current invoker is the campaign creator. @@ -801,6 +877,7 @@ fn panic_with_error(env: &Env, error: Error) -> ! { #[cfg(test)] mod test { + pub mod admin_action_tests; pub mod bump_storage_tests; pub mod claim_refund_tests; pub mod get_campaign_status_tests; diff --git a/campaign/src/storage.rs b/campaign/src/storage.rs index 22766a6b..7310efa7 100644 --- a/campaign/src/storage.rs +++ b/campaign/src/storage.rs @@ -1,7 +1,8 @@ // src/storage.rs use crate::types::{ - CampaignData, CampaignReport, DataKey, DonorRecord, Error, MilestoneData, MilestoneStatus, + AdminAction, CampaignData, CampaignReport, DataKey, DonorRecord, Error, MilestoneData, + MilestoneStatus, }; use soroban_sdk::{panic_with_error, Address, Env, Vec}; @@ -535,6 +536,15 @@ pub fn bump_all_persistent(env: &Env, milestone_count: u32) { if env.storage().persistent().has(&report_key) { bump_persistent(env, &report_key); } + // Issue #92 – admin governance keys. + let count_key = DataKey::AdminActionCount; + if env.storage().persistent().has(&count_key) { + bump_persistent(env, &count_key); + } + let signers_key = DataKey::AdminSigners; + if env.storage().persistent().has(&signers_key) { + bump_persistent(env, &signers_key); + } // Legacy per-index entries (pre-#118 layouts, not yet migrated). for i in 0..milestone_count { let key = DataKey::MilestoneData(i); @@ -543,3 +553,56 @@ pub fn bump_all_persistent(env: &Env, milestone_count: u32) { } } } + +// ─── Issue #92 – timelock + multi-sig admin actions ────────────────────────── + +/// Store an admin action proposal under its id. +pub fn set_admin_action(env: &Env, action_id: u64, action: &AdminAction) { + let key = DataKey::AdminAction(action_id); + env.storage().persistent().set(&key, action); + bump_persistent(env, &key); +} + +/// Fetch an admin action proposal, if it exists (executed actions are deleted). +pub fn get_admin_action(env: &Env, action_id: u64) -> Option { + env.storage() + .persistent() + .get(&DataKey::AdminAction(action_id)) +} + +/// Delete an admin action. Called on execution so an action cannot replay. +pub fn remove_admin_action(env: &Env, action_id: u64) { + env.storage() + .persistent() + .remove(&DataKey::AdminAction(action_id)); +} + +/// Number of admin actions ever proposed; the next proposal takes this id. +pub fn get_admin_action_count(env: &Env) -> u64 { + env.storage() + .persistent() + .get(&DataKey::AdminActionCount) + .unwrap_or(0) +} + +/// Increment the admin action counter, returning the id just consumed. +pub fn increment_admin_action_count(env: &Env) -> u64 { + let id = get_admin_action_count(env); + let key = DataKey::AdminActionCount; + env.storage().persistent().set(&key, &(id + 1)); + bump_persistent(env, &key); + id +} + +/// The stored admin signer set, if one has been configured. +/// `None` means the backwards-compatible default: `[creator]`, 1-of-1. +pub fn get_admin_signers_storage(env: &Env) -> Option> { + env.storage().persistent().get(&DataKey::AdminSigners) +} + +/// Replace the admin signer set. +pub fn set_admin_signers_storage(env: &Env, signers: &Vec
) { + let key = DataKey::AdminSigners; + env.storage().persistent().set(&key, signers); + bump_persistent(env, &key); +} diff --git a/campaign/src/test/admin_action_tests.rs b/campaign/src/test/admin_action_tests.rs new file mode 100644 index 00000000..bc0465b5 --- /dev/null +++ b/campaign/src/test/admin_action_tests.rs @@ -0,0 +1,479 @@ +//! Issue #92 – Tests for the timelock + multi-sig admin governance flow. +//! +//! Covers the full acceptance matrix: quorum enforcement (an action cannot +//! execute with <2 approvals under a multi-sig set), timelock enforcement +//! (cannot execute before `execute_after`), replay protection, payload +//! validation, and 1-of-1 backwards compatibility of the direct entrypoints. + +#![cfg(test)] + +use soroban_sdk::testutils::{Address as AddressTestUtils, Ledger}; +use soroban_sdk::{vec, Address, Bytes, Env, Vec}; + +use crate::storage::{is_frozen, set_campaign}; +use crate::types::{ActionKind, CampaignData, CampaignStatus, StellarAsset}; +use crate::CampaignContract; + +/// Base ledger timestamp; large enough to add deadlines on top of. +const BASE: u64 = 86400 * 365; + +/// One-hour timelock used by most tests. +const DELAY: u64 = 3600; + +fn make_env() -> Env { + let env = Env::default(); + env.ledger().set_timestamp(BASE); + env.mock_all_auths(); + env +} + +/// Register the contract and store an Active campaign; returns +/// `(contract_id, creator)`. Every subsequent contract invocation should use +/// its own `as_contract` frame (same-address re-auth inside one frame trips +/// "frame is already authorized"). +fn setup(env: &Env) -> (Address, Address) { + let contract_id = env.register_contract(None, CampaignContract); + let creator = Address::generate(env); + let campaign = CampaignData { + creator: creator.clone(), + goal_amount: 1000, + raised_amount: 0, + end_time: BASE + 30 * 86400, + status: CampaignStatus::Active, + accepted_assets: vec![ + env, + StellarAsset { + asset_code: soroban_sdk::String::from_str(env, "TST"), + issuer: Some(Address::generate(env)), + }, + ], + milestone_count: 0, + min_donation_amount: 0, + created_at_ledger: 0, + created_at_time: 0, + concluded_at_ledger: None, + }; + env.as_contract(&contract_id, || set_campaign(env, &campaign)); + (contract_id, creator) +} + +/// Configure a 2-signer multi-sig set (creator + one more); returns the +/// second signer. +fn setup_multisig(env: &Env, contract_id: &Address, creator: &Address) -> Address { + let second = Address::generate(env); + let signers: Vec
= vec![env, creator.clone(), second.clone()]; + env.as_contract(contract_id, || { + CampaignContract::set_admin_signers(env.clone(), signers) + }); + second +} + +fn empty_payload(env: &Env) -> Bytes { + Bytes::new(env) +} + +// ─── Signer set & backwards compatibility ──────────────────────────────────── + +/// Without a stored signer set the admin set defaults to `[creator]`. +#[test] +fn test_default_signers_is_creator_1_of_1() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + let signers = env.as_contract(&contract_id, || { + CampaignContract::get_admin_signers(env.clone()) + }); + assert_eq!(signers.len(), 1); + assert_eq!(signers.get(0).unwrap(), creator); +} + +/// Backwards compatibility: with the default single signer, the direct +/// `freeze()` / `unfreeze()` entrypoints keep working exactly as before. +#[test] +fn test_direct_freeze_still_works_with_single_signer() { + let env = make_env(); + let (contract_id, _creator) = setup(&env); + + env.as_contract(&contract_id, || CampaignContract::freeze(env.clone())); + let frozen = env.as_contract(&contract_id, || is_frozen(&env)); + assert!(frozen); + + env.as_contract(&contract_id, || CampaignContract::unfreeze(env.clone())); + let frozen = env.as_contract(&contract_id, || is_frozen(&env)); + assert!(!frozen); +} + +/// Once a multi-sig signer set is configured, direct admin calls are blocked +/// with `Error::MultisigActive` — the flow is the only path. +#[test] +#[should_panic(expected = "Error(Contract, #100)")] +fn test_direct_freeze_blocked_when_multisig_active() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + setup_multisig(&env, &contract_id, &creator); + + env.as_contract(&contract_id, || CampaignContract::freeze(env.clone())); +} + +/// The signer set rejects duplicates. +#[test] +#[should_panic(expected = "Error(Contract, #102)")] +fn test_set_admin_signers_rejects_duplicates() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + let signers: Vec
= vec![&env, creator.clone(), creator.clone()]; + env.as_contract(&contract_id, || { + CampaignContract::set_admin_signers(env.clone(), signers) + }); +} + +/// The signer set rejects an empty list. +#[test] +#[should_panic(expected = "Error(Contract, #102)")] +fn test_set_admin_signers_rejects_empty() { + let env = make_env(); + let (contract_id, _creator) = setup(&env); + env.as_contract(&contract_id, || { + CampaignContract::set_admin_signers(env.clone(), Vec::new(&env)) + }); +} + +// ─── Propose ───────────────────────────────────────────────────────────────── + +/// A non-signer cannot propose an action. +#[test] +#[should_panic(expected = "Error(Contract, #94)")] +fn test_propose_by_non_signer_rejected() { + let env = make_env(); + let (contract_id, _creator) = setup(&env); + let outsider = Address::generate(&env); + env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + outsider.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE + DELAY, + ) + }); +} + +/// An `Upgrade` proposal whose payload is not exactly 32 bytes is rejected. +#[test] +#[should_panic(expected = "Error(Contract, #99)")] +fn test_propose_upgrade_with_bad_payload_rejected() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Upgrade, + Bytes::from_array(&env, &[1, 2, 3]), + BASE + DELAY, + ) + }); +} + +/// An `execute_after` in the past is rejected at proposal time. +#[test] +#[should_panic(expected = "Error(Contract, #101)")] +fn test_propose_with_past_execute_after_rejected() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE - 1, + ) + }); +} + +/// Proposing stores the action, counts as the proposer's approval, and +/// increments the action counter. +#[test] +fn test_propose_records_action_and_first_approval() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + + let id = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE + DELAY, + ) + }); + assert_eq!(id, 0); + + let action = env.as_contract(&contract_id, || { + CampaignContract::get_admin_action(env.clone(), id) + }); + assert_eq!(action.kind, ActionKind::Freeze); + assert_eq!(action.execute_after, BASE + DELAY); + assert_eq!(action.voters.len(), 1); + assert!(action.voters.contains_key(creator.clone())); + + let count = env.as_contract(&contract_id, || { + CampaignContract::get_admin_action_count(env.clone()) + }); + assert_eq!(count, 1); +} + +// ─── Approve ───────────────────────────────────────────────────────────────── + +/// A signer cannot approve the same action twice (proposing counts as the +/// proposer's approval). +#[test] +#[should_panic(expected = "Error(Contract, #98)")] +fn test_double_approval_rejected() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + setup_multisig(&env, &contract_id, &creator); + + let id = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE + DELAY, + ) + }); + + env.as_contract(&contract_id, || { + CampaignContract::approve_admin_action(env.clone(), creator.clone(), id) + }); +} + +/// Approving a non-existent action panics with `ActionNotFound`. +#[test] +#[should_panic(expected = "Error(Contract, #95)")] +fn test_approve_unknown_action_rejected() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + env.as_contract(&contract_id, || { + CampaignContract::approve_admin_action(env.clone(), creator.clone(), 7) + }); +} + +// ─── Execute: quorum & timelock (acceptance criteria) ──────────────────────── + +/// Acceptance: with a 2-signer set, an action with only the proposer's +/// approval cannot execute — `InsufficientApprovals`. +#[test] +#[should_panic(expected = "Error(Contract, #97)")] +fn test_execute_with_one_of_two_approvals_rejected() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + setup_multisig(&env, &contract_id, &creator); + + let id = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE + DELAY, + ) + }); + + // Timelock elapsed, but quorum (2) not met: only the proposer approved. + env.ledger().set_timestamp(BASE + DELAY + 1); + env.as_contract(&contract_id, || { + CampaignContract::execute_admin_action(env.clone(), creator.clone(), id) + }); +} + +/// Acceptance: a fully-approved action cannot execute before `execute_after` +/// — `TimelockNotElapsed`. +#[test] +#[should_panic(expected = "Error(Contract, #96)")] +fn test_execute_before_timelock_rejected() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + let second = setup_multisig(&env, &contract_id, &creator); + + let id = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE + DELAY, + ) + }); + env.as_contract(&contract_id, || { + CampaignContract::approve_admin_action(env.clone(), second.clone(), id) + }); + + // Quorum met, but the timelock has not elapsed (now == BASE < BASE+DELAY). + env.as_contract(&contract_id, || { + CampaignContract::execute_admin_action(env.clone(), creator.clone(), id) + }); +} + +/// Happy path: propose freeze → second approval → warp past the timelock → +/// execute. The contract freezes; the flow then unfreezes it the same way. +#[test] +fn test_full_flow_freeze_then_unfreeze() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + let second = setup_multisig(&env, &contract_id, &creator); + + // Freeze via the flow. + let id = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE + DELAY, + ) + }); + env.as_contract(&contract_id, || { + CampaignContract::approve_admin_action(env.clone(), second.clone(), id) + }); + env.ledger().set_timestamp(BASE + DELAY + 1); + env.as_contract(&contract_id, || { + CampaignContract::execute_admin_action(env.clone(), second.clone(), id) + }); + assert!(env.as_contract(&contract_id, || is_frozen(&env))); + + // Unfreeze via the flow. + let id2 = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Unfreeze, + empty_payload(&env), + BASE + 2 * DELAY, + ) + }); + env.as_contract(&contract_id, || { + CampaignContract::approve_admin_action(env.clone(), second.clone(), id2) + }); + env.ledger().set_timestamp(BASE + 2 * DELAY + 1); + env.as_contract(&contract_id, || { + CampaignContract::execute_admin_action(env.clone(), creator.clone(), id2) + }); + assert!(!env.as_contract(&contract_id, || is_frozen(&env))); +} + +/// Replay protection: an executed action is deleted, so executing it again +/// panics with `ActionNotFound`. +#[test] +#[should_panic(expected = "Error(Contract, #95)")] +fn test_executed_action_cannot_replay() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + let second = setup_multisig(&env, &contract_id, &creator); + + let id = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE + DELAY, + ) + }); + env.as_contract(&contract_id, || { + CampaignContract::approve_admin_action(env.clone(), second.clone(), id) + }); + env.ledger().set_timestamp(BASE + DELAY + 1); + env.as_contract(&contract_id, || { + CampaignContract::execute_admin_action(env.clone(), creator.clone(), id) + }); + + // Second execution must find nothing. + env.as_contract(&contract_id, || { + CampaignContract::execute_admin_action(env.clone(), second.clone(), id) + }); +} + +/// 1-of-1 backwards compatibility inside the flow itself: with the default +/// single signer, the proposer's implicit approval alone meets the quorum. +#[test] +fn test_flow_works_with_single_signer_quorum_of_one() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + + let id = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE + DELAY, + ) + }); + env.ledger().set_timestamp(BASE + DELAY + 1); + env.as_contract(&contract_id, || { + CampaignContract::execute_admin_action(env.clone(), creator.clone(), id) + }); + assert!(env.as_contract(&contract_id, || is_frozen(&env))); +} + +/// ExtendDeadline flows end-to-end: the payload's big-endian `u64` becomes +/// the campaign's new `end_time` after quorum + timelock. +#[test] +fn test_extend_deadline_via_flow() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + let second = setup_multisig(&env, &contract_id, &creator); + + let new_end = BASE + 60 * 86400; + let payload = Bytes::from_array(&env, &new_end.to_be_bytes()); + + let id = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::ExtendDeadline, + payload, + BASE + DELAY, + ) + }); + env.as_contract(&contract_id, || { + CampaignContract::approve_admin_action(env.clone(), second.clone(), id) + }); + env.ledger().set_timestamp(BASE + DELAY + 1); + env.as_contract(&contract_id, || { + CampaignContract::execute_admin_action(env.clone(), second.clone(), id) + }); + + let campaign = env.as_contract(&contract_id, || crate::storage::get_campaign(&env).unwrap()); + assert_eq!(campaign.end_time, new_end); +} + +/// A non-signer cannot execute even a fully-approved, matured action. +#[test] +#[should_panic(expected = "Error(Contract, #94)")] +fn test_execute_by_non_signer_rejected() { + let env = make_env(); + let (contract_id, creator) = setup(&env); + let second = setup_multisig(&env, &contract_id, &creator); + + let id = env.as_contract(&contract_id, || { + CampaignContract::propose_admin_action( + env.clone(), + creator.clone(), + ActionKind::Freeze, + empty_payload(&env), + BASE + DELAY, + ) + }); + env.as_contract(&contract_id, || { + CampaignContract::approve_admin_action(env.clone(), second.clone(), id) + }); + env.ledger().set_timestamp(BASE + DELAY + 1); + + let outsider = Address::generate(&env); + env.as_contract(&contract_id, || { + CampaignContract::execute_admin_action(env.clone(), outsider.clone(), id) + }); +} diff --git a/campaign/src/types.rs b/campaign/src/types.rs index f794858a..b236b0ed 100644 --- a/campaign/src/types.rs +++ b/campaign/src/types.rs @@ -1,7 +1,7 @@ // src/types.rs use soroban_sdk::{ - contracterror, contracttype, panic_with_error, Address, BytesN, Env, String, Vec, + contracterror, contracttype, panic_with_error, Address, Bytes, BytesN, Env, Map, String, Vec, }; // ─── Error enum ─────────────────────────────────────────────────────────────── @@ -116,6 +116,31 @@ pub enum Error { // ── Asset block ───────────────────────────────────────────────────── 9x /// Donations in this asset are blocked by the admin. AssetBlocked = 90, + + // Codes 91–93 are reserved by the in-flight donation-receipt work (#158). + + // ── Issue #92 – timelock + multi-sig admin ────────────────────────────── + /// Caller is not a member of the admin signer set. + NotAdminSigner = 94, + /// No admin action exists under the given id (never proposed, or already + /// executed — actions are deleted on execution to prevent replay). + ActionNotFound = 95, + /// The action's `execute_after` timestamp has not been reached yet. + TimelockNotElapsed = 96, + /// The action has fewer approvals than the required quorum. + InsufficientApprovals = 97, + /// This signer has already approved the action. + AlreadyApproved = 98, + /// The action payload does not match its kind (e.g. an `Upgrade` payload + /// that is not exactly 32 bytes). + InvalidActionPayload = 99, + /// Direct admin calls are disabled while a multi-sig signer set (≥2 + /// signers) is configured; use the propose/approve/execute flow. + MultisigActive = 100, + /// `execute_after` lies in the past at proposal time. + InvalidExecuteAfter = 101, + /// The proposed signer set is invalid (empty or contains duplicates). + InvalidSigners = 102, } #[cfg(test)] @@ -167,6 +192,15 @@ mod error_code_tests { Error::InvalidAmount as u32, Error::ContractFrozen as u32, Error::AssetBlocked as u32, + Error::NotAdminSigner as u32, + Error::ActionNotFound as u32, + Error::TimelockNotElapsed as u32, + Error::InsufficientApprovals as u32, + Error::AlreadyApproved as u32, + Error::InvalidActionPayload as u32, + Error::MultisigActive as u32, + Error::InvalidExecuteAfter as u32, + Error::InvalidSigners as u32, ]; for (index, code) in campaign_codes.iter().enumerate() { assert!(!campaign_codes[index + 1..].contains(code)); @@ -324,6 +358,16 @@ pub enum DataKey { /// clients read one entry instead of recomputing from campaign + /// milestones + counters on every call. CachedReport, + + // ── Persistent (appended — issue #92) ─────────────────────────────────── + /// A proposed admin action (timelock + multi-sig flow), keyed by id. + /// Deleted on execution so an action can never be replayed. + AdminAction(u64), + /// Number of admin actions ever proposed; the next proposal takes this id. + AdminActionCount, + /// The admin signer set. Absent = `[creator]`, the backwards-compatible + /// 1-of-1 quorum. + AdminSigners, } // ─── Asset types ────────────────────────────────────────────────────────────── @@ -664,3 +708,38 @@ pub struct RefundProcessedEvent { pub asset: AssetInfo, pub ledger: u32, } + +// ─── Issue #92 – timelock + multi-sig admin governance ─────────────────────── + +/// The kind of privileged operation an [`AdminAction`] performs. +/// +/// Encodes by variant name (`contracttype`), so appending new kinds is safe. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum ActionKind { + /// Replace the contract WASM. Payload: the 32-byte new WASM hash. + Upgrade, + /// Freeze the contract (block all mutating operations). Payload: empty. + Freeze, + /// Unfreeze the contract. Payload: empty. + Unfreeze, + /// Extend the campaign deadline. Payload: the new end time as 8 bytes, + /// big-endian `u64`. + ExtendDeadline, +} + +/// Issue #92 – A proposed admin action moving through the timelock + +/// multi-sig flow: proposed, approved by the signer quorum, then executed +/// once `execute_after` has passed. +#[contracttype] +#[derive(Clone, Debug)] +pub struct AdminAction { + /// Which privileged operation this action performs. + pub kind: ActionKind, + /// Kind-specific payload (see [`ActionKind`] variants for the encoding). + pub payload: Bytes, + /// Ledger timestamp before which the action cannot execute (timelock). + pub execute_after: u64, + /// Signers who have approved. Proposing counts as the first approval. + pub voters: Map, +}