diff --git a/swaptrade-contracts/counter/src/governance/governance.rs b/swaptrade-contracts/counter/src/governance/governance.rs index 308c8e3..6ce7bac 100644 --- a/swaptrade-contracts/counter/src/governance/governance.rs +++ b/swaptrade-contracts/counter/src/governance/governance.rs @@ -268,6 +268,10 @@ pub struct MultiSigProposal { pub approvals: HashSet, pub executed: bool, pub rejected: bool, + /// Minimum signatures required for execution + pub required_signatures: usize, + /// Total authorized signers at time of creation + pub total_signers: usize, } impl MultiSigProposal { @@ -276,7 +280,7 @@ impl MultiSigProposal { } pub fn is_approved(&self) -> bool { - self.approvals.len() >= MULTISIG_THRESHOLD + self.approvals.len() >= self.required_signatures } } @@ -330,6 +334,8 @@ impl MultiSigCoordinator { approvals, executed: false, rejected: false, + required_signatures: MULTISIG_THRESHOLD, + total_signers: self.authorized_signers.len(), }); Ok(proposal_id) @@ -347,6 +353,11 @@ impl MultiSigCoordinator { if proposal.executed { return Err("Already executed".into()); } if proposal.rejected { return Err("Proposal rejected".into()); } + // Prevent duplicate signatures + if proposal.approvals.contains(&signer) { + return Err(format!("'{}' has already approved this proposal", signer)); + } + proposal.approvals.insert(signer); Ok(proposal.approvals.len()) } @@ -359,7 +370,7 @@ impl MultiSigCoordinator { if proposal.rejected { return Err("Proposal rejected".into()); } if !proposal.is_approved() { return Err(format!( - "Insufficient approvals: {}/{}", proposal.approval_count(), MULTISIG_THRESHOLD + "Insufficient approvals: {}/{}", proposal.approval_count(), proposal.required_signatures )); } diff --git a/swaptrade-contracts/counter/src/governance/voting.rs b/swaptrade-contracts/counter/src/governance/voting.rs index 71e7710..d586703 100644 --- a/swaptrade-contracts/counter/src/governance/voting.rs +++ b/swaptrade-contracts/counter/src/governance/voting.rs @@ -11,6 +11,7 @@ pub enum ProposalStatus { Passed, Rejected, Expired, + Executed, } #[derive(Debug, Clone)] @@ -86,6 +87,18 @@ impl Proposal { self.status = ProposalStatus::Rejected; } } + + /// Execute a passed proposal. Prevents double execution. + pub fn execute(&mut self) -> Result<(), String> { + if self.status == ProposalStatus::Executed { + return Err("Proposal has already been executed".to_string()); + } + if self.status != ProposalStatus::Passed { + return Err(format!("Proposal cannot be executed, current status: {:?}", self.status)); + } + self.status = ProposalStatus::Executed; + Ok(()) + } } pub struct GovernanceVoting { @@ -127,6 +140,11 @@ impl GovernanceVoting { Ok(proposal.status.clone()) } + pub fn execute_proposal(&mut self, proposal_id: u64) -> Result<(), String> { + let proposal = self.proposals.get_mut(&proposal_id).ok_or("proposal not found")?; + proposal.execute() + } + pub fn get(&self, proposal_id: u64) -> Option<&Proposal> { self.proposals.get(&proposal_id) } diff --git a/swaptrade-contracts/counter/src/governance_multisig_tests.rs b/swaptrade-contracts/counter/src/governance_multisig_tests.rs new file mode 100644 index 0000000..0aadbc6 --- /dev/null +++ b/swaptrade-contracts/counter/src/governance_multisig_tests.rs @@ -0,0 +1,192 @@ +// tests/governance_multisig_tests.rs +//! Tests for multi-signature governance execution (Issue #166) +//! and prevention of double execution (Issue #167) + +#[cfg(test)] +mod tests { + use crate::governance::{ + MultiSigCoordinator, MultiSigProposal, MULTISIG_THRESHOLD, MULTISIG_TOTAL, + }; + + #[test] + fn test_multisig_requires_minimum_signatures() { + let signers: Vec = (0..MULTISIG_TOTAL) + .map(|i| format!("signer_{}", i)) + .collect(); + let mut coordinator = MultiSigCoordinator::new(signers.clone()); + + // Create a proposal + let payload = b"test payload"; + let proposal_id = coordinator + .propose("signer_0", "Test proposal", payload) + .unwrap(); + + // Proposal should not be approved yet (only 1 signature from proposer) + let proposal = coordinator.proposals.get(&proposal_id).unwrap(); + assert!(!proposal.is_approved()); + assert_eq!(proposal.approval_count(), 1); + + // Add more signatures + coordinator.approve(&proposal_id, "signer_1").unwrap(); + coordinator.approve(&proposal_id, "signer_2").unwrap(); + + // Now should be approved (3 signatures) + let proposal = coordinator.proposals.get(&proposal_id).unwrap(); + assert!(proposal.is_approved()); + assert_eq!(proposal.approval_count(), MULTISIG_THRESHOLD); + } + + #[test] + fn test_multisig_fails_without_quorum() { + let signers: Vec = (0..MULTISIG_TOTAL) + .map(|i| format!("signer_{}", i)) + .collect(); + let mut coordinator = MultiSigCoordinator::new(signers.clone()); + + let payload = b"test payload"; + let proposal_id = coordinator + .propose("signer_0", "Test proposal", payload) + .unwrap(); + + // Only add 1 more signature (total 2, need 3) + coordinator.approve(&proposal_id, "signer_1").unwrap(); + + // Try to execute - should fail + let result = coordinator.execute(&proposal_id, payload); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .contains("Insufficient approvals")); + } + + #[test] + fn test_multisig_succeeds_with_required_signatures() { + let signers: Vec = (0..MULTISIG_TOTAL) + .map(|i| format!("signer_{}", i)) + .collect(); + let mut coordinator = MultiSigCoordinator::new(signers.clone()); + + let payload = b"test payload"; + let proposal_id = coordinator + .propose("signer_0", "Test proposal", payload) + .unwrap(); + + // Add required signatures + coordinator.approve(&proposal_id, "signer_1").unwrap(); + coordinator.approve(&proposal_id, "signer_2").unwrap(); + + // Execute should succeed + let result = coordinator.execute(&proposal_id, payload); + assert!(result.is_ok()); + + // Verify proposal is marked as executed + let proposal = coordinator.proposals.get(&proposal_id).unwrap(); + assert!(proposal.executed); + } + + #[test] + fn test_multisig_prevents_duplicate_signatures() { + let signers: Vec = (0..MULTISIG_TOTAL) + .map(|i| format!("signer_{}", i)) + .collect(); + let mut coordinator = MultiSigCoordinator::new(signers.clone()); + + let payload = b"test payload"; + let proposal_id = coordinator + .propose("signer_0", "Test proposal", payload) + .unwrap(); + + // Try to approve twice with same signer + let result1 = coordinator.approve(&proposal_id, "signer_1"); + assert!(result1.is_ok()); + + let result2 = coordinator.approve(&proposal_id, "signer_1"); + assert!(result2.is_err()); + assert!(result2 + .unwrap_err() + .contains("already approved")); + } + + #[test] + fn test_multisig_rejects_unauthorized_signer() { + let signers: Vec = (0..MULTISIG_TOTAL) + .map(|i| format!("signer_{}", i)) + .collect(); + let mut coordinator = MultiSigCoordinator::new(signers); + + let payload = b"test payload"; + let proposal_id = coordinator + .propose("signer_0", "Test proposal", payload) + .unwrap(); + + // Unauthorized signer tries to approve + let result = coordinator.approve(&proposal_id, "unauthorized_signer"); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("not an authorized signer")); + } + + #[test] + fn test_multisig_tracks_approval_count_correctly() { + let signers: Vec = (0..MULTISIG_TOTAL) + .map(|i| format!("signer_{}", i)) + .collect(); + let mut coordinator = MultiSigCoordinator::new(signers.clone()); + + let payload = b"test payload"; + let proposal_id = coordinator + .propose("signer_0", "Test proposal", payload) + .unwrap(); + + // Check initial count (proposer auto-approves) + let proposal = coordinator.proposals.get(&proposal_id).unwrap(); + assert_eq!(proposal.approval_count(), 1); + + // Add signatures one by one + for i in 1..MULTISIG_TOTAL { + coordinator + .approve(&proposal_id, &format!("signer_{}", i)) + .unwrap(); + let proposal = coordinator.proposals.get(&proposal_id).unwrap(); + assert_eq!(proposal.approval_count(), i + 1); + } + } + + #[test] + fn test_multisig_execution_prevented_after_rejection() { + let signers: Vec = (0..MULTISIG_TOTAL) + .map(|i| format!("signer_{}", i)) + .collect(); + let mut coordinator = MultiSigCoordinator::new(signers.clone()); + + let payload = b"test payload"; + let proposal_id = coordinator + .propose("signer_0", "Test proposal", payload) + .unwrap(); + + // Mark as rejected + let proposal = coordinator.proposals.get_mut(&proposal_id).unwrap(); + proposal.rejected = true; + + // Try to execute + let result = coordinator.execute(&proposal_id, payload); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Proposal rejected")); + } + + #[test] + fn test_multisig_stores_total_signers_at_creation() { + let signers: Vec = (0..MULTISIG_TOTAL) + .map(|i| format!("signer_{}", i)) + .collect(); + let mut coordinator = MultiSigCoordinator::new(signers.clone()); + + let payload = b"test payload"; + let proposal_id = coordinator + .propose("signer_0", "Test proposal", payload) + .unwrap(); + + let proposal = coordinator.proposals.get(&proposal_id).unwrap(); + assert_eq!(proposal.total_signers, MULTISIG_TOTAL); + assert_eq!(proposal.required_signatures, MULTISIG_THRESHOLD); + } +} diff --git a/swaptrade-contracts/counter/src/kyc.rs b/swaptrade-contracts/counter/src/kyc.rs index 7f1ae2b..ef3054b 100644 --- a/swaptrade-contracts/counter/src/kyc.rs +++ b/swaptrade-contracts/counter/src/kyc.rs @@ -65,6 +65,8 @@ pub struct KYCRecord { pub updated_by: Option
, /// Reason for rejection (if applicable) pub rejection_reason: Option, + /// Timestamp when pending request expires (if status is Pending) + pub expires_at: Option, } impl KYCRecord { @@ -76,6 +78,7 @@ impl KYCRecord { finalized_at: None, updated_by: None, rejection_reason: None, + expires_at: None, } } @@ -83,6 +86,14 @@ impl KYCRecord { pub fn is_finalized(&self) -> bool { self.finalized_at.is_some() } + + /// Check if this record has expired (only applies to Pending status) + pub fn is_expired(&self, current_time: u64) -> bool { + if let Some(expires_at) = self.expires_at { + return current_time >= expires_at; + } + false + } } /// Governance override request for terminal state changes @@ -119,6 +130,8 @@ pub enum KYCStorageKey { OverrideCounter, /// Timelock duration in seconds TimelockDuration, + /// Pending KYC expiry duration in seconds + PendingExpiryDuration, } /// Timelock duration for governance overrides (7 days in seconds) @@ -127,6 +140,12 @@ pub const DEFAULT_TIMELOCK_DURATION: u64 = 7 * 24 * 60 * 60; /// Minimum timelock duration (1 day) pub const MIN_TIMELOCK_DURATION: u64 = 24 * 60 * 60; +/// Default pending KYC expiry duration (30 days in seconds) +pub const DEFAULT_PENDING_EXPIRY_DURATION: u64 = 30 * 24 * 60 * 60; + +/// Minimum pending KYC expiry duration (7 days) +pub const MIN_PENDING_EXPIRY_DURATION: u64 = 7 * 24 * 60 * 60; + /// KYC system errors #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum KYCError { @@ -148,6 +167,10 @@ pub enum KYCError { OverrideAlreadyExecuted = 1007, /// Invalid timelock duration InvalidTimelockDuration = 1008, + /// KYC request has expired + RequestExpired = 1009, + /// Invalid expiry duration + InvalidExpiryDuration = 1010, } /// KYC system implementation @@ -297,18 +320,33 @@ impl KYCSystem { return Err(KYCError::TerminalStateImmutable); } + // Check if pending request has expired + let timestamp = env.ledger().timestamp(); + if record.status == KYCStatus::Pending && record.is_expired(timestamp) { + // Emit expiry event + env.events().publish( + (symbol_short!("kyc"), symbol_short!("expired")), + user.clone(), + ); + return Err(KYCError::RequestExpired); + } + // Validate state transition if !record.status.can_transition_to(&new_status) { return Err(KYCError::InvalidStateTransition); } // Update record - let timestamp = env.ledger().timestamp(); record.status = new_status.clone(); record.updated_at = timestamp; record.updated_by = Some(operator.clone()); record.rejection_reason = None; + // Clear expiry when moving from Pending to another state + if new_status != KYCStatus::Pending { + record.expires_at = None; + } + if new_status.is_terminal() { record.finalized_at = Some(timestamp); if new_status == KYCStatus::Rejected { @@ -338,17 +376,21 @@ impl KYCSystem { return Err(KYCError::InvalidStateTransition); } + let timestamp = env.ledger().timestamp(); + let expiry_duration = Self::get_pending_expiry_duration(env); + let mut new_record = record; new_record.status = KYCStatus::Pending; - new_record.updated_at = env.ledger().timestamp(); + new_record.updated_at = timestamp; new_record.updated_by = None; new_record.rejection_reason = None; + new_record.expires_at = Some(timestamp + expiry_duration); Self::save_record(env, user, &new_record); env.events().publish( (symbol_short!("kyc"), symbol_short!("submitted")), - user.clone(), + (user.clone(), new_record.expires_at), ); Ok(()) @@ -410,6 +452,34 @@ impl KYCSystem { .unwrap_or(DEFAULT_TIMELOCK_DURATION) } + /// Set pending KYC expiry duration (admin only) + pub fn set_pending_expiry_duration( + env: &Env, + admin: &Address, + duration: u64, + ) -> Result<(), KYCError> { + admin.require_auth(); + crate::admin::require_admin(env, admin).map_err(|_| KYCError::NotKYCOperator)?; + + if duration < MIN_PENDING_EXPIRY_DURATION { + return Err(KYCError::InvalidExpiryDuration); + } + + env.storage() + .persistent() + .set(&KYCStorageKey::PendingExpiryDuration, &duration); + + Ok(()) + } + + /// Get pending KYC expiry duration + pub fn get_pending_expiry_duration(env: &Env) -> u64 { + env.storage() + .persistent() + .get(&KYCStorageKey::PendingExpiryDuration) + .unwrap_or(DEFAULT_PENDING_EXPIRY_DURATION) + } + /// Propose governance override for terminal state change pub fn propose_override( env: &Env, diff --git a/swaptrade-contracts/counter/src/kyc_tests.rs b/swaptrade-contracts/counter/src/kyc_tests.rs index 8ef7cfb..b55ea16 100644 --- a/swaptrade-contracts/counter/src/kyc_tests.rs +++ b/swaptrade-contracts/counter/src/kyc_tests.rs @@ -92,6 +92,108 @@ fn expect_kyc_panic(f: impl FnOnce()) { assert!(result.is_err()); } +#[test] +fn test_kyc_pending_request_expires_after_duration() { + let (env, contract_id, admin, operator, user, _) = setup_contract(); + + // Set short expiry duration for testing (7 days minimum) + with_contract(&env, &contract_id, || { + KYCSystem::set_pending_expiry_duration(&env, &admin, MIN_PENDING_EXPIRY_DURATION).unwrap(); + }); + + // Submit KYC + submit_kyc(&env, &contract_id, &user); + + let record = get_record(&env, &contract_id, &user); + assert_eq!(record.status, KYCStatus::Pending); + assert!(record.expires_at.is_some()); + + // Move forward in time past expiry + env.ledger().with_mut(|li| { + li.timestamp += MIN_PENDING_EXPIRY_DURATION + 1; + }); + + // Try to approve expired request - should fail + with_contract(&env, &contract_id, || { + assert_eq!( + KYCSystem::update_status(&env, &operator, &user, KYCStatus::InReview, None), + Err(KYCError::RequestExpired) + ); + }); +} + +#[test] +fn test_kyc_expiry_boundary_exact_expiry_time() { + let (env, contract_id, admin, operator, user, _) = setup_contract(); + + // Set short expiry duration for testing + with_contract(&env, &contract_id, || { + KYCSystem::set_pending_expiry_duration(&env, &admin, MIN_PENDING_EXPIRY_DURATION).unwrap(); + }); + + // Submit KYC + submit_kyc(&env, &contract_id, &user); + + let record = get_record(&env, &contract_id, &user); + let expiry_time = record.expires_at.unwrap(); + + // Move to exactly expiry time - should still be valid + env.ledger().with_mut(|li| { + li.timestamp = expiry_time - 1; + }); + + // Should still be able to approve (1 second before expiry) + with_contract(&env, &contract_id, || { + KYCSystem::update_status(&env, &operator, &user, KYCStatus::InReview, None).unwrap(); + }); + + let record = get_record(&env, &contract_id, &user); + assert_eq!(record.status, KYCStatus::InReview); + assert!(record.expires_at.is_none()); // Cleared after transition +} + +#[test] +fn test_kyc_expired_request_emits_event() { + let (env, contract_id, admin, operator, user, _) = setup_contract(); + + // Set short expiry duration + with_contract(&env, &contract_id, || { + KYCSystem::set_pending_expiry_duration(&env, &admin, MIN_PENDING_EXPIRY_DURATION).unwrap(); + }); + + // Submit KYC + submit_kyc(&env, &contract_id, &user); + + // Move past expiry + env.ledger().with_mut(|li| { + li.timestamp += MIN_PENDING_EXPIRY_DURATION + 1; + }); + + // Attempt to approve - this should emit an expiry event + with_contract(&env, &contract_id, || { + let result = KYCSystem::update_status(&env, &operator, &user, KYCStatus::InReview, None); + assert_eq!(result, Err(KYCError::RequestExpired)); + }); +} + +#[test] +fn test_kyc_expiry_cleared_on_status_transition() { + let (env, contract_id, _, operator, user, _) = setup_contract(); + + // Submit KYC + submit_kyc(&env, &contract_id, &user); + + let record = get_record(&env, &contract_id, &user); + assert!(record.expires_at.is_some()); + + // Move to InReview + move_to_review(&env, &contract_id, &operator, &user); + + let record = get_record(&env, &contract_id, &user); + assert_eq!(record.status, KYCStatus::InReview); + assert!(record.expires_at.is_none()); // Should be cleared +} + #[test] fn test_transition_matrix_matches_expected_fsm() { assert!(KYCStatus::Unverified.can_transition_to(&KYCStatus::Pending)); @@ -296,6 +398,7 @@ fn test_seeded_terminal_record_cannot_be_mutated_through_controlled_flow() { finalized_at: Some(env.ledger().timestamp()), updated_by: Some(operator.clone()), rejection_reason: None, + expires_at: None, }, ); diff --git a/swaptrade-contracts/counter/src/lib.rs b/swaptrade-contracts/counter/src/lib.rs index e47cf98..0581344 100644 --- a/swaptrade-contracts/counter/src/lib.rs +++ b/swaptrade-contracts/counter/src/lib.rs @@ -17,6 +17,9 @@ mod kyc; mod kyc_tests; mod liquidity_pool; mod rate_limit; +mod state_snapshot; +#[cfg(test)] +mod state_snapshot_tests; mod storage; mod batch { include!("../batch.rs"); diff --git a/swaptrade-contracts/counter/src/state_snapshot.rs b/swaptrade-contracts/counter/src/state_snapshot.rs new file mode 100644 index 0000000..6a56e8e --- /dev/null +++ b/swaptrade-contracts/counter/src/state_snapshot.rs @@ -0,0 +1,185 @@ +// src/state_snapshot.rs +//! State Snapshot Pattern for Critical Operations +//! +//! This module implements a state snapshot pattern to ensure critical operations +//! use consistent state and prevent race conditions or partial updates. +//! +//! Key principles: +//! - Read all required state before mutation +//! - Avoid interleaved updates +//! - Use local variables for state reads +//! - Validate before commit + +use soroban_sdk::{contracttype, Env, Address, Symbol}; + +/// Snapshot of critical state for validation +#[contracttype] +#[derive(Clone, Debug)] +pub struct StateSnapshot { + /// Timestamp when snapshot was taken + pub timestamp: u64, + /// Block number when snapshot was taken + pub block_number: u32, + /// Snapshot ID for tracking + pub snapshot_id: u64, +} + +/// State snapshot manager +pub struct StateSnapshotManager; + +impl StateSnapshotManager { + /// Create a new state snapshot + pub fn create_snapshot(env: &Env) -> StateSnapshot { + StateSnapshot { + timestamp: env.ledger().timestamp(), + block_number: env.ledger().sequence(), + snapshot_id: Self::get_next_snapshot_id(env), + } + } + + /// Validate that the current state is consistent with the snapshot + /// Returns true if state is still valid (no significant changes) + pub fn validate_snapshot(env: &Env, snapshot: &StateSnapshot) -> bool { + let current_block = env.ledger().sequence(); + // Allow snapshots within the same block or next block for consistency + current_block <= snapshot.block_number + 1 + } + + /// Get next snapshot ID + fn get_next_snapshot_id(env: &Env) -> u64 { + let key = (soroban_sdk::symbol_short!("snap"), soroban_sdk::symbol_short!("id")); + let current_id: u64 = env.storage().temporary().get(&key).unwrap_or(0); + let next_id = current_id + 1; + env.storage().temporary().set(&key, &next_id); + next_id + } +} + +/// Atomic operation executor with state validation +pub struct AtomicOperation; + +impl AtomicOperation { + /// Execute an atomic operation with state validation + /// + /// This function ensures that: + /// 1. All state is read before any mutations + /// 2. State is validated before committing changes + /// 3. All changes are applied together (atomic) + pub fn execute(env: &Env, operation: F) -> R + where + F: FnOnce(&Env, &StateSnapshot) -> R, + { + // Step 1: Create snapshot of current state + let snapshot = StateSnapshotManager::create_snapshot(env); + + // Step 2: Execute operation with snapshot + let result = operation(env, &snapshot); + + // Step 3: Validate snapshot is still valid + assert!( + StateSnapshotManager::validate_snapshot(env, &snapshot), + "State changed during operation execution" + ); + + result + } + + /// Execute an atomic operation that can fail with validation + pub fn execute_validated( + env: &Env, + operation: F, + validator: impl FnOnce(&Env, &StateSnapshot, &R) -> Result<(), E>, + ) -> Result + where + F: FnOnce(&Env, &StateSnapshot) -> R, + E: core::fmt::Debug, + { + // Step 1: Create snapshot + let snapshot = StateSnapshotManager::create_snapshot(env); + + // Step 2: Execute operation + let result = operation(env, &snapshot); + + // Step 3: Validate result + validator(env, &snapshot, &result)?; + + // Step 4: Validate snapshot still valid + assert!( + StateSnapshotManager::validate_snapshot(env, &snapshot), + "State changed during operation execution" + ); + + Ok(result) + } +} + +/// State consistency checker for critical operations +pub struct StateConsistencyChecker; + +impl StateConsistencyChecker { + /// Check if a state transition is valid + pub fn validate_transition( + old_state: &T, + new_state: &T, + allowed_transitions: &[(T, T)], + ) -> bool { + allowed_transitions.iter().any(|(from, to)| { + from == old_state && to == new_state + }) + } + + /// Validate that all required preconditions are met before state mutation + pub fn validate_preconditions(preconditions: F) -> bool + where + F: FnOnce() -> bool, + { + preconditions() + } + + /// Execute state mutation with pre and post validation + pub fn execute_with_validation( + operation: F, + validator: V, + ) -> Result + where + F: FnOnce() -> R, + V: FnOnce(&R) -> bool, + { + // Execute operation + let result = operation(); + + // Validate result + if validator(&result) { + Ok(result) + } else { + Err("State validation failed after operation") + } + } +} + +/// Read-consistency guard for critical state reads +pub struct ReadConsistencyGuard { + pub snapshot: StateSnapshot, +} + +impl ReadConsistencyGuard { + /// Create a new read consistency guard + pub fn new(env: &Env) -> Self { + Self { + snapshot: StateSnapshotManager::create_snapshot(env), + } + } + + /// Validate that reads are still consistent + pub fn validate(&self, env: &Env) -> bool { + StateSnapshotManager::validate_snapshot(env, &self.snapshot) + } + + /// Ensure consistency or panic + pub fn ensure_consistent(&self, env: &Env) { + assert!( + self.validate(env), + "Read consistency check failed: state changed during operation" + ); + } +} diff --git a/swaptrade-contracts/counter/src/state_snapshot_tests.rs b/swaptrade-contracts/counter/src/state_snapshot_tests.rs new file mode 100644 index 0000000..563d20c --- /dev/null +++ b/swaptrade-contracts/counter/src/state_snapshot_tests.rs @@ -0,0 +1,205 @@ +// tests/state_snapshot_tests.rs +//! Tests for state snapshot pattern and consistency (Issue #168) + +#[cfg(test)] +mod tests { + use crate::state_snapshot::{ + AtomicOperation, ReadConsistencyGuard, StateConsistencyChecker, StateSnapshotManager, + }; + use soroban_sdk::testutils::Ledger; + use soroban_sdk::Env; + + #[test] + fn test_snapshot_creation() { + let env = Env::default(); + let snapshot = env.as_contract(&Address::generate(&env), || { + StateSnapshotManager::create_snapshot(&env) + }); + + assert!(snapshot.timestamp > 0); + assert!(snapshot.block_number > 0); + assert!(snapshot.snapshot_id > 0); + } + + #[test] + fn test_snapshot_validation_same_block() { + let env = Env::default(); + let snapshot = env.as_contract(&Address::generate(&env), || { + StateSnapshotManager::create_snapshot(&env) + }); + + // Should be valid in same block + let valid = env.as_contract(&Address::generate(&env), || { + StateSnapshotManager::validate_snapshot(&env, &snapshot) + }); + assert!(valid); + } + + #[test] + fn test_snapshot_validation_next_block() { + let env = Env::default(); + let snapshot = env.as_contract(&Address::generate(&env), || { + StateSnapshotManager::create_snapshot(&env) + }); + + // Advance one block + env.ledger().with_mut(|li| { + li.sequence += 1; + }); + + // Should still be valid + let valid = env.as_contract(&Address::generate(&env), || { + StateSnapshotManager::validate_snapshot(&env, &snapshot) + }); + assert!(valid); + } + + #[test] + fn test_snapshot_invalid_after_multiple_blocks() { + let env = Env::default(); + let snapshot = env.as_contract(&Address::generate(&env), || { + StateSnapshotManager::create_snapshot(&env) + }); + + // Advance multiple blocks + env.ledger().with_mut(|li| { + li.sequence += 5; + }); + + // Should be invalid + let valid = env.as_contract(&Address::generate(&env), || { + StateSnapshotManager::validate_snapshot(&env, &snapshot) + }); + assert!(!valid); + } + + #[test] + fn test_read_consistency_guard() { + let env = Env::default(); + let guard = env.as_contract(&Address::generate(&env), || { + ReadConsistencyGuard::new(&env) + }); + + // Should be valid initially + let valid = env.as_contract(&Address::generate(&env), || { + guard.validate(&env) + }); + assert!(valid); + + // Advance one block - still valid + env.ledger().with_mut(|li| { + li.sequence += 1; + }); + + let valid = env.as_contract(&Address::generate(&env), || { + guard.validate(&env) + }); + assert!(valid); + } + + #[test] + fn test_state_consistency_checker_validates_transition() { + let allowed_transitions = vec![ + (0, 1), + (1, 2), + (2, 3), + ]; + + // Valid transition + assert!(StateConsistencyChecker::validate_transition( + &0, + &1, + &allowed_transitions + )); + + // Invalid transition + assert!(!StateConsistencyChecker::validate_transition( + &0, + &2, + &allowed_transitions + )); + } + + #[test] + fn test_state_consistency_checker_preconditions() { + let result = StateConsistencyChecker::validate_preconditions(|| { + true + }); + assert!(result); + + let result = StateConsistencyChecker::validate_preconditions(|| { + false + }); + assert!(!result); + } + + #[test] + fn test_execute_with_validation_success() { + let result = StateConsistencyChecker::execute_with_validation( + || 42, + |value| *value == 42, + ); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), 42); + } + + #[test] + fn test_execute_with_validation_failure() { + let result = StateConsistencyChecker::execute_with_validation( + || 42, + |value| *value == 100, + ); + assert!(result.is_err()); + } + + #[test] + fn test_snapshot_ids_increment() { + let env = Env::default(); + + let snapshot1 = env.as_contract(&Address::generate(&env), || { + StateSnapshotManager::create_snapshot(&env) + }); + + let snapshot2 = env.as_contract(&Address::generate(&env), || { + StateSnapshotManager::create_snapshot(&env) + }); + + assert_eq!(snapshot2.snapshot_id, snapshot1.snapshot_id + 1); + } + + #[test] + fn test_atomic_operation_executes_successfully() { + let env = Env::default(); + + let result = env.as_contract(&Address::generate(&env), |env| { + AtomicOperation::execute(&env, |env, snapshot| { + // Operation can use both env and snapshot + assert!(snapshot.timestamp > 0); + env.ledger().sequence() + }) + }); + + assert!(result > 0); + } + + #[test] + fn test_read_consistency_guard_panics_on_invalid() { + let env = Env::default(); + let guard = env.as_contract(&Address::generate(&env), || { + ReadConsistencyGuard::new(&env) + }); + + // Advance multiple blocks to make it invalid + env.ledger().with_mut(|li| { + li.sequence += 10; + }); + + // Should panic when ensuring consistency + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + env.as_contract(&Address::generate(&env), || { + guard.ensure_consistent(&env); + }) + })); + assert!(result.is_err()); + } +} diff --git a/swaptrade-contracts/counter/src/voting_execution_tests.rs b/swaptrade-contracts/counter/src/voting_execution_tests.rs new file mode 100644 index 0000000..911782d --- /dev/null +++ b/swaptrade-contracts/counter/src/voting_execution_tests.rs @@ -0,0 +1,189 @@ +// tests/voting_execution_tests.rs +//! Tests for preventing double execution of governance proposals (Issue #167) + +#[cfg(test)] +mod tests { + use crate::governance::voting::{GovernanceVoting, ProposalStatus}; + + #[test] + fn test_proposal_cannot_be_executed_twice() { + let mut gov = GovernanceVoting::new(); + let id = gov.create_proposal( + "Test proposal".to_string(), + "alice".to_string(), + 2, + 3600, + 0, + ); + + // Vote to pass + gov.vote(id, "alice", true, 100).unwrap(); + gov.vote(id, "bob", true, 200).unwrap(); + + // Finalize - should pass + let status = gov.finalize(id, 300).unwrap(); + assert_eq!(status, ProposalStatus::Passed); + + // Execute first time - should succeed + let result = gov.execute_proposal(id); + assert!(result.is_ok()); + + // Check status is now Executed + let proposal = gov.get(id).unwrap(); + assert_eq!(proposal.status, ProposalStatus::Executed); + + // Try to execute again - should fail + let result = gov.execute_proposal(id); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("already been executed")); + } + + #[test] + fn test_failed_proposal_cannot_be_executed() { + let mut gov = GovernanceVoting::new(); + let id = gov.create_proposal( + "Test proposal".to_string(), + "alice".to_string(), + 2, + 3600, + 0, + ); + + // Vote against + gov.vote(id, "alice", false, 100).unwrap(); + gov.vote(id, "bob", false, 200).unwrap(); + + // Finalize - should be rejected + let status = gov.finalize(id, 300).unwrap(); + assert_eq!(status, ProposalStatus::Rejected); + + // Try to execute - should fail + let result = gov.execute_proposal(id); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("cannot be executed")); + } + + #[test] + fn test_expired_proposal_cannot_be_executed() { + let mut gov = GovernanceVoting::new(); + let id = gov.create_proposal( + "Test proposal".to_string(), + "alice".to_string(), + 2, + 60, + 0, + ); + + // Try to vote after expiry + let result = gov.vote(id, "alice", true, 120); + assert!(result.is_err()); + + // Finalize - should be expired + let status = gov.finalize(id, 120).unwrap(); + assert_eq!(status, ProposalStatus::Expired); + + // Try to execute - should fail + let result = gov.execute_proposal(id); + assert!(result.is_err()); + } + + #[test] + fn test_active_proposal_cannot_be_executed() { + let mut gov = GovernanceVoting::new(); + let id = gov.create_proposal( + "Test proposal".to_string(), + "alice".to_string(), + 2, + 3600, + 0, + ); + + // Only one vote - not finalized yet + gov.vote(id, "alice", true, 100).unwrap(); + + // Try to execute before finalization - should fail + let result = gov.execute_proposal(id); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("cannot be executed")); + } + + #[test] + fn test_execution_state_transitions_are_correct() { + let mut gov = GovernanceVoting::new(); + let id = gov.create_proposal( + "Test proposal".to_string(), + "alice".to_string(), + 2, + 3600, + 0, + ); + + // Initial state + let proposal = gov.get(id).unwrap(); + assert_eq!(proposal.status, ProposalStatus::Active); + + // Vote and finalize + gov.vote(id, "alice", true, 100).unwrap(); + gov.vote(id, "bob", true, 200).unwrap(); + gov.finalize(id, 300).unwrap(); + + let proposal = gov.get(id).unwrap(); + assert_eq!(proposal.status, ProposalStatus::Passed); + + // Execute + gov.execute_proposal(id).unwrap(); + + let proposal = gov.get(id).unwrap(); + assert_eq!(proposal.status, ProposalStatus::Executed); + } + + #[test] + fn test_multiple_proposals_have_independent_execution_state() { + let mut gov = GovernanceVoting::new(); + + let id1 = gov.create_proposal( + "Proposal 1".to_string(), + "alice".to_string(), + 2, + 3600, + 0, + ); + + let id2 = gov.create_proposal( + "Proposal 2".to_string(), + "bob".to_string(), + 2, + 3600, + 0, + ); + + // Pass both proposals + gov.vote(id1, "alice", true, 100).unwrap(); + gov.vote(id1, "bob", true, 200).unwrap(); + gov.finalize(id1, 300).unwrap(); + + gov.vote(id2, "alice", true, 100).unwrap(); + gov.vote(id2, "bob", true, 200).unwrap(); + gov.finalize(id2, 300).unwrap(); + + // Execute first proposal + gov.execute_proposal(id1).unwrap(); + + // Verify first is executed, second is still passed + let proposal1 = gov.get(id1).unwrap(); + assert_eq!(proposal1.status, ProposalStatus::Executed); + + let proposal2 = gov.get(id2).unwrap(); + assert_eq!(proposal2.status, ProposalStatus::Passed); + + // Execute second proposal + gov.execute_proposal(id2).unwrap(); + + let proposal2 = gov.get(id2).unwrap(); + assert_eq!(proposal2.status, ProposalStatus::Executed); + + // Verify first cannot be executed again + let result = gov.execute_proposal(id1); + assert!(result.is_err()); + } +}