From 3a127169ea25d0e3e83e7ff8b83810bc34bbc785 Mon Sep 17 00:00:00 2001 From: Ugooweb Date: Mon, 17 Aug 2026 00:40:02 +0100 Subject: [PATCH] fix: snapshot fee_bps at deposit time to prevent retroactive fee changes --- contracts/escrow/src/lib.rs | 16 +++++++------ contracts/escrow/src/test.rs | 32 +++++++++++++++++++++++++ contracts/escrow/src/types.rs | 1 + contracts/maintenance-pool/src/lib.rs | 10 ++++++++ contracts/milestones/src/lib.rs | 16 +++++++------ contracts/milestones/src/test.rs | 34 +++++++++++++++++++++++++++ contracts/milestones/src/types.rs | 1 + 7 files changed, 96 insertions(+), 14 deletions(-) diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index c9f95cb..0b946b4 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -100,6 +100,12 @@ impl EscrowContract { .set(&contribution_key, &Contribution { sponsor, amount }); extend_ttl(&env, &contribution_key); + let fee_bps: u32 = env + .storage() + .instance() + .get(&DataKey::FeeBps) + .ok_or(Error::NotInitialized)?; + let escrow = Escrow { token, amount, @@ -107,6 +113,7 @@ impl EscrowContract { created_at: env.ledger().timestamp(), deadline, contributor_count: 1, + fee_bps, }; env.storage().persistent().set(&key, &escrow); extend_ttl(&env, &key); @@ -192,7 +199,7 @@ impl EscrowContract { EscrowStatus::Funded => {} } - let payouts = compute_split(&env, escrow.amount, &recipients)?; + let payouts = compute_split(&env, escrow.amount, &recipients, escrow.fee_bps)?; let treasury: Address = env.storage().instance().get(&DataKey::Treasury).unwrap(); let token_client = token::Client::new(&env, &escrow.token); let contract_address = env.current_contract_address(); @@ -375,6 +382,7 @@ pub(crate) fn compute_split( env: &Env, total: i128, recipients: &Vec<(Address, u32)>, + fee_bps: u32, ) -> Result { if recipients.is_empty() { return Err(Error::InvalidSplit); @@ -388,12 +396,6 @@ pub(crate) fn compute_split( return Err(Error::InvalidSplit); } - let fee_bps: u32 = env - .storage() - .instance() - .get(&DataKey::FeeBps) - .ok_or(Error::NotInitialized)?; - let fee = total * (fee_bps as i128) / BPS_DENOMINATOR; let distributable = total - fee; diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 7fd4390..65609ab 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -732,3 +732,35 @@ fn test_get_contribution_enumerates_each_contributor() { let err = client.try_get_contribution(&108u64, &2u32); assert_eq!(err, Err(Ok(Error::EscrowNotFound))); } + +#[test] +fn test_release_uses_fee_bps_from_fund_time_not_current_value() { + let env = Env::default(); + env.mock_all_auths(); + let (_, _admin, treasury, client) = setup(&env); + + let token_admin = Address::generate(&env); + let (token_addr, asset_client, token_client) = create_token(&env, &token_admin); + let sponsor = Address::generate(&env); + asset_client.mint(&sponsor, &10_000_000_000i128); + + let contributor = Address::generate(&env); + + // Initial fee is 500 bps (5%) from setup + client.fund(&999u64, &sponsor, &token_addr, &10_000_000_000i128, &1_000u64); + + let escrow = client.get_escrow(&999u64); + assert_eq!(escrow.fee_bps, 500u32); + + // Change global fee_bps to 10% (1000 bps) to simulate #20 + env.as_contract(&client.address, || { + env.storage().instance().set(&DataKey::FeeBps, &1000u32); + }); + + let recipients = vec![&env, (contributor.clone(), 10_000u32)]; + client.release(&999u64, &recipients); + + // The snapshot value of 5% should be applied + assert_eq!(token_client.balance(&treasury), 500_000_000i128); + assert_eq!(token_client.balance(&contributor), 9_500_000_000i128); +} diff --git a/contracts/escrow/src/types.rs b/contracts/escrow/src/types.rs index 11923e5..af63a42 100644 --- a/contracts/escrow/src/types.rs +++ b/contracts/escrow/src/types.rs @@ -17,6 +17,7 @@ pub struct Escrow { pub created_at: u64, pub deadline: u64, pub contributor_count: u32, + pub fee_bps: u32, } /// One sponsor's contribution toward a (possibly crowdfunded) escrow. diff --git a/contracts/maintenance-pool/src/lib.rs b/contracts/maintenance-pool/src/lib.rs index 505e0be..fd62bac 100644 --- a/contracts/maintenance-pool/src/lib.rs +++ b/contracts/maintenance-pool/src/lib.rs @@ -113,6 +113,16 @@ impl MaintenancePoolContract { /// to `recipient` (a maintainer), as authorized off-chain by the /// backend oracle for completed maintenance work. Rejects if the pool /// balance is insufficient. + /// + /// Note on protocol fees: Unlike single-issue Escrows or Milestones, + /// a Maintenance Pool's balance is a blended pool of deposits made + /// over time, potentially by different sponsors under different historical + /// fee rates. Tracking individual deposit fee rates and applying them + /// proportionally at withdrawal time would add significant complexity. + /// Therefore, withdrawals always use the *current* global fee_bps at the + /// time of withdrawal, not the rate at deposit time. Sponsors should be + /// aware that the effective fee applied to their deposit may change if + /// the protocol fee is updated before funds are withdrawn. pub fn withdraw(env: Env, pool_id: u64, recipient: Address, amount: i128) -> Result<(), Error> { require_admin(&env)?.require_auth(); diff --git a/contracts/milestones/src/lib.rs b/contracts/milestones/src/lib.rs index a4b2ad9..b572afa 100644 --- a/contracts/milestones/src/lib.rs +++ b/contracts/milestones/src/lib.rs @@ -72,6 +72,12 @@ impl MilestonesContract { let token_client = token::Client::new(&env, &token); token_client.transfer(&sponsor, env.current_contract_address(), &total_budget); + let fee_bps: u32 = env + .storage() + .instance() + .get(&DataKey::FeeBps) + .ok_or(Error::NotInitialized)?; + let milestone = Milestone { sponsor, token, @@ -80,6 +86,7 @@ impl MilestonesContract { created_at: env.ledger().timestamp(), closed: false, allocations: Map::new(&env), + fee_bps, }; env.storage().persistent().set(&key, &milestone); extend_ttl(&env, &key); @@ -160,7 +167,7 @@ impl MilestonesContract { .get(issue_id) .ok_or(Error::IssueNotAllocated)?; - let payouts = compute_split(&env, amount, &recipients)?; + let payouts = compute_split(&env, amount, &recipients, milestone.fee_bps)?; let treasury: Address = env.storage().instance().get(&DataKey::Treasury).unwrap(); let token_client = token::Client::new(&env, &milestone.token); let contract_address = env.current_contract_address(); @@ -241,6 +248,7 @@ fn compute_split( env: &Env, total: i128, recipients: &Vec<(Address, u32)>, + fee_bps: u32, ) -> Result { if recipients.is_empty() { return Err(Error::InvalidSplit); @@ -254,12 +262,6 @@ fn compute_split( return Err(Error::InvalidSplit); } - let fee_bps: u32 = env - .storage() - .instance() - .get(&DataKey::FeeBps) - .ok_or(Error::NotInitialized)?; - let fee = total * (fee_bps as i128) / BPS_DENOMINATOR; let distributable = total - fee; diff --git a/contracts/milestones/src/test.rs b/contracts/milestones/src/test.rs index 69aaa4f..dfa738f 100644 --- a/contracts/milestones/src/test.rs +++ b/contracts/milestones/src/test.rs @@ -248,3 +248,37 @@ fn test_cancel_milestone_requires_admin_auth() { let result = client.try_cancel_milestone(&9u64); assert!(result.is_err()); } + +#[test] +fn test_release_uses_fee_bps_from_fund_time_not_current_value() { + let env = Env::default(); + env.mock_all_auths(); + let (_admin, treasury, client) = setup(&env); + + let token_admin = Address::generate(&env); + let (token_addr, asset_client, token_client) = create_token(&env, &token_admin); + let sponsor = Address::generate(&env); + asset_client.mint(&sponsor, &10_000_000_000i128); + + client.create_milestone(&10u64, &sponsor, &token_addr, &10_000_000_000i128); + client.allocate(&10u64, &1001u64, &100_0000000i128); + + let milestone = client.get_milestone(&10u64); + assert_eq!(milestone.fee_bps, 500u32); + + // Change global fee_bps to 10% (1000 bps) + env.as_contract(&client.address, || { + env.storage().instance().set(&DataKey::FeeBps, &1000u32); + }); + + let contributor = Address::generate(&env); + client.release_issue( + &10u64, + &1001u64, + &vec![&env, (contributor.clone(), 10_000u32)], + ); + + // 5% snapshot should be applied, meaning treasury gets 5% of 100_0000000 (5_0000000) + assert_eq!(token_client.balance(&treasury), 5_0000000i128); + assert_eq!(token_client.balance(&contributor), 95_0000000i128); +} diff --git a/contracts/milestones/src/types.rs b/contracts/milestones/src/types.rs index 05ecbba..e31cbe1 100644 --- a/contracts/milestones/src/types.rs +++ b/contracts/milestones/src/types.rs @@ -16,6 +16,7 @@ pub struct Milestone { /// issue_id -> allocated amount (0 once released and removed from the /// "open" set is not necessary; we track release via `IssueStatus`). pub allocations: Map, + pub fee_bps: u32, } #[contracttype]