From fba702200e78286e4869867f25d50bc0b3a5d171 Mon Sep 17 00:00:00 2001 From: Dillon Ofili Date: Thu, 30 Jul 2026 07:04:26 +0000 Subject: [PATCH 1/2] fix: use 1-address-1-vote model to prevent flash-loan voting (#469) Replace token-weighted voting with 1-address-1-vote in cast_vote and verify_with_votes. Each eligible token holder now contributes exactly 1 to the vote count regardless of their balance, closing the flash-loan attack vector where an attacker borrows a large balance, votes with inflated weight, and returns the tokens before verification. Changes: - cast_vote: weight = 1 instead of weight = balance - verify_with_votes: threshold computed from vote counts, not token weights - Updated tests and proptests for the new 1-address-1-vote model - Updated overflow regression test (#354) for safe weight addition Closes #469 --- src/lib.rs | 5 +- src/tests/mod.rs | 2 +- src/tests/test_regressions.rs | 14 +++- src/tests/test_voting.rs | 140 ++++++++++++++------------------ src/tests/test_voting_verify.rs | 8 +- src/voting.rs | 41 +++++----- 6 files changed, 98 insertions(+), 112 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4e58b6ba..b94d5b96 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -617,7 +617,10 @@ impl ProofOfHeart { queries::get_creator_stats(&env, creator) } - pub fn get_contributor_portfolio(env: Env, contributor: Address) -> soroban_sdk::Vec<(u32, i128, String, bool)> { + pub fn get_contributor_portfolio( + env: Env, + contributor: Address, + ) -> soroban_sdk::Vec<(u32, i128, String, bool)> { queries::get_contributor_portfolio(&env, contributor) } diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 416e975b..c06debd9 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -6,9 +6,9 @@ mod test_benchmark; mod test_bookmarks; mod test_campaign_update; mod test_campaigns; -mod test_cap_interactions; mod test_cancel_after_goal_met; mod test_cancel_revenue_orphan; +mod test_cap_interactions; mod test_contributions; mod test_creator_buckets; mod test_lifecycle; diff --git a/src/tests/test_regressions.rs b/src/tests/test_regressions.rs index 4042c339..dda0005a 100644 --- a/src/tests/test_regressions.rs +++ b/src/tests/test_regressions.rs @@ -273,9 +273,13 @@ fn test_set_personal_cap_cannot_exceed_max_contribution_per_user() { assert_eq!(res2.unwrap_err().unwrap(), Error::ValidationFailed); } -// ── #354 vote weight checked addition ── +// ── #354 / #469 vote weight safe addition ── +/// With the 1-address-1-vote model (#469), vote weight is incremented by 1 +/// regardless of token balance. Even when the stored weight is near i128::MAX, +/// a vote still succeeds because we only add 1 (not the voter's balance), +/// preventing flash-loan attacks that inflate voting weight. #[test] -fn test_vote_weight_overflow_fails() { +fn test_vote_weight_does_not_overflow_with_1_address_1_vote() { let (env, _admin, creator, contributor, _, _token, token_admin, client) = setup_env(); let campaign_id = client.create_campaign(&make_campaign_params_simple(&env, &creator)); @@ -289,8 +293,10 @@ fn test_vote_weight_overflow_fails() { token_admin.mint(&contributor, &501); - let res = client.try_vote_on_campaign(&campaign_id, &contributor, &true); - assert_eq!(res.unwrap_err().unwrap(), Error::Overflow); + // With 1-address-1-vote, the weight only increments by 1, so no overflow occurs + // even when ApproveWeight is near i128::MAX (#469). + client.vote_on_campaign(&campaign_id, &contributor, &true); + assert_eq!(client.get_approve_votes(&campaign_id), 1); } // ── #360 resume_campaign admin-path coverage ────────────────────────────────── diff --git a/src/tests/test_voting.rs b/src/tests/test_voting.rs index 86773b0f..206f4672 100644 --- a/src/tests/test_voting.rs +++ b/src/tests/test_voting.rs @@ -512,17 +512,18 @@ fn test_vote_on_campaign_after_withdraw_fails() { } #[test] -fn test_vote_on_campaign_token_weighted() { +fn test_vote_on_campaign_one_address_one_vote() { let (env, _admin, creator, contributor1, contributor2, _token, token_admin, client) = setup_env(); + // contributor1 has 5000 tokens, contributor2 has only 1000 — both get 1 vote (#469). token_admin.mint(&contributor1, &5000); token_admin.mint(&contributor2, &1000); let campaign_id = client.create_campaign(&make_params( creator.clone(), - String::from_str(&env, "Weighted Vote Test"), - String::from_str(&env, "Test token-weighted voting"), + String::from_str(&env, "1-Address-1-Vote Test"), + String::from_str(&env, "Test 1-address-1-vote model"), 1000, 30, Category::Learner, @@ -534,6 +535,7 @@ fn test_vote_on_campaign_token_weighted() { client.vote_on_campaign(&campaign_id, &contributor1, &true); client.vote_on_campaign(&campaign_id, &contributor2, &false); + // Each voter contributes exactly 1 to the count regardless of balance. assert_eq!(client.get_approve_votes(&campaign_id), 1); assert_eq!(client.get_reject_votes(&campaign_id), 1); } @@ -790,10 +792,11 @@ fn test_min_voting_balance_threshold_enforcement() { // ── Pure arithmetic helpers ────────────────────────────────────────────────── -/// Calculate approval percentage in basis points (0-10000) -fn calculate_approval_bps(approve_weight: i128, total_weight: i128) -> u32 { - if total_weight > 0 { - ((approve_weight * 10_000) / total_weight) as u32 +/// Calculate approval percentage in basis points (0-10000) from vote counts. +/// Uses u64 to avoid overflow when multiplying by 10_000. +fn calculate_approval_bps(approve_votes: u32, total_votes: u32) -> u32 { + if total_votes > 0 { + ((approve_votes as u64 * 10_000) / total_votes as u64) as u32 } else { 0 } @@ -816,11 +819,6 @@ fn arb_vote_count() -> impl Strategy { 0u32..=1_000_000u32 } -/// Token weights: 0 to 10 billion stroops -fn arb_token_weight() -> impl Strategy { - 0i128..=10_000_000_000i128 -} - /// Approval threshold in basis points (0-10000, i.e., 0-100%) fn arb_threshold_bps() -> impl Strategy { 0u32..=10_000u32 @@ -836,11 +834,11 @@ fn arb_min_quorum() -> impl Strategy { proptest! { #[test] fn prop_approval_bps_in_valid_range( - approve_weight in arb_token_weight(), - reject_weight in arb_token_weight(), + approve_votes in arb_vote_count(), + reject_votes in arb_vote_count(), ) { - let total_weight = approve_weight + reject_weight; - let approval_bps = calculate_approval_bps(approve_weight, total_weight); + let total_votes = approve_votes.saturating_add(reject_votes); + let approval_bps = calculate_approval_bps(approve_votes, total_votes); prop_assert!( approval_bps <= 10_000, "approval_bps ({}) must be <= 10000", @@ -849,8 +847,8 @@ proptest! { } #[test] - fn prop_full_approval_gives_max_bps(weight in arb_token_weight()) { - let approval_bps = calculate_approval_bps(weight, weight); + fn prop_full_approval_gives_max_bps(votes in 1u32..=1_000_000u32) { + let approval_bps = calculate_approval_bps(votes, votes); prop_assert_eq!( approval_bps, 10_000, "100% approval should give 10000 bps" @@ -858,18 +856,18 @@ proptest! { } #[test] - fn prop_zero_approval_gives_zero_bps(reject_weight in arb_token_weight()) { - let approval_bps = calculate_approval_bps(0, reject_weight); + fn prop_zero_approval_gives_zero_bps(reject_votes in 1u32..=1_000_000u32) { + let approval_bps = calculate_approval_bps(0, reject_votes); prop_assert_eq!(approval_bps, 0, "0% approval should give 0 bps"); } #[test] - fn prop_half_approval_gives_half_bps(weight in 2i128..=10_000_000_000i128) { - let half = weight / 2; - let approval_bps = calculate_approval_bps(half, weight); + fn prop_half_approval_gives_half_bps(votes in 10u32..=1_000_000u32) { + let half = votes / 2; + let approval_bps = calculate_approval_bps(half, votes); // Allow for rounding error of 1 bps prop_assert!( - (4_999..=5_000).contains(&approval_bps), + (4_900..=5_000).contains(&approval_bps), "50% approval should give ~5000 bps, got {}", approval_bps ); @@ -904,29 +902,20 @@ proptest! { prop_assert!(total.is_some(), "vote count addition should not overflow"); } - #[test] - fn prop_weight_no_overflow( - approve_weight in 0i128..=5_000_000_000i128, - reject_weight in 0i128..=5_000_000_000i128, - ) { - let total = approve_weight.checked_add(reject_weight); - prop_assert!(total.is_some(), "weight addition should not overflow"); - } - #[test] fn prop_approval_monotonic( - base_approve in 0i128..=1_000_000i128, - extra_approve in 0i128..=1_000_000i128, - reject_weight in 1i128..=1_000_000i128, + base_approve in 0u32..=500_000u32, + extra_approve in 0u32..=500_000u32, + reject_votes in 1u32..=500_000u32, ) { - let bps1 = calculate_approval_bps(base_approve, base_approve + reject_weight); + let bps1 = calculate_approval_bps(base_approve, base_approve.saturating_add(reject_votes)); let bps2 = calculate_approval_bps( - base_approve + extra_approve, - base_approve + extra_approve + reject_weight + base_approve.saturating_add(extra_approve), + base_approve.saturating_add(extra_approve).saturating_add(reject_votes), ); prop_assert!( bps2 >= bps1, - "adding approval weight should not decrease approval bps: {} -> {}", + "adding approval votes should not decrease approval bps: {} -> {}", bps1, bps2 ); } @@ -935,14 +924,11 @@ proptest! { fn prop_verification_requires_both_conditions( approve_votes in arb_vote_count(), reject_votes in arb_vote_count(), - approve_weight in arb_token_weight(), - reject_weight in arb_token_weight(), min_quorum in arb_min_quorum(), threshold_bps in 5_000u32..=10_000u32, // 50-100% ) { let total_votes = approve_votes.saturating_add(reject_votes); - let total_weight = approve_weight.saturating_add(reject_weight); - let approval_bps = calculate_approval_bps(approve_weight, total_weight); + let approval_bps = calculate_approval_bps(approve_votes, total_votes); let quorum_met = is_quorum_met(total_votes, min_quorum); let threshold_met = is_threshold_met(approval_bps, threshold_bps); @@ -954,40 +940,32 @@ proptest! { } } - /// Property test for issue #211: - /// Verify that voting weights always equal the sum of token-balances of voters - /// who chose the same side. - /// - /// This test generates a set of voters with their balances and voting choices, - /// then verifies the invariant: - /// approve_weight = sum(balances of voters who approved) - /// reject_weight = sum(balances of voters who rejected) + /// Property test for the 1-address-1-vote model (#469): + /// Every voter contributes exactly 1 to the count regardless of token balance. + /// This confirms that the vote counts equal the number of voters on each side. #[test] - fn prop_voting_weights_equal_sum_of_balances( - // Generate random voters with their balances and choices - approval_balances in prop::collection::vec(1i128..=1_000_000i128, 0..20), - rejection_balances in prop::collection::vec(1i128..=1_000_000i128, 0..20), + fn prop_one_address_one_vote_invariant( + // Generate random numbers of approving and rejecting voters + approve_count in 0u32..=10_000u32, + reject_count in 0u32..=10_000u32, ) { - // Calculate expected weights - let expected_approve_weight: i128 = approval_balances.iter().sum(); - let expected_reject_weight: i128 = rejection_balances.iter().sum(); - - // In the actual voting implementation (from voting.rs cast_vote): - // - When approve=true: approve_weight += voter_balance - // - When approve=false: reject_weight += voter_balance - // This test verifies that summing balances of each group produces the correct weight - // - // The invariant is: - // approve_weight = sum of all voter balances who approved - // reject_weight = sum of all voter balances who rejected - prop_assert!( - expected_approve_weight >= 0, - "approve_weight must be non-negative" - ); - prop_assert!( - expected_reject_weight >= 0, - "reject_weight must be non-negative" - ); + // In the 1-address-1-vote model: + // - Each approving voter adds exactly 1 to approve_count + // - Each rejecting voter adds exactly 1 to reject_count + // - approval_bps is computed from counts, not balances + let total_votes = approve_count.saturating_add(reject_count); + let approval_bps = calculate_approval_bps(approve_count, total_votes); + prop_assert!(approval_bps <= 10_000); + + // If all votes approve, approval should be 10000 bps + if reject_count == 0 && approve_count > 0 { + prop_assert_eq!(approval_bps, 10_000); + } + + // If all votes reject, approval should be 0 bps + if approve_count == 0 && reject_count > 0 { + prop_assert_eq!(approval_bps, 0); + } } } @@ -997,16 +975,16 @@ mod unit_tests { #[test] fn test_approval_bps_calculation() { - // 60% approval - assert_eq!(calculate_approval_bps(600, 1000), 6000); + // 60% approval (3 approve / 5 total) + assert_eq!(calculate_approval_bps(3, 5), 6000); // 100% approval - assert_eq!(calculate_approval_bps(1000, 1000), 10000); + assert_eq!(calculate_approval_bps(1, 1), 10000); // 0% approval - assert_eq!(calculate_approval_bps(0, 1000), 0); + assert_eq!(calculate_approval_bps(0, 1), 0); - // Zero total weight + // Zero total votes assert_eq!(calculate_approval_bps(0, 0), 0); } diff --git a/src/tests/test_voting_verify.rs b/src/tests/test_voting_verify.rs index b4fa3142..09854db9 100644 --- a/src/tests/test_voting_verify.rs +++ b/src/tests/test_voting_verify.rs @@ -138,17 +138,18 @@ fn test_vote_on_campaign_after_withdraw_fails() { } #[test] -fn test_vote_on_campaign_token_weighted() { +fn test_vote_on_campaign_one_address_one_vote() { let (env, _admin, creator, contributor1, contributor2, _token, token_admin, client) = setup_env(); + // contributor1 has 5000 tokens, contributor2 has only 1000 — both get 1 vote (#469). token_admin.mint(&contributor1, &5000); token_admin.mint(&contributor2, &1000); let campaign_id = client.create_campaign(&make_params( creator.clone(), - String::from_str(&env, "Weighted Vote Test"), - String::from_str(&env, "Test token-weighted voting"), + String::from_str(&env, "1-Address-1-Vote Test"), + String::from_str(&env, "Test 1-address-1-vote model"), 1000, 30, Category::Learner, @@ -160,6 +161,7 @@ fn test_vote_on_campaign_token_weighted() { client.vote_on_campaign(&campaign_id, &contributor1, &true); client.vote_on_campaign(&campaign_id, &contributor2, &false); + // Each voter contributes exactly 1 to the count regardless of balance. assert_eq!(client.get_approve_votes(&campaign_id), 1); assert_eq!(client.get_reject_votes(&campaign_id), 1); } diff --git a/src/voting.rs b/src/voting.rs index d3b51916..e940ce42 100644 --- a/src/voting.rs +++ b/src/voting.rs @@ -55,12 +55,17 @@ pub fn set_params( /// Records a vote (approve or reject) from a token-holding voter. /// +/// Voting uses a 1-address-1-vote model (#469): every eligible token holder +/// gets exactly one vote, regardless of their token balance. This prevents +/// flash-loan attacks where an attacker borrows a large balance, votes with +/// inflated weight, and returns the tokens before verification. +/// /// # Errors /// * `CampaignNotFound` - No campaign with the given ID. /// * `CampaignAlreadyVerified` - The campaign is already verified. /// * `CampaignNotActive` - The campaign is cancelled or inactive. /// * `DeadlinePassed` - The voting period has closed (deadline exceeded). -/// * `NotTokenHolder` - The voter holds no tokens. +/// * `NotTokenHolder` - The voter holds no tokens or is below the minimum. /// * `AlreadyVoted` - The voter has already cast a vote on this campaign. pub fn cast_vote(env: &Env, campaign_id: u32, voter: Address, approve: bool) -> Result<(), Error> { voter.require_auth(); @@ -89,13 +94,15 @@ pub fn cast_vote(env: &Env, campaign_id: u32, voter: Address, approve: bool) -> return Err(Error::AlreadyVoted); } + // 1-address-1-vote: each voter contributes exactly 1 to the weight sum + // regardless of token balance (#469). if approve { let new_count = get_approve_votes(env, campaign_id) .checked_add(1) .ok_or(Error::Overflow)?; set_approve_votes(env, campaign_id, new_count); let new_weight = get_approve_weight(env, campaign_id) - .checked_add(balance) + .checked_add(1) .ok_or(Error::Overflow)?; set_approve_weight(env, campaign_id, new_weight); } else { @@ -104,17 +111,16 @@ pub fn cast_vote(env: &Env, campaign_id: u32, voter: Address, approve: bool) -> .ok_or(Error::Overflow)?; set_reject_votes(env, campaign_id, new_count); let new_weight = get_reject_weight(env, campaign_id) - .checked_add(balance) + .checked_add(1) .ok_or(Error::Overflow)?; set_reject_weight(env, campaign_id, new_weight); } set_has_voted(env, campaign_id, &voter); - let vote_weight = balance; env.events().publish( ("campaign_vote_cast", campaign_id, voter), - (approve, balance, vote_weight), + (approve, 1i128, 1i128), ); Ok(()) @@ -174,24 +180,15 @@ pub fn verify_with_votes(env: &Env, campaign_id: u32) -> Result<(), Error> { return Err(Error::VotingQuorumNotMet); } - // Use token-weighted sums for the approval-threshold check. - let approve_weight = get_approve_weight(env, campaign_id); - let reject_weight = get_reject_weight(env, campaign_id); - let total_weight = approve_weight - .checked_add(reject_weight) - .ok_or(Error::Overflow)?; - + // 1-address-1-vote (#469): threshold is computed from vote counts, not + // token balances, so flash-loaned tokens cannot inflate the approval + // percentage. `total_votes` is guaranteed > 0 because of the quorum + // check above, so division by zero is impossible. let threshold = effective_approval_threshold_bps(env, campaign.category); - let approval_bps = if total_weight > 0 { - // Use checked arithmetic to avoid silent overflow/truncation when - // approve_weight is a large i128 (e.g. whale holders on 18-decimal tokens). - approve_weight - .checked_mul(crate::BPS_DENOMINATOR as i128) - .and_then(|n| n.checked_div(total_weight)) - .unwrap_or(0) as u32 - } else { - 0 - }; + let approval_bps = ((approve_votes as u64) + .checked_mul(crate::BPS_DENOMINATOR as u64) + .and_then(|n| n.checked_div(total_votes as u64)) + .unwrap_or(0)) as u32; if approval_bps < threshold { return Err(Error::VotingThresholdNotMet); } From cf4b1efb02a5316e547c9485e32d78682ffdb4b2 Mon Sep 17 00:00:00 2001 From: Dillon Ofili Date: Thu, 30 Jul 2026 07:24:40 +0000 Subject: [PATCH 2/2] test: fix campaign_metadata_updated event unpack to 4-tuple (#510) Tests were unpacking the campaign_metadata_updated event as a 2-tuple (String, String) but the event was updated to emit 4 strings (old_title, old_desc, new_title, new_desc) for consistent indexer shape. --- src/tests/test_campaign_update.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/tests/test_campaign_update.rs b/src/tests/test_campaign_update.rs index 15180645..99eba167 100644 --- a/src/tests/test_campaign_update.rs +++ b/src/tests/test_campaign_update.rs @@ -59,10 +59,12 @@ fn test_update_campaign_emits_title_and_description() { let events = env.events().all(); let last_event = events.last().unwrap(); - let payload: (String, String) = soroban_sdk::FromVal::from_val(&env, &last_event.2); + // #510: campaign_metadata_updated emits (old_title, old_desc, new_title, new_desc) + let payload: (String, String, String, String) = + soroban_sdk::FromVal::from_val(&env, &last_event.2); - assert_eq!(payload.0, new_title); - assert_eq!(payload.1, new_desc); + assert_eq!(payload.2, new_title); + assert_eq!(payload.3, new_desc); } #[test] @@ -94,9 +96,11 @@ fn test_update_campaign_event_tracks_latest_description() { let events = env.events().all(); let last_event = events.last().unwrap(); - let payload: (String, String) = soroban_sdk::FromVal::from_val(&env, &last_event.2); - assert_eq!(payload.0, String::from_str(&env, "Title V3")); - assert_eq!(payload.1, String::from_str(&env, "Description V3")); + // #510: campaign_metadata_updated emits (old_title, old_desc, new_title, new_desc) + let payload: (String, String, String, String) = + soroban_sdk::FromVal::from_val(&env, &last_event.2); + assert_eq!(payload.2, String::from_str(&env, "Title V3")); + assert_eq!(payload.3, String::from_str(&env, "Description V3")); } #[test]