From bc9480ed25a3ed0976b644326088d6b3bed13269 Mon Sep 17 00:00:00 2001 From: job-soft Date: Sat, 29 Aug 2026 01:19:51 +0000 Subject: [PATCH] feat(compliance,treasury): sanctions-oracle design, bulk-op cooldown, withdrawal velocity limit, split dispute resolution - docs(compliance): design external sanctions-list integration pattern (#453) - feat(compliance): add rate limiting to compliance bulk allow/block operations (#454) - feat(treasury): add configurable withdrawal velocity limit (#455) - feat(treasury): add partial (split) dispute resolution outcome (#456) Bumps crates/multisig to 0.3.0 (new TreasuryError variants, DisputeStatus::ResolvedSplit, Dispute::claimant_share_bps, withdrawal-limit DataKey variants) and updates the treasury ABI version-lock test accordingly. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EABdyUmnzWCP8FeX4MefJS --- Cargo.lock | 2 +- contracts/compliance/src/lib.rs | 34 ++++- contracts/treasury/README.md | 6 +- contracts/treasury/src/deposits.rs | 49 +++++++ contracts/treasury/src/disputes.rs | 138 +++++++++++++----- contracts/treasury/src/lib.rs | 37 +++++ .../tests/multisig_version_lock_test.rs | 12 +- crates/multisig/Cargo.toml | 2 +- crates/multisig/src/lib.rs | 17 +++ docs/compliance-sanctions-design.md | 114 +++++++++++++++ 10 files changed, 372 insertions(+), 39 deletions(-) create mode 100644 docs/compliance-sanctions-design.md diff --git a/Cargo.lock b/Cargo.lock index db8f240..4b2f4df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -292,7 +292,7 @@ dependencies = [ [[package]] name = "comebackhere-multisig" -version = "0.2.0" +version = "0.3.0" dependencies = [ "soroban-sdk", ] diff --git a/contracts/compliance/src/lib.rs b/contracts/compliance/src/lib.rs index e3bfc89..abcd9ea 100644 --- a/contracts/compliance/src/lib.rs +++ b/contracts/compliance/src/lib.rs @@ -61,6 +61,10 @@ pub enum DataKey { BlockCount, /// Reserved for a future tiered-compliance model; not yet used by any entrypoint. Tier(Address), + /// Timestamp of the caller's last `bulk_allow_addresses` call. See [`BULK_OP_COOLDOWN_SECS`]. + LastBulkAllow(Address), + /// Timestamp of the caller's last `bulk_block_addresses` call. See [`BULK_OP_COOLDOWN_SECS`]. + LastBulkBlock(Address), } /// Coarse classification of an address's compliance state. @@ -108,7 +112,7 @@ pub struct AddressStatus { /// Primary error type for the compliance contract. /// /// Variants must only be appended at the end (highest numeric value) to preserve -/// on-chain backwards compatibility. Range: 1..=5 (see `ARCHITECTURE.md`). +/// on-chain backwards compatibility. Range: 1..=6 (see `ARCHITECTURE.md`). #[contracterror] #[derive(Copy, Clone, Debug, PartialEq)] #[repr(u32)] @@ -118,6 +122,9 @@ pub enum ContractError { AlreadyInitialized = 3, BatchTooLarge = 4, AddressIndexFull = 5, + /// A bulk allow/block call was made before [`BULK_OP_COOLDOWN_SECS`] elapsed since the + /// caller's previous bulk call (see #454). + BulkOperationCooldown = 6, } /// Upper bound on the number of distinct addresses tracked in `DataKey::AddressIndex`. @@ -131,6 +138,15 @@ const MAX_TRACKED_ADDRESSES: u32 = 50_000; /// the batch caps used elsewhere in the workspace (see #8/#21/#29). pub const MAX_BATCH_SIZE: u32 = 50; +/// Minimum time (seconds) a caller must wait between successive calls to the *same* bulk +/// entrypoint (`bulk_allow_addresses` or `bulk_block_addresses`). `MAX_BATCH_SIZE` bounds how +/// many addresses a single call can affect, but without a time dimension a compromised admin +/// key could still call one bulk entrypoint repeatedly in quick succession and affect an +/// unbounded number of addresses in aggregate (see #454). Tracked separately per entrypoint +/// (rather than shared) so that legitimate admin flows — e.g. allowing a batch and then +/// immediately blocking a different batch — are not penalized for using both in succession. +pub const BULK_OP_COOLDOWN_SECS: u64 = 60; + #[contract] pub struct ComplianceContract; @@ -164,6 +180,7 @@ impl ComplianceContract { ) -> Result<(), ContractError> { Self::require_admin(&env, &admin)?; Self::require_not_paused(&env)?; + Self::check_bulk_op_cooldown(&env, DataKey::LastBulkAllow(admin.clone()))?; if addresses.len() > MAX_BATCH_SIZE { return Err(ContractError::BatchTooLarge); } @@ -348,6 +365,7 @@ impl ComplianceContract { addresses: Vec
, ) -> Result<(), ContractError> { Self::require_admin(&env, &admin)?; + Self::check_bulk_op_cooldown(&env, DataKey::LastBulkBlock(admin.clone()))?; if addresses.len() > MAX_BATCH_SIZE { return Err(ContractError::BatchTooLarge); } @@ -842,6 +860,20 @@ impl ComplianceContract { Err(ContractError::Unauthorized) } + /// Enforces [`BULK_OP_COOLDOWN_SECS`] between successive calls keyed by `key` + /// (a `LastBulkAllow`/`LastBulkBlock` variant), and records `now` as the new + /// last-call timestamp on success. + fn check_bulk_op_cooldown(env: &Env, key: DataKey) -> Result<(), ContractError> { + let now = env.ledger().timestamp(); + if let Some(last) = env.storage().instance().get::<_, u64>(&key) { + if now < last.saturating_add(BULK_OP_COOLDOWN_SECS) { + return Err(ContractError::BulkOperationCooldown); + } + } + env.storage().instance().set(&key, &now); + Ok(()) + } + fn require_not_paused(env: &Env) -> Result<(), ContractError> { let paused: bool = env .storage() diff --git a/contracts/treasury/README.md b/contracts/treasury/README.md index 698f474..8da2cce 100644 --- a/contracts/treasury/README.md +++ b/contracts/treasury/README.md @@ -23,9 +23,13 @@ The Treasury contract manages funds and settlements using a multi-signature appr | `unpause` | `admin` | `admin: Address` | `()` | `Unauthorized` | | `raise_dispute` | `claimant` | `claimant: Address, settlement_id: u64, counterparty: Address, amount: i128` | `u64` | `ContractPaused`, `Unauthorized`, `InvalidAmount` | | `resolve_dispute` | `admin` | `admin: Address, dispute_id: u64, in_favor_of_claimant: bool` | `()` | `Unauthorized`, `ContractPaused`, `DisputeNotFound`, `DisputeAlreadyResolved` | +| `resolve_dispute_split` | `admin` | `admin: Address, dispute_id: u64, claimant_bps: u32, token_contract: Address` | `()` | `Unauthorized`, `ContractPaused`, `DisputeNotFound`, `DisputeAlreadyResolved`, `InvalidSplitRatio` | | `vote_dispute_resolution` | `signer` | `signer: Address, dispute_id: u64, in_favor_of_claimant: bool` | `()` | `ContractPaused`, `UnauthorizedSigner`, `DisputeNotFound`, `DisputeAlreadyResolved`, `ResolutionDirectionMismatch` | | `deposit` | `from` | `from: Address, token_contract: Address, amount: i128` | `()` | `ContractPaused`, `Unauthorized`, `InvalidAmount` | -| `withdraw` | `to` | `to: Address, token_contract: Address, amount: i128` | `()` | `ContractPaused`, `Unauthorized`, `InvalidAmount`, `InsufficientBalance` | +| `withdraw` | `to` | `to: Address, token_contract: Address, amount: i128` | `()` | `ContractPaused`, `Unauthorized`, `InvalidAmount`, `InsufficientBalance`, `DestinationNotAllowed`, `WithdrawalLimitExceeded` | +| `withdraw_all` | `admin` | `admin: Address, token_contract: Address, recipient: Address` | `()` | `Unauthorized`, `NotPaused`, `WithdrawalLimitExceeded` | +| `set_withdrawal_limit` | `admin` | `admin: Address, limit: i128, window_secs: u64` | `()` | `Unauthorized` | +| `get_withdrawal_limit` | None | None | `(i128, u64)` | None | | `add_allowed_token` | `admin` | `admin: Address, token: Address` | `()` | `Unauthorized` | | `remove_allowed_token` | `admin` | `admin: Address, token: Address` | `()` | `Unauthorized` | | `get_balance` | None | `address: Address` | `i128` | None | diff --git a/contracts/treasury/src/deposits.rs b/contracts/treasury/src/deposits.rs index 1d5720a..8f19c2f 100644 --- a/contracts/treasury/src/deposits.rs +++ b/contracts/treasury/src/deposits.rs @@ -51,6 +51,7 @@ impl TreasuryContract { if balance < amount { soroban_sdk::panic_with_error!(env, TreasuryError::InsufficientBalance); } + enforce_withdrawal_limit(&env, &to, amount); balance = balance.checked_sub(amount).unwrap_or_else(|| { soroban_sdk::panic_with_error!(env, TreasuryError::ArithmeticOverflow) }); @@ -90,6 +91,7 @@ impl TreasuryContract { let token_client = token::Client::new(&env, &token_contract); let balance = token_client.balance(&treasury); if balance > 0 { + enforce_withdrawal_limit(&env, &recipient, balance); token_client.transfer(&treasury, &recipient, &balance); } env.events() @@ -97,6 +99,53 @@ impl TreasuryContract { } } +/// Enforces the admin-configured withdrawal-per-window cap (see +/// `TreasuryContract::set_withdrawal_limit`) against `amount` for `tracked`, rolling the +/// window over once it has elapsed. A `limit <= 0` (the default) leaves withdrawals uncapped. +/// Panics: `WithdrawalLimitExceeded`, `ArithmeticOverflow`. +fn enforce_withdrawal_limit(env: &Env, tracked: &Address, amount: i128) { + let limit: i128 = env + .storage() + .instance() + .get(&DataKey::WithdrawalLimitPerWindow) + .unwrap_or(0); + if limit <= 0 { + return; + } + let window_secs: u64 = env + .storage() + .instance() + .get(&DataKey::WithdrawalWindowSecs) + .unwrap_or(0); + let now = env.ledger().timestamp(); + let window_start: u64 = env + .storage() + .persistent() + .get(&DataKey::WithdrawalWindowStart(tracked.clone())) + .unwrap_or(0); + let window_elapsed = window_secs == 0 || now >= window_start.saturating_add(window_secs); + let withdrawn_so_far: i128 = if window_elapsed { + env.storage() + .persistent() + .set(&DataKey::WithdrawalWindowStart(tracked.clone()), &now); + 0 + } else { + env.storage() + .persistent() + .get(&DataKey::WithdrawnInWindow(tracked.clone())) + .unwrap_or(0) + }; + let new_total = withdrawn_so_far.checked_add(amount).unwrap_or_else(|| { + soroban_sdk::panic_with_error!(env, TreasuryError::ArithmeticOverflow) + }); + if new_total > limit { + soroban_sdk::panic_with_error!(env, TreasuryError::WithdrawalLimitExceeded); + } + env.storage() + .persistent() + .set(&DataKey::WithdrawnInWindow(tracked.clone()), &new_total); +} + fn deposit_one(env: &Env, from: &Address, token_contract: &Address, amount: i128) { if amount <= 0 { soroban_sdk::panic_with_error!(env, TreasuryError::InvalidAmount); diff --git a/contracts/treasury/src/disputes.rs b/contracts/treasury/src/disputes.rs index 4becc6e..01ec4f3 100644 --- a/contracts/treasury/src/disputes.rs +++ b/contracts/treasury/src/disputes.rs @@ -4,7 +4,10 @@ use crate::{ TreasuryContractClient, TreasuryError, }; use multisig::{meets_threshold, record_approval, require_authorized_signer}; -use soroban_sdk::{contractimpl, Address, Env, Symbol, Vec}; +use soroban_sdk::{contractimpl, token, Address, Env, Symbol, Vec}; + +/// Basis-points denominator for `resolve_dispute_split`'s ratio (10_000 = 100.00%). +pub const BPS_DENOMINATOR: u32 = 10_000; #[contractimpl] impl TreasuryContract { @@ -57,6 +60,7 @@ impl TreasuryContract { resolution_weight: 0, resolution_for_claimant: false, dispute_expires_at: expires_at, + claimant_share_bps: 0, }; env.storage() .persistent() @@ -140,41 +144,69 @@ impl TreasuryContract { .set(&DataKey::Dispute(dispute_id), &dispute); env.events() .publish((Symbol::new(&env, "dispute_resolved"), dispute_id), dispute); - if let Some(mut settlement) = env + release_settlement_hold_if_no_open_disputes(&env, settlement_id); + } + + /// Resolves an open dispute by splitting `dispute.amount` between claimant and + /// counterparty according to `claimant_bps` (out of [`BPS_DENOMINATOR`]) instead of the + /// binary claimant/counterparty outcome `resolve_dispute` supports (admin-only). + /// + /// Design note: like `resolve_dispute`, this only ever pays out `dispute.amount` (the + /// amount raised with the dispute, not necessarily equal to the associated settlement's + /// own `amount`) directly from the treasury's token balance to the two parties; it does + /// not itself execute the settlement. Releasing the settlement hold below only means the + /// settlement becomes eligible for its own `execute_settlement`/`partially_execute_settlement` + /// payout again — callers must ensure the disputed amount and the settlement's payout are + /// not double-counted when both eventually execute, exactly as with a binary resolution. + /// Panics: `DisputeNotFound`, `DisputeAlreadyResolved`, `ContractPaused`, `InvalidSplitRatio`. + /// Emits: `dispute_resolved_split`. + pub fn resolve_dispute_split( + env: Env, + admin: Address, + dispute_id: u64, + claimant_bps: u32, + token_contract: Address, + ) { + require_admin(&env, &admin); + require_not_paused(&env); + if claimant_bps > BPS_DENOMINATOR { + soroban_sdk::panic_with_error!(env, TreasuryError::InvalidSplitRatio); + } + let mut dispute: Dispute = env .storage() .persistent() - .get::(&DataKey::Settlement(settlement_id)) - { - if settlement.status == SettlementStatus::OnHold { - let dispute_count: u64 = env - .storage() - .instance() - .get(&DataKey::DisputeCount) - .unwrap_or(0); - let mut has_open = false; - let mut i = 1u64; - while i <= dispute_count { - if let Some(d) = env - .storage() - .persistent() - .get::(&DataKey::Dispute(i)) - { - if d.settlement_id == settlement_id && d.status == DisputeStatus::Raised { - has_open = true; - break; - } - } - i += 1; - } - if !has_open { - settlement.status = SettlementStatus::Pending; - settlement.hold_reason = SettlementHoldReason::None; - env.storage() - .persistent() - .set(&DataKey::Settlement(settlement_id), &settlement); - } - } + .get(&DataKey::Dispute(dispute_id)) + .unwrap_or_else(|| soroban_sdk::panic_with_error!(env, TreasuryError::DisputeNotFound)); + if dispute.status != DisputeStatus::Raised { + soroban_sdk::panic_with_error!(env, TreasuryError::DisputeAlreadyResolved); + } + let claimant_amount = dispute + .amount + .checked_mul(claimant_bps as i128) + .unwrap_or_else(|| { + soroban_sdk::panic_with_error!(env, TreasuryError::ArithmeticOverflow) + }) + / BPS_DENOMINATOR as i128; + let counterparty_amount = dispute.amount - claimant_amount; + let treasury = env.current_contract_address(); + let token_client = token::Client::new(&env, &token_contract); + if claimant_amount > 0 { + token_client.transfer(&treasury, &dispute.claimant, &claimant_amount); + } + if counterparty_amount > 0 { + token_client.transfer(&treasury, &dispute.counterparty, &counterparty_amount); } + 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. @@ -231,3 +263,43 @@ impl TreasuryContract { ); } } + +/// Shared by `resolve_dispute` and `resolve_dispute_split`: releases `settlement_id` from +/// `OnHold` back to `Pending` once no `Raised` dispute references it anymore. +fn release_settlement_hold_if_no_open_disputes(env: &Env, settlement_id: u64) { + if let Some(mut settlement) = env + .storage() + .persistent() + .get::(&DataKey::Settlement(settlement_id)) + { + if settlement.status == SettlementStatus::OnHold { + let dispute_count: u64 = env + .storage() + .instance() + .get(&DataKey::DisputeCount) + .unwrap_or(0); + let mut has_open = false; + let mut i = 1u64; + while i <= dispute_count { + if let Some(d) = env + .storage() + .persistent() + .get::(&DataKey::Dispute(i)) + { + if d.settlement_id == settlement_id && d.status == DisputeStatus::Raised { + has_open = true; + break; + } + } + i += 1; + } + if !has_open { + settlement.status = SettlementStatus::Pending; + settlement.hold_reason = SettlementHoldReason::None; + env.storage() + .persistent() + .set(&DataKey::Settlement(settlement_id), &settlement); + } + } + } +} diff --git a/contracts/treasury/src/lib.rs b/contracts/treasury/src/lib.rs index 1d64c07..d8a7a4b 100644 --- a/contracts/treasury/src/lib.rs +++ b/contracts/treasury/src/lib.rs @@ -110,6 +110,43 @@ impl TreasuryContract { env.events() .publish((Symbol::new(&env, "treasury_unpaused"),), admin); } + + /// Configures the maximum amount withdrawable per rolling time window (admin-only). + /// Applies to both `withdraw` (tracked per recipient `to`) and `withdraw_all` (tracked + /// per `recipient`) — see `deposits.rs`. Passing `limit <= 0` disables the cap + /// (the default at initialization is uncapped), trading off protection against a + /// compromised-but-authorized withdrawer for the ability to move arbitrarily large + /// legitimate withdrawals in a single call; admins needing large one-off withdrawals + /// should raise the limit first rather than relying on an uncapped default long-term. + /// Emits: `withdrawal_limit_set`. + pub fn set_withdrawal_limit(env: Env, admin: Address, limit: i128, window_secs: u64) { + require_admin(&env, &admin); + env.storage() + .instance() + .set(&DataKey::WithdrawalLimitPerWindow, &limit); + env.storage() + .instance() + .set(&DataKey::WithdrawalWindowSecs, &window_secs); + env.events().publish( + (Symbol::new(&env, "withdrawal_limit_set"),), + (limit, window_secs), + ); + } + + /// Returns the currently configured `(limit, window_secs)`. `limit <= 0` means uncapped. + pub fn get_withdrawal_limit(env: Env) -> (i128, u64) { + let limit: i128 = env + .storage() + .instance() + .get(&DataKey::WithdrawalLimitPerWindow) + .unwrap_or(0); + let window_secs: u64 = env + .storage() + .instance() + .get(&DataKey::WithdrawalWindowSecs) + .unwrap_or(0); + (limit, window_secs) + } } /// Maximum number of tokens allowed in the allowlist to prevent unbounded storage growth. diff --git a/contracts/treasury/tests/multisig_version_lock_test.rs b/contracts/treasury/tests/multisig_version_lock_test.rs index 7d3f6fa..c09bc39 100644 --- a/contracts/treasury/tests/multisig_version_lock_test.rs +++ b/contracts/treasury/tests/multisig_version_lock_test.rs @@ -24,7 +24,7 @@ use treasury::{ /// Expected version of `crates/multisig` (see its `Cargo.toml`). Bump this only /// alongside a review of every exhaustive match below - if they still compile, /// the ABI-relevant shape of multisig's types is unchanged. -const EXPECTED_MULTISIG_VERSION: &str = "0.2.0"; +const EXPECTED_MULTISIG_VERSION: &str = "0.3.0"; const MULTISIG_CARGO_TOML: &str = include_str!("../../../crates/multisig/Cargo.toml"); @@ -98,6 +98,8 @@ 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); // No wildcard arm: adding, removing, or renaming a variant fails this compile. fn assert_exhaustive(err: TreasuryError) { @@ -134,7 +136,9 @@ fn treasury_error_shape_is_unchanged() { | TreasuryError::DestinationNotAllowed | TreasuryError::InsufficientBalance | TreasuryError::NotPaused - | TreasuryError::RotationProposalCooldown => {} + | TreasuryError::RotationProposalCooldown + | TreasuryError::WithdrawalLimitExceeded + | TreasuryError::InvalidSplitRatio => {} } } assert_exhaustive(TreasuryError::AlreadyOnHold); @@ -168,6 +172,7 @@ fn assert_dispute_status_exhaustive(status: DisputeStatus) { DisputeStatus::ResolvedClaimant => {} DisputeStatus::ResolvedCounterparty => {} DisputeStatus::Expired => {} + DisputeStatus::ResolvedSplit => {} } } @@ -212,6 +217,7 @@ fn dispute_enum_variants_are_unchanged() { DisputeStatus::ResolvedClaimant, DisputeStatus::ResolvedCounterparty, DisputeStatus::Expired, + DisputeStatus::ResolvedSplit, ] { assert_dispute_status_exhaustive(status); } @@ -286,6 +292,7 @@ fn dispute_struct_shape_is_unchanged() { resolution_weight, resolution_for_claimant, dispute_expires_at, + claimant_share_bps, } = dispute; assert_eq!(id, did); @@ -298,6 +305,7 @@ fn dispute_struct_shape_is_unchanged() { assert_eq!(resolution_weight, 0); assert!(!resolution_for_claimant); assert_eq!(dispute_expires_at, 1_000); + assert_eq!(claimant_share_bps, 0); } /// Builds a real `SignerRotationProposal` through the deployed contract and diff --git a/crates/multisig/Cargo.toml b/crates/multisig/Cargo.toml index 8ff25ed..85d0b61 100644 --- a/crates/multisig/Cargo.toml +++ b/crates/multisig/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "comebackhere-multisig" -version = "0.2.0" +version = "0.3.0" edition.workspace = true license.workspace = true repository.workspace = true diff --git a/crates/multisig/src/lib.rs b/crates/multisig/src/lib.rs index c520e95..923e6ad 100644 --- a/crates/multisig/src/lib.rs +++ b/crates/multisig/src/lib.rs @@ -40,6 +40,9 @@ pub enum TreasuryError { InsufficientBalance = 31, NotPaused = 32, RotationProposalCooldown = 33, + // Appended (not renumbered) — see scripts/check-enum-ordering.sh (#74). + WithdrawalLimitExceeded = 34, + InvalidSplitRatio = 35, } // Issue #48: reason codes attached to a held settlement; None means not on hold @@ -72,6 +75,9 @@ pub enum DisputeStatus { ResolvedClaimant, ResolvedCounterparty, Expired, + /// The disputed amount was split between claimant and counterparty; see + /// `Dispute::claimant_share_bps` for the ratio and `resolve_dispute_split` (#456). + ResolvedSplit, } #[contracttype] @@ -100,6 +106,9 @@ pub struct Dispute { pub resolution_weight: u32, pub resolution_for_claimant: bool, pub dispute_expires_at: u64, + /// Claimant's share of `amount` in basis points (0..=10_000), set when `status` is + /// `ResolvedSplit`; meaningless (always 0) for every other status. See #456. + pub claimant_share_bps: u32, } #[contracttype] @@ -141,6 +150,14 @@ pub enum DataKey { WithdrawalAllowlist, LastRotationProposal(Address), PartialApprovedTotal(u64), + /// Admin-configured max amount withdrawable per rolling window; `0` means uncapped (#455). + WithdrawalLimitPerWindow, + /// Window length (seconds) paired with `WithdrawalLimitPerWindow`. + WithdrawalWindowSecs, + /// Start timestamp of the current withdrawal window for a given tracked address. + WithdrawalWindowStart(Address), + /// Amount withdrawn so far within the current window for a given tracked address. + WithdrawnInWindow(Address), } /// Returns the approval weight assigned to `signer`, or `0` if not registered. diff --git a/docs/compliance-sanctions-design.md b/docs/compliance-sanctions-design.md new file mode 100644 index 0000000..0f74d32 --- /dev/null +++ b/docs/compliance-sanctions-design.md @@ -0,0 +1,114 @@ +# Compliance: External Sanctions-List Integration — Design + +> **Status:** Design proposal · No behavioral contract changes accompany this PR. +> **Issue:** #453 +> **Branch:** `docs/compliance-sanctions-oracle-design` + +## 1. Problem + +`compliance`'s allow/block state is entirely admin-managed: every address-level +decision requires an explicit `allow_address` / `block_address` (or their +batch/timed variants) call. There is no mechanism to react to an *external* +sanctions-list update (e.g. a new OFAC addition) without an admin manually +noticing it and manually blocking each newly-sanctioned address — which does +not scale and introduces unbounded delay between a real-world sanctions event +and this protocol reflecting it. + +This document designs an oracle/attestation pattern for feeding external +sanctions-list updates into the existing `block_address` mechanism. It +deliberately does **not** implement real off-chain data-source integration — +that depends on data-source and legal decisions out of scope for a contract +design doc (which feed, which jurisdiction's list, licensing, legal liability +for false positives/negatives). + +## 2. Trust model options considered + +| Option | Description | Trade-off | +|---|---|---| +| **A. Reuse existing `Operator` role** | The already-defined but unused `DataKey::Operator` (see `address_status`'s `require_admin_or_operator`) is granted a new capability: submitting sanctions updates. | No new storage/role plumbing. But `Operator` is a single address today — a single sanctions feed key becomes a single point of compromise, same class of risk as the admin key it's meant to reduce reliance on. | +| **B. Dedicated multisig "attestor set"** | A new weighted-signer set (mirroring `treasury`'s `multisig` crate: `signer_weight`, `record_approval`, `meets_threshold`) requires N-of-M attestors to agree before a sanctions batch applies. | Matches the trust bar of an irreversible, high-consequence action (blocking funds). Adds a second signer registry to a contract that otherwise has none — real complexity cost. | +| **C. External oracle contract** | A separate on-chain contract (or existing bridge/oracle infra) is trusted as a single cross-contract caller, verified via `require_auth()` on the oracle's own contract address. | Cleanest separation of concerns; sanctions-feed logic lives outside `compliance`. Requires that oracle contract to exist and be independently trustworthy/audited — nothing in this repo provides one today. | + +## 3. Recommendation + +**Start with Option A (reuse `Operator`), with an explicit upgrade path to +Option B.** Rationale: + +- `Operator` already exists in storage (`DataKey::Operator`) and already has a + defined trust tier below `Admin` (see `address_status`'s + `require_admin_or_operator`) — extending its authority to sanctions + submission is additive, not a new primitive. +- The realistic near-term operator of a sanctions feed is a single automated + service (e.g. an indexer polling OFAC/SDN and relaying diffs), not a + committee — a single authorized submitter matches that shape today. +- Option B's multisig attestor set is the right answer once more than one + external feed/party needs to co-sign updates, but building it now would be + speculative: `compliance` has no other multisig surface, and introducing one + purely for this feature before a second real attestor exists is scope creep + the issue itself explicitly cautions against ("rather than fully + implementing actual off-chain sanctions-list integration end to end"). +- Option C is the eventual target if/when a shared cross-protocol oracle + contract exists, but nothing in this workspace provides one, and building a + bespoke oracle contract is a separate, larger design effort. + +## 4. Proposed shape (for the follow-up implementation issue) + +Entrypoints, all gated on `require_admin_or_operator` (i.e. callable by +`Admin` or the designated sanctions-feed `Operator`): + +```rust +/// Applies a batch of sanctions-list updates in one call. Each entry blocks an +/// address with a structured `reason` tag (distinct from the free-text +/// `BlockReason` used by manual `block_address`) so blocks originating from an +/// external feed are distinguishable from admin-initiated blocks in +/// `export_snapshot` / indexers. +pub fn apply_sanctions_update( + env: Env, + caller: Address, // admin or operator + addresses: Vec
, + list_id: Symbol, // e.g. "OFAC_SDN"; identifies the source feed + effective_at: u64, // ledger timestamp the update should be attributed to +) -> Result<(), ContractError>; +``` + +Design notes carried over from the existing bulk entrypoints: + +- Reuse `MAX_BATCH_SIZE` and `MAX_TRACKED_ADDRESSES` guards (#8/#21/#29, #48) — + a sanctions batch is not exempt from the storage-growth bound. +- Reuse the `BULK_OP_COOLDOWN_SECS` velocity cap added for `bulk_block_addresses` + (#454) — an external feed is not a more trusted caller than the admin key, + and a compromised or malfunctioning feed should be bounded the same way. +- Emit a distinct event (`sanctions_block_applied`) rather than reusing + `address_blocked`, so downstream indexers can separate "admin decided" from + "external list said so" without parsing `BlockReason` free text. +- `list_id` is stored (new `DataKey::SanctionsSource(Address)`, analogous to + `BlockReason(Address)`) so a later `clear_address` / dispute-style appeal + process can show provenance. +- No automatic *unblock* path is proposed: removing an address from an + external list should still require an explicit admin `clear_address` call — + auto-unblocking on "the feed stopped listing it" is a materially different + trust decision (a missing/failed feed poll must never silently unblock) and + is out of scope here. + +## 5. Threat model + +- **Compromised operator key**: bounded by the existing `MAX_BATCH_SIZE` cap + plus the reused `BULK_OP_COOLDOWN_SECS` velocity cap (#454) — same blast + radius as a compromised admin key today, not worse. +- **Malicious/compromised feed pushing false positives**: the design + intentionally keeps `block_address`'s existing semantics (an address can + always be `clear_address`'d back by the real admin), and does not grant the + operator any new *irreversible* capability — only `Admin` can permanently + clear a block. +- **Stale/absent feed (false negatives)**: out of scope for this contract — + detecting that a feed has stopped publishing is an off-chain monitoring + concern, not something `compliance` can observe on its own. + +## 6. Disposition + +Per this issue's scope, this PR is **design-only**. The `apply_sanctions_update` +entrypoint and `DataKey::SanctionsSource` described in §4 are deferred to a +dedicated follow-up implementation issue, to be reviewed against this document +before any contract code is written — consistent with the issue's own +instruction to "propose the approach in the PR description and get it +reviewed before writing significant contract code."