From 93a0406e31eadae0e27078333231b71b8de08563 Mon Sep 17 00:00:00 2001 From: Farez Abel Date: Sun, 18 Oct 2026 02:39:59 +0000 Subject: [PATCH 1/4] feat(split): add fee waiver system with expiry and audit log (#922) - Add FeeWaiver record (grantor, grant time, expiry, reason) per whitelisted creator - Add grant_fee_waiver / extend_fee_waiver admin controls (SuperAdmin) - Expired waivers no longer exempt creators from platform fees at release - Add permissionless purge_expired_fee_waivers; grants purge lapsed entries first - Add bounded fee waiver audit log and fw_exp / fw_purge events - Add get_fee_waiver, list_fee_waivers, get_fee_waiver_audit_log read functions --- .claude/settings.json | 20 ++ contracts/split/src/events.rs | 28 +++ contracts/split/src/lib.rs | 295 +++++++++++++++++++++++----- contracts/split/src/test.rs | 146 ++++++++++++++ contracts/split/src/types.rs | 34 ++++ docs/ISSUE_922_FEE_WAIVER_SYSTEM.md | 47 +++++ 6 files changed, 526 insertions(+), 44 deletions(-) create mode 100644 .claude/settings.json create mode 100644 docs/ISSUE_922_FEE_WAIVER_SYSTEM.md diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..3c7ef49 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,20 @@ +{ + "permissions": { + "allow": [ + "Bash", + "Read", + "Edit", + "Write", + "WebFetch", + "Grep", + "Glob", + "LS", + "MultiEdit", + "NotebookRead", + "NotebookEdit", + "TodoRead", + "TodoWrite", + "WebSearch" + ] + } +} diff --git a/contracts/split/src/events.rs b/contracts/split/src/events.rs index 8a8f631..672e5b8 100644 --- a/contracts/split/src/events.rs +++ b/contracts/split/src/events.rs @@ -625,6 +625,34 @@ pub fn fee_waiver_revoked(env: &Env, creator: &Address) { ); } +/// Issue #922: Emitted when a fee waiver's expiry is set or extended. +/// Topics: (split, fw_exp, creator) +/// Data: expires_at (0 = never) +pub fn fee_waiver_expiry_set(env: &Env, creator: &Address, expires_at: u64) { + env.events().publish( + ( + symbol_short!("split"), + symbol_short!("fw_exp"), + creator.clone(), + ), + expires_at, + ); +} + +/// Issue #922: Emitted when an expired fee waiver is purged from the whitelist. +/// Topics: (split, fw_purge, creator) +/// Data: expires_at +pub fn fee_waiver_expired(env: &Env, creator: &Address, expires_at: u64) { + env.events().publish( + ( + symbol_short!("split"), + symbol_short!("fw_purge"), + creator.clone(), + ), + expires_at, + ); +} + /// Issue #285: Emitted when fee tiers are updated. /// Topics: (split, fee_tiers_updated) /// Data: count of tiers diff --git a/contracts/split/src/lib.rs b/contracts/split/src/lib.rs index ab78604..7741933 100644 --- a/contracts/split/src/lib.rs +++ b/contracts/split/src/lib.rs @@ -37,6 +37,8 @@ const STROOPS_PER_10K_INSTRUCTIONS: u64 = 1; /// Issue #296: Maximum entries in the per-creator fee waiver list. const MAX_FEE_WAIVER_ENTRIES: usize = 100; +/// Issue #922: Maximum entries retained in the fee waiver audit log (oldest dropped first). +const MAX_FEE_WAIVER_AUDIT_ENTRIES: u32 = 200; const DEFAULT_COMMITMENT_EXPIRY_LEDGERS: u32 = 100; /// Fixed-point scale for oracle-priced invoices: the oracle's `price()` return @@ -90,7 +92,8 @@ use types::{ AdminAction, AdminRole, AdminSet, AuditEntry, Bid, CircuitBreakerStatus, CloneOverrides, CompactInvoice, CompactMigrateResult, CompletionProof, ComputeEstimate, ConfidentialPayment, ContributionResult, CreateInvoiceParams, CreatorStats, DelayedPayout, - DisputeOutcome, DisputeRecord, DisputeStatus, FeeBracket, FeeSplit, FeeTier, InstalmentPlan, + DisputeOutcome, DisputeRecord, DisputeStatus, FeeBracket, FeeSplit, FeeTier, FeeWaiver, + FeeWaiverAction, FeeWaiverAuditEntry, InstalmentPlan, Invoice, InvoiceCore, InvoiceExt, InvoiceExt2, InvoiceExt3, InvoiceHot, InvoiceOptions, InvoiceOptions2, InvoicePayment, InvoiceStats, InvoiceStatus, InvoiceTemplate, InvoiceTemplateRecord, LegacyInvoice, OverflowBehavior, OverfundingPolicy, Payment, @@ -181,6 +184,16 @@ fn creator_fee_waiver_key() -> Symbol { symbol_short!("cr_fw") } +/// Issue #922: Per-creator fee waiver record (grantor, expiry, reason). +fn fee_waiver_record_key(creator: &Address) -> (Symbol, Address) { + (symbol_short!("fw_rec"), creator.clone()) +} + +/// Issue #922: Bounded fee waiver audit log. +fn fee_waiver_audit_key() -> Symbol { + symbol_short!("fw_audit") +} + /// Issue #297: Circuit breaker active flag key. fn circuit_breaker_key() -> Symbol { symbol_short!("cb_flag") @@ -2305,6 +2318,142 @@ fn require_fn_not_paused(env: &Env, name: &Symbol) { } } +// --------------------------------------------------------------------------- +// Issue #922: Fee waiver helpers +// --------------------------------------------------------------------------- + +fn load_fee_waiver_list(env: &Env) -> Vec
{ + env.storage() + .persistent() + .get(&creator_fee_waiver_key()) + .unwrap_or_else(|| Vec::new(env)) +} + +fn fee_waiver_is_expired(env: &Env, waiver: &FeeWaiver) -> bool { + waiver.expires_at != 0 && env.ledger().timestamp() >= waiver.expires_at +} + +/// True when `creator` is whitelisted and the waiver has not expired. +/// Entries added before issue #922 carry no record and never expire. +fn creator_has_active_fee_waiver(env: &Env, creator: &Address) -> bool { + if !load_fee_waiver_list(env).iter().any(|a| a == *creator) { + return false; + } + match env + .storage() + .persistent() + .get::<_, FeeWaiver>(&fee_waiver_record_key(creator)) + { + Some(waiver) => !fee_waiver_is_expired(env, &waiver), + None => true, + } +} + +fn record_fee_waiver_audit( + env: &Env, + action: FeeWaiverAction, + creator: &Address, + actor: &Address, + expires_at: u64, +) { + let mut log: Vec = env + .storage() + .persistent() + .get(&fee_waiver_audit_key()) + .unwrap_or_else(|| Vec::new(env)); + log.push_back(FeeWaiverAuditEntry { + action, + creator: creator.clone(), + actor: actor.clone(), + timestamp: env.ledger().timestamp(), + ledger: env.ledger().sequence(), + expires_at, + }); + while log.len() > MAX_FEE_WAIVER_AUDIT_ENTRIES { + log.pop_front(); + } + env.storage().persistent().set(&fee_waiver_audit_key(), &log); +} + +/// Remove every expired waiver from the whitelist. Returns the number removed. +fn purge_expired_fee_waivers_internal(env: &Env, actor: &Address) -> u32 { + let waivers = load_fee_waiver_list(env); + let mut kept: Vec
= Vec::new(env); + let mut removed: u32 = 0; + for creator in waivers.iter() { + let record: Option = env + .storage() + .persistent() + .get(&fee_waiver_record_key(&creator)); + match record { + Some(waiver) if fee_waiver_is_expired(env, &waiver) => { + env.storage() + .persistent() + .remove(&fee_waiver_record_key(&creator)); + record_fee_waiver_audit( + env, + FeeWaiverAction::Expired, + &creator, + actor, + waiver.expires_at, + ); + events::fee_waiver_expired(env, &creator, waiver.expires_at); + removed += 1; + } + _ => kept.push_back(creator), + } + } + if removed > 0 { + env.storage() + .persistent() + .set(&creator_fee_waiver_key(), &kept); + } + removed +} + +/// Add or renew a waiver for `creator`. Shared by `add_fee_waiver` and `grant_fee_waiver`. +fn grant_fee_waiver_internal( + env: &Env, + admin: &Address, + creator: &Address, + expires_at: u64, + reason: String, +) { + assert!( + expires_at == 0 || expires_at > env.ledger().timestamp(), + "waiver expiry must be in the future" + ); + // Free capacity held by lapsed waivers before enforcing the size cap. + purge_expired_fee_waivers_internal(env, admin); + + let mut waivers = load_fee_waiver_list(env); + let already_listed = waivers.iter().any(|a| a == *creator); + if !already_listed { + assert!( + (waivers.len() as usize) < MAX_FEE_WAIVER_ENTRIES, + "fee waiver list full" + ); + waivers.push_back(creator.clone()); + env.storage() + .persistent() + .set(&creator_fee_waiver_key(), &waivers); + } + + let waiver = FeeWaiver { + creator: creator.clone(), + granted_by: admin.clone(), + granted_at: env.ledger().timestamp(), + expires_at, + reason, + }; + env.storage() + .persistent() + .set(&fee_waiver_record_key(creator), &waiver); + record_fee_waiver_audit(env, FeeWaiverAction::Granted, creator, admin, expires_at); + events::fee_waiver_granted(env, creator); + events::fee_waiver_expiry_set(env, creator, expires_at); +} + // --------------------------------------------------------------------------- // Group helpers // --------------------------------------------------------------------------- @@ -9543,14 +9692,8 @@ impl SplitContract { let funding_token_client = token::Client::new(env, &funding_token_for(invoice)); - let creator_waived: bool = { - let cfw: Vec
= env - .storage() - .persistent() - .get(&creator_fee_waiver_key()) - .unwrap_or_else(|| Vec::new(env)); - cfw.iter().any(|a| a == invoice.creator) - }; + // Issue #922: expired waivers no longer exempt the creator. + let creator_waived: bool = creator_has_active_fee_waiver(env, &invoice.creator); let total: i128 = invoice.amounts.iter().sum(); let funded = invoice.funded; @@ -10096,14 +10239,8 @@ impl SplitContract { let funding_token_client = token::Client::new(env, &funding_token_for(invoice)); // Issue #296: if creator has a fee waiver, platform_fee_bps is 0 for this invoice. - let creator_waived: bool = { - let cfw: Vec
= env - .storage() - .persistent() - .get(&creator_fee_waiver_key()) - .unwrap_or_else(|| Vec::new(env)); - cfw.iter().any(|a| a == invoice.creator) - }; + // Issue #922: expired waivers no longer exempt the creator. + let creator_waived: bool = creator_has_active_fee_waiver(env, &invoice.creator); let funded = invoice.funded; @@ -13758,38 +13895,66 @@ impl SplitContract { } // ----------------------------------------------------------------------- - // Issue #296: Per-creator fee waiver list + // Issue #296 / #922: Per-creator fee waiver whitelist // ----------------------------------------------------------------------- - /// Grant a fee waiver to a creator. Admin-only. Max 100 entries. + /// Grant a non-expiring fee waiver to a creator. Admin-only. Max 100 entries. + /// Idempotent: re-adding a creator who already holds an active waiver is a no-op. pub fn add_fee_waiver(env: Env, admin: Address, creator: Address) { require_admin_role(&env, &admin, AdminRole::SuperAdmin); - let mut waivers: Vec
= env - .storage() - .persistent() - .get(&creator_fee_waiver_key()) - .unwrap_or_else(|| Vec::new(&env)); + if creator_has_active_fee_waiver(&env, &creator) { + return; + } + grant_fee_waiver_internal(&env, &admin, &creator, 0, String::from_str(&env, "")); + } + + /// Issue #922: Grant (or renew) a fee waiver with an expiry and reason. Admin-only. + /// `expires_at` is a ledger timestamp; pass `0` for a waiver that never expires. + pub fn grant_fee_waiver( + env: Env, + admin: Address, + creator: Address, + expires_at: u64, + reason: String, + ) { + require_admin_role(&env, &admin, AdminRole::SuperAdmin); + grant_fee_waiver_internal(&env, &admin, &creator, expires_at, reason); + } + + /// Issue #922: Change the expiry of an existing, still-active waiver. Admin-only. + pub fn extend_fee_waiver(env: Env, admin: Address, creator: Address, expires_at: u64) { + require_admin_role(&env, &admin, AdminRole::SuperAdmin); assert!( - (waivers.len() as usize) < MAX_FEE_WAIVER_ENTRIES, - "fee waiver list full" + creator_has_active_fee_waiver(&env, &creator), + "no active fee waiver" ); - if !waivers.iter().any(|a| a == creator) { - waivers.push_back(creator.clone()); - env.storage() - .persistent() - .set(&creator_fee_waiver_key(), &waivers); - events::fee_waiver_granted(&env, &creator); - } + assert!( + expires_at == 0 || expires_at > env.ledger().timestamp(), + "waiver expiry must be in the future" + ); + let mut waiver: FeeWaiver = env + .storage() + .persistent() + .get(&fee_waiver_record_key(&creator)) + .unwrap_or(FeeWaiver { + creator: creator.clone(), + granted_by: admin.clone(), + granted_at: env.ledger().timestamp(), + expires_at: 0, + reason: String::from_str(&env, ""), + }); + waiver.expires_at = expires_at; + env.storage() + .persistent() + .set(&fee_waiver_record_key(&creator), &waiver); + record_fee_waiver_audit(&env, FeeWaiverAction::Extended, &creator, &admin, expires_at); + events::fee_waiver_expiry_set(&env, &creator, expires_at); } /// Revoke a fee waiver from a creator. Admin-only. pub fn remove_fee_waiver(env: Env, admin: Address, creator: Address) { require_admin_role(&env, &admin, AdminRole::SuperAdmin); - let waivers: Vec
= env - .storage() - .persistent() - .get(&creator_fee_waiver_key()) - .unwrap_or_else(|| Vec::new(&env)); + let waivers = load_fee_waiver_list(&env); let mut new_waivers: Vec
= Vec::new(&env); for a in waivers.iter() { if a != creator { @@ -13799,17 +13964,59 @@ impl SplitContract { env.storage() .persistent() .set(&creator_fee_waiver_key(), &new_waivers); + let expires_at = env + .storage() + .persistent() + .get::<_, FeeWaiver>(&fee_waiver_record_key(&creator)) + .map(|w| w.expires_at) + .unwrap_or(0); + env.storage() + .persistent() + .remove(&fee_waiver_record_key(&creator)); + record_fee_waiver_audit(&env, FeeWaiverAction::Revoked, &creator, &admin, expires_at); events::fee_waiver_revoked(&env, &creator); } - /// Returns true if the creator is on the fee waiver list. + /// Issue #922: Remove all expired waivers from the whitelist. Callable by anyone. + /// Returns the number of waivers removed. + pub fn purge_expired_fee_waivers(env: Env, caller: Address) -> u32 { + caller.require_auth(); + purge_expired_fee_waivers_internal(&env, &caller) + } + + /// Returns true if the creator holds an active (unexpired) fee waiver. pub fn has_fee_waiver(env: Env, creator: Address) -> bool { - let waivers: Vec
= env - .storage() + creator_has_active_fee_waiver(&env, &creator) + } + + /// Issue #922: Returns the waiver record for a whitelisted creator, if any. + /// Expired waivers that have not yet been purged are still returned. + pub fn get_fee_waiver(env: Env, creator: Address) -> Option { + if !load_fee_waiver_list(&env).iter().any(|a| a == creator) { + return None; + } + env.storage() .persistent() - .get(&creator_fee_waiver_key()) - .unwrap_or_else(|| Vec::new(&env)); - waivers.iter().any(|a| a == creator) + .get(&fee_waiver_record_key(&creator)) + } + + /// Issue #922: Returns all creators currently holding an active fee waiver. + pub fn list_fee_waivers(env: Env) -> Vec
{ + let mut active: Vec
= Vec::new(&env); + for creator in load_fee_waiver_list(&env).iter() { + if creator_has_active_fee_waiver(&env, &creator) { + active.push_back(creator); + } + } + active + } + + /// Issue #922: Returns the fee waiver audit log (most recent last, capped at 200). + pub fn get_fee_waiver_audit_log(env: Env) -> Vec { + env.storage() + .persistent() + .get(&fee_waiver_audit_key()) + .unwrap_or_else(|| Vec::new(&env)) } // ----------------------------------------------------------------------- diff --git a/contracts/split/src/test.rs b/contracts/split/src/test.rs index 4bd3a9f..75a80c1 100644 --- a/contracts/split/src/test.rs +++ b/contracts/split/src/test.rs @@ -8688,3 +8688,149 @@ fn test_create_invoice_payment_window_only_one_or_none_ok() { assert!(id3 >= 1); } + +// --------------------------------------------------------------------------- +// Issue #922: Fee waiver system (expiry + audit log) +// --------------------------------------------------------------------------- + +fn setup_fee_waiver_admin() -> (Env, Address, Address, Address) { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let admin = Address::generate(&env); + let treasury = Address::generate(&env); + c.initialize(&admin, &0_i128, &treasury, &token_id, &0_u32, &None, &0_u32, &0_u32, &0_u64); + env.ledger().set_timestamp(1_000); + (env, contract_id, token_id, admin) +} + +#[test] +fn test_922_grant_fee_waiver_stores_record() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + let reason = String::from_str(&env, "launch partner"); + + c.grant_fee_waiver(&admin, &creator, &5_000_u64, &reason); + + assert!(c.has_fee_waiver(&creator)); + let waiver = c.get_fee_waiver(&creator).expect("waiver record"); + assert_eq!(waiver.creator, creator); + assert_eq!(waiver.granted_by, admin); + assert_eq!(waiver.granted_at, 1_000); + assert_eq!(waiver.expires_at, 5_000); + assert_eq!(waiver.reason, reason); + assert_eq!(c.list_fee_waivers().len(), 1); +} + +#[test] +fn test_922_waiver_expires_at_timestamp() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + + c.grant_fee_waiver(&admin, &creator, &2_000_u64, &String::from_str(&env, "")); + assert!(c.has_fee_waiver(&creator)); + + env.ledger().set_timestamp(2_000); + assert!(!c.has_fee_waiver(&creator), "waiver must lapse at expires_at"); + assert_eq!(c.list_fee_waivers().len(), 0); +} + +#[test] +#[should_panic(expected = "waiver expiry must be in the future")] +fn test_922_grant_with_past_expiry_rejected() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + c.grant_fee_waiver(&admin, &creator, &500_u64, &String::from_str(&env, "")); +} + +#[test] +fn test_922_purge_removes_only_expired_waivers() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let expiring = Address::generate(&env); + let permanent = Address::generate(&env); + + c.grant_fee_waiver(&admin, &expiring, &1_500_u64, &String::from_str(&env, "")); + c.add_fee_waiver(&admin, &permanent); + + env.ledger().set_timestamp(1_600); + let removed = c.purge_expired_fee_waivers(&Address::generate(&env)); + assert_eq!(removed, 1); + assert!(c.get_fee_waiver(&expiring).is_none()); + assert!(c.has_fee_waiver(&permanent)); +} + +#[test] +fn test_922_extend_fee_waiver_updates_expiry() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + + c.grant_fee_waiver(&admin, &creator, &2_000_u64, &String::from_str(&env, "")); + c.extend_fee_waiver(&admin, &creator, &9_000_u64); + + env.ledger().set_timestamp(5_000); + assert!(c.has_fee_waiver(&creator)); + assert_eq!(c.get_fee_waiver(&creator).unwrap().expires_at, 9_000); +} + +#[test] +fn test_922_audit_log_records_lifecycle() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + + c.grant_fee_waiver(&admin, &creator, &2_000_u64, &String::from_str(&env, "")); + c.extend_fee_waiver(&admin, &creator, &3_000_u64); + c.remove_fee_waiver(&admin, &creator); + + let log = c.get_fee_waiver_audit_log(); + assert_eq!(log.len(), 3); + assert_eq!(log.get(0).unwrap().action, types::FeeWaiverAction::Granted); + assert_eq!(log.get(1).unwrap().action, types::FeeWaiverAction::Extended); + assert_eq!(log.get(2).unwrap().action, types::FeeWaiverAction::Revoked); + assert_eq!(log.get(2).unwrap().actor, admin); + assert_eq!(log.get(2).unwrap().creator, creator); +} + +#[test] +fn test_922_expired_waiver_charges_platform_fee() { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); + + let admin = Address::generate(&env); + let treasury = Address::generate(&env); + let creator = Address::generate(&env); + let payer = Address::generate(&env); + let recipient = Address::generate(&env); + + StellarAssetClient::new(&env, &token_id).mint(&payer, &100); + env.ledger().set_timestamp(1_000); + c.initialize(&admin, &0_i128, &treasury, &token_id, &1_000_u32, &None, &0_u32, &0_u32, &0_u64); + c.grant_fee_waiver(&admin, &creator, &1_500_u64, &String::from_str(&env, "")); + + env.ledger().set_timestamp(2_000); + let id = make_invoice(&env, &c, &creator, &recipient, 100, &token_id, 9_999); + c.pay(&payer, &id, &100_i128, &0_u64, &false, &false, &None); + + assert!(tk.balance(&recipient) < 100, "expired waiver must not zero the platform fee"); +} + +#[test] +fn test_922_purge_frees_capacity_for_new_waivers() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + + for _ in 0..100 { + c.grant_fee_waiver(&admin, &Address::generate(&env), &1_500_u64, &String::from_str(&env, "")); + } + env.ledger().set_timestamp(1_600); + // List is full of expired entries; granting purges them first. + let creator = Address::generate(&env); + c.add_fee_waiver(&admin, &creator); + assert!(c.has_fee_waiver(&creator)); + assert_eq!(c.list_fee_waivers().len(), 1); +} diff --git a/contracts/split/src/types.rs b/contracts/split/src/types.rs index 564bc0b..83d0911 100644 --- a/contracts/split/src/types.rs +++ b/contracts/split/src/types.rs @@ -1661,6 +1661,40 @@ pub struct CircuitBreakerStatus { pub reason: Option, } +/// Issue #922: Fee waiver record for a whitelisted creator. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct FeeWaiver { + pub creator: Address, + pub granted_by: Address, + pub granted_at: u64, + /// Ledger timestamp at which the waiver lapses; `0` means it never expires. + pub expires_at: u64, + pub reason: String, +} + +/// Issue #922: Action recorded in the fee waiver audit log. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub enum FeeWaiverAction { + Granted, + Extended, + Revoked, + Expired, +} + +/// Issue #922: A single fee waiver audit log entry. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct FeeWaiverAuditEntry { + pub action: FeeWaiverAction, + pub creator: Address, + pub actor: Address, + pub timestamp: u64, + pub ledger: u32, + pub expires_at: u64, +} + /// Issue #295: A single confidential payment record stored per payer. #[contracttype] #[derive(Clone, Debug)] diff --git a/docs/ISSUE_922_FEE_WAIVER_SYSTEM.md b/docs/ISSUE_922_FEE_WAIVER_SYSTEM.md new file mode 100644 index 0000000..17af3cc --- /dev/null +++ b/docs/ISSUE_922_FEE_WAIVER_SYSTEM.md @@ -0,0 +1,47 @@ +# Issue #922: Fee Waiver System + +Extends the per-creator fee waiver whitelist from issue #296 with expiry, waiver records and an audit log. + +## Storage + +| Key | Tier | Type | Purpose | +|-----|------|------|---------| +| `cr_fw` | persistent | `Vec
` | Whitelist index (max 100 entries, from #296) | +| `(fw_rec, creator)` | persistent | `FeeWaiver` | Grantor, grant time, expiry, reason | +| `fw_audit` | persistent | `Vec` | Audit log, capped at 200 entries (oldest dropped) | + +`FeeWaiver.expires_at` is a ledger timestamp; `0` means the waiver never expires. Whitelist +entries created before this change have no record and are treated as non-expiring. + +## Admin controls (SuperAdmin only) + +- `add_fee_waiver(admin, creator)`: grants a non-expiring waiver. Does nothing if the creator already has an active waiver. +- `grant_fee_waiver(admin, creator, expires_at, reason)`: grants or renews a waiver with an expiry. +- `extend_fee_waiver(admin, creator, expires_at)`: changes the expiry of an active waiver. +- `remove_fee_waiver(admin, creator)`: revokes the waiver and deletes its record. + +An expiry that is not in the future is rejected with `"waiver expiry must be in the future"`. + +## Expiration + +- A waiver is active only while `expires_at == 0 || now < expires_at`. +- `purge_expired_fee_waivers(caller) -> u32` is permissionless. It removes expired waivers from the whitelist and returns how many it removed. +- Granting a waiver purges expired entries first, so lapsed waivers stop counting against the 100-entry cap. + +## Fee checks + +Release (`_release_full`, `_release_tranches`) uses `creator_has_active_fee_waiver`, so an expired waiver no longer zeroes the platform fee, even before it is purged. + +## Read functions + +- `has_fee_waiver(creator) -> bool`: true only for active waivers. +- `get_fee_waiver(creator) -> Option` +- `list_fee_waivers() -> Vec
`: active waivers only. +- `get_fee_waiver_audit_log() -> Vec` + +## Audit log and events + +Every grant, extension, revocation and expiry purge appends a `FeeWaiverAuditEntry` +`{ action, creator, actor, timestamp, ledger, expires_at }`. + +Events: `fw_grant`, `fw_rev` (existing), `fw_exp` (expiry set or extended), `fw_purge` (expired waiver removed). From 2a0ecf22e4c1ec8e5edc8f206357db764c14e7c0 Mon Sep 17 00:00:00 2001 From: Farez Abel Date: Sun, 18 Oct 2026 02:44:03 +0000 Subject: [PATCH 2/4] feat(split): implement circuit breaker state machine (#923) - Add Closed / Open / HalfOpen states, kept in sync with the issue #297 flag - Add begin_circuit_recovery / complete_circuit_recovery with a probation period - Add report_circuit_failure: auto-trip on a failure threshold within a ledger window, immediate re-open on failure during recovery - Add per-operation exemption list honoured by require_fn_not_paused guards - Add configurable thresholds, transition history log and cb_state / cb_fail / cb_exmpt events - Extend CircuitBreakerStatus with state, ledgers and failure count --- contracts/split/src/events.rs | 52 ++- contracts/split/src/lib.rs | 365 ++++++++++++++++-- contracts/split/src/test.rs | 187 +++++++++ contracts/split/src/types.rs | 46 +++ ...ISSUE_923_CIRCUIT_BREAKER_STATE_MACHINE.md | 75 ++++ 5 files changed, 695 insertions(+), 30 deletions(-) create mode 100644 docs/ISSUE_923_CIRCUIT_BREAKER_STATE_MACHINE.md diff --git a/contracts/split/src/events.rs b/contracts/split/src/events.rs index 672e5b8..b9aae9d 100644 --- a/contracts/split/src/events.rs +++ b/contracts/split/src/events.rs @@ -26,8 +26,11 @@ //! dynamically. use crate::storage_keys::ev_seq_key; -use crate::types::{DisputeOutcome, FeeSplit, InvoicePhase, InvoiceStatus, OverfundingPolicy, RepScore, TimelockAction}; -use soroban_sdk::{contracttype, symbol_short, Address, BytesN, Env, String, Vec}; +use crate::types::{ + CircuitBreakerState, DisputeOutcome, FeeSplit, InvoicePhase, InvoiceStatus, OverfundingPolicy, + RepScore, TimelockAction, +}; +use soroban_sdk::{contracttype, symbol_short, Address, BytesN, Env, String, Symbol, Vec}; // --------------------------------------------------------------------------- // Event sequence helper (per-invoice, temporary-storage counter) @@ -597,6 +600,51 @@ pub fn circuit_breaker_deactivated(env: &Env) { .publish((symbol_short!("split"), symbol_short!("cb_dact")), ()); } +/// Issue #923: Emitted on every circuit breaker state transition. +/// Topics: (split, cb_state) +/// Data: (from, to, actor, reason, ledger) +pub fn circuit_breaker_state_changed( + env: &Env, + from: CircuitBreakerState, + to: CircuitBreakerState, + actor: &Address, + reason: &String, +) { + env.events().publish( + (symbol_short!("split"), symbol_short!("cb_state")), + (from, to, actor.clone(), reason.clone(), env.ledger().sequence()), + ); +} + +/// Issue #923: Emitted when a failure is reported against the circuit breaker. +/// Topics: (split, cb_fail, reporter) +/// Data: (failure_count, failure_threshold) +pub fn circuit_breaker_failure_reported( + env: &Env, + reporter: &Address, + failure_count: u32, + failure_threshold: u32, +) { + env.events().publish( + ( + symbol_short!("split"), + symbol_short!("cb_fail"), + reporter.clone(), + ), + (failure_count, failure_threshold), + ); +} + +/// Issue #923: Emitted when an operation is added to or removed from the exemption list. +/// Topics: (split, cb_exmpt) +/// Data: (function, exempt) +pub fn circuit_breaker_exemption_set(env: &Env, function: &Symbol, exempt: bool) { + env.events().publish( + (symbol_short!("split"), symbol_short!("cb_exmpt")), + (function.clone(), exempt), + ); +} + /// Issue #296: Emitted when a fee waiver is granted to a creator. /// Topics: (split, fw_grant, creator) /// Data: () diff --git a/contracts/split/src/lib.rs b/contracts/split/src/lib.rs index 7741933..40a68b4 100644 --- a/contracts/split/src/lib.rs +++ b/contracts/split/src/lib.rs @@ -39,6 +39,15 @@ const STROOPS_PER_10K_INSTRUCTIONS: u64 = 1; const MAX_FEE_WAIVER_ENTRIES: usize = 100; /// Issue #922: Maximum entries retained in the fee waiver audit log (oldest dropped first). const MAX_FEE_WAIVER_AUDIT_ENTRIES: u32 = 200; + +/// Issue #923: Default failures within the window that trip the circuit breaker. +const DEFAULT_CB_FAILURE_THRESHOLD: u32 = 3; +/// Issue #923: Default failure counting window (~1 hour at ~5 s/ledger). +const DEFAULT_CB_FAILURE_WINDOW_LEDGERS: u32 = 720; +/// Issue #923: Default HalfOpen probation before recovery can complete (~10 min). +const DEFAULT_CB_RECOVERY_LEDGERS: u32 = 120; +/// Issue #923: Maximum circuit breaker transitions retained in history. +const MAX_CB_HISTORY_ENTRIES: u32 = 50; const DEFAULT_COMMITMENT_EXPIRY_LEDGERS: u32 = 100; /// Fixed-point scale for oracle-priced invoices: the oracle's `price()` return @@ -89,7 +98,8 @@ use soroban_sdk::{ const MAX_PARENT_DEPTH: u32 = 10; use types::{ - AdminAction, AdminRole, AdminSet, AuditEntry, Bid, CircuitBreakerStatus, + AdminAction, AdminRole, AdminSet, AuditEntry, Bid, CircuitBreakerConfig, CircuitBreakerState, + CircuitBreakerStatus, CircuitBreakerTransition, CloneOverrides, CompactInvoice, CompactMigrateResult, CompletionProof, ComputeEstimate, ConfidentialPayment, ContributionResult, CreateInvoiceParams, CreatorStats, DelayedPayout, DisputeOutcome, DisputeRecord, DisputeStatus, FeeBracket, FeeSplit, FeeTier, FeeWaiver, @@ -204,6 +214,41 @@ fn circuit_breaker_reason_key() -> Symbol { symbol_short!("cb_rsn") } +/// Issue #923: Circuit breaker state machine position. +fn circuit_breaker_state_key() -> Symbol { + symbol_short!("cb_state") +} + +/// Issue #923: Circuit breaker activation / recovery configuration. +fn circuit_breaker_config_key() -> Symbol { + symbol_short!("cb_cfg") +} + +/// Issue #923: Operations (function symbols) allowed while the breaker is open. +fn circuit_breaker_exempt_key() -> Symbol { + symbol_short!("cb_exempt") +} + +/// Issue #923: (failure_count, window_start_ledger) for auto-trip detection. +fn circuit_breaker_failures_key() -> Symbol { + symbol_short!("cb_fails") +} + +/// Issue #923: Ledger at which the breaker last opened. +fn circuit_breaker_opened_key() -> Symbol { + symbol_short!("cb_open") +} + +/// Issue #923: Ledger at which HalfOpen recovery began. +fn circuit_breaker_recovery_key() -> Symbol { + symbol_short!("cb_recov") +} + +/// Issue #923: Bounded history of state transitions. +fn circuit_breaker_history_key() -> Symbol { + symbol_short!("cb_hist") +} + /// Issue #295: Confidential payment storage key per (invoice_id, payer). fn confidential_pay_key(invoice_id: u64, payer: &Address) -> (Symbol, u64, Address) { (symbol_short!("conf_pay"), invoice_id, payer.clone()) @@ -2240,12 +2285,19 @@ fn assert_not_paused(env: &Env) -> Result<(), ContractError> { return Err(ContractError::ContractPaused); } // Issue #297: also check circuit breaker. - let cb_active: bool = env - .storage() - .persistent() - .get(&circuit_breaker_key()) - .unwrap_or(false); - if cb_active { + if circuit_breaker_blocks(env, None) { + return Err(ContractError::ContractPaused); + } + Ok(()) +} + +/// Issue #923: Same as [`assert_not_paused`], but lets `function` through an +/// open circuit breaker when it is on the exemption list. +fn assert_not_paused_for(env: &Env, function: &Symbol) -> Result<(), ContractError> { + if is_paused(env) { + return Err(ContractError::ContractPaused); + } + if circuit_breaker_blocks(env, Some(function)) { return Err(ContractError::ContractPaused); } Ok(()) @@ -2307,7 +2359,8 @@ fn require_admin_role_unguarded(env: &Env, admin: &Address, min_role: AdminRole) } fn require_fn_not_paused(env: &Env, name: &Symbol) { - require_not_paused(env); + migrations::require_schema_current(env); + assert_not_paused_for(env, name).expect("contract is paused"); let paused_fns: Vec = env .storage() .persistent() @@ -2318,6 +2371,114 @@ fn require_fn_not_paused(env: &Env, name: &Symbol) { } } +// --------------------------------------------------------------------------- +// Issue #923: Circuit breaker helpers +// --------------------------------------------------------------------------- + +/// True when the breaker is open and `function` (if given) is not exempt. +/// With `function == None` no exemption applies. +fn circuit_breaker_blocks(env: &Env, function: Option<&Symbol>) -> bool { + let active: bool = env + .storage() + .persistent() + .get(&circuit_breaker_key()) + .unwrap_or(false); + if !active { + return false; + } + match function { + Some(f) => !load_circuit_breaker_exemptions(env).iter().any(|e| e == *f), + None => true, + } +} + +fn load_circuit_breaker_exemptions(env: &Env) -> Vec { + env.storage() + .persistent() + .get(&circuit_breaker_exempt_key()) + .unwrap_or_else(|| Vec::new(env)) +} + +/// Current state. Contracts that only ever set the issue #297 flag have no +/// stored state, so it is derived from the flag. +fn load_circuit_breaker_state(env: &Env) -> CircuitBreakerState { + env.storage() + .persistent() + .get(&circuit_breaker_state_key()) + .unwrap_or_else(|| { + let active: bool = env + .storage() + .persistent() + .get(&circuit_breaker_key()) + .unwrap_or(false); + if active { + CircuitBreakerState::Open + } else { + CircuitBreakerState::Closed + } + }) +} + +fn load_circuit_breaker_config(env: &Env) -> CircuitBreakerConfig { + env.storage() + .persistent() + .get(&circuit_breaker_config_key()) + .unwrap_or(CircuitBreakerConfig { + failure_threshold: DEFAULT_CB_FAILURE_THRESHOLD, + failure_window_ledgers: DEFAULT_CB_FAILURE_WINDOW_LEDGERS, + recovery_ledgers: DEFAULT_CB_RECOVERY_LEDGERS, + }) +} + +/// Move the breaker to `to`, keeping the issue #297 flag in sync, and log the transition. +fn transition_circuit_breaker( + env: &Env, + to: CircuitBreakerState, + actor: &Address, + reason: &String, +) { + let from = load_circuit_breaker_state(env); + let storage = env.storage().persistent(); + let ledger = env.ledger().sequence(); + + storage.set(&circuit_breaker_state_key(), &to); + storage.set(&circuit_breaker_key(), &(to == CircuitBreakerState::Open)); + storage.remove(&circuit_breaker_failures_key()); + match to { + CircuitBreakerState::Open => { + storage.set(&circuit_breaker_reason_key(), reason); + storage.set(&circuit_breaker_opened_key(), &ledger); + storage.remove(&circuit_breaker_recovery_key()); + } + CircuitBreakerState::HalfOpen => { + storage.set(&circuit_breaker_recovery_key(), &ledger); + } + CircuitBreakerState::Closed => { + storage.remove(&circuit_breaker_reason_key()); + storage.remove(&circuit_breaker_opened_key()); + storage.remove(&circuit_breaker_recovery_key()); + } + } + + let mut history: Vec = storage + .get(&circuit_breaker_history_key()) + .unwrap_or_else(|| Vec::new(env)); + history.push_back(CircuitBreakerTransition { + from, + to, + actor: actor.clone(), + reason: reason.clone(), + ledger, + timestamp: env.ledger().timestamp(), + }); + while history.len() > MAX_CB_HISTORY_ENTRIES { + history.pop_front(); + } + storage.set(&circuit_breaker_history_key(), &history); + + events::circuit_breaker_state_changed(env, from, to, actor, reason); +} + // --------------------------------------------------------------------------- // Issue #922: Fee waiver helpers // --------------------------------------------------------------------------- @@ -13852,46 +14013,194 @@ impl SplitContract { } // ----------------------------------------------------------------------- - // Issue #297: Contract-wide circuit breaker + // Issue #297 / #923: Contract-wide circuit breaker // ----------------------------------------------------------------------- - /// Activate the circuit breaker. Admin-only. Halts all mutating entry points. + /// Open the circuit breaker. Admin-only. Halts all guarded mutating entry + /// points except those on the exemption list. Valid from any state; + /// calling it while already open updates the reason. pub fn activate_circuit_breaker(env: Env, admin: Address, reason: String) { require_admin_role(&env, &admin, AdminRole::SuperAdmin); + transition_circuit_breaker(&env, CircuitBreakerState::Open, &admin, &reason); + events::circuit_breaker_activated(&env, &reason); + } + + /// Close the circuit breaker immediately, skipping recovery probation. + /// Admin-only emergency override; valid from any state. + pub fn deactivate_circuit_breaker(env: Env, admin: Address) { + require_admin_role(&env, &admin, AdminRole::SuperAdmin); + transition_circuit_breaker( + &env, + CircuitBreakerState::Closed, + &admin, + &String::from_str(&env, "manual deactivation"), + ); + events::circuit_breaker_deactivated(&env); + } + + /// Issue #923: Start recovery (Open -> HalfOpen). Admin-only. + /// Operations resume, but any failure reported during probation re-opens the breaker. + pub fn begin_circuit_recovery(env: Env, admin: Address) { + require_admin_role(&env, &admin, AdminRole::SuperAdmin); + assert!( + load_circuit_breaker_state(&env) == CircuitBreakerState::Open, + "invalid circuit breaker transition" + ); + transition_circuit_breaker( + &env, + CircuitBreakerState::HalfOpen, + &admin, + &String::from_str(&env, "recovery started"), + ); + } + + /// Issue #923: Finish recovery (HalfOpen -> Closed) once the configured + /// probation period has elapsed. Admin-only. + pub fn complete_circuit_recovery(env: Env, admin: Address) { + require_admin_role(&env, &admin, AdminRole::SuperAdmin); + assert!( + load_circuit_breaker_state(&env) == CircuitBreakerState::HalfOpen, + "invalid circuit breaker transition" + ); + let started: u32 = env + .storage() + .persistent() + .get(&circuit_breaker_recovery_key()) + .unwrap_or(0); + let config = load_circuit_breaker_config(&env); + assert!( + env.ledger().sequence() >= started.saturating_add(config.recovery_ledgers), + "recovery period not elapsed" + ); + transition_circuit_breaker( + &env, + CircuitBreakerState::Closed, + &admin, + &String::from_str(&env, "recovery complete"), + ); + events::circuit_breaker_deactivated(&env); + } + + /// Issue #923: Report an operational failure or anomaly. Operator+ only. + /// + /// Activation conditions: + /// - `Closed`: the breaker opens once `failure_threshold` failures are + /// reported within `failure_window_ledgers` (a threshold of 0 disables this). + /// - `HalfOpen`: a single failure re-opens the breaker. + /// - `Open`: the report is logged only. + /// + /// Returns the breaker state after the report. + pub fn report_circuit_failure( + env: Env, + reporter: Address, + reason: String, + ) -> CircuitBreakerState { + require_admin_role(&env, &reporter, AdminRole::Operator); + let state = load_circuit_breaker_state(&env); + let config = load_circuit_breaker_config(&env); + let ledger = env.ledger().sequence(); + + let (mut count, mut window_start): (u32, u32) = env + .storage() + .persistent() + .get(&circuit_breaker_failures_key()) + .unwrap_or((0, ledger)); + if ledger.saturating_sub(window_start) >= config.failure_window_ledgers { + count = 0; + window_start = ledger; + } + count = count.saturating_add(1); + events::circuit_breaker_failure_reported(&env, &reporter, count, config.failure_threshold); + + let should_trip = match state { + CircuitBreakerState::Open => false, + CircuitBreakerState::HalfOpen => true, + CircuitBreakerState::Closed => { + config.failure_threshold > 0 && count >= config.failure_threshold + } + }; + if should_trip { + transition_circuit_breaker(&env, CircuitBreakerState::Open, &reporter, &reason); + events::circuit_breaker_activated(&env, &reason); + return CircuitBreakerState::Open; + } env.storage() .persistent() - .set(&circuit_breaker_key(), &true); + .set(&circuit_breaker_failures_key(), &(count, window_start)); + state + } + + /// Issue #923: Configure activation and recovery parameters. Admin-only. + pub fn set_circuit_breaker_config(env: Env, admin: Address, config: CircuitBreakerConfig) { + require_admin_role(&env, &admin, AdminRole::SuperAdmin); + assert!(config.failure_window_ledgers > 0, "failure window must be positive"); env.storage() .persistent() - .set(&circuit_breaker_reason_key(), &reason.clone()); - events::circuit_breaker_activated(&env, &reason); + .set(&circuit_breaker_config_key(), &config); } - /// Deactivate the circuit breaker. Admin-only. - pub fn deactivate_circuit_breaker(env: Env, admin: Address) { + /// Issue #923: Returns the active circuit breaker configuration. + pub fn get_circuit_breaker_config(env: Env) -> CircuitBreakerConfig { + load_circuit_breaker_config(&env) + } + + /// Issue #923: Allow (or stop allowing) a critical operation while the breaker is open. + /// `function` is the guard name the entry point checks, e.g. `refund`. Admin-only. + pub fn set_circuit_breaker_exemption(env: Env, admin: Address, function: Symbol, exempt: bool) { require_admin_role(&env, &admin, AdminRole::SuperAdmin); + let current = load_circuit_breaker_exemptions(&env); + let mut updated: Vec = Vec::new(&env); + for f in current.iter() { + if f != function { + updated.push_back(f); + } + } + if exempt { + updated.push_back(function.clone()); + } env.storage() .persistent() - .set(&circuit_breaker_key(), &false); + .set(&circuit_breaker_exempt_key(), &updated); + events::circuit_breaker_exemption_set(&env, &function, exempt); + } + + /// Issue #923: Returns the operations exempt from the circuit breaker. + pub fn get_circuit_breaker_exemptions(env: Env) -> Vec { + load_circuit_breaker_exemptions(&env) + } + + /// Issue #923: Returns recorded state transitions (oldest first, capped at 50). + pub fn get_circuit_breaker_history(env: Env) -> Vec { env.storage() .persistent() - .remove(&circuit_breaker_reason_key()); - events::circuit_breaker_deactivated(&env); + .get(&circuit_breaker_history_key()) + .unwrap_or_else(|| Vec::new(&env)) } /// Returns the current circuit breaker status. /// Read-only — not affected by the circuit breaker. pub fn get_circuit_breaker_status(env: Env) -> CircuitBreakerStatus { - let active: bool = env - .storage() - .persistent() - .get(&circuit_breaker_key()) - .unwrap_or(false); - let reason: Option = env - .storage() - .persistent() - .get(&circuit_breaker_reason_key()); - CircuitBreakerStatus { active, reason } + let storage = env.storage().persistent(); + let active: bool = storage.get(&circuit_breaker_key()).unwrap_or(false); + let reason: Option = storage.get(&circuit_breaker_reason_key()); + let config = load_circuit_breaker_config(&env); + let failure_count = match storage.get::<_, (u32, u32)>(&circuit_breaker_failures_key()) { + Some((count, window_start)) + if env.ledger().sequence().saturating_sub(window_start) + < config.failure_window_ledgers => + { + count + } + _ => 0, + }; + CircuitBreakerStatus { + active, + reason, + state: load_circuit_breaker_state(&env), + opened_at_ledger: storage.get(&circuit_breaker_opened_key()).unwrap_or(0), + recovery_started_ledger: storage.get(&circuit_breaker_recovery_key()).unwrap_or(0), + failure_count, + } } // ----------------------------------------------------------------------- diff --git a/contracts/split/src/test.rs b/contracts/split/src/test.rs index 75a80c1..43e0e50 100644 --- a/contracts/split/src/test.rs +++ b/contracts/split/src/test.rs @@ -8834,3 +8834,190 @@ fn test_922_purge_frees_capacity_for_new_waivers() { assert!(c.has_fee_waiver(&creator)); assert_eq!(c.list_fee_waivers().len(), 1); } + +// --------------------------------------------------------------------------- +// Issue #923: Circuit breaker state machine +// --------------------------------------------------------------------------- + +fn setup_circuit_breaker() -> (Env, Address, Address, Address) { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let admin = Address::generate(&env); + let treasury = Address::generate(&env); + env.ledger().set_timestamp(1_000); + env.ledger().set_sequence_number(100); + c.initialize(&admin, &0_i128, &treasury, &token_id, &0_u32, &None, &0_u32, &0_u32, &0_u64); + (env, contract_id, token_id, admin) +} + +#[test] +fn test_923_state_defaults_closed() { + let (env, contract_id, _token_id, _admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + let status = c.get_circuit_breaker_status(); + assert_eq!(status.state, types::CircuitBreakerState::Closed); + assert_eq!(status.failure_count, 0); +} + +#[test] +fn test_923_activate_opens_and_records_ledger() { + let (env, contract_id, _token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + c.activate_circuit_breaker(&admin, &String::from_str(&env, "exploit")); + let status = c.get_circuit_breaker_status(); + assert_eq!(status.state, types::CircuitBreakerState::Open); + assert!(status.active); + assert_eq!(status.opened_at_ledger, 100); +} + +#[test] +fn test_923_recovery_flow_half_open_then_closed() { + let (env, contract_id, _token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + c.activate_circuit_breaker(&admin, &String::from_str(&env, "exploit")); + + c.begin_circuit_recovery(&admin); + let status = c.get_circuit_breaker_status(); + assert_eq!(status.state, types::CircuitBreakerState::HalfOpen); + assert!(!status.active, "operations resume during recovery"); + assert_eq!(status.recovery_started_ledger, 100); + + env.ledger().set_sequence_number(100 + 120); + c.complete_circuit_recovery(&admin); + let status = c.get_circuit_breaker_status(); + assert_eq!(status.state, types::CircuitBreakerState::Closed); + assert!(status.reason.is_none()); +} + +#[test] +#[should_panic(expected = "recovery period not elapsed")] +fn test_923_complete_recovery_before_probation_fails() { + let (env, contract_id, _token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + c.activate_circuit_breaker(&admin, &String::from_str(&env, "exploit")); + c.begin_circuit_recovery(&admin); + env.ledger().set_sequence_number(150); + c.complete_circuit_recovery(&admin); +} + +#[test] +#[should_panic(expected = "invalid circuit breaker transition")] +fn test_923_begin_recovery_when_closed_fails() { + let (env, contract_id, _token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + c.begin_circuit_recovery(&admin); +} + +#[test] +fn test_923_failure_threshold_trips_breaker() { + let (env, contract_id, _token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + let reason = String::from_str(&env, "oracle anomaly"); + + assert_eq!(c.report_circuit_failure(&admin, &reason), types::CircuitBreakerState::Closed); + assert_eq!(c.report_circuit_failure(&admin, &reason), types::CircuitBreakerState::Closed); + assert_eq!(c.get_circuit_breaker_status().failure_count, 2); + assert_eq!(c.report_circuit_failure(&admin, &reason), types::CircuitBreakerState::Open); + + let status = c.get_circuit_breaker_status(); + assert!(status.active); + assert_eq!(status.reason, Some(reason)); +} + +#[test] +fn test_923_failures_outside_window_do_not_accumulate() { + let (env, contract_id, _token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + let reason = String::from_str(&env, "anomaly"); + + c.report_circuit_failure(&admin, &reason); + c.report_circuit_failure(&admin, &reason); + env.ledger().set_sequence_number(100 + 720); + assert_eq!(c.report_circuit_failure(&admin, &reason), types::CircuitBreakerState::Closed); + assert_eq!(c.get_circuit_breaker_status().failure_count, 1); +} + +#[test] +fn test_923_failure_during_recovery_reopens() { + let (env, contract_id, _token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + c.activate_circuit_breaker(&admin, &String::from_str(&env, "exploit")); + c.begin_circuit_recovery(&admin); + + let state = c.report_circuit_failure(&admin, &String::from_str(&env, "still broken")); + assert_eq!(state, types::CircuitBreakerState::Open); +} + +#[test] +fn test_923_zero_threshold_disables_auto_trip() { + let (env, contract_id, _token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + c.set_circuit_breaker_config( + &admin, + &types::CircuitBreakerConfig { + failure_threshold: 0, + failure_window_ledgers: 720, + recovery_ledgers: 10, + }, + ); + for _ in 0..5 { + c.report_circuit_failure(&admin, &String::from_str(&env, "noise")); + } + assert_eq!(c.get_circuit_breaker_status().state, types::CircuitBreakerState::Closed); +} + +#[test] +fn test_923_exempt_refund_allowed_while_open() { + let (env, contract_id, token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); + let creator = Address::generate(&env); + let payer = Address::generate(&env); + let recipient = Address::generate(&env); + StellarAssetClient::new(&env, &token_id).mint(&payer, &500); + + let id = make_invoice(&env, &c, &creator, &recipient, 200, &token_id, 2_000); + c.pay(&payer, &id, &50_i128, &0_u64, &false, &false, &None); + + c.activate_circuit_breaker(&admin, &String::from_str(&env, "exploit")); + c.set_circuit_breaker_exemption(&admin, &symbol_short!("refund"), &true); + assert_eq!(c.get_circuit_breaker_exemptions().len(), 1); + + env.ledger().set_timestamp(3_000); + c.refund(&id); + assert_eq!(tk.balance(&payer), 500, "exempt refund must go through while open"); +} + +#[test] +#[should_panic(expected = "ContractPaused")] +fn test_923_non_exempt_pay_blocked_while_open() { + let (env, contract_id, token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + let payer = Address::generate(&env); + let recipient = Address::generate(&env); + StellarAssetClient::new(&env, &token_id).mint(&payer, &500); + + let id = make_invoice(&env, &c, &creator, &recipient, 200, &token_id, 9_999); + c.activate_circuit_breaker(&admin, &String::from_str(&env, "exploit")); + c.set_circuit_breaker_exemption(&admin, &symbol_short!("refund"), &true); + c.pay(&payer, &id, &50_i128, &0_u64, &false, &false, &None); +} + +#[test] +fn test_923_history_logs_transitions() { + let (env, contract_id, _token_id, admin) = setup_circuit_breaker(); + let c = client(&env, &contract_id); + c.activate_circuit_breaker(&admin, &String::from_str(&env, "exploit")); + c.begin_circuit_recovery(&admin); + c.deactivate_circuit_breaker(&admin); + + let history = c.get_circuit_breaker_history(); + assert_eq!(history.len(), 3); + let first = history.get(0).unwrap(); + assert_eq!(first.from, types::CircuitBreakerState::Closed); + assert_eq!(first.to, types::CircuitBreakerState::Open); + assert_eq!(first.actor, admin); + assert_eq!(history.get(2).unwrap().to, types::CircuitBreakerState::Closed); + assert!(has_circuit_breaker_event(&env, "cb_state")); +} diff --git a/contracts/split/src/types.rs b/contracts/split/src/types.rs index 83d0911..40f2cf2 100644 --- a/contracts/split/src/types.rs +++ b/contracts/split/src/types.rs @@ -1659,6 +1659,52 @@ pub struct ComputeEstimate { pub struct CircuitBreakerStatus { pub active: bool, pub reason: Option, + /// Issue #923: current state machine position. + pub state: CircuitBreakerState, + /// Issue #923: ledger at which the breaker last opened (0 if never / closed). + pub opened_at_ledger: u32, + /// Issue #923: ledger at which recovery (HalfOpen) began (0 if not recovering). + pub recovery_started_ledger: u32, + /// Issue #923: failures reported in the current failure window. + pub failure_count: u32, +} + +/// Issue #923: Circuit breaker state machine. +/// +/// - `Closed`: normal operation. +/// - `Open`: tripped; all guarded mutating operations fail except exempt ones. +/// - `HalfOpen`: recovery probation; operations resume, but any reported +/// failure re-opens the breaker immediately. +#[contracttype] +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum CircuitBreakerState { + Closed, + Open, + HalfOpen, +} + +/// Issue #923: Activation and recovery parameters for the circuit breaker. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct CircuitBreakerConfig { + /// Failures within `failure_window_ledgers` that trip the breaker. `0` disables auto-trip. + pub failure_threshold: u32, + /// Length of the failure counting window, in ledgers. + pub failure_window_ledgers: u32, + /// Minimum ledgers the breaker must spend in `HalfOpen` before recovery can complete. + pub recovery_ledgers: u32, +} + +/// Issue #923: A recorded circuit breaker state transition. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct CircuitBreakerTransition { + pub from: CircuitBreakerState, + pub to: CircuitBreakerState, + pub actor: Address, + pub reason: String, + pub ledger: u32, + pub timestamp: u64, } /// Issue #922: Fee waiver record for a whitelisted creator. diff --git a/docs/ISSUE_923_CIRCUIT_BREAKER_STATE_MACHINE.md b/docs/ISSUE_923_CIRCUIT_BREAKER_STATE_MACHINE.md new file mode 100644 index 0000000..1a36e1d --- /dev/null +++ b/docs/ISSUE_923_CIRCUIT_BREAKER_STATE_MACHINE.md @@ -0,0 +1,75 @@ +# Issue #923: Circuit Breaker Pattern + +Extends the emergency circuit breaker from issue #297 into a full state machine with an +exemption list, recovery procedure, automatic activation conditions and a transition log. + +## State machine + +``` + activate / failure threshold reached + Closed ───────────────────────────────────────▶ Open + ▲ │ ▲ + │ complete_circuit_recovery │ │ failure reported + │ (after recovery_ledgers) │ │ during probation + │ ▼ │ + └──────────────────────────────────────── HalfOpen + begin_circuit_recovery ◀─┘ + + deactivate_circuit_breaker: any state ──▶ Closed (emergency override) +``` + +| State | Guarded mutating operations | +|-------|-----------------------------| +| `Closed` | Allowed | +| `Open` | Blocked with `ContractPaused`, except exempt operations | +| `HalfOpen` | Allowed; any reported failure re-opens the breaker | + +The issue #297 `cb_flag` stays in sync (`true` only in `Open`), so every existing +`require_not_paused` / `check_not_paused` guard keeps working unchanged. + +## Admin functions + +| Function | Role | Transition | +|----------|------|------------| +| `activate_circuit_breaker(admin, reason)` | SuperAdmin | any → `Open` | +| `deactivate_circuit_breaker(admin)` | SuperAdmin | any → `Closed` | +| `begin_circuit_recovery(admin)` | SuperAdmin | `Open` → `HalfOpen` | +| `complete_circuit_recovery(admin)` | SuperAdmin | `HalfOpen` → `Closed`, once `recovery_ledgers` have passed | +| `set_circuit_breaker_config(admin, config)` | SuperAdmin | n/a | +| `set_circuit_breaker_exemption(admin, function, exempt)` | SuperAdmin | n/a | +| `report_circuit_failure(reporter, reason)` | Operator+ | see below | + +Invalid transitions panic with `"invalid circuit breaker transition"`. Completing recovery +too early panics with `"recovery period not elapsed"`. + +## Activation conditions + +`report_circuit_failure` counts failures in a rolling window of `failure_window_ledgers`: + +- `Closed`: the breaker opens when the count reaches `failure_threshold` (`0` turns auto-trip off). +- `HalfOpen`: a single report re-opens the breaker. +- `Open`: the report is counted and emitted only. + +Defaults: `failure_threshold = 3`, `failure_window_ledgers = 720` (~1 h), +`recovery_ledgers = 120` (~10 min). + +## Exemptions + +Exemptions are keyed by the guard name an entry point passes to `require_fn_not_paused`: +`pay`, `pay_tok`, `brg_pay`, `release` or `refund`. For example, exempting `refund` keeps +`refund` and `claim_refund` working while the breaker is open, so payers can still recover +funds. Entry points that use the global guard (`require_not_paused`), and `create_invoice`, +are never exempt. + +## Monitoring + +- `get_circuit_breaker_status()` returns `{ active, reason, state, opened_at_ledger, recovery_started_ledger, failure_count }`. +- `get_circuit_breaker_history()` returns the last 50 `CircuitBreakerTransition { from, to, actor, reason, ledger, timestamp }` entries. +- `get_circuit_breaker_config()`, `get_circuit_breaker_exemptions()` + +## Events + +- `cb_state`: every transition `(from, to, actor, reason, ledger)` +- `cb_fail`: failure reported `(failure_count, failure_threshold)` +- `cb_exmpt`: exemption changed `(function, exempt)` +- `cb_act` / `cb_dact` (existing): breaker opened / closed From ebca16a971cc07351a41351c401610dfc9a6b33e Mon Sep 17 00:00:00 2001 From: Farez Abel Date: Sun, 18 Oct 2026 02:46:51 +0000 Subject: [PATCH 3/4] feat(split): add runtime compute budget estimation (#924) - Add per-operation linear cost model with built-in defaults and admin overrides - Add estimate_operation_cost with calibration learned from reported usage - Add record_compute_usage tracking (min / max / last / totals / error vs estimate) - Cache estimates in temporary storage, invalidated by a model version counter - Add per-operation profiling, get_compute_cost_report and reset_operation_profile --- contracts/split/src/lib.rs | 350 +++++++++++++++++++- contracts/split/src/test.rs | 113 +++++++ contracts/split/src/types.rs | 68 ++++ docs/ISSUE_924_COMPUTE_BUDGET_ESTIMATION.md | 67 ++++ 4 files changed, 597 insertions(+), 1 deletion(-) create mode 100644 docs/ISSUE_924_COMPUTE_BUDGET_ESTIMATION.md diff --git a/contracts/split/src/lib.rs b/contracts/split/src/lib.rs index 40a68b4..72ca19b 100644 --- a/contracts/split/src/lib.rs +++ b/contracts/split/src/lib.rs @@ -35,6 +35,11 @@ const INSTRUCTIONS_BASE: u64 = 1_000_000; /// Issue #298: Stroops per 10_000 instructions (rough estimate based on Soroban fee schedule). const STROOPS_PER_10K_INSTRUCTIONS: u64 = 1; +/// Issue #924: Estimate counted as accurate when within this error of actual (10%). +const COST_ESTIMATE_TOLERANCE_BPS: u64 = 1_000; +/// Issue #924: Maximum distinct operations tracked by the profiler. +const MAX_PROFILED_OPERATIONS: u32 = 32; + /// Issue #296: Maximum entries in the per-creator fee waiver list. const MAX_FEE_WAIVER_ENTRIES: usize = 100; /// Issue #922: Maximum entries retained in the fee waiver audit log (oldest dropped first). @@ -99,7 +104,8 @@ const MAX_PARENT_DEPTH: u32 = 10; use types::{ AdminAction, AdminRole, AdminSet, AuditEntry, Bid, CircuitBreakerConfig, CircuitBreakerState, - CircuitBreakerStatus, CircuitBreakerTransition, + CircuitBreakerStatus, CircuitBreakerTransition, CostEstimate, OperationCostModel, + OperationCostReport, OperationProfile, CloneOverrides, CompactInvoice, CompactMigrateResult, CompletionProof, ComputeEstimate, ConfidentialPayment, ContributionResult, CreateInvoiceParams, CreatorStats, DelayedPayout, DisputeOutcome, DisputeRecord, DisputeStatus, FeeBracket, FeeSplit, FeeTier, FeeWaiver, @@ -249,6 +255,36 @@ fn circuit_breaker_history_key() -> Symbol { symbol_short!("cb_hist") } +/// Issue #924: Admin-configured cost model override for an operation. +fn cost_model_key(operation: &Symbol) -> (Symbol, Symbol) { + (symbol_short!("cost_mdl"), operation.clone()) +} + +/// Issue #924: Version counter bumped whenever a model or calibration changes. +fn cost_model_version_key() -> Symbol { + symbol_short!("cost_ver") +} + +/// Issue #924: Per-operation profiling data. +fn cost_profile_key(operation: &Symbol) -> (Symbol, Symbol) { + (symbol_short!("cost_prf"), operation.clone()) +} + +/// Issue #924: Operations that have profiling data. +fn cost_profiled_ops_key() -> Symbol { + symbol_short!("cost_ops") +} + +/// Issue #924: Cached estimate per (operation, recipients, payers, items) — temporary storage. +fn cost_cache_key( + operation: &Symbol, + recipients: u32, + payers: u32, + items: u32, +) -> (Symbol, Symbol, u32, u32, u32) { + (symbol_short!("cost_c"), operation.clone(), recipients, payers, items) +} + /// Issue #295: Confidential payment storage key per (invoice_id, payer). fn confidential_pay_key(invoice_id: u64, payer: &Address) -> (Symbol, u64, Address) { (symbol_short!("conf_pay"), invoice_id, payer.clone()) @@ -2371,6 +2407,141 @@ fn require_fn_not_paused(env: &Env, name: &Symbol) { } } +// --------------------------------------------------------------------------- +// Issue #924: Compute cost estimation helpers +// --------------------------------------------------------------------------- + +/// Built-in cost models, derived from the `estimate_compute` constants. +fn default_cost_model(operation: &Symbol) -> Option { + let shard_overhead = INSTRUCTIONS_PER_SHARD * SHARD_COUNT; + let model = if *operation == symbol_short!("create") { + OperationCostModel { + base_insns: INSTRUCTIONS_BASE, + per_recipient_insns: 200_000, + per_payer_insns: 0, + per_item_insns: 0, + base_mem_bytes: 128 * 1024, + per_unit_mem_bytes: 64 * 1024, + } + } else if *operation == symbol_short!("pay") { + OperationCostModel { + base_insns: INSTRUCTIONS_BASE + shard_overhead, + per_recipient_insns: INSTRUCTIONS_PER_RECIPIENT / 2, + per_payer_insns: 0, + per_item_insns: 0, + base_mem_bytes: 256 * 1024, + per_unit_mem_bytes: 16 * 1024, + } + } else if *operation == symbol_short!("release") { + OperationCostModel { + base_insns: INSTRUCTIONS_BASE + shard_overhead, + per_recipient_insns: INSTRUCTIONS_PER_RECIPIENT, + per_payer_insns: 0, + per_item_insns: 0, + base_mem_bytes: 256 * 1024, + per_unit_mem_bytes: 32 * 1024, + } + } else if *operation == symbol_short!("refund") { + OperationCostModel { + base_insns: INSTRUCTIONS_BASE + shard_overhead, + per_recipient_insns: 0, + per_payer_insns: 350_000, + per_item_insns: 0, + base_mem_bytes: 256 * 1024, + per_unit_mem_bytes: 32 * 1024, + } + } else if *operation == symbol_short!("batch_pay") { + // One token transfer + setup, then internal accounting per batched invoice. + OperationCostModel { + base_insns: INSTRUCTIONS_BASE + shard_overhead, + per_recipient_insns: 0, + per_payer_insns: 0, + per_item_insns: 400_000, + base_mem_bytes: 256 * 1024, + per_unit_mem_bytes: 32 * 1024, + } + } else { + return None; + }; + Some(model) +} + +fn load_cost_model(env: &Env, operation: &Symbol) -> OperationCostModel { + env.storage() + .persistent() + .get(&cost_model_key(operation)) + .or_else(|| default_cost_model(operation)) + .expect("unknown operation") +} + +fn cost_model_version(env: &Env) -> u32 { + env.storage() + .persistent() + .get(&cost_model_version_key()) + .unwrap_or(0) +} + +fn bump_cost_model_version(env: &Env) { + let next = cost_model_version(env).wrapping_add(1); + env.storage() + .persistent() + .set(&cost_model_version_key(), &next); +} + +fn load_operation_profile(env: &Env, operation: &Symbol) -> Option { + env.storage().persistent().get(&cost_profile_key(operation)) +} + +/// actual / raw-estimate ratio from profiling data, in bps (10_000 when unprofiled). +fn cost_calibration_bps(profile: &Option) -> u32 { + match profile { + Some(p) if p.total_raw_estimated_insns > 0 => { + let bps = (p.total_actual_insns as u128) * 10_000 + / (p.total_raw_estimated_insns as u128); + bps.min(u32::MAX as u128) as u32 + } + _ => 10_000, + } +} + +fn raw_cost_insns(model: &OperationCostModel, recipients: u32, payers: u32, items: u32) -> u64 { + model + .base_insns + .saturating_add((recipients as u64).saturating_mul(model.per_recipient_insns)) + .saturating_add((payers as u64).saturating_mul(model.per_payer_insns)) + .saturating_add((items as u64).saturating_mul(model.per_item_insns)) +} + +/// Compute an estimate from the current model and calibration (no caching). +fn compute_cost_estimate( + env: &Env, + operation: &Symbol, + recipients: u32, + payers: u32, + items: u32, +) -> CostEstimate { + let model = load_cost_model(env, operation); + let raw_cpu_insns = raw_cost_insns(&model, recipients, payers, items); + let calibration_bps = cost_calibration_bps(&load_operation_profile(env, operation)); + let cpu_insns = ((raw_cpu_insns as u128) * (calibration_bps as u128) / 10_000) + .min(u64::MAX as u128) as u64; + let units = (recipients as u64) + (payers as u64) + (items as u64); + let mem_bytes = model + .base_mem_bytes + .saturating_add(units.saturating_mul(model.per_unit_mem_bytes)); + CostEstimate { + operation: operation.clone(), + cpu_insns, + raw_cpu_insns, + mem_bytes, + fee_stroops: (cpu_insns as i128 / 10_000) * STROOPS_PER_10K_INSTRUCTIONS as i128, + calibration_bps, + within_budget: cpu_insns <= INSTRUCTION_BUDGET_LIMIT, + cached: false, + model_version: cost_model_version(env), + } +} + // --------------------------------------------------------------------------- // Issue #923: Circuit breaker helpers // --------------------------------------------------------------------------- @@ -14012,6 +14183,183 @@ impl SplitContract { } } + // ----------------------------------------------------------------------- + // Issue #924: Runtime compute cost estimation, tracking and profiling + // ----------------------------------------------------------------------- + + /// Estimate the cost of `operation` (`create`, `pay`, `release`, `refund`, + /// `batch_pay`, or any operation with an admin-defined model). + /// + /// The raw model estimate is scaled by the calibration learned from + /// `record_compute_usage`. Results are cached in temporary storage and + /// invalidated whenever a model or calibration changes. + pub fn estimate_operation_cost( + env: Env, + operation: Symbol, + recipient_count: u32, + payer_count: u32, + item_count: u32, + ) -> CostEstimate { + let key = cost_cache_key(&operation, recipient_count, payer_count, item_count); + let version = cost_model_version(&env); + if let Some(mut cached) = env.storage().temporary().get::<_, CostEstimate>(&key) { + if cached.model_version == version { + cached.cached = true; + return cached; + } + } + let estimate = + compute_cost_estimate(&env, &operation, recipient_count, payer_count, item_count); + env.storage().temporary().set(&key, &estimate); + estimate + } + + /// Report the measured instruction count of an executed operation. Operator+ only. + /// + /// Updates the operation's profile (min / max / last / totals), scores the + /// current calibrated estimate against the measurement, and refreshes the + /// calibration used by future estimates. + pub fn record_compute_usage( + env: Env, + reporter: Address, + operation: Symbol, + recipient_count: u32, + payer_count: u32, + item_count: u32, + actual_insns: u64, + ) -> OperationProfile { + require_admin_role(&env, &reporter, AdminRole::Operator); + assert!(actual_insns > 0, "actual instructions must be positive"); + + let estimate = + compute_cost_estimate(&env, &operation, recipient_count, payer_count, item_count); + let error = estimate.cpu_insns.abs_diff(actual_insns); + let error_bps = ((error as u128) * 10_000 / (actual_insns as u128)).min(u32::MAX as u128) + as u32; + + let mut profile = load_operation_profile(&env, &operation).unwrap_or(OperationProfile { + operation: operation.clone(), + samples: 0, + total_raw_estimated_insns: 0, + total_actual_insns: 0, + min_actual_insns: u64::MAX, + max_actual_insns: 0, + last_actual_insns: 0, + last_error_bps: 0, + within_tolerance: 0, + }); + if profile.samples == 0 { + let mut ops: Vec = env + .storage() + .persistent() + .get(&cost_profiled_ops_key()) + .unwrap_or_else(|| Vec::new(&env)); + if !ops.iter().any(|o| o == operation) { + assert!(ops.len() < MAX_PROFILED_OPERATIONS, "too many profiled operations"); + ops.push_back(operation.clone()); + env.storage().persistent().set(&cost_profiled_ops_key(), &ops); + } + } + profile.samples = profile.samples.saturating_add(1); + profile.total_raw_estimated_insns = profile + .total_raw_estimated_insns + .saturating_add(estimate.raw_cpu_insns); + profile.total_actual_insns = profile.total_actual_insns.saturating_add(actual_insns); + profile.min_actual_insns = profile.min_actual_insns.min(actual_insns); + profile.max_actual_insns = profile.max_actual_insns.max(actual_insns); + profile.last_actual_insns = actual_insns; + profile.last_error_bps = error_bps; + if (error_bps as u64) <= COST_ESTIMATE_TOLERANCE_BPS { + profile.within_tolerance = profile.within_tolerance.saturating_add(1); + } + env.storage() + .persistent() + .set(&cost_profile_key(&operation), &profile); + bump_cost_model_version(&env); + + env.events().publish( + (symbol_short!("split"), symbol_short!("cost_rec"), operation), + (actual_insns, estimate.cpu_insns, error_bps), + ); + profile + } + + /// Override the cost model for an operation (or define a new one). SuperAdmin only. + /// Invalidates every cached estimate. + pub fn set_operation_cost_model( + env: Env, + admin: Address, + operation: Symbol, + model: OperationCostModel, + ) { + require_admin_role(&env, &admin, AdminRole::SuperAdmin); + env.storage() + .persistent() + .set(&cost_model_key(&operation), &model); + bump_cost_model_version(&env); + } + + /// Returns the cost model in effect for an operation. Panics on unknown operations. + pub fn get_operation_cost_model(env: Env, operation: Symbol) -> OperationCostModel { + load_cost_model(&env, &operation) + } + + /// Returns the profiling data for an operation, if any usage has been recorded. + pub fn get_operation_profile(env: Env, operation: Symbol) -> Option { + load_operation_profile(&env, &operation) + } + + /// Clear the profiling data (and so the calibration) of an operation. SuperAdmin only. + pub fn reset_operation_profile(env: Env, admin: Address, operation: Symbol) { + require_admin_role(&env, &admin, AdminRole::SuperAdmin); + env.storage() + .persistent() + .remove(&cost_profile_key(&operation)); + let ops: Vec = env + .storage() + .persistent() + .get(&cost_profiled_ops_key()) + .unwrap_or_else(|| Vec::new(&env)); + let mut kept: Vec = Vec::new(&env); + for o in ops.iter() { + if o != operation { + kept.push_back(o); + } + } + env.storage().persistent().set(&cost_profiled_ops_key(), &kept); + bump_cost_model_version(&env); + } + + /// Summarise profiling data for every tracked operation. + pub fn get_compute_cost_report(env: Env) -> Vec { + let ops: Vec = env + .storage() + .persistent() + .get(&cost_profiled_ops_key()) + .unwrap_or_else(|| Vec::new(&env)); + let mut report: Vec = Vec::new(&env); + for operation in ops.iter() { + let profile = load_operation_profile(&env, &operation); + if let Some(p) = profile.clone() { + if p.samples == 0 { + continue; + } + let samples = p.samples as u64; + report.push_back(OperationCostReport { + operation: operation.clone(), + samples: p.samples, + avg_actual_insns: p.total_actual_insns / samples, + avg_raw_estimated_insns: p.total_raw_estimated_insns / samples, + min_actual_insns: p.min_actual_insns, + max_actual_insns: p.max_actual_insns, + calibration_bps: cost_calibration_bps(&profile), + accuracy_bps: ((p.within_tolerance as u64) * 10_000 / samples) as u32, + }); + } + } + report + } + // ----------------------------------------------------------------------- // Issue #297 / #923: Contract-wide circuit breaker // ----------------------------------------------------------------------- diff --git a/contracts/split/src/test.rs b/contracts/split/src/test.rs index 43e0e50..e193e72 100644 --- a/contracts/split/src/test.rs +++ b/contracts/split/src/test.rs @@ -9021,3 +9021,116 @@ fn test_923_history_logs_transitions() { assert_eq!(history.get(2).unwrap().to, types::CircuitBreakerState::Closed); assert!(has_circuit_breaker_event(&env, "cb_state")); } + +// --------------------------------------------------------------------------- +// Issue #924: Compute budget estimation +// --------------------------------------------------------------------------- + +#[test] +fn test_924_estimate_uses_default_model() { + let (env, contract_id, _token_id, _admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + + let est = c.estimate_operation_cost(&symbol_short!("release"), &4_u32, &0_u32, &0_u32); + let expected = INSTRUCTIONS_BASE + INSTRUCTIONS_PER_SHARD * SHARD_COUNT + 4 * INSTRUCTIONS_PER_RECIPIENT; + assert_eq!(est.raw_cpu_insns, expected); + assert_eq!(est.cpu_insns, expected, "uncalibrated estimate equals raw"); + assert_eq!(est.calibration_bps, 10_000); + assert!(est.within_budget); + assert!(!est.cached); +} + +#[test] +fn test_924_estimate_is_cached_until_model_changes() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let op = symbol_short!("pay"); + + let first = c.estimate_operation_cost(&op, &2_u32, &0_u32, &0_u32); + assert!(!first.cached); + let second = c.estimate_operation_cost(&op, &2_u32, &0_u32, &0_u32); + assert!(second.cached); + assert_eq!(second.cpu_insns, first.cpu_insns); + + c.set_operation_cost_model( + &admin, + &op, + &types::OperationCostModel { + base_insns: 10, + per_recipient_insns: 5, + per_payer_insns: 0, + per_item_insns: 0, + base_mem_bytes: 0, + per_unit_mem_bytes: 0, + }, + ); + let third = c.estimate_operation_cost(&op, &2_u32, &0_u32, &0_u32); + assert!(!third.cached, "model change must invalidate the cache"); + assert_eq!(third.raw_cpu_insns, 20); +} + +#[test] +fn test_924_calibration_brings_estimate_within_ten_percent() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let op = symbol_short!("release"); + + let raw = c.estimate_operation_cost(&op, &3_u32, &0_u32, &0_u32).raw_cpu_insns; + // Real usage runs 30% above the model. + let actual = raw * 13 / 10; + c.record_compute_usage(&admin, &op, &3_u32, &0_u32, &0_u32, &actual); + + let est = c.estimate_operation_cost(&op, &3_u32, &0_u32, &0_u32); + assert_eq!(est.calibration_bps, 13_000); + let error = est.cpu_insns.abs_diff(actual); + assert!(error * 10 <= actual, "calibrated estimate must be within 10% of actual"); +} + +#[test] +fn test_924_profile_tracks_samples() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let op = symbol_short!("refund"); + + assert!(c.get_operation_profile(&op).is_none()); + c.record_compute_usage(&admin, &op, &0_u32, &2_u32, &0_u32, &2_000_000_u64); + c.record_compute_usage(&admin, &op, &0_u32, &4_u32, &0_u32, &3_000_000_u64); + + let p = c.get_operation_profile(&op).unwrap(); + assert_eq!(p.samples, 2); + assert_eq!(p.min_actual_insns, 2_000_000); + assert_eq!(p.max_actual_insns, 3_000_000); + assert_eq!(p.last_actual_insns, 3_000_000); + assert_eq!(p.total_actual_insns, 5_000_000); +} + +#[test] +fn test_924_report_summarises_tracked_operations() { + let (env, contract_id, _token_id, admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + let op = symbol_short!("create"); + + let raw = c.estimate_operation_cost(&op, &2_u32, &0_u32, &0_u32).raw_cpu_insns; + c.record_compute_usage(&admin, &op, &2_u32, &0_u32, &0_u32, &raw); + c.record_compute_usage(&admin, &op, &2_u32, &0_u32, &0_u32, &raw); + + let report = c.get_compute_cost_report(); + assert_eq!(report.len(), 1); + let row = report.get(0).unwrap(); + assert_eq!(row.operation, op); + assert_eq!(row.samples, 2); + assert_eq!(row.avg_actual_insns, raw); + assert_eq!(row.calibration_bps, 10_000); + assert_eq!(row.accuracy_bps, 10_000); + + c.reset_operation_profile(&admin, &op); + assert_eq!(c.get_compute_cost_report().len(), 0); +} + +#[test] +#[should_panic(expected = "unknown operation")] +fn test_924_unknown_operation_rejected() { + let (env, contract_id, _token_id, _admin) = setup_fee_waiver_admin(); + let c = client(&env, &contract_id); + c.estimate_operation_cost(&symbol_short!("nope"), &1_u32, &0_u32, &0_u32); +} diff --git a/contracts/split/src/types.rs b/contracts/split/src/types.rs index 40f2cf2..a3777a6 100644 --- a/contracts/split/src/types.rs +++ b/contracts/split/src/types.rs @@ -1653,6 +1653,74 @@ pub struct ComputeEstimate { pub fee_stroops: i128, } +/// Issue #924: Linear cost model for one operation. +/// +/// `cpu = base_insns + recipients * per_recipient_insns + payers * per_payer_insns +/// + items * per_item_insns` +/// `mem = base_mem_bytes + (recipients + payers + items) * per_unit_mem_bytes` +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct OperationCostModel { + pub base_insns: u64, + pub per_recipient_insns: u64, + pub per_payer_insns: u64, + pub per_item_insns: u64, + pub base_mem_bytes: u64, + pub per_unit_mem_bytes: u64, +} + +/// Issue #924: Result of `estimate_operation_cost`. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct CostEstimate { + pub operation: Symbol, + /// Calibrated instruction estimate (raw estimate scaled by `calibration_bps`). + pub cpu_insns: u64, + /// Uncalibrated instruction estimate straight from the cost model. + pub raw_cpu_insns: u64, + pub mem_bytes: u64, + pub fee_stroops: i128, + /// Observed actual / raw estimate ratio in basis points (10_000 = model is exact). + pub calibration_bps: u32, + pub within_budget: bool, + /// True when served from the estimate cache. + pub cached: bool, + /// Cost model version the estimate was computed against. + pub model_version: u32, +} + +/// Issue #924: Per-operation profiling data built from reported actual usage. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct OperationProfile { + pub operation: Symbol, + pub samples: u32, + pub total_raw_estimated_insns: u64, + pub total_actual_insns: u64, + pub min_actual_insns: u64, + pub max_actual_insns: u64, + pub last_actual_insns: u64, + /// Error of the calibrated estimate for the last sample, in basis points of actual. + pub last_error_bps: u32, + /// Samples whose calibrated estimate was within 10% of actual. + pub within_tolerance: u32, +} + +/// Issue #924: One row of `get_compute_cost_report`. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct OperationCostReport { + pub operation: Symbol, + pub samples: u32, + pub avg_actual_insns: u64, + pub avg_raw_estimated_insns: u64, + pub min_actual_insns: u64, + pub max_actual_insns: u64, + pub calibration_bps: u32, + /// Share of samples estimated within 10%, in basis points. + pub accuracy_bps: u32, +} + /// Issue #297: Circuit breaker status returned by get_circuit_breaker_status(). #[contracttype] #[derive(Clone, Debug)] diff --git a/docs/ISSUE_924_COMPUTE_BUDGET_ESTIMATION.md b/docs/ISSUE_924_COMPUTE_BUDGET_ESTIMATION.md new file mode 100644 index 0000000..2c192ac --- /dev/null +++ b/docs/ISSUE_924_COMPUTE_BUDGET_ESTIMATION.md @@ -0,0 +1,67 @@ +# Issue #924: Compute Budget Estimation + +Runtime cost estimation with a configurable cost model, usage tracking, calibration, +caching and per-operation profiling. This complements the static `estimate_compute` +(#351) and `simulate_release` (#298) entry points. + +## Cost model + +Each operation has an `OperationCostModel`: + +``` +cpu = base_insns + + recipients * per_recipient_insns + + payers * per_payer_insns + + items * per_item_insns +mem = base_mem_bytes + (recipients + payers + items) * per_unit_mem_bytes +``` + +Built-in models, derived from the `estimate_compute` constants: + +| Operation | base | per recipient | per payer | per item | +|-----------|------|---------------|-----------|----------| +| `create` | 1,000,000 | 200,000 | – | – | +| `pay` | 1,800,000 | 250,000 | – | – | +| `release` | 1,800,000 | 500,000 | – | – | +| `refund` | 1,800,000 | – | 350,000 | – | +| `batch_pay` | 1,800,000 | – | – | 400,000 | + +`set_operation_cost_model(admin, operation, model)` (SuperAdmin) can override a built-in +model or define a new operation. Unknown operations panic with `"unknown operation"`. + +## Estimation + +`estimate_operation_cost(operation, recipient_count, payer_count, item_count) -> CostEstimate` +returns `{ operation, cpu_insns, raw_cpu_insns, mem_bytes, fee_stroops, calibration_bps, within_budget, cached, model_version }`. + +- `raw_cpu_insns` comes straight from the model. +- `cpu_insns = raw_cpu_insns * calibration_bps / 10_000`. +- `within_budget` checks against the 100M instruction per-transaction limit. + +## Tracking and calibration + +Contracts cannot read their own instruction meter, so measured costs (from transaction +simulation or ledger meta) are reported by an Operator: + +`record_compute_usage(reporter, operation, recipient_count, payer_count, item_count, actual_insns) -> OperationProfile` + +Each report: + +1. Scores the current calibrated estimate against `actual_insns` (`last_error_bps`). If the error is within 10%, `within_tolerance` goes up by one. +2. Adds the sample to the operation's totals, min, max and last. +3. Recomputes `calibration_bps = total_actual / total_raw_estimated`. This is what keeps estimates within 10% of measured usage. + +## Caching + +Estimates are cached in temporary storage, keyed by `(operation, recipients, payers, items)`. +A global `cost_ver` counter goes up on every model change, usage report or profile reset. +A cached entry is served (`cached = true`) only while its `model_version` still matches. + +## Reporting and profiling + +- `get_operation_profile(operation) -> Option`: raw profiling data. +- `get_compute_cost_report() -> Vec`: one row per tracked operation, with average actual and estimated cost, min, max, calibration and `accuracy_bps` (the share of samples estimated within 10%). +- `reset_operation_profile(admin, operation)` (SuperAdmin) clears profiling and calibration for an operation. +- `get_operation_cost_model(operation)` + +Each usage report emits a `cost_rec` event `(actual_insns, estimated_insns, error_bps)`. From ad49fa2ba5132a92d6606851e9315b49af0c4dd1 Mon Sep 17 00:00:00 2001 From: Farez Abel Date: Sun, 18 Oct 2026 02:50:34 +0000 Subject: [PATCH 4/4] feat(split): implement payment batching (#925) - Add PaymentBatch struct with open / add / remove / cancel collection flow - Validate items on add and again at execution; merge repeated invoice entries - Enforce configurable batch size limit (default 25, max 50) - Execute atomically with a single token transfer via shared _pool_pay accounting - Report per-batch cost savings vs individual payments and track cumulative stats --- contracts/split/src/events.rs | 37 ++++ contracts/split/src/lib.rs | 339 +++++++++++++++++++++++++++-- contracts/split/src/test.rs | 199 +++++++++++++++++ contracts/split/src/types.rs | 52 +++++ docs/ISSUE_925_PAYMENT_BATCHING.md | 80 +++++++ 5 files changed, 693 insertions(+), 14 deletions(-) create mode 100644 docs/ISSUE_925_PAYMENT_BATCHING.md diff --git a/contracts/split/src/events.rs b/contracts/split/src/events.rs index b9aae9d..ccb4a67 100644 --- a/contracts/split/src/events.rs +++ b/contracts/split/src/events.rs @@ -600,6 +600,43 @@ pub fn circuit_breaker_deactivated(env: &Env) { .publish((symbol_short!("split"), symbol_short!("cb_dact")), ()); } +/// Issue #925: Emitted when a payment batch is opened. +/// Topics: (split, pb_open, payer) +/// Data: (batch_id, token) +pub fn payment_batch_opened(env: &Env, batch_id: u64, payer: &Address, token: &Address) { + env.events().publish( + (symbol_short!("split"), symbol_short!("pb_open"), payer.clone()), + (batch_id, token.clone()), + ); +} + +/// Issue #925: Emitted when a payment batch is executed. +/// Topics: (split, pb_exec, payer) +/// Data: (batch_id, item_count, total_amount, savings_bps) +pub fn payment_batch_executed( + env: &Env, + batch_id: u64, + payer: &Address, + item_count: u32, + total_amount: i128, + savings_bps: u32, +) { + env.events().publish( + (symbol_short!("split"), symbol_short!("pb_exec"), payer.clone()), + (batch_id, item_count, total_amount, savings_bps), + ); +} + +/// Issue #925: Emitted when a payment batch is cancelled. +/// Topics: (split, pb_cncl, payer) +/// Data: batch_id +pub fn payment_batch_cancelled(env: &Env, batch_id: u64, payer: &Address) { + env.events().publish( + (symbol_short!("split"), symbol_short!("pb_cncl"), payer.clone()), + batch_id, + ); +} + /// Issue #923: Emitted on every circuit breaker state transition. /// Topics: (split, cb_state) /// Data: (from, to, actor, reason, ledger) diff --git a/contracts/split/src/lib.rs b/contracts/split/src/lib.rs index 72ca19b..dc93748 100644 --- a/contracts/split/src/lib.rs +++ b/contracts/split/src/lib.rs @@ -40,6 +40,11 @@ const COST_ESTIMATE_TOLERANCE_BPS: u64 = 1_000; /// Issue #924: Maximum distinct operations tracked by the profiler. const MAX_PROFILED_OPERATIONS: u32 = 32; +/// Issue #925: Default maximum invoices per payment batch. +const DEFAULT_PAYMENT_BATCH_LIMIT: u32 = 25; +/// Issue #925: Hard upper bound an admin may raise the batch limit to. +const MAX_PAYMENT_BATCH_LIMIT: u32 = 50; + /// Issue #296: Maximum entries in the per-creator fee waiver list. const MAX_FEE_WAIVER_ENTRIES: usize = 100; /// Issue #922: Maximum entries retained in the fee waiver audit log (oldest dropped first). @@ -103,7 +108,8 @@ use soroban_sdk::{ const MAX_PARENT_DEPTH: u32 = 10; use types::{ - AdminAction, AdminRole, AdminSet, AuditEntry, Bid, CircuitBreakerConfig, CircuitBreakerState, + AdminAction, AdminRole, AdminSet, AuditEntry, BatchExecutionReport, BatchingStats, Bid, + CircuitBreakerConfig, CircuitBreakerState, CircuitBreakerStatus, CircuitBreakerTransition, CostEstimate, OperationCostModel, OperationCostReport, OperationProfile, CloneOverrides, CompactInvoice, CompactMigrateResult, CompletionProof, ComputeEstimate, @@ -113,7 +119,7 @@ use types::{ Invoice, InvoiceCore, InvoiceExt, InvoiceExt2, InvoiceExt3, InvoiceHot, InvoiceOptions, InvoiceOptions2, InvoicePayment, InvoiceStats, InvoiceStatus, InvoiceTemplate, InvoiceTemplateRecord, LegacyInvoice, OverflowBehavior, OverfundingPolicy, Payment, - PaymentCertificate, PaymentCommitment, PaymentProof, PaymentRecord, PendingAdminAction, + PaymentBatch, PaymentBatchStatus, PaymentCertificate, PaymentCommitment, PaymentProof, PaymentRecord, PendingAdminAction, ProtocolFeeConfig, QueuedAction, RebateTier, Recipient, RepScore, ResolveAction, ResolveRule, Role, SimulateReleaseResult, SplitRule, SubscriptionParams, TimelockAction, Tombstone, Tranche, TransferRecord, TreasuryRecord, UpgradeProposal, @@ -255,6 +261,26 @@ fn circuit_breaker_history_key() -> Symbol { symbol_short!("cb_hist") } +/// Issue #925: Next payment batch id counter. +fn payment_batch_next_id_key() -> Symbol { + symbol_short!("pb_next") +} + +/// Issue #925: Payment batch record. +fn payment_batch_key(batch_id: u64) -> (Symbol, u64) { + (symbol_short!("pay_bat"), batch_id) +} + +/// Issue #925: Admin-configured maximum invoices per batch. +fn payment_batch_limit_key() -> Symbol { + symbol_short!("pb_limit") +} + +/// Issue #925: Cumulative batching metrics. +fn payment_batch_stats_key() -> Symbol { + symbol_short!("pb_stats") +} + /// Issue #924: Admin-configured cost model override for an operation. fn cost_model_key(operation: &Symbol) -> (Symbol, Symbol) { (symbol_short!("cost_mdl"), operation.clone()) @@ -2407,6 +2433,46 @@ fn require_fn_not_paused(env: &Env, name: &Symbol) { } } +// --------------------------------------------------------------------------- +// Issue #925: Payment batching helpers +// --------------------------------------------------------------------------- + +fn load_payment_batch(env: &Env, batch_id: u64) -> PaymentBatch { + env.storage() + .persistent() + .get(&payment_batch_key(batch_id)) + .expect("batch not found") +} + +/// Load a batch the payer may still modify. +fn load_open_payment_batch(env: &Env, batch_id: u64, payer: &Address) -> PaymentBatch { + let batch = load_payment_batch(env, batch_id); + assert!(batch.payer == *payer, "not batch owner"); + assert!(batch.status == PaymentBatchStatus::Open, "batch is not open"); + batch +} + +fn payment_batch_limit(env: &Env) -> u32 { + env.storage() + .persistent() + .get(&payment_batch_limit_key()) + .unwrap_or(DEFAULT_PAYMENT_BATCH_LIMIT) +} + +fn load_batching_stats(env: &Env) -> BatchingStats { + env.storage() + .persistent() + .get(&payment_batch_stats_key()) + .unwrap_or(BatchingStats { + batches_executed: 0, + items_processed: 0, + total_volume: 0, + transfers_saved: 0, + estimated_insns_saved: 0, + last_savings_bps: 0, + }) +} + // --------------------------------------------------------------------------- // Issue #924: Compute cost estimation helpers // --------------------------------------------------------------------------- @@ -8406,17 +8472,29 @@ impl SplitContract { pub fn pool_pay(env: Env, payer: Address, payments: Vec) { require_not_paused(&env); payer.require_auth(); + Self::_pool_pay(&env, &payer, &payments, symbol_short!("pool_pay")); + } + /// Shared accounting for `pool_pay` and `execute_payment_batch` (issue #925). + /// Validates every payment, performs one token transfer and credits each + /// invoice; any failure panics so the whole call reverts. Returns the total + /// transferred. Callers are responsible for auth and pause checks. + fn _pool_pay( + env: &Env, + payer: &Address, + payments: &Vec, + action: Symbol, + ) -> i128 { assert!(!payments.is_empty(), "payments must not be empty"); // Determine the shared token from the first invoice. - let first_inv = load_invoice(&env, payments.get(0).unwrap().invoice_id); + let first_inv = load_invoice(env, payments.get(0).unwrap().invoice_id); let shared_token = funding_token_for(&first_inv); // Validate all payments and compute total. let mut total: i128 = 0; for p in payments.iter() { - let inv = load_invoice(&env, p.invoice_id); + let inv = load_invoice(env, p.invoice_id); assert!( inv.status == InvoiceStatus::Pending, "invoice is not pending" @@ -8441,19 +8519,19 @@ impl SplitContract { } // Single token transfer from payer to contract. - let token_client = token::Client::new(&env, &shared_token); - token_client.transfer(&payer, &env.current_contract_address(), &total); + let token_client = token::Client::new(env, &shared_token); + token_client.transfer(payer, &env.current_contract_address(), &total); // Update each invoice via internal accounting (no further token transfers). for p in payments.iter() { - let mut inv = load_invoice(&env, p.invoice_id); + let mut inv = load_invoice(env, p.invoice_id); // Write payment to sharded storage (issue #177). - let shard_id = compute_shard_id(&env, &payer); + let shard_id = compute_shard_id(env, payer); let mut shard_payments: Vec = env .storage() .persistent() .get::<(Symbol, u64, u64), Vec>(&pay_shard_key(p.invoice_id, shard_id)) - .unwrap_or_else(|| Vec::new(&env)); + .unwrap_or_else(|| Vec::new(env)); shard_payments.push_back(Payment { payer: payer.clone(), amount: p.amount, @@ -8474,8 +8552,8 @@ impl SplitContract { .persistent() .set(&cumulative_key, &(cumulative + p.amount)); - append_audit_entry(&env, p.invoice_id, symbol_short!("pool_pay"), &payer); - events::payment_received(&env, p.invoice_id, &payer, p.amount, &shared_token); + append_audit_entry(env, p.invoice_id, action.clone(), payer); + events::payment_received(env, p.invoice_id, payer, p.amount, &shared_token); let inv_total: i128 = inv.amounts.iter().sum(); if inv.funded >= inv_total { @@ -8495,14 +8573,247 @@ impl SplitContract { < (inv.amounts.iter().sum::() * inv.min_funding_bps as i128 / 10_000)); if guarded { - save_invoice(&env, p.invoice_id, &inv); + save_invoice(env, p.invoice_id, &inv); } else { - Self::_release(&env, p.invoice_id, &mut inv, &payer); + Self::_release(env, p.invoice_id, &mut inv, payer); } } else { - save_invoice(&env, p.invoice_id, &inv); + save_invoice(env, p.invoice_id, &inv); } } + total + } + + // ----------------------------------------------------------------------- + // Issue #925: Payment batching + // ----------------------------------------------------------------------- + + /// Open an empty payment batch for `payer` settling in `token`. Returns the batch id. + pub fn open_payment_batch(env: Env, payer: Address, token: Address) -> u64 { + require_not_paused(&env); + payer.require_auth(); + let id: u64 = env + .storage() + .persistent() + .get(&payment_batch_next_id_key()) + .unwrap_or(1); + env.storage() + .persistent() + .set(&payment_batch_next_id_key(), &(id + 1)); + let batch = PaymentBatch { + id, + payer: payer.clone(), + token: token.clone(), + items: Vec::new(&env), + total: 0, + status: PaymentBatchStatus::Open, + created_at: env.ledger().timestamp(), + executed_at: 0, + }; + env.storage().persistent().set(&payment_batch_key(id), &batch); + events::payment_batch_opened(&env, id, &payer, &token); + id + } + + /// Add a payment toward `invoice_id` to an open batch. + /// + /// The invoice must be pending, not disputed, before its deadline and + /// funded in the batch token; the combined batched amount must not exceed + /// its remaining balance. Adding to an invoice already in the batch merges + /// the amounts. Panics with `BatchLimitExceeded` past the batch size limit. + pub fn add_to_payment_batch( + env: Env, + payer: Address, + batch_id: u64, + invoice_id: u64, + amount: i128, + ) { + require_not_paused(&env); + payer.require_auth(); + let mut batch = load_open_payment_batch(&env, batch_id, &payer); + assert!(amount > 0, "payment amount must be positive"); + + let inv = load_invoice(&env, invoice_id); + assert!(inv.status == InvoiceStatus::Pending, "invoice is not pending"); + assert!(!inv.disputed, "invoice is disputed"); + assert!( + env.ledger().timestamp() <= inv.deadline, + "invoice deadline has passed" + ); + assert!( + funding_token_for(&inv) == batch.token, + "all invoices must use the same token" + ); + + let mut existing: Option = None; + for (i, item) in batch.items.iter().enumerate() { + if item.invoice_id == invoice_id { + existing = Some(i as u32); + } + } + let batched_so_far = existing + .map(|i| batch.items.get(i).unwrap().amount) + .unwrap_or(0); + let new_amount = batched_so_far + .checked_add(amount) + .expect("payment amount overflow"); + let inv_total: i128 = inv.amounts.iter().sum(); + assert!( + inv.funded + new_amount <= inv_total, + "payment exceeds remaining balance" + ); + + match existing { + Some(i) => batch.items.set( + i, + InvoicePayment { + invoice_id, + amount: new_amount, + }, + ), + None => { + if batch.items.len() >= payment_batch_limit(&env) { + panic!("BatchLimitExceeded"); + } + batch.items.push_back(InvoicePayment { invoice_id, amount }); + } + } + batch.total = batch.total.checked_add(amount).expect("batch total overflow"); + env.storage() + .persistent() + .set(&payment_batch_key(batch_id), &batch); + } + + /// Remove an invoice's payment from an open batch. + pub fn remove_from_payment_batch(env: Env, payer: Address, batch_id: u64, invoice_id: u64) { + payer.require_auth(); + let mut batch = load_open_payment_batch(&env, batch_id, &payer); + let mut kept: Vec = Vec::new(&env); + let mut total: i128 = 0; + for item in batch.items.iter() { + if item.invoice_id != invoice_id { + total += item.amount; + kept.push_back(item); + } + } + assert!(kept.len() < batch.items.len(), "invoice not in batch"); + batch.items = kept; + batch.total = total; + env.storage() + .persistent() + .set(&payment_batch_key(batch_id), &batch); + } + + /// Cancel an open batch. No funds have moved, so nothing is refunded. + pub fn cancel_payment_batch(env: Env, payer: Address, batch_id: u64) { + payer.require_auth(); + let mut batch = load_open_payment_batch(&env, batch_id, &payer); + batch.status = PaymentBatchStatus::Cancelled; + env.storage() + .persistent() + .set(&payment_batch_key(batch_id), &batch); + events::payment_batch_cancelled(&env, batch_id, &payer); + } + + /// Settle every payment in the batch atomically with a single token transfer. + /// + /// All items are re-validated before any funds move; if any item is no + /// longer valid (e.g. the invoice was funded or expired meanwhile) the + /// whole call reverts and the batch stays open. Returns a cost comparison + /// against paying each invoice individually. + pub fn execute_payment_batch(env: Env, payer: Address, batch_id: u64) -> BatchExecutionReport { + require_fn_not_paused(&env, &symbol_short!("pay")); + require_not_frozen(&env); + payer.require_auth(); + let mut batch = load_open_payment_batch(&env, batch_id, &payer); + assert!(!batch.items.is_empty(), "payments must not be empty"); + + // Estimate cost before execution, while recipient counts are still readable. + let item_count = batch.items.len(); + let pay_op = symbol_short!("pay"); + let mut unbatched_cpu_insns: u64 = 0; + let mut recipients_total: u32 = 0; + for item in batch.items.iter() { + let recipients = load_invoice(&env, item.invoice_id).recipients.len(); + recipients_total = recipients_total.saturating_add(recipients); + unbatched_cpu_insns = unbatched_cpu_insns.saturating_add( + compute_cost_estimate(&env, &pay_op, recipients, 0, 0).cpu_insns, + ); + } + let batched_cpu_insns = compute_cost_estimate( + &env, + &symbol_short!("batch_pay"), + recipients_total, + 0, + item_count, + ) + .cpu_insns; + + batch.status = PaymentBatchStatus::Executed; + batch.executed_at = env.ledger().timestamp(); + env.storage() + .persistent() + .set(&payment_batch_key(batch_id), &batch); + + let total = Self::_pool_pay(&env, &payer, &batch.items, symbol_short!("batch_pay")); + + let saved_insns = unbatched_cpu_insns.saturating_sub(batched_cpu_insns); + let savings_bps = if unbatched_cpu_insns == 0 { + 0 + } else { + ((saved_insns as u128) * 10_000 / (unbatched_cpu_insns as u128)) as u32 + }; + let report = BatchExecutionReport { + batch_id, + item_count, + total_amount: total, + token_transfers: 1, + transfers_saved: item_count - 1, + batched_cpu_insns, + unbatched_cpu_insns, + savings_bps, + }; + + let mut stats = load_batching_stats(&env); + stats.batches_executed += 1; + stats.items_processed += item_count as u64; + stats.total_volume = stats.total_volume.saturating_add(total); + stats.transfers_saved += (item_count - 1) as u64; + stats.estimated_insns_saved = stats.estimated_insns_saved.saturating_add(saved_insns); + stats.last_savings_bps = savings_bps; + env.storage() + .persistent() + .set(&payment_batch_stats_key(), &stats); + + events::payment_batch_executed(&env, batch_id, &payer, item_count, total, savings_bps); + report + } + + /// Set the maximum invoices per batch (1..=50). SuperAdmin only. + pub fn set_payment_batch_limit(env: Env, admin: Address, limit: u32) { + require_admin_role(&env, &admin, AdminRole::SuperAdmin); + assert!( + limit > 0 && limit <= MAX_PAYMENT_BATCH_LIMIT, + "batch limit out of range" + ); + env.storage() + .persistent() + .set(&payment_batch_limit_key(), &limit); + } + + /// Returns the maximum invoices per batch. + pub fn get_payment_batch_limit(env: Env) -> u32 { + payment_batch_limit(&env) + } + + /// Returns a payment batch by id. Panics with "batch not found" if absent. + pub fn get_payment_batch(env: Env, batch_id: u64) -> PaymentBatch { + load_payment_batch(&env, batch_id) + } + + /// Returns cumulative batching metrics. + pub fn get_batching_stats(env: Env) -> BatchingStats { + load_batching_stats(&env) } // ----------------------------------------------------------------------- diff --git a/contracts/split/src/test.rs b/contracts/split/src/test.rs index e193e72..0d5cff2 100644 --- a/contracts/split/src/test.rs +++ b/contracts/split/src/test.rs @@ -9134,3 +9134,202 @@ fn test_924_unknown_operation_rejected() { let c = client(&env, &contract_id); c.estimate_operation_cost(&symbol_short!("nope"), &1_u32, &0_u32, &0_u32); } + +// --------------------------------------------------------------------------- +// Issue #925: Payment batching +// --------------------------------------------------------------------------- + +fn setup_payment_batch() -> (Env, Address, Address, Address, Address) { + let (env, contract_id, token_id) = setup(); + let c = client(&env, &contract_id); + let admin = Address::generate(&env); + let treasury = Address::generate(&env); + let payer = Address::generate(&env); + env.ledger().set_timestamp(1_000); + c.initialize(&admin, &0_i128, &treasury, &token_id, &0_u32, &None, &0_u32, &0_u32, &0_u64); + StellarAssetClient::new(&env, &token_id).mint(&payer, &10_000); + (env, contract_id, token_id, admin, payer) +} + +#[test] +fn test_925_batch_collects_and_executes_atomically() { + let (env, contract_id, token_id, _admin, payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); + let creator = Address::generate(&env); + let r1 = Address::generate(&env); + let r2 = Address::generate(&env); + let r3 = Address::generate(&env); + + let id1 = make_invoice(&env, &c, &creator, &r1, 100, &token_id, 9_999); + let id2 = make_invoice(&env, &c, &creator, &r2, 200, &token_id, 9_999); + let id3 = make_invoice(&env, &c, &creator, &r3, 300, &token_id, 9_999); + + let batch_id = c.open_payment_batch(&payer, &token_id); + c.add_to_payment_batch(&payer, &batch_id, &id1, &100_i128); + c.add_to_payment_batch(&payer, &batch_id, &id2, &50_i128); + c.add_to_payment_batch(&payer, &batch_id, &id3, &300_i128); + + let batch = c.get_payment_batch(&batch_id); + assert_eq!(batch.items.len(), 3); + assert_eq!(batch.total, 450); + assert_eq!(batch.status, types::PaymentBatchStatus::Open); + + let report = c.execute_payment_batch(&payer, &batch_id); + assert_eq!(report.item_count, 3); + assert_eq!(report.total_amount, 450); + assert_eq!(report.token_transfers, 1); + assert_eq!(report.transfers_saved, 2); + + assert_eq!(tk.balance(&payer), 10_000 - 450); + assert_eq!(tk.balance(&r1), 100, "fully funded invoice auto-releases"); + assert_eq!(c.get_invoice(&id2).funded, 50); + assert_eq!(c.get_payment_batch(&batch_id).status, types::PaymentBatchStatus::Executed); +} + +#[test] +fn test_925_batching_saves_at_least_twenty_percent() { + let (env, contract_id, token_id, _admin, payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + + let batch_id = c.open_payment_batch(&payer, &token_id); + for _ in 0..5 { + let id = make_invoice(&env, &c, &creator, &Address::generate(&env), 1_000, &token_id, 9_999); + c.add_to_payment_batch(&payer, &batch_id, &id, &10_i128); + } + let report = c.execute_payment_batch(&payer, &batch_id); + assert!(report.batched_cpu_insns < report.unbatched_cpu_insns); + assert!(report.savings_bps >= 2_000, "expected >= 20% savings, got {} bps", report.savings_bps); + + let stats = c.get_batching_stats(); + assert_eq!(stats.batches_executed, 1); + assert_eq!(stats.items_processed, 5); + assert_eq!(stats.total_volume, 50); + assert_eq!(stats.transfers_saved, 4); + assert_eq!(stats.last_savings_bps, report.savings_bps); +} + +#[test] +fn test_925_adding_same_invoice_merges_amounts() { + let (env, contract_id, token_id, _admin, payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + let id = make_invoice(&env, &c, &creator, &Address::generate(&env), 500, &token_id, 9_999); + + let batch_id = c.open_payment_batch(&payer, &token_id); + c.add_to_payment_batch(&payer, &batch_id, &id, &100_i128); + c.add_to_payment_batch(&payer, &batch_id, &id, &150_i128); + + let batch = c.get_payment_batch(&batch_id); + assert_eq!(batch.items.len(), 1); + assert_eq!(batch.items.get(0).unwrap().amount, 250); + assert_eq!(batch.total, 250); +} + +#[test] +#[should_panic(expected = "payment exceeds remaining balance")] +fn test_925_merged_amount_cannot_overfund() { + let (env, contract_id, token_id, _admin, payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + let id = make_invoice(&env, &c, &creator, &Address::generate(&env), 200, &token_id, 9_999); + + let batch_id = c.open_payment_batch(&payer, &token_id); + c.add_to_payment_batch(&payer, &batch_id, &id, &150_i128); + c.add_to_payment_batch(&payer, &batch_id, &id, &100_i128); +} + +#[test] +#[should_panic(expected = "BatchLimitExceeded")] +fn test_925_batch_size_limit_enforced() { + let (env, contract_id, token_id, admin, payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + c.set_payment_batch_limit(&admin, &2_u32); + assert_eq!(c.get_payment_batch_limit(), 2); + + let batch_id = c.open_payment_batch(&payer, &token_id); + for _ in 0..3 { + let id = make_invoice(&env, &c, &creator, &Address::generate(&env), 100, &token_id, 9_999); + c.add_to_payment_batch(&payer, &batch_id, &id, &10_i128); + } +} + +#[test] +#[should_panic(expected = "batch limit out of range")] +fn test_925_batch_limit_above_cap_rejected() { + let (env, contract_id, _token_id, admin, _payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + c.set_payment_batch_limit(&admin, &51_u32); +} + +#[test] +fn test_925_invalid_item_reverts_whole_batch() { + let (env, contract_id, token_id, _admin, payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + let tk = token_client(&env, &token_id); + let creator = Address::generate(&env); + let other_payer = Address::generate(&env); + StellarAssetClient::new(&env, &token_id).mint(&other_payer, &1_000); + + let id1 = make_invoice(&env, &c, &creator, &Address::generate(&env), 100, &token_id, 9_999); + let id2 = make_invoice(&env, &c, &creator, &Address::generate(&env), 100, &token_id, 9_999); + + let batch_id = c.open_payment_batch(&payer, &token_id); + c.add_to_payment_batch(&payer, &batch_id, &id1, &50_i128); + c.add_to_payment_batch(&payer, &batch_id, &id2, &80_i128); + + // id2 is partly funded elsewhere, so the batched 80 no longer fits. + c.pay(&other_payer, &id2, &50_i128, &0_u64, &false, &false, &None); + + let result = c.try_execute_payment_batch(&payer, &batch_id); + assert!(result.is_err(), "execution must fail when any item is invalid"); + assert_eq!(tk.balance(&payer), 10_000, "no funds move on failure"); + assert_eq!(c.get_invoice(&id1).funded, 0, "valid items are rolled back too"); + assert_eq!(c.get_payment_batch(&batch_id).status, types::PaymentBatchStatus::Open); +} + +#[test] +#[should_panic(expected = "batch is not open")] +fn test_925_cancelled_batch_cannot_execute() { + let (env, contract_id, token_id, _admin, payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + let id = make_invoice(&env, &c, &creator, &Address::generate(&env), 100, &token_id, 9_999); + + let batch_id = c.open_payment_batch(&payer, &token_id); + c.add_to_payment_batch(&payer, &batch_id, &id, &50_i128); + c.cancel_payment_batch(&payer, &batch_id); + c.execute_payment_batch(&payer, &batch_id); +} + +#[test] +#[should_panic(expected = "not batch owner")] +fn test_925_only_owner_can_modify_batch() { + let (env, contract_id, token_id, _admin, payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + let id = make_invoice(&env, &c, &creator, &Address::generate(&env), 100, &token_id, 9_999); + + let batch_id = c.open_payment_batch(&payer, &token_id); + c.add_to_payment_batch(&Address::generate(&env), &batch_id, &id, &50_i128); +} + +#[test] +fn test_925_remove_from_batch_updates_total() { + let (env, contract_id, token_id, _admin, payer) = setup_payment_batch(); + let c = client(&env, &contract_id); + let creator = Address::generate(&env); + let id1 = make_invoice(&env, &c, &creator, &Address::generate(&env), 100, &token_id, 9_999); + let id2 = make_invoice(&env, &c, &creator, &Address::generate(&env), 100, &token_id, 9_999); + + let batch_id = c.open_payment_batch(&payer, &token_id); + c.add_to_payment_batch(&payer, &batch_id, &id1, &40_i128); + c.add_to_payment_batch(&payer, &batch_id, &id2, &60_i128); + c.remove_from_payment_batch(&payer, &batch_id, &id1); + + let batch = c.get_payment_batch(&batch_id); + assert_eq!(batch.items.len(), 1); + assert_eq!(batch.total, 60); +} diff --git a/contracts/split/src/types.rs b/contracts/split/src/types.rs index a3777a6..ff78dbd 100644 --- a/contracts/split/src/types.rs +++ b/contracts/split/src/types.rs @@ -1653,6 +1653,58 @@ pub struct ComputeEstimate { pub fee_stroops: i128, } +/// Issue #925: Lifecycle of a payment batch. +#[contracttype] +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum PaymentBatchStatus { + Open, + Executed, + Cancelled, +} + +/// Issue #925: Payments collected for one payer, settled with a single token transfer. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct PaymentBatch { + pub id: u64, + pub payer: Address, + pub token: Address, + /// One entry per invoice; repeated additions to the same invoice are merged. + pub items: Vec, + pub total: i128, + pub status: PaymentBatchStatus, + pub created_at: u64, + pub executed_at: u64, +} + +/// Issue #925: Outcome and cost comparison of one executed batch. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct BatchExecutionReport { + pub batch_id: u64, + pub item_count: u32, + pub total_amount: i128, + /// Token transfers performed (always 1) vs. one per item when paid individually. + pub token_transfers: u32, + pub transfers_saved: u32, + pub batched_cpu_insns: u64, + pub unbatched_cpu_insns: u64, + /// Estimated instruction savings vs. individual `pay` calls, in basis points. + pub savings_bps: u32, +} + +/// Issue #925: Cumulative batching metrics. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct BatchingStats { + pub batches_executed: u64, + pub items_processed: u64, + pub total_volume: i128, + pub transfers_saved: u64, + pub estimated_insns_saved: u64, + pub last_savings_bps: u32, +} + /// Issue #924: Linear cost model for one operation. /// /// `cpu = base_insns + recipients * per_recipient_insns + payers * per_payer_insns diff --git a/docs/ISSUE_925_PAYMENT_BATCHING.md b/docs/ISSUE_925_PAYMENT_BATCHING.md new file mode 100644 index 0000000..5897841 --- /dev/null +++ b/docs/ISSUE_925_PAYMENT_BATCHING.md @@ -0,0 +1,80 @@ +# Issue #925: Payment Batching + +A payer can collect payments to several invoices into a batch, then settle them all with +one token transfer. This builds on `pool_pay` (issue #3), whose accounting now lives in a +shared `_pool_pay` helper so both paths validate and record payments the same way. + +## Data model + +```rust +pub enum PaymentBatchStatus { Open, Executed, Cancelled } + +pub struct PaymentBatch { + pub id: u64, + pub payer: Address, + pub token: Address, + pub items: Vec, // one entry per invoice + pub total: i128, + pub status: PaymentBatchStatus, + pub created_at: u64, + pub executed_at: u64, +} +``` + +## Collection + +| Function | Notes | +|----------|-------| +| `open_payment_batch(payer, token) -> u64` | Creates an empty `Open` batch | +| `add_to_payment_batch(payer, batch_id, invoice_id, amount)` | Validates the item and appends it. Adding to an invoice already in the batch merges the amounts | +| `remove_from_payment_batch(payer, batch_id, invoice_id)` | Removes the item and updates `total` | +| `cancel_payment_batch(payer, batch_id)` | Marks the batch `Cancelled`. No funds have moved | + +Only the batch owner can change a batch (`"not batch owner"`), and only while it is +`Open` (`"batch is not open"`). + +## Validation + +Each item is checked when it is added and again at execution: + +- The invoice is `Pending`, not disputed, and before its deadline. +- The invoice is funded in the batch token. +- `amount > 0`. +- The batched amount for the invoice (merged additions included) fits within the invoice's remaining balance. + +## Batch size limits + +- The default limit is 25 invoices per batch. `set_payment_batch_limit(admin, limit)` (SuperAdmin) can change it within `1..=50`. +- Adding an invoice beyond the limit panics with `BatchLimitExceeded`. Merging into an existing item does not count toward the limit. + +## Atomicity + +`execute_payment_batch(payer, batch_id)`: + +1. Re-validates every item before any funds move. +2. Makes a single `token.transfer(payer → contract)` for the batch total. +3. Credits each invoice through the same sharded payment, audit (`batch_pay`) and auto-release path as `pool_pay`. + +If any step panics, the whole transaction reverts. No funds move, no invoice is credited, and +the batch stays `Open`. Execution is blocked by the pause and circuit breaker guards under +the `pay` guard name. + +## Monitoring + +`execute_payment_batch` returns a `BatchExecutionReport`: + +| Field | Meaning | +|-------|---------| +| `item_count`, `total_amount` | Batch size and value | +| `token_transfers`, `transfers_saved` | 1 transfer, compared with one transfer per item | +| `batched_cpu_insns` | `batch_pay` cost estimate (issue #924 model) | +| `unbatched_cpu_insns` | Sum of `pay` estimates if each invoice were paid individually | +| `savings_bps` | `(unbatched - batched) / unbatched` | + +With the default models, a 2-invoice batch saves about 36% of estimated instructions and a +5-invoice batch about 63%, above the 20% target. + +`get_batching_stats()` gives cumulative figures: `batches_executed`, `items_processed`, +`total_volume`, `transfers_saved`, `estimated_insns_saved` and `last_savings_bps`. + +Events: `pb_open`, `pb_exec` `(batch_id, item_count, total_amount, savings_bps)`, `pb_cncl`.