From 58f4c9d145217a14cb014ce62f7b6e9a5a800aed Mon Sep 17 00:00:00 2001 From: Anonfedora Date: Mon, 30 Mar 2026 10:50:05 +0100 Subject: [PATCH] feat: multisig proposal expiry --- docs/multisig-proposal-expiry.md | 72 ++++++++++++++++++++++++++++++++ src/chunking_tests.rs | 1 + src/lib.rs | 45 +++++++++++++++++++- src/test.rs | 12 +++--- src/test_utils.rs | 7 ++-- 5 files changed, 127 insertions(+), 10 deletions(-) create mode 100644 docs/multisig-proposal-expiry.md diff --git a/docs/multisig-proposal-expiry.md b/docs/multisig-proposal-expiry.md new file mode 100644 index 000000000..7b31562c7 --- /dev/null +++ b/docs/multisig-proposal-expiry.md @@ -0,0 +1,72 @@ +# Multisig Proposal Expiry + +## Overview + +The contract includes a minimal multisig admin module used for sensitive administrative changes (e.g. freezing the contract, rotating owners, changing threshold, changing proposal duration). + +Each multisig proposal has a deterministic `expiry` timestamp stored on-chain. After expiry, the proposal can no longer be approved or executed. + +## Storage and Types + +- Proposals are stored in persistent storage under `DataKey::MultisigProposal(u32)`. +- Proposal payload is represented by: + - `ProposalAction` (the requested admin change) + - `Proposal` (proposal metadata including approvals and expiry) + +The `Proposal` struct includes: + +- `id: u32` +- `action: ProposalAction` +- `proposer: Address` +- `approvals: Vec
` +- `executed: bool` +- `expiry: u64` + +## Expiry Semantics + +### How expiry is computed + +When creating a proposal via `propose_action`, expiry is calculated as: + +- `now = env.ledger().timestamp()` +- `expiry = now + proposal_duration` + +The implementation uses checked arithmetic (`checked_add`) and fails if the addition would overflow. + +`proposal_duration` is set during `init_multisig` and can later be updated through a multisig proposal (`ProposalAction::SetProposalDuration`). + +### When a proposal is considered expired + +A proposal is considered expired when: + +- `env.ledger().timestamp() >= proposal.expiry` + +This boundary is intentional and deterministic. If `now == expiry`, the proposal is already expired. + +### What expiry blocks + +If a proposal is expired, the following entrypoints return `Err(RevoraError::ProposalExpired)`: + +- `approve_action` +- `execute_action` + +`get_proposal` remains readable regardless of expiry. + +## Security Assumptions and Abuse Paths + +- Expiry prevents execution of stale proposals after long inactivity or after off-chain coordination context has changed. +- Expiry does not delete proposal data. Proposals remain in storage for auditability. +- An attacker cannot extend the life of an already-created proposal. Updating `MultisigProposalDuration` only affects proposals created after the update. +- Expiry is based on ledger timestamp. This assumes the ledger timestamp is the authoritative time source for the chain. + +## Test Coverage + +Deterministic tests validate expiry behavior at the exact boundary (`now == expiry`): + +- Approval fails at expiry. +- Execution fails at expiry. + +See `src/test.rs`: + +- `multisig_approve_fails_after_expiry_boundary` +- `multisig_execute_fails_after_expiry_boundary` diff --git a/src/chunking_tests.rs b/src/chunking_tests.rs index d14819f73..192ef7ec3 100644 --- a/src/chunking_tests.rs +++ b/src/chunking_tests.rs @@ -1,3 +1,4 @@ +#![cfg(test)] #![allow(dead_code, unused_variables, unused_imports)] use crate::{RevoraRevenueShare, RevoraRevenueShareClient}; diff --git a/src/lib.rs b/src/lib.rs index 3179f7fe6..0cd326b92 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -73,6 +73,8 @@ pub enum RevoraError { SignatureReplay = 28, /// Off-chain signer key has not been registered. SignerKeyNotRegistered = 29, + /// Multisig proposal has expired. + ProposalExpired = 30, } // ── Event symbols ──────────────────────────────────────────── @@ -120,6 +122,7 @@ const EVENT_CLAIM_DELAY_SET: Symbol = symbol_short!("delay_set"); const EVENT_PROPOSAL_CREATED: Symbol = symbol_short!("prop_new"); const EVENT_PROPOSAL_APPROVED: Symbol = symbol_short!("prop_app"); const EVENT_PROPOSAL_EXECUTED: Symbol = symbol_short!("prop_exe"); +const EVENT_DURATION_SET: Symbol = symbol_short!("dur_set"); #[contracttype] #[derive(Clone, Debug, PartialEq)] @@ -129,6 +132,7 @@ pub enum ProposalAction { SetThreshold(u32), AddOwner(Address), RemoveOwner(Address), + SetProposalDuration(u64), } #[contracttype] @@ -139,6 +143,7 @@ pub struct Proposal { pub proposer: Address, pub approvals: Vec
, pub executed: bool, + pub expiry: u64, } const EVENT_SNAP_CONFIG: Symbol = symbol_short!("snap_cfg"); @@ -416,6 +421,8 @@ pub enum DataKey { MultisigProposal(u32), /// Multisig proposal count. MultisigProposalCount, + /// Multisig proposal duration in seconds. + MultisigProposalDuration, /// Whether snapshot distribution is enabled for an offering. SnapshotConfig(OfferingId), @@ -3067,6 +3074,7 @@ impl RevoraRevenueShare { caller: Address, owners: Vec
, threshold: u32, + proposal_duration: u64, ) -> Result<(), RevoraError> { caller.require_auth(); if env.storage().persistent().has(&DataKey::MultisigThreshold) { @@ -3081,6 +3089,7 @@ impl RevoraRevenueShare { env.storage().persistent().set(&DataKey::MultisigThreshold, &threshold); env.storage().persistent().set(&DataKey::MultisigOwners, &owners); env.storage().persistent().set(&DataKey::MultisigProposalCount, &0_u32); + env.storage().persistent().set(&DataKey::MultisigProposalDuration, &proposal_duration); Ok(()) } @@ -3097,6 +3106,14 @@ impl RevoraRevenueShare { let count_key = DataKey::MultisigProposalCount; let id: u32 = env.storage().persistent().get(&count_key).unwrap_or(0); + let duration: u64 = env + .storage() + .persistent() + .get(&DataKey::MultisigProposalDuration) + .ok_or(RevoraError::NotInitialized)?; + let now = env.ledger().timestamp(); + let expiry = now.checked_add(duration).ok_or(RevoraError::InvalidAmount)?; + // Proposer's vote counts as the first approval automatically let mut initial_approvals = Vec::new(&env); initial_approvals.push_back(proposer.clone()); @@ -3107,12 +3124,13 @@ impl RevoraRevenueShare { proposer: proposer.clone(), approvals: initial_approvals, executed: false, + expiry, }; env.storage().persistent().set(&DataKey::MultisigProposal(id), &proposal); env.storage().persistent().set(&count_key, &(id + 1)); - env.events().publish((EVENT_PROPOSAL_CREATED, proposer.clone()), id); + env.events().publish((EVENT_PROPOSAL_CREATED, proposer.clone()), (id, expiry)); env.events().publish((EVENT_PROPOSAL_APPROVED, proposer), id); Ok(id) } @@ -3134,6 +3152,10 @@ impl RevoraRevenueShare { return Err(RevoraError::LimitReached); } + if env.ledger().timestamp() >= proposal.expiry { + return Err(RevoraError::ProposalExpired); + } + // Check for duplicate approvals for i in 0..proposal.approvals.len() { if proposal.approvals.get(i).unwrap() == approver { @@ -3158,6 +3180,10 @@ impl RevoraRevenueShare { return Err(RevoraError::LimitReached); } + if env.ledger().timestamp() >= proposal.expiry { + return Err(RevoraError::ProposalExpired); + } + let threshold: u32 = env .storage() .persistent() @@ -3208,6 +3234,18 @@ impl RevoraRevenueShare { } env.storage().persistent().set(&DataKey::MultisigOwners, &new_owners); } + ProposalAction::SetProposalDuration(new_duration) => { + if new_duration == 0 { + return Err(RevoraError::InvalidAmount); + } + env.storage() + .persistent() + .set(&DataKey::MultisigProposalDuration, &new_duration); + env.events().publish( + (EVENT_DURATION_SET, proposal.proposer.clone()), + new_duration, + ); + } } proposal.executed = true; @@ -3993,8 +4031,13 @@ mod vesting_test; #[cfg(test)] mod test_utils; +#[cfg(test)] mod chunking_tests; +#[cfg(test)] mod test; +#[cfg(test)] mod test_auth; +#[cfg(test)] mod test_cross_contract; +#[cfg(test)] mod test_namespaces; diff --git a/src/test.rs b/src/test.rs index e82a881ad..aa7a7acb2 100644 --- a/src/test.rs +++ b/src/test.rs @@ -4592,8 +4592,8 @@ fn multisig_setup() -> (Env, RevoraRevenueShareClient<'static>, Address, Address owners.push_back(owner2.clone()); owners.push_back(owner3.clone()); - // 2-of-3 threshold - client.init_multisig(&caller, &owners, &2); + // 2-of-3 threshold with 86400s (1 day) duration + client.init_multisig(&caller, &owners, &2, &86400); (env, client, owner1, owner2, owner3, caller) } @@ -4616,7 +4616,7 @@ fn multisig_init_twice_fails() { let mut owners2 = Vec::new(&env); owners2.push_back(owner1.clone()); - let r = client.try_init_multisig(&caller, &owners2, &1); + let r = client.try_init_multisig(&caller, &owners2, &1, &86400); assert!(r.is_err()); } @@ -4633,7 +4633,7 @@ fn multisig_init_zero_threshold_fails() { let mut owners = Vec::new(&env); owners.push_back(owner.clone()); - let r = client.try_init_multisig(&caller, &owners, &0); + let r = client.try_init_multisig(&caller, &owners, &0, &86400); assert!(r.is_err()); } @@ -4651,7 +4651,7 @@ fn multisig_init_threshold_exceeds_owners_fails() { let mut owners = Vec::new(&env); owners.push_back(owner.clone()); // threshold=2 but only 1 owner - let r = client.try_init_multisig(&caller, &owners, &2); + let r = client.try_init_multisig(&caller, &owners, &2, &86400); assert!(r.is_err()); } @@ -4664,7 +4664,7 @@ fn multisig_init_empty_owners_fails() { let issuer = caller.clone(); let owners = Vec::new(&env); - let r = client.try_init_multisig(&caller, &owners, &1); + let r = client.try_init_multisig(&caller, &owners, &1, &86400); assert!(r.is_err()); } diff --git a/src/test_utils.rs b/src/test_utils.rs index 19892913c..a705080ed 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -1,11 +1,12 @@ #![cfg(test)] #![allow(warnings)] // Silences the unused variable errors failing the CI -use soroban_sdk::{testutils::Address as _, Address, Env}; use crate::{RevoraRevenueShare, RevoraRevenueShareClient}; +use soroban_sdk::{testutils::Address as _, Address, Env}; /// Core test utilities avoiding self-referential struct lifetime errors. -pub fn setup_context() -> (Env, RevoraRevenueShareClient<'static>, Address, Address, Address, Address) { +pub fn setup_context( +) -> (Env, RevoraRevenueShareClient<'static>, Address, Address, Address, Address) { let env = Env::default(); env.mock_all_auths(); let contract_id = env.register_contract(None, RevoraRevenueShare); @@ -14,4 +15,4 @@ pub fn setup_context() -> (Env, RevoraRevenueShareClient<'static>, Address, Addr let token = Address::generate(&env); let payout_asset = Address::generate(&env); (env, client, contract_id, issuer, token, payout_asset) -} \ No newline at end of file +}