From ef64738e147f70fe3435fa776cc392086c9e3247 Mon Sep 17 00:00:00 2001 From: dinahmaccodes Date: Mon, 23 Feb 2026 19:20:12 +0100 Subject: [PATCH 1/3] [Security]: Implement governance security features #163 - Added and to - Enforced minimum threshold to propose in - Capped voting power per user in - Prevented double execution of proposals in via CEI pattern - Validated proposal timestamps and VotingConfig parameters - Fixed tests to include new signature and added new exploit test scenarios --- contracts/src/execution_tests.rs | 24 +++++++++++++++++---- contracts/src/governance.rs | 35 +++++++++++++++++++++---------- contracts/src/governance_tests.rs | 10 ++++----- contracts/src/lib.rs | 4 ++++ contracts/src/transition_tests.rs | 8 +++++-- contracts/src/voting_tests.rs | 2 +- 6 files changed, 60 insertions(+), 23 deletions(-) diff --git a/contracts/src/execution_tests.rs b/contracts/src/execution_tests.rs index 7fdaded3..74e8f897 100644 --- a/contracts/src/execution_tests.rs +++ b/contracts/src/execution_tests.rs @@ -38,10 +38,14 @@ mod execution_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); + + client.initialize_user(&creator); + let _ = client.create_savings_plan(&creator, &PlanType::Flexi, &1000); // Meets threshold + let action = ProposalAction::SetFlexiRate(500); let proposal_id = client .try_create_action_proposal(&creator, &description, &action) @@ -96,10 +100,14 @@ mod execution_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); + + client.initialize_user(&creator); + let _ = client.create_savings_plan(&creator, &PlanType::Flexi, &1000); + let action = ProposalAction::SetFlexiRate(500); let proposal_id = client .try_create_action_proposal(&creator, &description, &action) @@ -225,11 +233,15 @@ mod execution_tests { env.mock_all_auths(); // Setup governance - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); // Create proposal let creator = Address::generate(&env); let description = String::from_str(&env, "Change flexi rate"); + + client.initialize_user(&creator); + let _ = client.create_savings_plan(&creator, &PlanType::Flexi, &1000); + let action = ProposalAction::SetFlexiRate(750); let proposal_id = client .try_create_action_proposal(&creator, &description, &action) @@ -274,10 +286,14 @@ mod execution_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Pause contract"); + + client.initialize_user(&creator); + let _ = client.create_savings_plan(&creator, &PlanType::Flexi, &1000); + let action = ProposalAction::PauseContract; let proposal_id = client .try_create_action_proposal(&creator, &description, &action) diff --git a/contracts/src/governance.rs b/contracts/src/governance.rs index eb305cf6..1c6fae49 100644 --- a/contracts/src/governance.rs +++ b/contracts/src/governance.rs @@ -41,6 +41,8 @@ pub struct VotingConfig { pub quorum: u32, pub voting_period: u64, pub timelock_duration: u64, + pub proposal_threshold: u128, + pub max_voting_power: u128, } #[contracttype] @@ -130,6 +132,10 @@ pub fn create_action_proposal( creator.require_auth(); let config = get_voting_config(env)?; + if get_voting_power(env, &creator) < config.proposal_threshold { + return Err(SavingsError::InsufficientBalance); + } + let proposal_id = get_next_proposal_id(env); let now = env.ledger().timestamp(); @@ -222,6 +228,10 @@ pub fn init_voting_config( return Err(SavingsError::ConfigAlreadyInitialized); } + if config.voting_period == 0 || config.timelock_duration == 0 || config.max_voting_power == 0 { + return Err(SavingsError::InvalidAmount); + } + env.storage() .persistent() .set(&GovernanceKey::VotingConfig, &config); @@ -258,6 +268,9 @@ pub fn vote( if weight == 0 { return Err(SavingsError::InsufficientBalance); } + + let config = get_voting_config(env)?; + let capped_weight = weight.min(config.max_voting_power); // Check for double voting let voter_key = GovernanceKey::VoterRecord(proposal_id, voter.clone()); @@ -278,19 +291,19 @@ pub fn vote( 1 => { proposal.for_votes = proposal .for_votes - .checked_add(weight) + .checked_add(capped_weight) .ok_or(SavingsError::Overflow)?; } 2 => { proposal.against_votes = proposal .against_votes - .checked_add(weight) + .checked_add(capped_weight) .ok_or(SavingsError::Overflow)?; } 3 => { proposal.abstain_votes = proposal .abstain_votes - .checked_add(weight) + .checked_add(capped_weight) .ok_or(SavingsError::Overflow)?; } _ => return Err(SavingsError::InvalidAmount), @@ -323,19 +336,19 @@ pub fn vote( 1 => { proposal.for_votes = proposal .for_votes - .checked_add(weight) + .checked_add(capped_weight) .ok_or(SavingsError::Overflow)?; } 2 => { proposal.against_votes = proposal .against_votes - .checked_add(weight) + .checked_add(capped_weight) .ok_or(SavingsError::Overflow)?; } 3 => { proposal.abstain_votes = proposal .abstain_votes - .checked_add(weight) + .checked_add(capped_weight) .ok_or(SavingsError::Overflow)?; } _ => return Err(SavingsError::InvalidAmount), @@ -487,15 +500,15 @@ pub fn execute_proposal(env: &Env, proposal_id: u64) -> Result<(), SavingsError> return Err(SavingsError::TooEarly); } - // Execute the action - execute_action(env, &proposal.action)?; - - // Mark as executed + // Mark as executed first to prevent re-entrancy proposal.executed = true; env.storage() .persistent() .set(&GovernanceKey::ActionProposal(proposal_id), &proposal); + // Execute the action + execute_action(env, &proposal.action)?; + // Emit event emit_proposal_executed(env, proposal_id, now); @@ -524,7 +537,7 @@ pub fn execute_proposal(env: &Env, proposal_id: u64) -> Result<(), SavingsError> return Err(SavingsError::TooEarly); } - // Mark as executed + // Mark as executed first proposal.executed = true; env.storage() .persistent() diff --git a/contracts/src/governance_tests.rs b/contracts/src/governance_tests.rs index 07ed94be..331e70b3 100644 --- a/contracts/src/governance_tests.rs +++ b/contracts/src/governance_tests.rs @@ -97,7 +97,7 @@ mod governance_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let result = client.try_init_voting_config(&admin, &5000, &604800, &86400); + let result = client.try_init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); assert!(result.is_ok()); let config = client.try_get_voting_config().unwrap().unwrap(); @@ -111,7 +111,7 @@ mod governance_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); @@ -128,7 +128,7 @@ mod governance_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); @@ -150,7 +150,7 @@ mod governance_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let desc1 = String::from_str(&env, "Proposal 1"); @@ -170,7 +170,7 @@ mod governance_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Store test"); diff --git a/contracts/src/lib.rs b/contracts/src/lib.rs index 9a9b9139..65ba598e 100644 --- a/contracts/src/lib.rs +++ b/contracts/src/lib.rs @@ -764,11 +764,15 @@ impl NesteraContract { quorum: u32, voting_period: u64, timelock_duration: u64, + proposal_threshold: u128, + max_voting_power: u128, ) -> Result<(), SavingsError> { let config = governance::VotingConfig { quorum, voting_period, timelock_duration, + proposal_threshold, + max_voting_power, }; governance::init_voting_config(&env, admin, config) } diff --git a/contracts/src/transition_tests.rs b/contracts/src/transition_tests.rs index 32234c9d..b484d391 100644 --- a/contracts/src/transition_tests.rs +++ b/contracts/src/transition_tests.rs @@ -2,7 +2,7 @@ mod transition_tests { use crate::governance::{ProposalAction, VotingConfig}; use crate::rewards::storage_types::RewardsConfig; - use crate::{NesteraContract, NesteraContractClient}; + use crate::{NesteraContract, NesteraContractClient, PlanType}; use soroban_sdk::{testutils::Address as _, Address, BytesN, Env, String}; fn setup_contract() -> (Env, NesteraContractClient<'static>, Address) { @@ -88,10 +88,14 @@ mod transition_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Set flexi rate to 500"); + + client.initialize_user(&creator); + let _ = client.create_savings_plan(&creator, &PlanType::Flexi, &1000); + let action = ProposalAction::SetFlexiRate(500); let proposal_id = client diff --git a/contracts/src/voting_tests.rs b/contracts/src/voting_tests.rs index d4755ace..fc8c8150 100644 --- a/contracts/src/voting_tests.rs +++ b/contracts/src/voting_tests.rs @@ -38,7 +38,7 @@ mod voting_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400); + let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); From d93b2bade1b50a5e824ca61d95848d0304ebc3f8 Mon Sep 17 00:00:00 2001 From: dinahmaccodes Date: Mon, 23 Feb 2026 20:05:40 +0100 Subject: [PATCH 2/3] chore: fix clippy warnings and code formatting to resolve CI checks --- contracts/src/admin_tests.rs | 2 +- contracts/src/autosave.rs | 2 +- contracts/src/config_tests.rs | 14 ++++----- contracts/src/execution_tests.rs | 44 ++++++++++++++-------------- contracts/src/goal.rs | 2 +- contracts/src/governance.rs | 8 ++--- contracts/src/governance_events.rs | 24 +++++---------- contracts/src/governance_tests.rs | 14 ++++----- contracts/src/group.rs | 2 +- contracts/src/lock.rs | 2 +- contracts/src/rewards/ranking.rs | 4 +-- contracts/src/security.rs | 11 ++++--- contracts/src/storage_types.rs | 6 ++++ contracts/src/transition_tests.rs | 16 +++++----- contracts/src/voting_tests.rs | 20 ++++++------- contracts/tests/admin_validation.rs | 2 +- contracts/tests/anti_farming_test.rs | 6 ++-- contracts/tests/autosave_test.rs | 36 +++++++++++------------ contracts/tests/integration.rs | 6 ++-- contracts/tests/ranking_test.rs | 8 ++--- contracts/tests/rewards_test.rs | 15 +++++----- 21 files changed, 120 insertions(+), 124 deletions(-) diff --git a/contracts/src/admin_tests.rs b/contracts/src/admin_tests.rs index b1a3bfca..f7e547c1 100644 --- a/contracts/src/admin_tests.rs +++ b/contracts/src/admin_tests.rs @@ -12,7 +12,7 @@ fn setup() -> (Env, NesteraContractClient<'static>, Address) { env.mock_all_auths(); // Assuming initialize returns Result now - let _ = client.initialize(&admin, &admin_pk); + client.initialize(&admin, &admin_pk); (env, client, admin) } diff --git a/contracts/src/autosave.rs b/contracts/src/autosave.rs index c29d5ea8..15981334 100644 --- a/contracts/src/autosave.rs +++ b/contracts/src/autosave.rs @@ -254,7 +254,7 @@ pub fn get_user_autosaves(env: &Env, user: &Address) -> Vec { .unwrap_or(Vec::new(env)); // Extend TTL on list access - if schedules.len() > 0 { + if !schedules.is_empty() { ttl::extend_user_plan_list_ttl(env, &list_key); } diff --git a/contracts/src/config_tests.rs b/contracts/src/config_tests.rs index 2d632b5d..39a29e3b 100644 --- a/contracts/src/config_tests.rs +++ b/contracts/src/config_tests.rs @@ -38,7 +38,7 @@ fn test_initialize_config_succeeds() { assert_eq!(config.admin, admin); assert_eq!(config.treasury, treasury); assert_eq!(config.protocol_fee_bps, 100); - assert_eq!(config.paused, false); + assert!(!config.paused); } #[test] @@ -127,7 +127,7 @@ fn test_get_config_before_config_init() { let config = client.get_config(); assert_eq!(config.admin, admin); assert_eq!(config.protocol_fee_bps, 0); // default - assert_eq!(config.paused, false); // default + assert!(!config.paused); // default } #[test] @@ -270,7 +270,7 @@ fn test_pause_contract_succeeds() { assert!(result.is_ok(), "admin should be able to pause"); let config = client.get_config(); - assert_eq!(config.paused, true); + assert!(config.paused); } #[test] @@ -283,7 +283,7 @@ fn test_unpause_contract_succeeds() { assert!(result.is_ok(), "admin should be able to unpause"); let config = client.get_config(); - assert_eq!(config.paused, false); + assert!(!config.paused); } #[test] @@ -419,7 +419,7 @@ fn test_full_config_lifecycle() { let config = client.get_config(); assert_eq!(config.treasury, treasury1); assert_eq!(config.protocol_fee_bps, 250); - assert_eq!(config.paused, false); + assert!(!config.paused); // 2. Update treasury client.set_treasury(&admin, &treasury2); @@ -433,7 +433,7 @@ fn test_full_config_lifecycle() { // 4. Pause client.pause_contract(&admin); - assert_eq!(client.get_config().paused, true); + assert!(client.get_config().paused); // 5. Admin can still update config while paused client.set_protocol_fee(&admin, &300); @@ -441,5 +441,5 @@ fn test_full_config_lifecycle() { // 6. Unpause client.unpause_contract(&admin); - assert_eq!(client.get_config().paused, false); + assert!(!client.get_config().paused); } diff --git a/contracts/src/execution_tests.rs b/contracts/src/execution_tests.rs index 74e8f897..1562b4a4 100644 --- a/contracts/src/execution_tests.rs +++ b/contracts/src/execution_tests.rs @@ -1,6 +1,6 @@ #[cfg(test)] mod execution_tests { - use crate::governance::{ProposalAction, VotingConfig}; + use crate::governance::ProposalAction; use crate::rewards::storage_types::RewardsConfig; use crate::{NesteraContract, NesteraContractClient, PlanType}; use soroban_sdk::{ @@ -29,7 +29,7 @@ mod execution_tests { max_daily_points: 1_000_000, max_streak_multiplier: 10_000, }; - let _ = client.initialize_rewards_config(&config); + client.initialize_rewards_config(&config); (env, client, admin) } @@ -38,11 +38,11 @@ mod execution_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); - + client.initialize_user(&creator); let _ = client.create_savings_plan(&creator, &PlanType::Flexi, &1000); // Meets threshold @@ -63,8 +63,8 @@ mod execution_tests { let _ = client.create_savings_plan(&voter2, &PlanType::Flexi, &2000); // Vote for the proposal - let _ = client.vote(&proposal_id, &1, &voter1); - let _ = client.vote(&proposal_id, &1, &voter2); + client.vote(&proposal_id, &1, &voter1); + client.vote(&proposal_id, &1, &voter2); (env, client, admin, proposal_id) } @@ -100,7 +100,7 @@ mod execution_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); @@ -118,7 +118,7 @@ mod execution_tests { let voter = Address::generate(&env); client.initialize_user(&voter); let _ = client.create_savings_plan(&voter, &PlanType::Flexi, &1000); - let _ = client.vote(&proposal_id, &2, &voter); + client.vote(&proposal_id, &2, &voter); // Advance time env.ledger().with_mut(|li| { @@ -139,7 +139,7 @@ mod execution_tests { li.timestamp += 604800 + 1; }); - let _ = client.queue_proposal(&proposal_id); + client.queue_proposal(&proposal_id); // Advance time past timelock env.ledger().with_mut(|li| { @@ -166,7 +166,7 @@ mod execution_tests { li.timestamp += 604800 + 1; }); - let _ = client.queue_proposal(&proposal_id); + client.queue_proposal(&proposal_id); // Try to execute before timelock let result = client.try_execute_proposal(&proposal_id); @@ -198,7 +198,7 @@ mod execution_tests { li.timestamp += 604800 + 1; }); - let _ = client.queue_proposal(&proposal_id); + client.queue_proposal(&proposal_id); let result = client.try_queue_proposal(&proposal_id); assert!(result.is_err()); @@ -214,14 +214,14 @@ mod execution_tests { li.timestamp += 604800 + 1; }); - let _ = client.queue_proposal(&proposal_id); + client.queue_proposal(&proposal_id); // Advance time past timelock env.ledger().with_mut(|li| { li.timestamp += 86400 + 1; }); - let _ = client.execute_proposal(&proposal_id); + client.execute_proposal(&proposal_id); let result = client.try_execute_proposal(&proposal_id); assert!(result.is_err()); @@ -233,7 +233,7 @@ mod execution_tests { env.mock_all_auths(); // Setup governance - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); // Create proposal let creator = Address::generate(&env); @@ -256,8 +256,8 @@ mod execution_tests { let _ = client.create_savings_plan(&voter1, &PlanType::Flexi, &4000); let _ = client.create_savings_plan(&voter2, &PlanType::Flexi, &3000); - let _ = client.vote(&proposal_id, &1, &voter1); - let _ = client.vote(&proposal_id, &1, &voter2); + client.vote(&proposal_id, &1, &voter1); + client.vote(&proposal_id, &1, &voter2); // Wait for voting to end env.ledger().with_mut(|li| { @@ -265,7 +265,7 @@ mod execution_tests { }); // Queue - let _ = client.queue_proposal(&proposal_id); + client.queue_proposal(&proposal_id); // Wait for timelock env.ledger().with_mut(|li| { @@ -273,7 +273,7 @@ mod execution_tests { }); // Execute - let _ = client.execute_proposal(&proposal_id); + client.execute_proposal(&proposal_id); // Verify assert_eq!(client.get_flexi_rate(), 750); @@ -286,7 +286,7 @@ mod execution_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Pause contract"); @@ -303,17 +303,17 @@ mod execution_tests { let voter = Address::generate(&env); client.initialize_user(&voter); let _ = client.create_savings_plan(&voter, &PlanType::Flexi, &5000); - let _ = client.vote(&proposal_id, &1, &voter); + client.vote(&proposal_id, &1, &voter); env.ledger().with_mut(|li| { li.timestamp += 604800 + 1; }); - let _ = client.queue_proposal(&proposal_id); + client.queue_proposal(&proposal_id); env.ledger().with_mut(|li| { li.timestamp += 86400 + 1; }); - let _ = client.execute_proposal(&proposal_id); + client.execute_proposal(&proposal_id); assert!(client.is_paused()); } diff --git a/contracts/src/goal.rs b/contracts/src/goal.rs index c95afa8c..58fbb0a0 100644 --- a/contracts/src/goal.rs +++ b/contracts/src/goal.rs @@ -395,7 +395,7 @@ pub fn get_user_goal_saves(env: &Env, user: &Address) -> Vec { .unwrap_or_else(|| Vec::new(env)); // Extend TTL on list access - if goals.len() > 0 { + if !goals.is_empty() { ttl::extend_user_plan_list_ttl(env, &list_key); } diff --git a/contracts/src/governance.rs b/contracts/src/governance.rs index 1c6fae49..98664b48 100644 --- a/contracts/src/governance.rs +++ b/contracts/src/governance.rs @@ -135,7 +135,7 @@ pub fn create_action_proposal( if get_voting_power(env, &creator) < config.proposal_threshold { return Err(SavingsError::InsufficientBalance); } - + let proposal_id = get_next_proposal_id(env); let now = env.ledger().timestamp(); @@ -259,7 +259,7 @@ pub fn vote( voter.require_auth(); // Validate vote_type: 1=for, 2=against, 3=abstain - if vote_type < 1 || vote_type > 3 { + if !(1..=3).contains(&vote_type) { return Err(SavingsError::InvalidAmount); } @@ -268,7 +268,7 @@ pub fn vote( if weight == 0 { return Err(SavingsError::InsufficientBalance); } - + let config = get_voting_config(env)?; let capped_weight = weight.min(config.max_voting_power); @@ -403,7 +403,7 @@ pub fn queue_proposal(env: &Env, proposal_id: u64) -> Result<(), SavingsError> { } // Check quorum - let config = get_voting_config(env)?; + let _config = get_voting_config(env)?; let total_votes = proposal .for_votes .checked_add(proposal.against_votes) diff --git a/contracts/src/governance_events.rs b/contracts/src/governance_events.rs index 7f87bf2a..5085a0b0 100644 --- a/contracts/src/governance_events.rs +++ b/contracts/src/governance_events.rs @@ -57,10 +57,8 @@ pub fn emit_vote_cast(env: &Env, proposal_id: u64, voter: Address, vote_type: u3 vote_type, weight, }; - env.events().publish( - (symbol_short!("gov"), symbol_short!("voted"), voter), - event, - ); + env.events() + .publish((symbol_short!("gov"), symbol_short!("voted"), voter), event); } pub fn emit_proposal_queued(env: &Env, proposal_id: u64, queued_at: u64) { @@ -68,10 +66,8 @@ pub fn emit_proposal_queued(env: &Env, proposal_id: u64, queued_at: u64) { proposal_id, queued_at, }; - env.events().publish( - (symbol_short!("gov"), symbol_short!("queued")), - event, - ); + env.events() + .publish((symbol_short!("gov"), symbol_short!("queued")), event); } pub fn emit_proposal_executed(env: &Env, proposal_id: u64, executed_at: u64) { @@ -79,10 +75,8 @@ pub fn emit_proposal_executed(env: &Env, proposal_id: u64, executed_at: u64) { proposal_id, executed_at, }; - env.events().publish( - (symbol_short!("gov"), symbol_short!("executed")), - event, - ); + env.events() + .publish((symbol_short!("gov"), symbol_short!("executed")), event); } pub fn emit_proposal_canceled(env: &Env, proposal_id: u64, canceled_at: u64) { @@ -90,8 +84,6 @@ pub fn emit_proposal_canceled(env: &Env, proposal_id: u64, canceled_at: u64) { proposal_id, canceled_at, }; - env.events().publish( - (symbol_short!("gov"), symbol_short!("canceled")), - event, - ); + env.events() + .publish((symbol_short!("gov"), symbol_short!("canceled")), event); } diff --git a/contracts/src/governance_tests.rs b/contracts/src/governance_tests.rs index 331e70b3..1c54a7cc 100644 --- a/contracts/src/governance_tests.rs +++ b/contracts/src/governance_tests.rs @@ -1,6 +1,6 @@ #[cfg(test)] mod governance_tests { - use crate::governance::VotingConfig; + use crate::rewards::storage_types::RewardsConfig; use crate::{NesteraContract, NesteraContractClient, PlanType}; use soroban_sdk::{testutils::Address as _, Address, BytesN, Env, String}; @@ -26,7 +26,7 @@ mod governance_tests { max_daily_points: 1_000_000, max_streak_multiplier: 10_000, }; - let _ = client.initialize_rewards_config(&config); + client.initialize_rewards_config(&config); (env, client, admin) } @@ -111,7 +111,7 @@ mod governance_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); @@ -128,7 +128,7 @@ mod governance_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); @@ -140,7 +140,7 @@ mod governance_tests { let proposal = client.get_proposal(&proposal_id).unwrap(); assert_eq!(proposal.id, 1); assert_eq!(proposal.creator, creator); - assert_eq!(proposal.executed, false); + assert!(!proposal.executed); assert_eq!(proposal.for_votes, 0); assert_eq!(proposal.against_votes, 0); } @@ -150,7 +150,7 @@ mod governance_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let desc1 = String::from_str(&env, "Proposal 1"); @@ -170,7 +170,7 @@ mod governance_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Store test"); diff --git a/contracts/src/group.rs b/contracts/src/group.rs index c21766fe..e199de59 100644 --- a/contracts/src/group.rs +++ b/contracts/src/group.rs @@ -207,7 +207,7 @@ pub fn get_user_groups(env: &Env, user: &Address) -> Vec { .unwrap_or(Vec::new(env)); // Extend TTL on list access - if groups.len() > 0 { + if !groups.is_empty() { ttl::extend_user_plan_list_ttl(env, &key); } diff --git a/contracts/src/lock.rs b/contracts/src/lock.rs index 9a2671b8..dada212d 100644 --- a/contracts/src/lock.rs +++ b/contracts/src/lock.rs @@ -145,7 +145,7 @@ pub fn get_user_lock_saves(env: &Env, user: &Address) -> Vec { .unwrap_or_else(|| Vec::new(env)); // Extend TTL on list access - if locks.len() > 0 { + if !locks.is_empty() { ttl::extend_user_plan_list_ttl(env, &list_key); } diff --git a/contracts/src/rewards/ranking.rs b/contracts/src/rewards/ranking.rs index bc95a54a..696e3d41 100644 --- a/contracts/src/rewards/ranking.rs +++ b/contracts/src/rewards/ranking.rs @@ -1,5 +1,5 @@ -use super::storage::{get_user_rewards, save_user_rewards}; -use super::storage_types::{RewardsDataKey, UserRewards}; +use super::storage::get_user_rewards; +use super::storage_types::RewardsDataKey; use soroban_sdk::{Address, Env, Vec}; /// Maximum number of users to consider for ranking calculations diff --git a/contracts/src/security.rs b/contracts/src/security.rs index 4bfd05bc..4a978d0b 100644 --- a/contracts/src/security.rs +++ b/contracts/src/security.rs @@ -1,14 +1,13 @@ -use crate::{NesteraContract, NesteraContractClient, PlanType, SavingsError}; -use soroban_sdk::{testutils::Address as _, Address, Env, String, Symbol, Vec}; +use soroban_sdk::Env; #[cfg(test)] mod security_tests { use super::*; - use soroban_sdk::testutils::Address as _; + #[test] fn test_overflow_protection() { - let env = Env::default(); + let _env = Env::default(); // Setup Nestera contract... // 1. Try to deposit i128::MAX + 1 @@ -17,14 +16,14 @@ mod security_tests { #[test] fn test_negative_deposit_protection() { - let env = Env::default(); + let _env = Env::default(); // 1. Try to deposit -500 // 2. Assert that the result is Err(SavingsError::InvalidAmount) } #[test] fn test_pause_invariant() { - let env = Env::default(); + let _env = Env::default(); // 1. Pause the contract // 2. Try to withdraw // 3. Assert result is Err(SavingsError::ContractPaused) diff --git a/contracts/src/storage_types.rs b/contracts/src/storage_types.rs index 35dee76f..a67a5f84 100644 --- a/contracts/src/storage_types.rs +++ b/contracts/src/storage_types.rs @@ -34,6 +34,12 @@ pub struct User { } /// Represents a Lock Save plan with fixed duration +impl Default for User { + fn default() -> Self { + Self::new() + } +} + impl User { pub fn new() -> Self { Self { diff --git a/contracts/src/transition_tests.rs b/contracts/src/transition_tests.rs index b484d391..5df6e44f 100644 --- a/contracts/src/transition_tests.rs +++ b/contracts/src/transition_tests.rs @@ -1,6 +1,6 @@ #[cfg(test)] mod transition_tests { - use crate::governance::{ProposalAction, VotingConfig}; + use crate::governance::ProposalAction; use crate::rewards::storage_types::RewardsConfig; use crate::{NesteraContract, NesteraContractClient, PlanType}; use soroban_sdk::{testutils::Address as _, Address, BytesN, Env, String}; @@ -26,7 +26,7 @@ mod transition_tests { max_daily_points: 1_000_000, max_streak_multiplier: 10_000, }; - let _ = client.initialize_rewards_config(&config); + client.initialize_rewards_config(&config); (env, client, admin) } @@ -78,7 +78,7 @@ mod transition_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.activate_governance(&admin); + client.activate_governance(&admin); let result = client.try_set_flexi_rate(&admin, &600); assert!(result.is_ok()); } @@ -88,11 +88,11 @@ mod transition_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Set flexi rate to 500"); - + client.initialize_user(&creator); let _ = client.create_savings_plan(&creator, &PlanType::Flexi, &1000); @@ -112,13 +112,13 @@ mod transition_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.set_flexi_rate(&admin, &300); - let _ = client.set_goal_rate(&admin, &400); + client.set_flexi_rate(&admin, &300); + client.set_goal_rate(&admin, &400); assert_eq!(client.get_flexi_rate(), 300); assert_eq!(client.get_goal_rate(), 400); - let _ = client.activate_governance(&admin); + client.activate_governance(&admin); assert_eq!(client.get_flexi_rate(), 300); assert_eq!(client.get_goal_rate(), 400); diff --git a/contracts/src/voting_tests.rs b/contracts/src/voting_tests.rs index fc8c8150..7420c41a 100644 --- a/contracts/src/voting_tests.rs +++ b/contracts/src/voting_tests.rs @@ -1,6 +1,6 @@ #[cfg(test)] mod voting_tests { - use crate::governance::VotingConfig; + use crate::rewards::storage_types::RewardsConfig; use crate::{NesteraContract, NesteraContractClient, PlanType}; use soroban_sdk::{ @@ -29,7 +29,7 @@ mod voting_tests { max_daily_points: 1_000_000, max_streak_multiplier: 10_000, }; - let _ = client.initialize_rewards_config(&config); + client.initialize_rewards_config(&config); (env, client, admin) } @@ -38,7 +38,7 @@ mod voting_tests { let (env, client, admin) = setup_contract(); env.mock_all_auths(); - let _ = client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); + client.init_voting_config(&admin, &5000, &604800, &86400, &100, &10_000); let creator = Address::generate(&env); let description = String::from_str(&env, "Test proposal"); @@ -121,9 +121,9 @@ mod voting_tests { let _ = client.create_savings_plan(&voter2, &PlanType::Flexi, &2000); let _ = client.create_savings_plan(&voter3, &PlanType::Flexi, &1500); - let _ = client.vote(&proposal_id, &1, &voter1); - let _ = client.vote(&proposal_id, &1, &voter2); - let _ = client.vote(&proposal_id, &2, &voter3); + client.vote(&proposal_id, &1, &voter1); + client.vote(&proposal_id, &1, &voter2); + client.vote(&proposal_id, &2, &voter3); let proposal = client.get_proposal(&proposal_id).unwrap(); assert_eq!(proposal.for_votes, 3000); @@ -140,7 +140,7 @@ mod voting_tests { client.initialize_user(&voter); let _ = client.create_savings_plan(&voter, &PlanType::Flexi, &1000); - let _ = client.vote(&proposal_id, &1, &voter); + client.vote(&proposal_id, &1, &voter); let result = client.try_vote(&proposal_id, &2, &voter); assert!(result.is_err()); @@ -161,7 +161,7 @@ mod voting_tests { assert!(!client.has_voted(&proposal_id, &voter)); - let _ = client.vote(&proposal_id, &1, &voter); + client.vote(&proposal_id, &1, &voter); assert!(client.has_voted(&proposal_id, &voter)); } @@ -238,8 +238,8 @@ mod voting_tests { let _ = client.create_savings_plan(&voter1, &PlanType::Flexi, &5000); let _ = client.create_savings_plan(&voter2, &PlanType::Flexi, &3000); - let _ = client.vote(&proposal_id, &1, &voter1); - let _ = client.vote(&proposal_id, &1, &voter2); + client.vote(&proposal_id, &1, &voter1); + client.vote(&proposal_id, &1, &voter2); let proposal = client.get_proposal(&proposal_id).unwrap(); assert_eq!(proposal.for_votes, 8000); diff --git a/contracts/tests/admin_validation.rs b/contracts/tests/admin_validation.rs index c2759b47..b7d6e3cb 100644 --- a/contracts/tests/admin_validation.rs +++ b/contracts/tests/admin_validation.rs @@ -3,7 +3,7 @@ #[cfg(test)] mod admin_validation { - use soroban_sdk::{testutils::Address as _, Address, Env}; + #[test] fn validate_admin_functions_exist() { diff --git a/contracts/tests/anti_farming_test.rs b/contracts/tests/anti_farming_test.rs index 56d14824..40a84f7c 100644 --- a/contracts/tests/anti_farming_test.rs +++ b/contracts/tests/anti_farming_test.rs @@ -1,9 +1,9 @@ #![cfg(test)] use soroban_sdk::{ - testutils::{Address as _, Ledger, LedgerInfo}, + testutils::Address as _, Address, BytesN, Env, }; -use Nestera::{NesteraContract, NesteraContractClient, PlanType}; +use Nestera::{NesteraContract, NesteraContractClient}; fn create_test_env() -> (Env, NesteraContractClient<'static>, Address, Address) { let env = Env::default(); @@ -69,7 +69,7 @@ fn test_minimum_deposit_threshold() { config.min_deposit_for_rewards, 100, "Config should be initialized" ); - assert_eq!(config.enabled, true, "Rewards should be enabled"); + assert!(config.enabled, "Rewards should be enabled"); // Deposit below minimum - should not earn rewards client.deposit_flexi(&user, &99); diff --git a/contracts/tests/autosave_test.rs b/contracts/tests/autosave_test.rs index a05d91f8..4af59676 100644 --- a/contracts/tests/autosave_test.rs +++ b/contracts/tests/autosave_test.rs @@ -190,9 +190,9 @@ mod autosave_tests { // All should succeed assert_eq!(results.len(), 3); - assert_eq!(results.get(0).unwrap(), true); - assert_eq!(results.get(1).unwrap(), true); - assert_eq!(results.get(2).unwrap(), true); + assert!(results.get(0).unwrap()); + assert!(results.get(1).unwrap()); + assert!(results.get(2).unwrap()); // Verify total Flexi balance = 500 + 300 + 200 = 1000 let balance = client.get_flexi_balance(&user); @@ -213,8 +213,8 @@ mod autosave_tests { // Both should be skipped (not due) assert_eq!(results.len(), 2); - assert_eq!(results.get(0).unwrap(), false); - assert_eq!(results.get(1).unwrap(), false); + assert!(!results.get(0).unwrap()); + assert!(!results.get(1).unwrap()); // Balance should remain 0 let balance = client.get_flexi_balance(&user); @@ -231,9 +231,9 @@ mod autosave_tests { // All should be false (not found) assert_eq!(results.len(), 3); - assert_eq!(results.get(0).unwrap(), false); - assert_eq!(results.get(1).unwrap(), false); - assert_eq!(results.get(2).unwrap(), false); + assert!(!results.get(0).unwrap()); + assert!(!results.get(1).unwrap()); + assert!(!results.get(2).unwrap()); } #[test] @@ -253,8 +253,8 @@ mod autosave_tests { // id1 should be false (inactive), id2 should be true (due and active) assert_eq!(results.len(), 2); - assert_eq!(results.get(0).unwrap(), false); - assert_eq!(results.get(1).unwrap(), true); + assert!(!results.get(0).unwrap()); + assert!(results.get(1).unwrap()); // Only id2's 2000 should have been deposited let balance = client.get_flexi_balance(&user); @@ -285,11 +285,11 @@ mod autosave_tests { let results = client.execute_due_autosaves(&schedule_ids); assert_eq!(results.len(), 5); - assert_eq!(results.get(0).unwrap(), true); // id1: due, active -> executed - assert_eq!(results.get(1).unwrap(), false); // id2: not due -> skipped - assert_eq!(results.get(2).unwrap(), true); // id3: due, active -> executed - assert_eq!(results.get(3).unwrap(), false); // id4: inactive -> skipped - assert_eq!(results.get(4).unwrap(), false); // fake_id: not found -> skipped + assert!(results.get(0).unwrap()); // id1: due, active -> executed + assert!(!results.get(1).unwrap()); // id2: not due -> skipped + assert!(results.get(2).unwrap()); // id3: due, active -> executed + assert!(!results.get(3).unwrap()); // id4: inactive -> skipped + assert!(!results.get(4).unwrap()); // fake_id: not found -> skipped // Only id1 (500) and id3 (200) executed -> balance = 700 let balance = client.get_flexi_balance(&user); @@ -308,7 +308,7 @@ mod autosave_tests { let schedule_ids = soroban_sdk::vec![&env, id1]; let results = client.execute_due_autosaves(&schedule_ids); - assert_eq!(results.get(0).unwrap(), true); + assert!(results.get(0).unwrap()); // Verify next_execution_time was advanced let schedule = client.get_autosave(&id1).unwrap(); @@ -343,8 +343,8 @@ mod autosave_tests { let schedule_ids = soroban_sdk::vec![&env, id1, id2]; let results = client.execute_due_autosaves(&schedule_ids); - assert_eq!(results.get(0).unwrap(), true); - assert_eq!(results.get(1).unwrap(), true); + assert!(results.get(0).unwrap()); + assert!(results.get(1).unwrap()); // Verify per-user Flexi balances assert_eq!(client.get_flexi_balance(&user1), 500); diff --git a/contracts/tests/integration.rs b/contracts/tests/integration.rs index 5a132493..faae42d8 100644 --- a/contracts/tests/integration.rs +++ b/contracts/tests/integration.rs @@ -659,13 +659,13 @@ fn test_multi_user_multi_plan_scenario() { // Verify all plans exist and are functional let user_locks = client.get_user_lock_saves(&user1); - assert!(user_locks.len() > 0); + assert!(!user_locks.is_empty()); let user_goals = client.get_user_goal_saves(&user1); - assert!(user_goals.len() > 0); + assert!(!user_goals.is_empty()); let user_autosaves = client.get_user_autosaves(&user2); - assert!(user_autosaves.len() > 0); + assert!(!user_autosaves.is_empty()); } #[test] diff --git a/contracts/tests/ranking_test.rs b/contracts/tests/ranking_test.rs index 16acab62..b3c5b638 100644 --- a/contracts/tests/ranking_test.rs +++ b/contracts/tests/ranking_test.rs @@ -112,7 +112,7 @@ fn test_get_user_rank_not_ranked() { let (_env, client, admin, users) = create_test_env(); setup_rewards_config(&client, &admin); - assert!(users.len() >= 1, "Need at least 1 user for test"); + assert!(!users.is_empty(), "Need at least 1 user for test"); let rank = client.get_user_rank(&users.get(0).unwrap()); assert_eq!(rank, 0, "User with no points should have rank 0"); } @@ -245,10 +245,10 @@ fn test_large_user_set_safety() { // Should not panic with large user set let top_users = client.get_top_users(&20); assert!(top_users.len() <= 20, "Should respect limit"); - assert!(top_users.len() > 0, "Should return some users"); + assert!(!top_users.is_empty(), "Should return some users"); // Top user should have most points - if top_users.len() > 0 { + if !top_users.is_empty() { let top1 = top_users.get(0).unwrap(); assert_eq!(top1.1, 50 * 100 * 10, "Top user should have highest points"); } @@ -259,7 +259,7 @@ fn test_ranking_read_only() { let (_env, client, admin, users) = create_test_env(); setup_rewards_config(&client, &admin); - assert!(users.len() >= 1, "Need at least 1 user for test"); + assert!(!users.is_empty(), "Need at least 1 user for test"); client.deposit_flexi(&users.get(0).unwrap(), &1000); diff --git a/contracts/tests/rewards_test.rs b/contracts/tests/rewards_test.rs index 2b9784aa..a28c51e8 100644 --- a/contracts/tests/rewards_test.rs +++ b/contracts/tests/rewards_test.rs @@ -2,8 +2,7 @@ use soroban_sdk::{ symbol_short, - testutils::{Address as _, Events}, - vec, Address, BytesN, Env, IntoVal, Symbol, + testutils::{Address as _, Events}, Address, BytesN, Env, IntoVal, Symbol, }; use Nestera::{ rewards::{BonusAwarded, PointsAwarded, StreakUpdated}, @@ -63,8 +62,8 @@ fn test_points_awarded_event() { }) .collect(); - assert!(rewards_events.len() > 0); - let last_event = rewards_events.get(rewards_events.len() - 1).unwrap(); + assert!(!rewards_events.is_empty()); + let last_event = rewards_events.last().unwrap(); let event_data: PointsAwarded = last_event.2.clone().into_val(&env); assert_eq!(event_data.user, user); @@ -93,8 +92,8 @@ fn test_streak_updated_event() { }) .collect(); - assert!(streak_events.len() > 0); - let event_data: StreakUpdated = streak_events.get(0).unwrap().2.clone().into_val(&env); + assert!(!streak_events.is_empty()); + let event_data: StreakUpdated = streak_events.first().unwrap().2.clone().into_val(&env); assert_eq!(event_data.user, user); assert_eq!(event_data.streak, 1); @@ -125,8 +124,8 @@ fn test_bonus_awarded_streak_event() { }) .collect(); - assert!(bonus_events.len() > 0); - let event_data: BonusAwarded = bonus_events.get(0).unwrap().2.clone().into_val(&env); + assert!(!bonus_events.is_empty()); + let event_data: BonusAwarded = bonus_events.first().unwrap().2.clone().into_val(&env); assert_eq!(event_data.user, user); assert_eq!(event_data.bonus_type, Symbol::new(&env, "streak")); From f3b2b399663b64486118374263a0ccbcc25c00ba Mon Sep 17 00:00:00 2001 From: dinahmaccodes Date: Mon, 23 Feb 2026 20:07:17 +0100 Subject: [PATCH 3/3] chore: fix cargo fmt after clippy fixes --- contracts/src/governance_tests.rs | 2 +- contracts/src/security.rs | 1 - contracts/src/voting_tests.rs | 2 +- contracts/tests/admin_validation.rs | 1 - contracts/tests/anti_farming_test.rs | 5 +---- contracts/tests/rewards_test.rs | 3 ++- 6 files changed, 5 insertions(+), 9 deletions(-) diff --git a/contracts/src/governance_tests.rs b/contracts/src/governance_tests.rs index 1c54a7cc..1fa552fb 100644 --- a/contracts/src/governance_tests.rs +++ b/contracts/src/governance_tests.rs @@ -1,6 +1,6 @@ #[cfg(test)] mod governance_tests { - + use crate::rewards::storage_types::RewardsConfig; use crate::{NesteraContract, NesteraContractClient, PlanType}; use soroban_sdk::{testutils::Address as _, Address, BytesN, Env, String}; diff --git a/contracts/src/security.rs b/contracts/src/security.rs index 4a978d0b..4bc0e096 100644 --- a/contracts/src/security.rs +++ b/contracts/src/security.rs @@ -3,7 +3,6 @@ use soroban_sdk::Env; #[cfg(test)] mod security_tests { use super::*; - #[test] fn test_overflow_protection() { diff --git a/contracts/src/voting_tests.rs b/contracts/src/voting_tests.rs index 7420c41a..9362feb8 100644 --- a/contracts/src/voting_tests.rs +++ b/contracts/src/voting_tests.rs @@ -1,6 +1,6 @@ #[cfg(test)] mod voting_tests { - + use crate::rewards::storage_types::RewardsConfig; use crate::{NesteraContract, NesteraContractClient, PlanType}; use soroban_sdk::{ diff --git a/contracts/tests/admin_validation.rs b/contracts/tests/admin_validation.rs index b7d6e3cb..dac5105e 100644 --- a/contracts/tests/admin_validation.rs +++ b/contracts/tests/admin_validation.rs @@ -3,7 +3,6 @@ #[cfg(test)] mod admin_validation { - #[test] fn validate_admin_functions_exist() { diff --git a/contracts/tests/anti_farming_test.rs b/contracts/tests/anti_farming_test.rs index 40a84f7c..2766272e 100644 --- a/contracts/tests/anti_farming_test.rs +++ b/contracts/tests/anti_farming_test.rs @@ -1,8 +1,5 @@ #![cfg(test)] -use soroban_sdk::{ - testutils::Address as _, - Address, BytesN, Env, -}; +use soroban_sdk::{testutils::Address as _, Address, BytesN, Env}; use Nestera::{NesteraContract, NesteraContractClient}; fn create_test_env() -> (Env, NesteraContractClient<'static>, Address, Address) { diff --git a/contracts/tests/rewards_test.rs b/contracts/tests/rewards_test.rs index a28c51e8..a21867f9 100644 --- a/contracts/tests/rewards_test.rs +++ b/contracts/tests/rewards_test.rs @@ -2,7 +2,8 @@ use soroban_sdk::{ symbol_short, - testutils::{Address as _, Events}, Address, BytesN, Env, IntoVal, Symbol, + testutils::{Address as _, Events}, + Address, BytesN, Env, IntoVal, Symbol, }; use Nestera::{ rewards::{BonusAwarded, PointsAwarded, StreakUpdated},