From 6b1921f003b5a56ca379c36dcce24a7aba57b6df Mon Sep 17 00:00:00 2001 From: Japheth Adamu Date: Sat, 17 Oct 2026 16:12:54 +0000 Subject: [PATCH] Fix fees, dispute persistence, and analytics capacity --- contracts/analytics/src/lib.rs | 28 +++++---- contracts/analytics/tests/time_series.rs | 1 + contracts/disputes/Cargo.toml | 4 ++ contracts/disputes/src/lib.rs | 67 ++++++++++++++++++--- contracts/disputes/tests/dispute_storage.rs | 36 +++++++++++ contracts/fees/src/lib.rs | 3 + contracts/fees/tests/auth_boundary.rs | 17 ++++++ contracts/markets/src/lib.rs | 34 +++++++++++ 8 files changed, 169 insertions(+), 21 deletions(-) create mode 100644 contracts/disputes/tests/dispute_storage.rs diff --git a/contracts/analytics/src/lib.rs b/contracts/analytics/src/lib.rs index 6bd21cbb..4d31321f 100644 --- a/contracts/analytics/src/lib.rs +++ b/contracts/analytics/src/lib.rs @@ -380,20 +380,20 @@ impl Analytics { .get(points_key) .unwrap_or_else(|| Vec::new(env)); - if points.len() >= max_capacity { + let summary = env + .storage() + .persistent() + .get::(summary_key); + let current_count = summary.as_ref().map(|s| s.total_count).unwrap_or(0); + if current_count >= max_capacity { return Err(ContractError::StoreFull); } - points.push_back(DataPoint { timestamp, value }); - env.storage().persistent().set(points_key, &points); - - // Update summary - let summary = if let Some(mut s) = env - .storage() - .persistent() - .get::(summary_key) - { - s.total_count += 1; + let next_summary = if let Some(mut s) = summary { + s.total_count = s + .total_count + .checked_add(1) + .ok_or(ContractError::Overflow)?; s.total_sum = s .total_sum .checked_add(value) @@ -419,7 +419,11 @@ impl Analytics { } }; - env.storage().persistent().set(summary_key, &summary); + points.push_back(DataPoint { timestamp, value }); + env.storage().persistent().set(points_key, &points); + env.storage() + .persistent() + .set(summary_key, &next_summary); Ok(()) } diff --git a/contracts/analytics/tests/time_series.rs b/contracts/analytics/tests/time_series.rs index 5719940b..4301f0bc 100644 --- a/contracts/analytics/tests/time_series.rs +++ b/contracts/analytics/tests/time_series.rs @@ -233,6 +233,7 @@ fn test_capacity_limits() { client.try_record_metric_at(&metric, &40, &400), Err(Ok(ContractError::StoreFull)) ); + assert_eq!(client.get_metric_summary(&metric).total_count, 3); } #[test] diff --git a/contracts/disputes/Cargo.toml b/contracts/disputes/Cargo.toml index 9f11aaba..cc57f2e4 100644 --- a/contracts/disputes/Cargo.toml +++ b/contracts/disputes/Cargo.toml @@ -23,6 +23,10 @@ path = "tests/err_stab.rs" name = "dispute_edge_cases" path = "tests/dispute_edge_cases.rs" +[[test]] +name = "dispute_storage" +path = "tests/dispute_storage.rs" + [[test]] name = "xcontract" path = "tests/xcontract.rs" diff --git a/contracts/disputes/src/lib.rs b/contracts/disputes/src/lib.rs index 9ea0e7d3..f660f708 100644 --- a/contracts/disputes/src/lib.rs +++ b/contracts/disputes/src/lib.rs @@ -50,6 +50,7 @@ pub enum DisputeStatus { #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] pub struct Dispute { + pub id: u32, pub market_id: Symbol, pub creator: Symbol, pub status: DisputeStatus, @@ -161,6 +162,26 @@ public struct EscalationRecord { pub resolved: bool, } +#[contracttype] +#[derive(Clone)] +enum DataKey { + DisputeCounter, + Dispute(u32), +} + +fn load_dispute(env: &Env, id: u32) -> Dispute { + env.storage() + .persistent() + .get(&DataKey::Dispute(id)) + .unwrap_or_else(|| panic!("dispute not found")) +} + +fn store_dispute(env: &Env, dispute: &Dispute) { + env.storage() + .persistent() + .set(&DataKey::Dispute(dispute.id), dispute); +} + #[contract] pub struct DisputesContract; @@ -253,24 +274,48 @@ impl DisputesContract { } } + /// Returns a dispute by its persistent ID, if it exists. + pub fn get_dispute(env: Env, id: u32) -> Option { + env.storage().persistent().get(&DataKey::Dispute(id)) + } + /// Open a new dispute for a market. The dispute begins in the - /// `Open` status; community voting has not yet started. - pub fn open_dispute(_env: Env, market_id: Symbol, creator: Symbol) -> Dispute { - Dispute { + /// `Open` status; community voting has not yet started. Its allocated ID + /// can be used to retrieve the persisted dispute and identify it in later + /// lifecycle transitions. + pub fn open_dispute(env: Env, market_id: Symbol, creator: Symbol) -> Dispute { + let previous_id: u32 = env + .storage() + .persistent() + .get(&DataKey::DisputeCounter) + .unwrap_or(0); + let id = previous_id + .checked_add(1) + .unwrap_or_else(|| panic!("dispute ID overflow")); + let dispute = Dispute { + id, market_id, creator, status: DisputeStatus::Open, outcome: DisputeOutcome::Pending, - } + }; + env.storage() + .persistent() + .set(&DataKey::DisputeCounter, &id); + store_dispute(&env, &dispute); + dispute } /// Transition a dispute from `Open` to `Voting`. Once voting has - /// begun the dispute can no longer be withdrawn by its creator. - pub fn start_voting(_env: Env, mut dispute: Dispute) -> Dispute { + /// begun the dispute can no longer be withdrawn by its creator. The + /// persisted record is authoritative; only the supplied ID is used. + pub fn start_voting(env: Env, dispute: Dispute) -> Dispute { + let mut dispute = load_dispute(&env, dispute.id); if dispute.status != DisputeStatus::Open { panic!("dispute is not open"); } dispute.status = DisputeStatus::Voting; + store_dispute(&env, &dispute); dispute } @@ -279,7 +324,8 @@ impl DisputesContract { /// before community voting has begun). This prevents a creator from /// opening a frivolous dispute to delay resolution and then /// cancelling it mid-vote to avoid an unfavourable ruling. - pub fn withdraw_dispute(_env: Env, mut dispute: Dispute, caller: Symbol) -> Dispute { + pub fn withdraw_dispute(env: Env, dispute: Dispute, caller: Symbol) -> Dispute { + let mut dispute = load_dispute(&env, dispute.id); if caller != dispute.creator { panic!("only the dispute creator may withdraw"); } @@ -287,16 +333,18 @@ impl DisputesContract { panic!"cannot withdraw a dispute once voting has begun"); } dispute.status = DisputeStatus::Withdrawn; + store_dispute(&env, &dispute); dispute } /// Record a binding resolution for a dispute. Only permitted while /// the dispute is in the `Voting` status. pub fn resolve_dispute( - _env: Env, - mut dispute: Dispute, + env: Env, + dispute: Dispute, outcome: DisputeOutcome, ) -> Dispute { + let mut dispute = load_dispute(&env, dispute.id); if dispute.status != DisputeStatus::Voting { panic!("dispute is not in voting"); } @@ -305,6 +353,7 @@ impl DisputesContract { } dispute.status = DisputeStatus::Resolved; dispute.outcome = outcome; + store_dispute(&env, &dispute); dispute } } diff --git a/contracts/disputes/tests/dispute_storage.rs b/contracts/disputes/tests/dispute_storage.rs new file mode 100644 index 00000000..2980aa24 --- /dev/null +++ b/contracts/disputes/tests/dispute_storage.rs @@ -0,0 +1,36 @@ +use disputes::{ + DisputeOutcome, DisputeStatus, DisputesContract, DisputesContractClient, +}; +use soroban_sdk::{Env, Symbol}; + +#[test] +fn dispute_lifecycle_is_persisted_and_uses_unique_ids() { + let env = Env::default(); + let contract_id = env.register(DisputesContract, ()); + let client = DisputesContractClient::new(&env, &contract_id); + let market_id = Symbol::new(&env, "MARKET1"); + let creator = Symbol::new(&env, "CREATOR"); + + let opened = client.open_dispute(&market_id, &creator); + assert_eq!(opened.id, 1); + assert_eq!(opened.status, DisputeStatus::Open); + assert_eq!(client.get_dispute(&opened.id), Some(opened.clone())); + + let mut stale_input = opened.clone(); + stale_input.creator = Symbol::new(&env, "FORGED"); + let voting = client.start_voting(&stale_input); + assert_eq!(voting.creator, creator); + assert_eq!(voting.status, DisputeStatus::Voting); + assert_eq!(client.get_dispute(&opened.id), Some(voting.clone())); + + let resolved = client.resolve_dispute(&voting, &DisputeOutcome::ChallengerWins); + assert_eq!(resolved.status, DisputeStatus::Resolved); + assert_eq!(resolved.outcome, DisputeOutcome::ChallengerWins); + assert_eq!(client.get_dispute(&opened.id), Some(resolved)); + + let second = client.open_dispute(&Symbol::new(&env, "MARKET2"), &creator); + assert_eq!(second.id, 2); + let withdrawn = client.withdraw_dispute(&second, &creator); + assert_eq!(withdrawn.status, DisputeStatus::Withdrawn); + assert_eq!(client.get_dispute(&second.id), Some(withdrawn)); +} \ No newline at end of file diff --git a/contracts/fees/src/lib.rs b/contracts/fees/src/lib.rs index b8348bff..327c8862 100644 --- a/contracts/fees/src/lib.rs +++ b/contracts/fees/src/lib.rs @@ -815,6 +815,9 @@ impl FeesContract { if config.min_fee_amount > config.max_fee_amount { return Err(ContractError::InvalidInput); } + if config.fees_enabled && config.min_fee_amount == 0 { + return Err(ContractError::InvalidInput); + } Ok(()) } } diff --git a/contracts/fees/tests/auth_boundary.rs b/contracts/fees/tests/auth_boundary.rs index 0915dae3..7404aabd 100644 --- a/contracts/fees/tests/auth_boundary.rs +++ b/contracts/fees/tests/auth_boundary.rs @@ -204,6 +204,23 @@ fn test_update_fee_config_rejects_negative_values() { } } +#[test] +fn test_update_fee_config_rejects_zero_minimum_when_fees_enabled() { + let env = Env::default(); + let admin = Address::generate(&env); + let client = register_and_initialize(&env, &admin); + + let mut config = default_fee_config(); + config.min_fee_amount = 0; + config.max_fee_amount = 0; + + let result = client.try_update_fee_config(&admin, &config); + assert!(matches!(result, Err(Ok(ContractError::InvalidInput)))); + + config.fees_enabled = false; + client.update_fee_config(&admin, &config); +} + // ============================================================ // Auth Boundary Tests — set_platform_fee // ============================================================ diff --git a/contracts/markets/src/lib.rs b/contracts/markets/src/lib.rs index 6553ddde..a8946288 100644 --- a/contracts/markets/src/lib.rs +++ b/contracts/markets/src/lib.rs @@ -6,6 +6,24 @@ //! Provides a prediction-market subsystem where every state-changing //! entrypoint enforces `require_auth` on the acting [`Address`]. //! +//! # User Balance Fee Model +//! +//! Deposits are fee-free: `deposit_user_balance` credits the full deposited +//! amount to the user's internal balance. Fees are charged on exit instead. +//! `withdraw_user_balance` charges the greater of the configured minimum fee +//! and the basis-point fee on the requested withdrawal amount, except when +//! dust or full-withdrawal rules waive the fee. It removes the requested amount +//! from the internal balance and returns the post-fee `net_amount`; it does not +//! transfer tokens. `collect_user_fees` is a separate admin-triggered charge +//! based on the user's full current balance; it deducts from that balance and +//! adds to the collected-fees counter, without transferring tokens out of the +//! contract. +//! +//! These two fee paths are not mutually exclusive. An admin collection does +//! not mark funds as already charged, so a later user withdrawal can charge +//! another fee on the remaining balance. Callers should not use both paths for +//! the same funds when only one fee is intended. +//! //! # Auth Matrix //! //! | Function | Required Role | @@ -1134,6 +1152,9 @@ impl MarketsContract { } /// Deposits funds into a user's internal withdrawable balance (#012). + /// + /// No fee is charged on deposit; the full `amount` is transferred in and + /// credited to the user's internal balance. pub fn deposit_user_balance(env: Env, user: Address, amount: i128) { user.require_auth(); @@ -1162,6 +1183,14 @@ impl MarketsContract { /// Withdraws funds from a user's balance with dust protection (#012). /// + /// The fee is calculated from the requested `amount` as the greater of + /// the configured minimum and the basis-point fee. The full requested + /// amount is removed from the internal balance, and the post-fee amount is + /// returned as `net_amount`; this method does not transfer tokens. The fee + /// is added to the collected-fees counter. This is independent of + /// `collect_user_fees`, so both paths can charge fees against the same + /// funds at different times. + /// /// # Dust Invariant /// If the user's available balance is `<= min_fee_amount`, it is classified /// as dust. To prevent funds from being permanently locked, the minimum fee @@ -1229,6 +1258,11 @@ impl MarketsContract { /// Admin fee collection from user account with dust-safe protection (#012). /// + /// This charges against the user's full current internal balance and adds + /// the amount to the collected-fees counter; it does not transfer tokens + /// out of the contract. It is independent of `withdraw_user_balance`, so + /// a later withdrawal can charge another fee on the remaining balance. + /// /// If a user balance is `<= min_fee_amount`, fees are not collected and 0 is returned, /// ensuring dust is never locked and `InsufficientBalance` is never thrown. pub fn collect_user_fees(env: Env, admin: Address, user: Address) -> i128 {