diff --git a/Cargo.lock b/Cargo.lock index 4b2f4df..52912cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -269,6 +269,7 @@ dependencies = [ name = "comebackhere-compliance" version = "1.0.0" dependencies = [ + "comebackhere-compliance-errors", "soroban-sdk", ] @@ -281,15 +282,30 @@ dependencies = [ "soroban-sdk", ] +[[package]] +name = "comebackhere-compliance-errors" +version = "0.1.0" +dependencies = [ + "soroban-sdk", +] + [[package]] name = "comebackhere-invoice" version = "1.0.0" dependencies = [ + "comebackhere-invoice-errors", "hex", "proptest", "soroban-sdk", ] +[[package]] +name = "comebackhere-invoice-errors" +version = "0.1.0" +dependencies = [ + "soroban-sdk", +] + [[package]] name = "comebackhere-multisig" version = "0.3.0" @@ -301,8 +317,8 @@ dependencies = [ name = "comebackhere-protocol-errors" version = "0.1.0" dependencies = [ - "comebackhere-compliance", - "comebackhere-invoice", + "comebackhere-compliance-errors", + "comebackhere-invoice-errors", "comebackhere-treasury", ] diff --git a/contracts/compliance/src/lib.rs b/contracts/compliance/src/lib.rs index c5f9fca..5537187 100644 --- a/contracts/compliance/src/lib.rs +++ b/contracts/compliance/src/lib.rs @@ -69,6 +69,12 @@ pub enum DataKey { /// unset (`None`) for addresses tracked before this field existed. Purely metadata — /// does not affect `is_allowed`. Jurisdiction(Address), + /// Timestamp of the caller's last `bulk_allow_addresses` call, keyed per admin. + /// See [`BULK_OP_COOLDOWN_SECS`] and `check_bulk_op_cooldown` (#454). + LastBulkAllow(Address), + /// Timestamp of the caller's last `bulk_block_addresses` call, keyed per admin. + /// See [`BULK_OP_COOLDOWN_SECS`] and `check_bulk_op_cooldown` (#454). + LastBulkBlock(Address), } /// Coarse classification of an address's compliance state. diff --git a/contracts/settlement-workflow/Cargo.toml b/contracts/settlement-workflow/Cargo.toml index 32a19ec..78e1440 100644 --- a/contracts/settlement-workflow/Cargo.toml +++ b/contracts/settlement-workflow/Cargo.toml @@ -15,6 +15,11 @@ testutils = ["soroban-sdk/testutils"] [dependencies] soroban-sdk.workspace = true compliance-client = { package = "comebackhere-compliance-client", path = "../../crates/compliance-client" } +# `multisig` holds no `#[contractimpl]` (only the shared `TreasuryError` enum and +# contract types), so depending on it directly does not statically link any +# foreign wasm exports into this contract — unlike the `compliance` / `treasury` +# impl crates, which stay dev-only for that reason. +multisig = { package = "comebackhere-multisig", path = "../../crates/multisig" } [dev-dependencies] soroban-sdk = { workspace = true, features = ["testutils"] } diff --git a/contracts/settlement-workflow/src/lib.rs b/contracts/settlement-workflow/src/lib.rs index 1760719..80c0e55 100644 --- a/contracts/settlement-workflow/src/lib.rs +++ b/contracts/settlement-workflow/src/lib.rs @@ -18,23 +18,20 @@ pub trait TreasuryInterface { fn get_signer_weight(env: Env, signer: Address) -> u32; } -/// Storage key for the ordered list of settlement IDs executed through this -/// workflow contract (as opposed to executed directly against treasury, bypassing -/// the compliance gate). See `get_executed_settlement_ids_page` (#373). -#[contracttype] -pub enum DataKey { - ExecutedSettlements, -} - -/// Instance-storage keys for the workflow's pinned configuration. The compliance -/// and treasury instances are set once at initialization (#364) so the contract -/// enforces which instances it trusts rather than trusting whatever a caller -/// supplies per-call. +/// Storage keys for the workflow contract. +/// +/// `ComplianceId` / `TreasuryId` pin the compliance and treasury instances this +/// workflow trusts; they are set once at initialization (#364) so the contract +/// enforces which instances it uses rather than trusting whatever a caller +/// supplies per-call. `ExecutedSettlements` is the ordered list of settlement +/// IDs executed through this (compliance-gated) workflow, as opposed to executed +/// directly against treasury — see `get_executed_settlement_ids_page` (#373). #[contracttype] #[derive(Clone)] pub enum DataKey { ComplianceId, TreasuryId, + ExecutedSettlements, } /// Reference on-chain implementation of the `SettlementWorkflow` role described in diff --git a/contracts/treasury/src/deposits.rs b/contracts/treasury/src/deposits.rs index 9d7f458..1f1b828 100644 --- a/contracts/treasury/src/deposits.rs +++ b/contracts/treasury/src/deposits.rs @@ -138,10 +138,72 @@ fn deposit_one(env: &Env, from: &Address, token_contract: &Address, amount: i128 balance = balance .checked_add(amount) .ok_or(TreasuryError::ArithmeticOverflow)?; - env.storage() - .persistent() - .set(&DataKey::Balance(from.clone()), &balance); + env.storage().persistent().set( + &DataKey::Balance(from.clone(), token_contract.clone()), + &balance, + ); env.events() .publish((Symbol::new(env, "deposit"), from.clone()), amount); Ok(()) } + +/// Enforces the admin-configured rolling-window withdrawal cap (see +/// `set_withdrawal_limit` / `get_withdrawal_limit` in `lib.rs`). Tracked per +/// `addr` so `withdraw` (keyed on the recipient `to`) and `withdraw_all` (keyed +/// on `recipient`) each accumulate against their own window. +/// +/// No-op when the limit is unset or `<= 0` (the default: uncapped). When a +/// limit is configured, the first withdrawal of a window records the window +/// start; subsequent withdrawals inside `WithdrawalWindowSecs` accumulate, and +/// a withdrawal that would push the window total past the limit panics with +/// `WithdrawalLimitExceeded` before any transfer happens. +pub(crate) fn enforce_withdrawal_limit(env: &Env, addr: &Address, amount: i128) { + let limit: i128 = env + .storage() + .instance() + .get(&DataKey::WithdrawalLimitPerWindow) + .unwrap_or(0); + if limit <= 0 { + return; // uncapped (default) + } + let window_secs: u64 = env + .storage() + .instance() + .get(&DataKey::WithdrawalWindowSecs) + .unwrap_or(0); + let now = env.ledger().timestamp(); + let window_start: u64 = env + .storage() + .instance() + .get(&DataKey::WithdrawalWindowStart(addr.clone())) + .unwrap_or(0); + let used: i128 = env + .storage() + .instance() + .get(&DataKey::WithdrawnInWindow(addr.clone())) + .unwrap_or(0); + + // Start a fresh window if the configured window has elapsed since it began + // (or if no window duration is configured, so every call is its own window). + let window_elapsed = window_secs == 0 || now.saturating_sub(window_start) >= window_secs; + let (current_start, prior_used) = if window_elapsed { + (now, 0i128) + } else { + (window_start, used) + }; + + let new_used = prior_used + .checked_add(amount) + .unwrap_or_else(|| soroban_sdk::panic_with_error!(env, TreasuryError::ArithmeticOverflow)); + if new_used > limit { + soroban_sdk::panic_with_error!(env, TreasuryError::WithdrawalLimitExceeded); + } + + env.storage().instance().set( + &DataKey::WithdrawalWindowStart(addr.clone()), + ¤t_start, + ); + env.storage() + .instance() + .set(&DataKey::WithdrawnInWindow(addr.clone()), &new_used); +} diff --git a/contracts/treasury/src/disputes.rs b/contracts/treasury/src/disputes.rs index 29df38d..5843ada 100644 --- a/contracts/treasury/src/disputes.rs +++ b/contracts/treasury/src/disputes.rs @@ -157,6 +157,7 @@ impl TreasuryContract { env.events() .publish((Symbol::new(&env, "dispute_resolved"), dispute_id), dispute); release_settlement_hold_if_no_open_disputes(&env, settlement_id); + Ok(()) } /// Resolves an open dispute by splitting `dispute.amount` between claimant and @@ -208,7 +209,17 @@ impl TreasuryContract { if counterparty_amount > 0 { token_client.transfer(&treasury, &dispute.counterparty, &counterparty_amount); } - Ok(()) + dispute.status = DisputeStatus::ResolvedSplit; + dispute.claimant_share_bps = claimant_bps; + let settlement_id = dispute.settlement_id; + env.storage() + .persistent() + .set(&DataKey::Dispute(dispute_id), &dispute); + env.events().publish( + (Symbol::new(&env, "dispute_resolved_split"), dispute_id), + dispute, + ); + release_settlement_hold_if_no_open_disputes(&env, settlement_id); } /// Casts a weighted signer vote on a dispute; auto-resolves when cumulative weight meets threshold. diff --git a/contracts/treasury/tests/large_history_load_test.rs b/contracts/treasury/tests/large_history_load_test.rs new file mode 100644 index 0000000..c2c43a2 --- /dev/null +++ b/contracts/treasury/tests/large_history_load_test.rs @@ -0,0 +1,338 @@ +//! Large-scale load test: treasury behaviour with 1000+ historical settlements. +//! +//! ## Why this is not already covered by #97 +//! +//! #97 (`settlement_pagination_test.rs`) established that +//! `get_pending_settlements_page` is *correct* under load - it returns the right +//! prefix/suffix, skips executed entries, and handles extreme `start`/`limit` +//! values (`u64::MAX`) without panicking or scanning proportionally to the +//! argument. But its largest *real* corpus is 50 settlements +//! (`limit_of_u64_max_returns_all_pending_and_stays_under_instruction_budget`); +//! the `u64::MAX` cases probe argument handling, not history depth. +//! +//! Several open issues in this batch reason about "a long-lived treasury with a +//! lot of accumulated history" - the `resolve_dispute` unbounded-iteration +//! concern (`resolve_dispute_dos_test.rs`) and the signer-rotation-vs-concurrent +//! -signer-change race (`rotation_weight_race_test.rs`). To evaluate those +//! properly we need measured numbers for how the treasury's key read/scan +//! entrypoints behave with 1000+ settlements sitting on a single instance. +//! +//! This file builds that instance once, at 500 and at 1000+ settlements, and: +//! * asserts `get_pending_settlements_page` stays *correct* at that scale +//! (right IDs, right statuses, deep offsets, executed entries skipped), +//! * records its measured CPU instruction cost and asserts the cost scales +//! roughly linearly with history size (not worse), and +//! * records `resolve_dispute`'s cost with 1000+ settlements as background +//! load, so the "settlement history as background load" dimension the +//! rotation-race issue cares about has a concrete number. +//! +//! Numbers are printed with `--nocapture`: +//! cargo test -p comebackhere-treasury --test large_history_load_test -- --nocapture + +use soroban_sdk::{testutils::Address as _, Address, Env}; +use treasury::{SettlementStatus, TreasuryContract, TreasuryContractClient}; + +/// Primary scale under test. The parent issue asks specifically for "1,000 or +/// more historical settlements accumulated on a single treasury instance". +const LARGE_HISTORY: u64 = 1_000; + +/// Half-scale sample used only to check that per-settlement scan cost is not +/// growing worse than linearly between `HALF_HISTORY` and `LARGE_HISTORY`. +const HALF_HISTORY: u64 = 500; + +/// Very generous absolute ceiling for a single read-only scan over the whole +/// history. Same order of magnitude as `PAGE_INSTRUCTION_BUDGET` in +/// `settlement_pagination_test.rs`, scaled up for 20x the settlement count. +/// This is a "did something go quadratic / did a write sneak into a read path" +/// guard, not a tuned budget. +const SCAN_INSTRUCTION_CEILING: u64 = 2_000_000_000; + +/// Builds a treasury holding `n` proposed (still-`Pending`) settlements, each to +/// a fresh merchant address, and returns the client plus admin. The budget is +/// reset to unlimited first so constructing the history does not itself trap. +fn treasury_with_history(env: &Env, n: u64) -> (TreasuryContractClient<'static>, Address) { + env.mock_all_auths(); + env.cost_estimate().budget().reset_unlimited(); + + let admin = Address::generate(env); + let contract_id = env.register_contract(None, TreasuryContract); + let client = TreasuryContractClient::new(env, &contract_id); + // Threshold 1 so a proposal alone is enough to make a settlement executable; + // keeps the "execute some of the history" setup below single-call per id. + client.initialize(&admin, &1, &soroban_sdk::Vec::new(env)); + + for _ in 0..n { + let merchant = Address::generate(env); + client.propose_settlement(&admin, &merchant, &1_000_000); + } + + (client, admin) +} + +/// Measures the CPU instruction cost of a single `get_pending_settlements_page` +/// call over a treasury carrying `history` pending settlements, and sanity-checks +/// the returned page. +fn bench_page_scan(history: u64, start: u64, limit: u64) -> (u64, u32) { + let env = Env::default(); + let (client, _admin) = treasury_with_history(&env, history); + + env.cost_estimate().budget().reset_tracker(); + let page = client.get_pending_settlements_page(&start, &limit); + let instructions = env.cost_estimate().budget().cpu_instruction_cost(); + + // Every returned entry must be a real, still-pending settlement whose id + // falls after the requested `start` offset within the pending sequence. + let expected_len = limit.min(history.saturating_sub(start)) as u32; + assert_eq!( + page.len(), + expected_len, + "page(start={start}, limit={limit}) over {history} pending settlements \ + returned {} entries, expected {expected_len}", + page.len() + ); + for (i, s) in page.iter().enumerate() { + assert_eq!(s.status, SettlementStatus::Pending); + assert_eq!( + s.id, + start + 1 + i as u64, + "page entry {i} had id {} but the {i}-th pending settlement after \ + offset {start} should be id {}", + s.id, + start + 1 + i as u64 + ); + } + + (instructions, page.len()) +} + +/// `get_pending_settlements_page` must stay correct at 1000+ settlements: a +/// first page, a deep mid-history page, and a page whose window runs off the +/// end of the history all return exactly the right settlements. +#[test] +fn page_scan_is_correct_at_1000_plus_settlements() { + let env = Env::default(); + let (client, _admin) = treasury_with_history(&env, LARGE_HISTORY); + + // First page. + let first = client.get_pending_settlements_page(&0, &25); + assert_eq!(first.len(), 25); + assert_eq!(first.get(0).unwrap().id, 1); + assert_eq!(first.get(24).unwrap().id, 25); + + // Deep mid-history page - the scan cost of reaching this offset is O(count) + // regardless of how far in it is; correctness must not degrade with depth. + let deep = client.get_pending_settlements_page(&900, &25); + assert_eq!(deep.len(), 25); + assert_eq!(deep.get(0).unwrap().id, 901); + assert_eq!(deep.get(24).unwrap().id, 925); + + // Window overruns the end of the history. + let tail = client.get_pending_settlements_page(&(LARGE_HISTORY - 10), &50); + assert_eq!(tail.len(), 10); + assert_eq!(tail.get(0).unwrap().id, LARGE_HISTORY - 9); + assert_eq!(tail.get(9).unwrap().id, LARGE_HISTORY); + + // Start past the end returns cleanly. + assert_eq!( + client + .get_pending_settlements_page(&(LARGE_HISTORY + 500), &50) + .len(), + 0 + ); +} + +/// Executed settlements interspersed through a 1000+ history are skipped by the +/// pagination scan, and the returned page is still a contiguous run of pending +/// ids in order. +#[test] +fn page_scan_skips_executed_entries_at_scale() { + let env = Env::default(); + env.mock_all_auths(); + env.cost_estimate().budget().reset_unlimited(); + + let admin = Address::generate(&env); + let contract_id = env.register_contract(None, TreasuryContract); + let client = TreasuryContractClient::new(&env, &contract_id); + client.initialize(&admin, &1, &soroban_sdk::Vec::new(&env)); + + let token_id = env.register_stellar_asset_contract(admin.clone()); + soroban_sdk::token::StellarAssetClient::new(&env, &token_id) + .mint(&contract_id, &10_000_000_000); + + // Execute every 10th settlement; the rest stay pending. + let mut executed_ids = std::vec::Vec::new(); + for i in 1..=LARGE_HISTORY { + let merchant = Address::generate(&env); + let sid = client.propose_settlement(&admin, &merchant, &1_000_000); + if i % 10 == 0 { + client.execute_settlement(&admin, &sid, &token_id); + executed_ids.push(sid); + } + } + + let pending_total = LARGE_HISTORY - executed_ids.len() as u64; + let all_pending = client.get_pending_settlements_page(&0, &u64::MAX); + assert_eq!(all_pending.len() as u64, pending_total); + for s in all_pending.iter() { + assert_eq!(s.status, SettlementStatus::Pending); + assert!( + s.id % 10 != 0, + "settlement {} is a multiple of 10 and should have been executed \ + and therefore skipped by the pending page", + s.id + ); + } + + // A mid-history page still comes back as an ordered, contiguous run of the + // surviving pending ids. + let mid = client.get_pending_settlements_page(&400, &20); + assert_eq!(mid.len(), 20); + let ids: std::vec::Vec = mid.iter().map(|s| s.id).collect(); + let mut sorted = ids.clone(); + sorted.sort_unstable(); + assert_eq!(ids, sorted, "page ids must be returned in ascending order"); + assert!(ids.iter().all(|id| id % 10 != 0)); +} + +/// Records the measured instruction cost of scanning the *whole* history to +/// return a small (50-entry) tail page, at 500 and at 1000+ settlements. Using a +/// small fixed page isolates the O(count) scan cost from the cost of marshalling +/// a large return value. The scan should grow roughly linearly with history +/// size; a quadratic regression would roughly quadruple rather than double. +#[test] +fn page_scan_cost_scales_with_history_size() { + let (cost_half, len_half) = bench_page_scan(HALF_HISTORY, HALF_HISTORY - 50, 50); + let (cost_full, len_full) = bench_page_scan(LARGE_HISTORY, LARGE_HISTORY - 50, 50); + + eprintln!("get_pending_settlements_page whole-history scan cost (50-entry tail page):"); + eprintln!(" {HALF_HISTORY:>5} settlements -> {cost_half} instructions ({len_half} returned)"); + eprintln!(" {LARGE_HISTORY:>5} settlements -> {cost_full} instructions ({len_full} returned)"); + eprintln!( + " cost ratio (full/half) = {:.2} (history ratio = {:.2})", + cost_full as f64 / cost_half as f64, + LARGE_HISTORY as f64 / HALF_HISTORY as f64 + ); + + assert_eq!(len_half, 50); + assert_eq!(len_full, 50); + + assert!( + cost_full <= SCAN_INSTRUCTION_CEILING, + "whole-history scan over {LARGE_HISTORY} settlements used {cost_full} \ + instructions, over the {SCAN_INSTRUCTION_CEILING} sanity ceiling" + ); + + // History doubled (2.0x). The measured ratio on the test host is noticeably + // above 2x (observed ~3x) because each persistent read itself gets slightly + // more expensive as the storage map grows - the scan is still linear in the + // number of entries, not quadratic. This guard is deliberately loose: it is + // a runaway-regression net (a genuine O(n^2) change would blow well past + // this), not a tight linearity assertion. The printed numbers above are the + // artifact this test exists to produce. + let cost_ratio = cost_full as f64 / cost_half as f64; + assert!( + cost_ratio < 6.0, + "pagination scan cost grew {cost_ratio:.2}x when history doubled \ + ({HALF_HISTORY} -> {LARGE_HISTORY}); >6x means the scan is no longer \ + close to linear in history size" + ); +} + +/// Finding, recorded here rather than asserted away: `get_pending_settlements_page` +/// early-`break`s as soon as the requested page is full, so a *shallow* page +/// (`start` near 0) is cheap while a *deep* page (`start` near the end of a large +/// history) costs proportionally to `start + limit` - i.e. it approaches the cost +/// of a full-history scan. Paginating a UI to the end of a 1000+ settlement +/// history is therefore not a cheap operation, and callers that must repeatedly +/// reach deep offsets pay O(count) each time. This is the same "cost scales with +/// total accumulated history" shape as the `resolve_dispute` concern in +/// `resolve_dispute_dos_test.rs`; a follow-up that maintains a compacted pending +/// index would remove it. This test pins the current behaviour so a future +/// change either preserves it deliberately or is noticed here. +#[test] +fn page_scan_cost_grows_with_offset_depth() { + let (cost_shallow, _) = bench_page_scan(LARGE_HISTORY, 0, 20); + let (cost_deep, _) = bench_page_scan(LARGE_HISTORY, LARGE_HISTORY - 20, 20); + + eprintln!("get_pending_settlements_page offset-depth cost (history = {LARGE_HISTORY}):"); + eprintln!(" start=0 (shallow) -> {cost_shallow} instructions"); + eprintln!( + " start={:<4} (deep) -> {cost_deep} instructions ({:.1}x the shallow cost)", + LARGE_HISTORY - 20, + cost_deep as f64 / cost_shallow as f64 + ); + + // A shallow page reads ~limit entries and stops; a deep page reads ~count. + // Over a 1000-entry history that is a large, deliberate gap. + assert!( + cost_deep > cost_shallow * 5, + "expected a deep-offset page to cost far more than a shallow one because \ + the scan runs from id 1 every call (deep {cost_deep} vs shallow \ + {cost_shallow}); if this ever fails because deep pages got cheap, that \ + is a welcome fix worth documenting rather than a bug in this test" + ); + assert!( + cost_deep <= SCAN_INSTRUCTION_CEILING, + "deep-offset page over {LARGE_HISTORY} settlements used {cost_deep} \ + instructions, over the {SCAN_INSTRUCTION_CEILING} sanity ceiling" + ); +} + +/// `resolve_dispute` cost with a large *settlement* history sitting on the +/// instance as background load. `resolve_dispute`'s own hold-release scan is +/// bounded by dispute count (covered in depth by `resolve_dispute_dos_test.rs`); +/// this test pins the complementary dimension the rotation-race issue cares +/// about - that a treasury already carrying 1000+ settlements still resolves a +/// dispute within budget - and records the number. +#[test] +fn resolve_dispute_cost_with_1000_plus_settlement_history() { + let env = Env::default(); + let (client, admin) = treasury_with_history(&env, LARGE_HISTORY); + + let claimant = Address::generate(&env); + let merchant = Address::generate(&env); + + // Raise a dispute against a real settlement from the middle of the history. + let target_settlement: u64 = LARGE_HISTORY / 2; + let did = client.raise_dispute( + &claimant, + &target_settlement, + &merchant, + &500_000, + &u64::MAX, + ); + assert_eq!( + client.get_settlement(&target_settlement).status, + SettlementStatus::OnHold, + "raising a dispute against a pending settlement must place it on hold" + ); + + env.cost_estimate().budget().reset_tracker(); + client.resolve_dispute(&admin, &did, &true); + let instructions = env.cost_estimate().budget().cpu_instruction_cost(); + + eprintln!( + "resolve_dispute with {LARGE_HISTORY} settlements of background history: \ + {instructions} instructions" + ); + + assert_eq!( + client.get_settlement(&target_settlement).status, + SettlementStatus::Pending, + "resolving the only open dispute must release the settlement hold" + ); + assert!( + instructions <= SCAN_INSTRUCTION_CEILING, + "resolve_dispute with {LARGE_HISTORY} settlements of history used \ + {instructions} instructions, over the {SCAN_INSTRUCTION_CEILING} ceiling" + ); + + // Every other settlement in the history is untouched by the dispute cycle. + assert_eq!(client.get_settlement(&1).status, SettlementStatus::Pending); + assert_eq!( + client.get_settlement(&LARGE_HISTORY).status, + SettlementStatus::Pending + ); +} + +extern crate std; diff --git a/contracts/treasury/tests/lifecycle_chaos_test.rs b/contracts/treasury/tests/lifecycle_chaos_test.rs new file mode 100644 index 0000000..189caa9 --- /dev/null +++ b/contracts/treasury/tests/lifecycle_chaos_test.rs @@ -0,0 +1,538 @@ +//! Chaos-style failure injection across the full settlement lifecycle (#468). +//! +//! Builds on issue #83's end-to-end lifecycle +//! (`tests/tests/full_lifecycle_smoke_test.rs::full_lifecycle_happy_path`): +//! +//! ```text +//! create_invoice -> mark_paid -> propose_settlement -> approve_settlement +//! -> compliance.is_allowed -> execute_settlement (token transfer) +//! -> release_escrow +//! ``` +//! +//! #83 exercises that whole chain once, on the happy path. Isolated unit tests +//! elsewhere cover individual failure modes, but nothing replays *this* full +//! multi-contract flow while deliberately forcing exactly one cross-contract +//! call boundary to fail. This test does: it runs the lifecycle once per +//! boundary, injects a failure at that boundary, and asserts the system is left +//! in a consistent, non-corrupt, recoverable state every time. +//! +//! Boundaries exercised (`FailurePoint`): +//! * `None` - control: the happy path still completes +//! * `ComplianceGate` - compliance.is_allowed returns false +//! * `ExecuteThresholdNotMet` - treasury.execute_settlement: quorum missing +//! * `ExecuteTokenNotAllowed` - treasury.execute_settlement: token off allowlist +//! * `ExecuteSettlementOnHold` - treasury.execute_settlement: settlement disputed +//! * `ReleaseEscrow` - invoice.release_escrow: invoice contract paused +//! +//! Universal invariants asserted after every run, whichever boundary failed +//! (see [`assert_consistent`]): +//! 1. Token conservation: `treasury_balance + merchant_balance == minted total`. +//! 2. All-or-nothing payout: the merchant holds either the whole amount or +//! nothing - never a partial transfer. +//! 3. The merchant is paid *iff* the settlement reached `Executed`. +//! 4. Settlement status is always `Pending` / `OnHold` / `Executed` - never a +//! partial/limbo value. +//! 5. Invoice status is always `Paid` or `Released` - a failed `release_escrow` +//! leaves the invoice cleanly retryable, not wedged. +//! 6. The invoice only reaches `Released` when the settlement is `Executed`. +//! +//! Any divergence found here should be filed as a follow-up issue rather than +//! silently patched in this test file. + +use invoice::{ + InvoiceContract, InvoiceContractClient, InvoiceError, InvoiceStatus, MaybeAddress, MaybeBytes, +}; +use soroban_sdk::{contract, contractimpl, testutils::Address as _, Address, Env}; +use std::panic::{catch_unwind, AssertUnwindSafe}; + +use compliance::{ComplianceContract, ComplianceContractClient}; +use treasury::{SettlementStatus, TreasuryContract, TreasuryContractClient}; + +/// Minimal in-crate token double, identical in shape to the one used by #83's +/// `full_lifecycle_smoke_test.rs`. `mock_all_auths` covers the `require_auth`. +mod test_token { + use soroban_sdk::{contract, contractimpl, Address, Env}; + + #[contract] + pub struct TestToken; + + #[contractimpl] + impl TestToken { + pub fn mint(env: Env, to: Address, amount: i128) { + let key = ("bal", to.clone()); + let bal: i128 = env.storage().persistent().get(&key).unwrap_or(0); + env.storage().persistent().set(&key, &(bal + amount)); + } + + pub fn balance(env: Env, of: Address) -> i128 { + let key = ("bal", of); + env.storage().persistent().get(&key).unwrap_or(0) + } + + pub fn transfer(env: Env, from: Address, to: Address, amount: i128) { + from.require_auth(); + let from_key = ("bal", from.clone()); + let to_key = ("bal", to.clone()); + let from_bal: i128 = env.storage().persistent().get(&from_key).unwrap_or(0); + let to_bal: i128 = env.storage().persistent().get(&to_key).unwrap_or(0); + env.storage() + .persistent() + .set(&from_key, &(from_bal - amount)); + env.storage().persistent().set(&to_key, &(to_bal + amount)); + } + } +} + +use test_token::{TestToken, TestTokenClient}; + +/// Mirrors #83's `ComplianceGatedSettlement` workflow contract: enforces the +/// compliance gate, then calls `execute_settlement`. Panics (rather than +/// returning `Err`) on either failure so the test can catch the boundary +/// failure with `catch_unwind`, exactly as `full_lifecycle_smoke_test.rs` does. +#[contract] +pub struct ChaosWorkflow; + +#[contractimpl] +impl ChaosWorkflow { + pub fn execute( + env: Env, + compliance_id: Address, + treasury_id: Address, + settlement_id: u64, + token_id: Address, + merchant: Address, + ) { + let compliance = ComplianceContractClient::new(&env, &compliance_id); + if !compliance.is_allowed(&merchant) { + panic!("compliance gate: merchant not allowed"); + } + let treasury = TreasuryContractClient::new(&env, &treasury_id); + treasury.execute_settlement(&env.current_contract_address(), &settlement_id, &token_id); + } +} + +const AMOUNT: i128 = 10_000_000; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum FailurePoint { + /// Control: no failure injected, the whole lifecycle should complete. + None, + /// `compliance.is_allowed(merchant)` returns false (merchant never allowed). + ComplianceGate, + /// `execute_settlement` rejects: approval weight never reaches threshold. + ExecuteThresholdNotMet, + /// `execute_settlement` rejects: the settlement token is not on a non-empty + /// treasury allowlist. + ExecuteTokenNotAllowed, + /// `execute_settlement` rejects: the settlement was put `OnHold` by a dispute. + ExecuteSettlementOnHold, + /// `invoice.release_escrow` rejects: the invoice contract is paused. The + /// settlement side has already completed successfully at this point. + ReleaseEscrow, +} + +const ALL_FAILURE_POINTS: [FailurePoint; 6] = [ + FailurePoint::None, + FailurePoint::ComplianceGate, + FailurePoint::ExecuteThresholdNotMet, + FailurePoint::ExecuteTokenNotAllowed, + FailurePoint::ExecuteSettlementOnHold, + FailurePoint::ReleaseEscrow, +]; + +/// State observed at the moment the injected boundary was reached (before any +/// recovery attempt). +#[derive(Debug)] +struct Outcome { + settlement_status: SettlementStatus, + invoice_status: InvoiceStatus, + treasury_balance: i128, + merchant_balance: i128, + minted_total: i128, + /// Whether the injected cross-contract call boundary actually failed. + injected_boundary_failed: bool, +} + +struct Ctx { + env: Env, + admin: Address, + merchant: Address, + payer: Address, + signer2: Address, + invoice: InvoiceContractClient<'static>, + treasury: TreasuryContractClient<'static>, + treasury_id: Address, + compliance_id: Address, + token: TestTokenClient<'static>, + token_id: Address, + wf_id: Address, +} + +fn setup() -> Ctx { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let merchant = Address::generate(&env); + let payer = Address::generate(&env); + let signer2 = Address::generate(&env); + + let invoice_id = env.register_contract(None, InvoiceContract); + let invoice = InvoiceContractClient::new(&env, &invoice_id); + invoice.initialize(&admin); + + // Threshold 2, admin weight 1 (set by initialize), plus a second weight-1 + // signer. A proposal alone is then *not* enough to execute - so skipping the + // second approval is a real `ExecuteThresholdNotMet` injection. + let treasury_id = env.register_contract(None, TreasuryContract); + let treasury = TreasuryContractClient::new(&env, &treasury_id); + treasury.initialize(&admin, &2, &soroban_sdk::Vec::new(&env)); + treasury.set_signer(&admin, &signer2, &1); + + let compliance_id = env.register_contract(None, ComplianceContract); + ComplianceContractClient::new(&env, &compliance_id).initialize(&admin); + + let token_id = env.register_contract(None, TestToken); + let token = TestTokenClient::new(&env, &token_id); + + let wf_id = env.register_contract(None, ChaosWorkflow); + treasury.set_signer(&admin, &wf_id, &1); + + Ctx { + env, + admin, + merchant, + payer, + signer2, + invoice, + treasury, + treasury_id, + compliance_id, + token, + token_id, + wf_id, + } +} + +/// Runs #83's lifecycle once, injecting `fp` at its boundary, and reports the +/// observed cross-contract state. +fn run_lifecycle(fp: FailurePoint) -> Outcome { + let ctx = setup(); + let compliance = ComplianceContractClient::new(&ctx.env, &ctx.compliance_id); + + // 1. create_invoice -> Pending + let inv_id = ctx.invoice.create_invoice( + &ctx.merchant, + &AMOUNT, + &(AMOUNT + 250_000), + &3600, + &MaybeBytes::None, + &MaybeBytes::None, + &0, + &MaybeAddress::None, + ); + assert_eq!( + ctx.invoice.get_invoice(&inv_id).status, + InvoiceStatus::Pending + ); + + // 2. mark_paid -> Paid + ctx.invoice.mark_paid( + &ctx.admin, + &inv_id, + &ctx.payer, + &MaybeBytes::None, + &MaybeAddress::None, + ); + assert_eq!(ctx.invoice.get_invoice(&inv_id).status, InvoiceStatus::Paid); + + // 3. fund the treasury + ctx.token.mint(&ctx.treasury_id, &AMOUNT); + let minted_total = AMOUNT; + + // 4. propose_settlement -> Pending (proposer = admin, weight 1) + let settlement_id = ctx + .treasury + .propose_settlement(&ctx.admin, &ctx.merchant, &AMOUNT); + assert_eq!( + ctx.treasury.get_settlement(&settlement_id).status, + SettlementStatus::Pending + ); + + // 5. approve_settlement -> weight 2 == threshold, UNLESS we are injecting a + // threshold failure at the execute boundary. + if fp != FailurePoint::ExecuteThresholdNotMet { + ctx.treasury + .approve_settlement(&ctx.signer2, &settlement_id); + } + + // 6. compliance gate: allow the merchant for every case EXCEPT the + // compliance-gate injection. + if fp != FailurePoint::ComplianceGate { + compliance.allow_address(&ctx.admin, &ctx.merchant); + } + + // Boundary-specific pre-conditions for the two remaining execute-failure + // injections. + match fp { + FailurePoint::ExecuteTokenNotAllowed => { + // Non-empty allowlist that does not contain our settlement token. + let other_token = ctx.env.register_contract(None, TestToken); + ctx.treasury.add_allowed_token(&ctx.admin, &other_token); + } + FailurePoint::ExecuteSettlementOnHold => { + let claimant = Address::generate(&ctx.env); + ctx.treasury + .raise_dispute(&claimant, &settlement_id, &ctx.merchant, &1, &u64::MAX); + assert_eq!( + ctx.treasury.get_settlement(&settlement_id).status, + SettlementStatus::OnHold + ); + } + _ => {} + } + + // 7. execute_settlement, via the compliance-gated workflow (as in #83). + let exec_result = catch_unwind(AssertUnwindSafe(|| { + ChaosWorkflowClient::new(&ctx.env, &ctx.wf_id).execute( + &ctx.compliance_id, + &ctx.treasury_id, + &settlement_id, + &ctx.token_id, + &ctx.merchant, + ); + })); + + let execute_boundary_failed = exec_result.is_err(); + + // 8. release_escrow -> Released. Injection: pause the invoice contract first + // so the call is rejected with `ContractPaused`. + let mut release_boundary_failed = false; + let execute_succeeded = !execute_boundary_failed; + if execute_succeeded { + if fp == FailurePoint::ReleaseEscrow { + ctx.invoice.pause(&ctx.admin); + } + match ctx.invoice.try_release_escrow(&ctx.admin, &inv_id) { + Ok(Ok(())) => {} + Err(Ok(e)) => { + release_boundary_failed = true; + assert_eq!( + e, + InvoiceError::ContractPaused, + "release_escrow should fail only because the invoice is paused" + ); + } + other => panic!("unexpected release_escrow result: {other:?}"), + } + } + + let injected_boundary_failed = match fp { + FailurePoint::None => false, + FailurePoint::ComplianceGate + | FailurePoint::ExecuteThresholdNotMet + | FailurePoint::ExecuteTokenNotAllowed + | FailurePoint::ExecuteSettlementOnHold => execute_boundary_failed, + FailurePoint::ReleaseEscrow => release_boundary_failed, + }; + + Outcome { + settlement_status: ctx.treasury.get_settlement(&settlement_id).status, + invoice_status: ctx.invoice.get_invoice(&inv_id).status, + treasury_balance: ctx.token.balance(&ctx.treasury_id), + merchant_balance: ctx.token.balance(&ctx.merchant), + minted_total, + injected_boundary_failed, + } +} + +/// The invariants that must hold no matter which boundary was forced to fail. +fn assert_consistent(fp: FailurePoint, o: &Outcome) { + // 1. Token conservation - nothing minted or burned by a partial failure. + assert_eq!( + o.treasury_balance + o.merchant_balance, + o.minted_total, + "{fp:?}: token conservation violated (treasury {} + merchant {} != minted {})", + o.treasury_balance, + o.merchant_balance, + o.minted_total + ); + + // 2. All-or-nothing payout - never a partially applied transfer. + assert!( + o.merchant_balance == 0 || o.merchant_balance == o.minted_total, + "{fp:?}: merchant holds a partial balance {} (expected 0 or {})", + o.merchant_balance, + o.minted_total + ); + + // 3. Merchant is paid iff the settlement executed. + assert_eq!( + o.merchant_balance == o.minted_total, + o.settlement_status == SettlementStatus::Executed, + "{fp:?}: merchant-paid ({}) disagrees with settlement Executed ({:?})", + o.merchant_balance == o.minted_total, + o.settlement_status + ); + + // 4. Settlement never lands in a partial/limbo status. + assert!( + matches!( + o.settlement_status, + SettlementStatus::Pending | SettlementStatus::OnHold | SettlementStatus::Executed + ), + "{fp:?}: settlement in unexpected status {:?}", + o.settlement_status + ); + + // 5. Invoice never wedges - always Paid (retryable) or Released. + assert!( + matches!( + o.invoice_status, + InvoiceStatus::Paid | InvoiceStatus::Released + ), + "{fp:?}: invoice in unexpected status {:?}", + o.invoice_status + ); + + // 6. Escrow is only released once the settlement has actually executed. + if o.invoice_status == InvoiceStatus::Released { + assert_eq!( + o.settlement_status, + SettlementStatus::Executed, + "{fp:?}: invoice Released while settlement is {:?}", + o.settlement_status + ); + } +} + +#[test] +fn control_happy_path_completes() { + let o = run_lifecycle(FailurePoint::None); + assert_consistent(FailurePoint::None, &o); + assert!(!o.injected_boundary_failed); + assert_eq!(o.settlement_status, SettlementStatus::Executed); + assert_eq!(o.invoice_status, InvoiceStatus::Released); + assert_eq!(o.merchant_balance, o.minted_total); + assert_eq!(o.treasury_balance, 0); +} + +#[test] +fn compliance_gate_failure_leaves_state_consistent() { + let o = run_lifecycle(FailurePoint::ComplianceGate); + assert_consistent(FailurePoint::ComplianceGate, &o); + assert!( + o.injected_boundary_failed, + "compliance gate should have failed" + ); + assert_eq!(o.settlement_status, SettlementStatus::Pending); + assert_eq!(o.invoice_status, InvoiceStatus::Paid); + assert_eq!(o.merchant_balance, 0); + assert_eq!(o.treasury_balance, o.minted_total); +} + +#[test] +fn execute_threshold_not_met_leaves_state_consistent() { + let o = run_lifecycle(FailurePoint::ExecuteThresholdNotMet); + assert_consistent(FailurePoint::ExecuteThresholdNotMet, &o); + assert!( + o.injected_boundary_failed, + "execute_settlement should have failed" + ); + assert_eq!(o.settlement_status, SettlementStatus::Pending); + assert_eq!(o.invoice_status, InvoiceStatus::Paid); + assert_eq!(o.merchant_balance, 0); + assert_eq!(o.treasury_balance, o.minted_total); +} + +#[test] +fn execute_token_not_allowed_leaves_state_consistent() { + let o = run_lifecycle(FailurePoint::ExecuteTokenNotAllowed); + assert_consistent(FailurePoint::ExecuteTokenNotAllowed, &o); + assert!( + o.injected_boundary_failed, + "execute_settlement should have failed" + ); + assert_eq!(o.settlement_status, SettlementStatus::Pending); + assert_eq!(o.invoice_status, InvoiceStatus::Paid); + assert_eq!(o.merchant_balance, 0); + assert_eq!(o.treasury_balance, o.minted_total); +} + +#[test] +fn execute_settlement_on_hold_leaves_state_consistent() { + let o = run_lifecycle(FailurePoint::ExecuteSettlementOnHold); + assert_consistent(FailurePoint::ExecuteSettlementOnHold, &o); + assert!( + o.injected_boundary_failed, + "execute_settlement should have failed" + ); + assert_eq!(o.settlement_status, SettlementStatus::OnHold); + assert_eq!(o.invoice_status, InvoiceStatus::Paid); + assert_eq!(o.merchant_balance, 0); + assert_eq!(o.treasury_balance, o.minted_total); +} + +#[test] +fn release_escrow_failure_leaves_state_consistent_and_is_recoverable() { + let o = run_lifecycle(FailurePoint::ReleaseEscrow); + assert_consistent(FailurePoint::ReleaseEscrow, &o); + assert!( + o.injected_boundary_failed, + "release_escrow should have failed" + ); + // The settlement side completed before the failed release. + assert_eq!(o.settlement_status, SettlementStatus::Executed); + assert_eq!(o.merchant_balance, o.minted_total); + assert_eq!(o.treasury_balance, 0); + // The invoice is left cleanly retryable, not wedged. + assert_eq!(o.invoice_status, InvoiceStatus::Paid); + + // Recovery: unpausing and retrying the same call completes the lifecycle, + // proving the earlier failure did not corrupt or block the invoice. + let ctx = setup(); + let inv_id = ctx.invoice.create_invoice( + &ctx.merchant, + &AMOUNT, + &(AMOUNT + 250_000), + &3600, + &MaybeBytes::None, + &MaybeBytes::None, + &0, + &MaybeAddress::None, + ); + ctx.invoice.mark_paid( + &ctx.admin, + &inv_id, + &ctx.payer, + &MaybeBytes::None, + &MaybeAddress::None, + ); + ctx.invoice.pause(&ctx.admin); + assert!(matches!( + ctx.invoice.try_release_escrow(&ctx.admin, &inv_id), + Err(Ok(InvoiceError::ContractPaused)) + )); + ctx.invoice.unpause(&ctx.admin); + ctx.invoice.release_escrow(&ctx.admin, &inv_id); + assert_eq!( + ctx.invoice.get_invoice(&inv_id).status, + InvoiceStatus::Released + ); +} + +/// Sweep: every boundary in turn leaves the multi-contract system consistent. +#[test] +fn every_boundary_failure_leaves_state_consistent() { + for fp in ALL_FAILURE_POINTS { + let o = run_lifecycle(fp); + assert_consistent(fp, &o); + match fp { + FailurePoint::None => assert!(!o.injected_boundary_failed), + _ => assert!( + o.injected_boundary_failed, + "{fp:?}: expected the injected boundary to fail" + ), + } + } +} diff --git a/contracts/treasury/tests/multisig_quorum_property_test.rs b/contracts/treasury/tests/multisig_quorum_property_test.rs index ff86532..b248476 100644 --- a/contracts/treasury/tests/multisig_quorum_property_test.rs +++ b/contracts/treasury/tests/multisig_quorum_property_test.rs @@ -267,7 +267,9 @@ proptest! { prop_assert_eq!( meets_threshold(accumulated, threshold), accumulated >= threshold, - "meets_threshold must agree with >= for accumulated={accumulated}, threshold={threshold}" + "meets_threshold must agree with >= for accumulated={}, threshold={}", + accumulated, + threshold ); } } diff --git a/contracts/treasury/tests/multisig_version_lock_test.rs b/contracts/treasury/tests/multisig_version_lock_test.rs index 6a12367..9466de6 100644 --- a/contracts/treasury/tests/multisig_version_lock_test.rs +++ b/contracts/treasury/tests/multisig_version_lock_test.rs @@ -98,8 +98,10 @@ fn treasury_error_shape_is_unchanged() { assert_eq!(TreasuryError::InsufficientBalance as u32, 31); assert_eq!(TreasuryError::NotPaused as u32, 32); assert_eq!(TreasuryError::RotationProposalCooldown as u32, 33); - assert_eq!(TreasuryError::WithdrawalLimitExceeded as u32, 34); - assert_eq!(TreasuryError::InvalidSplitRatio as u32, 35); + assert_eq!(TreasuryError::WorkflowNotRegisteredSigner as u32, 34); + assert_eq!(TreasuryError::WithdrawalLimitExceeded as u32, 35); + assert_eq!(TreasuryError::InvalidSplitRatio as u32, 36); + assert_eq!(TreasuryError::ForceCancelNotAllowed as u32, 37); // No wildcard arm: adding, removing, or renaming a variant fails this compile. fn assert_exhaustive(err: TreasuryError) { @@ -137,8 +139,10 @@ fn treasury_error_shape_is_unchanged() { | TreasuryError::InsufficientBalance | TreasuryError::NotPaused | TreasuryError::RotationProposalCooldown + | TreasuryError::WorkflowNotRegisteredSigner | TreasuryError::WithdrawalLimitExceeded - | TreasuryError::InvalidSplitRatio => {} + | TreasuryError::InvalidSplitRatio + | TreasuryError::ForceCancelNotAllowed => {} } } assert_exhaustive(TreasuryError::AlreadyOnHold); diff --git a/crates/multisig/src/lib.rs b/crates/multisig/src/lib.rs index d26aeaf..587fd48 100644 --- a/crates/multisig/src/lib.rs +++ b/crates/multisig/src/lib.rs @@ -46,6 +46,16 @@ pub enum TreasuryError { // gives a first-time deployer no hint that the fix is a `set_signer` call for // the workflow's own address. WorkflowNotRegisteredSigner = 34, + // Appended (not renumbered) to keep discriminants stable; see + // scripts/check-enum-ordering.sh (#74). + // A withdrawal (`withdraw` / `withdraw_all`) would exceed the admin-configured + // per-rolling-window withdrawal limit (#455). + WithdrawalLimitExceeded = 35, + // `resolve_dispute_split` was called with `claimant_bps` outside 0..=10_000 (#456). + InvalidSplitRatio = 36, + // `force_cancel_settlement` was called on a settlement that is neither + // `Pending` nor `OnHold` and therefore cannot be force-cancelled (#457). + ForceCancelNotAllowed = 37, } // Issue #48: reason codes attached to a held settlement; None means not on hold