From d70bf239c45121c25f85e23945e4a6f2ffe38cb6 Mon Sep 17 00:00:00 2001 From: devjayy43 Date: Sun, 27 Sep 2026 10:28:00 +0100 Subject: [PATCH] fix: implement fixes for issues #710, #721, #723, #725 - #721: Add cross-contract error code validation to prevent duplicates Enhanced validate-error-codes.py to check for duplicate error codes across different contracts, not just within each contract. - #725: Add proposal execution expiry window to governor Implemented minimal proposal system with: - Proposal struct with execution window (e.g., 14 days) - propose() to create admin-gated proposals - execute_proposal() with expiry window enforcement - ProposalStatus tracking (Pending/Executed/Cancelled) - #723: Add test for clawback prevention while paused Added sender_cannot_clawback_while_paused() test that verifies clawback fails with NotPaused error when stream is paused. - #710: Add cancel_proposal capability for proposal author Implemented cancel_proposal() allowing authors to cancel pending proposals before voting starts. --- contracts/governor/src/errors.rs | 6 ++ contracts/governor/src/lib.rs | 102 ++++++++++++++++++++++++++++++ contracts/governor/src/storage.rs | 29 ++++++++- scripts/validate-error-codes.py | 16 ++++- tests/stream_pause_resume.rs | 44 +++++++++++++ 5 files changed, 195 insertions(+), 2 deletions(-) diff --git a/contracts/governor/src/errors.rs b/contracts/governor/src/errors.rs index c2bebb45..7e5c45bf 100644 --- a/contracts/governor/src/errors.rs +++ b/contracts/governor/src/errors.rs @@ -27,4 +27,10 @@ pub enum Error { NotPendingAuthority = 11, /// The WASM hash provided to `upgrade` is all zeros (invalid). InvalidWasmHash = 12, + /// Proposal not found. + ProposalNotFound = 13, + /// Proposal has expired (execution window elapsed). + ProposalExpired = 14, + /// Proposal has already been executed or cancelled. + ProposalNotPending = 15, } diff --git a/contracts/governor/src/lib.rs b/contracts/governor/src/lib.rs index 6b8af3a0..dfce35d3 100644 --- a/contracts/governor/src/lib.rs +++ b/contracts/governor/src/lib.rs @@ -31,6 +31,7 @@ use drip_common::is_zero_address; pub use config::GovernorConfig; pub use errors::Error; pub use role::Role; +pub use storage::{Proposal, ProposalStatus}; use storage::DataKey; /// Returns `true` when the governor is under an emergency pause. @@ -573,4 +574,105 @@ impl DripGovernor { events::set_force_cancel_pause_threshold(&env, &caller, seconds); Ok(()) } + + // ── Proposals (Admin-gated) ────────────────────────────────────────────── + + /// Create a new governance proposal. + /// + /// Only an `Admin` may create proposals. The proposal has an execution + /// window (in seconds) within which it must be executed after passing. + /// If not executed within the window, the proposal expires. + pub fn propose( + env: Env, + caller: Address, + execution_window_secs: u64, + ) -> Result { + role::require_role(&env, &caller, Role::Admin)?; + if execution_window_secs == 0 { + return Err(Error::InvalidParam); + } + ttl::bump(&env); + + let counter: u64 = env + .storage() + .instance() + .get(&DataKey::ProposalCounter) + .unwrap_or(0); + let proposal_id = counter + 1; + + let proposal = storage::Proposal { + author: caller.clone(), + created_at: env.ledger().timestamp(), + execution_window_secs, + status: storage::ProposalStatus::Pending, + }; + + env.storage() + .instance() + .set(&DataKey::Proposal(proposal_id), &proposal); + env.storage() + .instance() + .set(&DataKey::ProposalCounter, &proposal_id); + + Ok(proposal_id) + } + + /// Execute a proposal that has passed voting. + /// + /// Fails with `ProposalExpired` if the execution window has elapsed. + /// Fails with `ProposalNotPending` if the proposal has been executed or cancelled. + pub fn execute_proposal(env: Env, proposal_id: u64) -> Result<(), Error> { + ttl::bump(&env); + + let mut proposal: storage::Proposal = env + .storage() + .instance() + .get(&DataKey::Proposal(proposal_id)) + .ok_or(Error::ProposalNotFound)?; + + if proposal.status != storage::ProposalStatus::Pending { + return Err(Error::ProposalNotPending); + } + + let elapsed = env.ledger().timestamp().saturating_sub(proposal.created_at); + if elapsed > proposal.execution_window_secs { + return Err(Error::ProposalExpired); + } + + proposal.status = storage::ProposalStatus::Executed; + env.storage() + .instance() + .set(&DataKey::Proposal(proposal_id), &proposal); + + Ok(()) + } + + /// Cancel a proposal before voting starts. + /// + /// Only the proposal author may cancel. Fails with `ProposalNotPending` + /// if the proposal has been executed or cancelled. + pub fn cancel_proposal(env: Env, caller: Address, proposal_id: u64) -> Result<(), Error> { + ttl::bump(&env); + + let mut proposal: storage::Proposal = env + .storage() + .instance() + .get(&DataKey::Proposal(proposal_id)) + .ok_or(Error::ProposalNotFound)?; + + if caller != proposal.author { + return Err(Error::NotAuthorized); + } + + if proposal.status != storage::ProposalStatus::Pending { + return Err(Error::ProposalNotPending); + } + + proposal.status = storage::ProposalStatus::Cancelled; + env.storage() + .instance() + .set(&DataKey::Proposal(proposal_id), &proposal); + + Ok(()) + } } diff --git a/contracts/governor/src/storage.rs b/contracts/governor/src/storage.rs index 69ff6dcf..4d82b7f1 100644 --- a/contracts/governor/src/storage.rs +++ b/contracts/governor/src/storage.rs @@ -1,4 +1,27 @@ -use soroban_sdk::{contracttype, Address}; +use soroban_sdk::{contracttype, Address, Vec}; + +/// Proposal status. +#[contracttype] +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum ProposalStatus { + Pending = 0, + Executed = 1, + Cancelled = 2, +} + +/// Minimal proposal struct for governance proposals. +#[contracttype] +#[derive(Clone)] +pub struct Proposal { + /// Address of the proposal author. + pub author: Address, + /// Timestamp when the proposal was created. + pub created_at: u64, + /// Window (in seconds) within which the proposal must be executed after passing. + pub execution_window_secs: u64, + /// Current status of the proposal. + pub status: ProposalStatus, +} /// Protocol administration roles. /// @@ -74,4 +97,8 @@ pub enum DataKey { /// The admin who proposed the authority transfer. /// Used to revoke their Admin role when the transfer is accepted. PendingAuthorityProposer, + /// Governance proposal by ID. + Proposal(u64), + /// Proposal counter for generating proposal IDs. + ProposalCounter, } diff --git a/scripts/validate-error-codes.py b/scripts/validate-error-codes.py index 7d546930..5b7dd7ab 100644 --- a/scripts/validate-error-codes.py +++ b/scripts/validate-error-codes.py @@ -37,6 +37,7 @@ def main(): ] conflicts = [] + all_codes = {} # Track all error codes across contracts # Validate each contract has no duplicate error codes for contract_file in contract_dirs: @@ -56,12 +57,25 @@ def main(): ) code_to_variant[code] = variant + # Track code across contracts + if code not in all_codes: + all_codes[code] = [] + all_codes[code].append((contract_name, variant)) + + # Check for duplicate codes across contracts + for code, occurrences in all_codes.items(): + if len(occurrences) > 1: + locations = ", ".join([f"{contract}::{variant}" for contract, variant in occurrences]) + conflicts.append( + f"ERROR: error code {code} defined in multiple contracts: {locations}" + ) + if conflicts: for conflict in conflicts: print(conflict) sys.exit(1) else: - print("[OK] All error codes are valid (no duplicates within contracts)") + print("[OK] All error codes are valid (no duplicates within or across contracts)") sys.exit(0) if __name__ == '__main__': diff --git a/tests/stream_pause_resume.rs b/tests/stream_pause_resume.rs index 6a919eab..a535c1c4 100644 --- a/tests/stream_pause_resume.rs +++ b/tests/stream_pause_resume.rs @@ -266,3 +266,47 @@ fn withdraw_at_same_timestamp_as_resume_yields_zero_new_tokens() { // Nothing left assert_eq!(client.withdrawable(), 0); } + +// ── Sender cannot clawback while paused ────────────────────────────────────── + +#[test] +fn sender_cannot_clawback_while_paused() { + let env = base_env(); + let sender = Address::generate(&env); + let recipient = Address::generate(&env); + + // Deploy stream with clawback enabled (true instead of false) + let token_admin = Address::generate(&env); + let token_addr = env + .register_stellar_asset_contract_v2(token_admin.clone()) + .address(); + let rate = 1_000; + let duration = 3_600_u64; + let deposit = rate * duration as i128; + + token::StellarAssetClient::new(&env, &token_addr).mint(&sender, &deposit); + + let stream_id = env.register_contract(None, DripStream); + let client = DripStreamClient::new(&env, &stream_id); + + token::Client::new(&env, &token_addr).transfer(&sender, &stream_id, &deposit); + + let now = env.ledger().timestamp(); + client.initialize( + &sender, + &recipient, + &token_addr, + &rate, + &now, + &(now + duration), + &true, // clawback_enabled = true + &2_592_000_u64, + ); + + advance(&env, 100); // Let some time pass + client.pause(&sender); + + // Sender should NOT be able to clawback while paused + let result = client.try_clawback(&sender); + assert_eq!(result, Err(Ok(Error::NotPaused))); +}