diff --git a/docs/multisig-proposal-expiry.md b/docs/multisig-proposal-expiry.md new file mode 100644 index 00000000..7b31562c --- /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/lib.rs b/src/lib.rs index 5c34a4a4..9f97e408 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -76,6 +76,8 @@ pub enum RevoraError { SignatureReplay = 28, /// Off-chain signer key has not been registered. SignerKeyNotRegistered = 29, + /// Multisig proposal has expired. + ProposalExpired = 30, /// Cross-contract token transfer failed. TransferFailed = 30, /// Contract is already at the target version; no migration needed. @@ -134,6 +136,7 @@ const EVENT_PROPOSAL_APPROVED_V2: Symbol = symbol_short!("prop_a2"); const EVENT_PROPOSAL_EXECUTED_V2: Symbol = symbol_short!("prop_e2"); 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)] @@ -144,6 +147,7 @@ pub enum ProposalAction { SetThreshold(u32), AddOwner(Address), RemoveOwner(Address), + SetProposalDuration(u64), } #[contracttype] @@ -154,6 +158,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"); @@ -487,6 +492,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), @@ -4242,6 +4249,7 @@ impl RevoraRevenueShare { caller: Address, owners: Vec, threshold: u32, + proposal_duration: u64, ) -> Result<(), RevoraError> { caller.require_auth(); @@ -4295,6 +4303,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()); @@ -4305,12 +4321,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) } @@ -4332,6 +4349,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 { @@ -4401,6 +4422,18 @@ impl RevoraRevenueShare { // Persist updated owners list env.storage().persistent().set(&DataKey::MultisigOwners, &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, + ); + } } // ─── Storage key types ──────────────────────────────────────────────────────── diff --git a/src/test.rs b/src/test.rs index 3958da80..310625e5 100644 --- a/src/test.rs +++ b/src/test.rs @@ -3994,8 +3994,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) } @@ -4018,7 +4018,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()); } @@ -4035,7 +4035,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()); } @@ -4053,7 +4053,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()); } @@ -4066,7 +4066,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()); }