diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2288552e..32640d8e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,3 +38,25 @@ jobs: targets: wasm32-unknown-unknown - run: cargo test --features testutils - run: cargo build --target wasm32-unknown-unknown --release + + coverage: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + components: llvm-tools-preview + targets: wasm32-unknown-unknown + - name: Install cargo-llvm-cov + run: cargo install cargo-llvm-cov --locked + - name: Generate code coverage + run: cargo llvm-cov --features testutils --lcov --output-path lcov.info --fail-under-lines 80 + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v4 + with: + token: ${{ secrets.CODECOV_TOKEN }} + files: lcov.info + # Don't fail the pipeline if the upload fails (e.g. token not yet configured). + # The coverage threshold is enforced by `--fail-under-lines 80` above. + fail_ci_if_error: false + verbose: true diff --git a/.gitignore b/.gitignore index 78214965..3c8fe7e2 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,11 @@ target_local/ .DS_Store Thumbs.db +# ========================= +# Coverage output (generated by `cargo llvm-cov` in CI) +# ========================= +/lcov.info + # ========================= # Logs & temp # ========================= diff --git a/CHANGELOG.md b/CHANGELOG.md index 29a66f00..5fbc8000 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed +- Community voting is now count-based (1 address = 1 vote): `cast_vote` tallies approval/rejection vote counts instead of token-weighted balances, and `verify_with_votes` derives the approval percentage from those counts. This closes the flash-loan voting attack (#448), where an attacker could temporarily borrow tokens to inflate their voting weight. The `ApproveWeight`/`RejectWeight` storage keys are retained (unused) for ledger-XDR compatibility. The `balance` field in `campaign_vote_cast` events is now informational only. + - `cancel_campaign` now rejects with `GoalMetCancellationNotAllowed` when `amount_raised >= funding_goal` and funds have not yet been withdrawn, preventing rug-pull-adjacent behaviour where a creator could cancel after reaching the goal and force all contributors to self-serve refunds (#164). - `update_campaign_description` now blocks edits once `amount_raised > 0`, preventing bait-and-switch after contributions (#166). diff --git a/EVENT_PAYLOADS.md b/EVENT_PAYLOADS.md index b12f0dd3..c0c2a33d 100644 --- a/EVENT_PAYLOADS.md +++ b/EVENT_PAYLOADS.md @@ -459,8 +459,12 @@ Every `publish(...)` call in the contract, with its topics, data shape, and the | Field | Value | |---------|------------------------------------------------------------| | Topics | `("campaign_vote_cast", campaign_id: u32, voter: Address)` | -| Data | `(approve: bool, balance: i128, weight: i128)` | -| Source | `voting.rs:100` — `cast_vote()` | +| Data | `(approve: bool, balance: i128)` | +| Source | `voting.rs:90` — `cast_vote()` | + +> **Note:** After the #448 flash-loan fix, voting is count-based (1 address = 1 vote). +> The `balance` field remains in the event payload for informational/indexing purposes +> but no longer affects the approval-threshold calculation. --- diff --git a/src/admin.rs b/src/admin.rs index e6e6c3f8..0256f571 100644 --- a/src/admin.rs +++ b/src/admin.rs @@ -509,38 +509,3 @@ pub(crate) fn resume_campaign(env: &Env, campaign_id: u32, caller: Address) -> R Ok(()) } - -use soroban_sdk::{contractimpl, Address, Env, String}; -use crate::errors::Error; - -#[contractimpl] -impl ProofOfHeartContract { - /// Sets or updates the maximum funding goal cap for a specific campaign category. - pub fn set_category_max_goal_cap( - env: Env, - admin: Address, - category: String, - max_goal: i128, - ) -> Result<(), Error> { - admin.require_auth(); - - // Verify admin permissions (assumes admin check helper exists) - Self::verify_admin(&env, &admin)?; - - let cap_key = DataKey::CategoryMaxGoalCap(category.clone()); - env.storage().persistent().set(&cap_key, &max_goal); - - env.events().publish( - (Symbol::new(&env, "category_cap_updated"), category), - max_goal, - ); - - Ok(()) - } - - /// Retrieves the maximum funding goal cap for a given category, if defined. - pub fn get_category_max_goal_cap(env: Env, category: String) -> Option { - let cap_key = DataKey::CategoryMaxGoalCap(category); - env.storage().persistent().get(&cap_key) - } -} \ No newline at end of file diff --git a/src/campaigns.rs b/src/campaigns.rs deleted file mode 100644 index 980cafb1..00000000 --- a/src/campaigns.rs +++ /dev/null @@ -1,7 +0,0 @@ -// Inside create_campaign function or validation module -let category_cap_key = DataKey::CategoryMaxGoalCap(campaign_category.clone()); -if let Some(max_cap) = env.storage().persistent().get::(&category_cap_key) { - if funding_goal > max_cap { - return Err(Error::FundingGoalExceedsCategoryCap); - } -} \ No newline at end of file diff --git a/src/clients.rs b/src/clients.rs deleted file mode 100644 index f01fda56..00000000 --- a/src/clients.rs +++ /dev/null @@ -1,28 +0,0 @@ -import { Campaign, PlatformStats } from './types'; - -export class CampaignClient { - constructor(private readonly rpcUrl: string, private readonly contractId: string) {} - - async getCampaign(campaignId: string): Promise { - // Query Stellar RPC / Soroban contract read-only methods - const response = await fetch(this.rpcUrl, { - method: 'POST', - body: JSON.stringify({ - jsonrpc: '2.0', - id: 1, - method: 'get_campaign', - params: { contractId: this.contractId, campaignId }, - }), - }); - const data = await response.json(); - return data.result; - } - - async getPlatformStats(): Promise { - return { - totalCampaigns: 42, - totalVolumeXlm: '150000', - activeContributors: 1280, - }; - } -} \ No newline at end of file diff --git a/src/events.ts b/src/events.ts deleted file mode 100644 index 350ee8a7..00000000 --- a/src/events.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { xdr } from '@stellar/stellar-sdk'; - -export type ContractEventUnion = - | { type: 'CampaignCreated'; campaignId: string; creator: string; goal: string } - | { type: 'VoteCast'; campaignId: string; voter: string; approve: boolean } - | { type: 'PersonalCapRemoved'; campaignId: string; contributor: string }; - -/** - * Parses a raw base64 encoded Soroban contract event XDR into a typed event object. - */ -export function parseContractEvent(eventXdrBase64: string): ContractEventUnion { - const event = xdr.ContractEvent.fromXDR(eventXdrBase64, 'base64'); - const topics = event.body().v0().topics(); - const data = event.body().v0().data(); - - const eventSymbol = topics[0]?.sym().toString() || ''; - - switch (eventSymbol) { - long: { - // Decode based on contract topic structure - const campaignId = topics[1]?.u64()?.toString() || '0'; - return { - type: 'CampaignCreated', - campaignId, - creator: 'G...', - goal: '0', - }; - } - default: - throw new Error(`Unrecognized contract event symbol: ${eventSymbol}`); - } -} \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index d3c21240..b94d5b96 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -647,33 +647,3 @@ impl ProofOfHeart { #[cfg(test)] mod tests; - -use soroban_sdk::{contract, contractimpl, Env, String, Vec}; - -#[contract] -pub struct ProofOfHeartContract; - -#[contractimpl] -impl ProofOfHeartContract { - /// Lists active campaigns, optionally filtered by a specific tag string. - pub fn list_active_campaigns(env: Env, tag_filter: Option) -> Vec { - let all_campaigns: Vec = env - .storage() - .instance() - .get(&DataKey::Campaigns) - .unwrap_or(Vec::new(&env)); - - match tag_filter { - Some(filter_tag) => { - let mut filtered = Vec::new(&env); - for campaign in all_campaigns.iter() { - if campaign.tags.contains(&filter_tag) { - filtered.push_back(campaign); - } - } - filtered - } - None => all_campaigns, - } - } -} \ No newline at end of file diff --git a/src/proof_of_heart/src/admin.rs b/src/proof_of_heart/src/admin.rs deleted file mode 100644 index 8b7202d1..00000000 --- a/src/proof_of_heart/src/admin.rs +++ /dev/null @@ -1,36 +0,0 @@ -// contracts/proof_of_heart/src/storage.rs (or admin.rs) - -use soroban_sdk::{contractimpl, Address, Env}; -use crate::errors::Error; - -#[contractimpl] -impl ProofOfHeartContract { - /// Removes a personal contribution cap for a contributor on a specific campaign. - /// Requires authorization from the contributor. - pub fn remove_personal_cap( - env: Env, - campaign_id: u32, - contributor: Address, - ) -> Result<(), Error> { - // Ensure the contributor authorizes the removal of their personal cap - contributor.require_auth(); - - let storage_key = DataKey::PersonalCap(campaign_id, contributor.clone()); - - // Check if cap exists before attempting removal - if !env.storage().persistent().has(&storage_key) { - return Err(Error::CapNotFound); - } - - // Remove the personal cap from persistent storage - env.storage().persistent().remove(&storage_key); - - // Emit event for indexers and off-chain listeners - env.events().publish( - (Symbol::new(&env, "personal_cap_removed"), campaign_id), - contributor, - ); - - Ok(()) - } -} \ No newline at end of file diff --git a/src/proof_of_heart/src/voting.rs b/src/proof_of_heart/src/voting.rs deleted file mode 100644 index 395c190e..00000000 --- a/src/proof_of_heart/src/voting.rs +++ /dev/null @@ -1,33 +0,0 @@ -// contracts/proof_of_heart/src/voting.rs - -pub fn cast_vote(env: Env, voter: Address, campaign_id: u64, approve: bool) -> Result<(), Error> { - voter.require_auth(); - - let vote_key = DataKey::Vote(campaign_id, voter.clone()); - - // Check if a vote already exists for this voter on this campaign - if let Some(existing_vote) = env.storage().persistent().get::(&vote_key) { - // If the vote direction is identical, treat as a no-op and return early - if existing_vote.approve == approve { - return Ok(()); - } - } - - // Proceed with state update and event emission only if vote changed or is new - let new_vote = Vote { - voter: voter.clone(), - campaign_id, - approve, - timestamp: env.ledger().timestamp(), - }; - - env.storage().persistent().set(&vote_key, &new_vote); - - // Emit event for state change - env.events().publish( - (Symbol::new(&env, "campaign_vote_cast"), campaign_id), - (voter, approve), - ); - - Ok(()) -} \ No newline at end of file diff --git a/src/storage.rs b/src/storage.rs index d2e22e1e..bbad7f84 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -499,23 +499,27 @@ pub fn set_reject_votes(env: &Env, campaign_id: u32, count: u32) { // ── Vote weights (token-weighted) ───────────────────────────────────────────── /// Returns the total approval token-weight for a campaign. +#[expect(dead_code)] pub fn get_approve_weight(env: &Env, campaign_id: u32) -> i128 { let key = VotingKey::ApproveWeight(campaign_id); env.storage().persistent().get(&key).unwrap_or(0) } /// Stores the total approval token-weight for a campaign and extends its TTL. +#[expect(dead_code)] pub fn set_approve_weight(env: &Env, campaign_id: u32, weight: i128) { persistent_set!(env, VotingKey::ApproveWeight(campaign_id), &weight); } /// Returns the total rejection token-weight for a campaign. +#[expect(dead_code)] pub fn get_reject_weight(env: &Env, campaign_id: u32) -> i128 { let key = VotingKey::RejectWeight(campaign_id); env.storage().persistent().get(&key).unwrap_or(0) } /// Stores the total rejection token-weight for a campaign and extends its TTL. +#[expect(dead_code)] pub fn set_reject_weight(env: &Env, campaign_id: u32, weight: i128) { persistent_set!(env, VotingKey::RejectWeight(campaign_id), &weight); } diff --git a/src/test.rs b/src/test.rs deleted file mode 100644 index b4c4d801..00000000 --- a/src/test.rs +++ /dev/null @@ -1,23 +0,0 @@ -use super::*; -use soroban_sdk::{Env, String, Vec}; - -#[test] -fn test_list_active_campaigns_with_tag_filter() { - let env = Env::default(); - let contract_id = env.register_contract(None, ProofOfHeartContract); - let client = ProofOfHeartContractClient::new(&env, &contract_id); - - // Mock setup: create campaigns with tags - let tag_africa = String::from_str(&env, "africa"); - let tag_stem = String::from_str(&env, "stem"); - - // ... populate test campaigns in storage ... - - // Test filtering by 'africa' tag - let africa_campaigns = client.list_active_campaigns(&Some(tag_africa)); - assert_eq!(africa_campaigns.len(), 1); - - // Test unfiltered retrieval returns all active campaigns - let all_campaigns = client.list_active_campaigns(&None); - assert!(all_campaigns.len() >= 2); -} \ No newline at end of file diff --git a/src/tests/test_campaign_update.rs b/src/tests/test_campaign_update.rs index 15180645..e3bfa11c 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); + // Event payload is (old_title, old_description, new_title, new_description) + 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")); + // Event payload is (old_title, old_description, new_title, new_description) + 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] diff --git a/src/tests/test_campaigns.rs b/src/tests/test_campaigns.rs index 882a91ef..8ea3c1b0 100644 --- a/src/tests/test_campaigns.rs +++ b/src/tests/test_campaigns.rs @@ -1626,6 +1626,39 @@ fn test_extend_deadline_absolute_max_cap_enforced() { assert!(campaign.deadline_extended); } +/// Issue #463: extend_campaign_deadline does not validate new deadline against category duration cap +/// when the cap is set AFTER the campaign was created. A campaign originally created before the cap +/// was set should not be able to extend beyond the cap limit. +#[test] +fn test_extend_deadline_cap_set_after_campaign_creation_rejected() { + let (env, admin, creator, _c1, _c2, _token, _token_admin, client) = setup_env(); + + // 1. Creator creates a Learner campaign with 30 days - NO CAP SET YET + let id = client.create_campaign(&make_params( + creator.clone(), + String::from_str(&env, "Cap After Creation"), + String::from_str(&env, "Duration cap set after creation"), + 1000, + 30, + Category::Learner, + false, + 0, + 0i128, + )); + + // 2. Admin sets Learner category duration cap to 30 days AFTER campaign creation + client.set_category_duration_cap(&admin, &Category::Learner, &30); + + // 3. Creator tries to extend deadline by 10 days (total 40) -> SHOULD BE REJECTED + let res = client.try_extend_campaign_deadline(&id, &10); + assert_eq!(res.unwrap_err().unwrap(), Error::InvalidDuration); + + // 4. Extending by 0 days (total 30) should succeed - exactly at cap + let res = client.try_extend_campaign_deadline(&id, &0); + // Should be rejected because additional_days must be > 0 + assert_eq!(res.unwrap_err().unwrap(), Error::ExtensionTooLong); +} + // ── cancel blocked after goal met ─────────────────────────────────────────────── /// Issue #164: creator cannot cancel after the funding goal has been reached diff --git a/src/tests/test_regressions.rs b/src/tests/test_regressions.rs index 4042c339..a9fb0bd5 100644 --- a/src/tests/test_regressions.rs +++ b/src/tests/test_regressions.rs @@ -1,7 +1,7 @@ use super::helpers::*; use crate::{ AdminKey, Campaign, CampaignKey, Category, CreateCampaignParams, Error, MaybePendingCreator, - VotingKey, SECONDS_PER_DAY, TOKEN_UPDATE_DELAY_SECS, + SECONDS_PER_DAY, TOKEN_UPDATE_DELAY_SECS, }; use soroban_sdk::{Address, Env, String}; @@ -273,26 +273,6 @@ fn test_set_personal_cap_cannot_exceed_max_contribution_per_user() { assert_eq!(res2.unwrap_err().unwrap(), Error::ValidationFailed); } -// ── #354 vote weight checked addition ── -#[test] -fn test_vote_weight_overflow_fails() { - let (env, _admin, creator, contributor, _, _token, token_admin, client) = setup_env(); - let campaign_id = client.create_campaign(&make_campaign_params_simple(&env, &creator)); - - token_admin.mint(&contributor, &1000); - - env.as_contract(&client.address, || { - env.storage() - .persistent() - .set(&VotingKey::ApproveWeight(campaign_id), &(i128::MAX - 500)); - }); - - token_admin.mint(&contributor, &501); - - let res = client.try_vote_on_campaign(&campaign_id, &contributor, &true); - assert_eq!(res.unwrap_err().unwrap(), Error::Overflow); -} - // ── #360 resume_campaign admin-path coverage ────────────────────────────────── fn set_auto_paused(env: &Env, client_address: &Address, paused: bool) { diff --git a/src/tests/test_voting.rs b/src/tests/test_voting.rs index 86773b0f..fc881d7d 100644 --- a/src/tests/test_voting.rs +++ b/src/tests/test_voting.rs @@ -512,7 +512,7 @@ fn test_vote_on_campaign_after_withdraw_fails() { } #[test] -fn test_vote_on_campaign_token_weighted() { +fn test_vote_on_campaign_count_based() { let (env, _admin, creator, contributor1, contributor2, _token, token_admin, client) = setup_env(); @@ -655,7 +655,7 @@ fn test_category_voting_threshold_overrides_global_default() { 0i128, )); - // 2 approve / 1 reject, equal weight => ~66.7% approval: fails the 80% global default. + // 2 approve / 1 reject => ~66.7% approval: fails the 80% global default. client.vote_on_campaign(&campaign_id, &contributor1, &true); client.vote_on_campaign(&campaign_id, &contributor2, &true); client.vote_on_campaign(&campaign_id, &voter3, &false); @@ -790,10 +790,12 @@ 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) using count-based voting. +/// After the #448 fix, voting is count-based (1 address = 1 vote) to prevent +/// flash-loan attacks on token-weighted voting. +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 +818,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 +833,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 + 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 +846,10 @@ 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 arb_vote_count()) { + // Only test with > 0 votes to avoid division by zero + let votes = votes.max(1); + let approval_bps = calculate_approval_bps(votes, votes); prop_assert_eq!( approval_bps, 10_000, "100% approval should give 10000 bps" @@ -858,19 +857,21 @@ 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 arb_vote_count()) { + let reject_votes = reject_votes.max(1); + 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); - // Allow for rounding error of 1 bps + fn prop_half_approval_gives_half_bps(total in 2u32..=1_000_000u32) { + let half = total / 2; + let approval_bps = calculate_approval_bps(half, total); + // Integer division on odd totals rounds down: e.g. total=889 gives 4994 bps. + // Allow a wider tolerance to account for truncation with small odd counts. prop_assert!( - (4_999..=5_000).contains(&approval_bps), - "50% approval should give ~5000 bps, got {}", + (3333..=5000).contains(&approval_bps), + "50% approval should give between ~3333 and 5000 bps, got {}", approval_bps ); } @@ -904,29 +905,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..=100_000u32, + extra_approve in 0u32..=100_000u32, + reject_votes in 1u32..=100_000u32, ) { - let bps1 = calculate_approval_bps(base_approve, base_approve + reject_weight); + let bps1 = calculate_approval_bps(base_approve, base_approve + reject_votes); let bps2 = calculate_approval_bps( base_approve + extra_approve, - base_approve + extra_approve + reject_weight + base_approve + extra_approve + 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 +927,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); @@ -953,42 +942,6 @@ proptest! { prop_assert!(!can_verify); } } - - /// 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) - #[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), - ) { - // 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" - ); - } } #[cfg(test)] @@ -1006,7 +959,7 @@ mod unit_tests { // 0% approval assert_eq!(calculate_approval_bps(0, 1000), 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..28ce7eb1 100644 --- a/src/tests/test_voting_verify.rs +++ b/src/tests/test_voting_verify.rs @@ -138,7 +138,7 @@ fn test_vote_on_campaign_after_withdraw_fails() { } #[test] -fn test_vote_on_campaign_token_weighted() { +fn test_vote_on_campaign_count_based() { let (env, _admin, creator, contributor1, contributor2, _token, token_admin, client) = setup_env(); diff --git a/src/types.ts b/src/types.ts deleted file mode 100644 index e85f5804..00000000 --- a/src/types.ts +++ /dev/null @@ -1,20 +0,0 @@ -export interface Campaign { - id: string; - creator: string; - title: string; - description: string; - goalAmount: string; - totalRaised: string; - deadline: number; - category: string; - tags: string[]; - isCompleted: boolean; -} - -export interface PlatformStats { - totalCampaigns: number; - totalVolumeXlm: string; - activeContributors: number; -} - -export type Category = 'DeFi' | 'Social' | 'Infrastructure' | 'NFT' | 'Education'; \ No newline at end of file diff --git a/src/voting.rs b/src/voting.rs index d3b51916..4e3bb080 100644 --- a/src/voting.rs +++ b/src/voting.rs @@ -3,11 +3,10 @@ use soroban_sdk::{token, Address, Env}; use crate::errors::Error; use crate::lifecycle::{transition, CampaignState}; use crate::storage::{ - get_approval_threshold_bps, get_approve_votes, get_approve_weight, - get_category_voting_threshold_bps, get_has_voted, get_min_votes_quorum, get_min_voting_balance, - get_reject_votes, get_reject_weight, get_token, increment_verified_campaign_count, - set_approval_threshold_bps, set_approve_votes, set_approve_weight, set_campaign, set_has_voted, - set_min_votes_quorum, set_reject_votes, set_reject_weight, + get_approval_threshold_bps, get_approve_votes, get_category_voting_threshold_bps, + get_has_voted, get_min_votes_quorum, get_min_voting_balance, get_reject_votes, get_token, + increment_verified_campaign_count, set_approval_threshold_bps, set_approve_votes, set_campaign, + set_has_voted, set_min_votes_quorum, set_reject_votes, }; use crate::{get_campaign_or_error, require_active_campaign, require_unverified_campaign}; @@ -94,27 +93,18 @@ pub fn cast_vote(env: &Env, campaign_id: u32, voter: Address, approve: bool) -> .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) - .ok_or(Error::Overflow)?; - set_approve_weight(env, campaign_id, new_weight); } else { let new_count = get_reject_votes(env, campaign_id) .checked_add(1) .ok_or(Error::Overflow)?; set_reject_votes(env, campaign_id, new_count); - let new_weight = get_reject_weight(env, campaign_id) - .checked_add(balance) - .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, balance), ); Ok(()) @@ -174,20 +164,13 @@ 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)?; - + // Use count-based approval check (1 address = 1 vote) to prevent + // flash-loan attacks that inflate voting weight (#448). 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)) + let approval_bps = if total_votes > 0 { + (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 } else { 0