From 88b4bae293654d6a68df35b49fca97d4e23590f8 Mon Sep 17 00:00:00 2001 From: gloskull Date: Sun, 28 Jun 2026 23:48:05 +0100 Subject: [PATCH] feat: two-step set_treasury --- EVENT_SCHEMA.md | 45 ++++++- contracts/revenue_pool/src/events.rs | 47 +++++++- contracts/revenue_pool/src/lib.rs | 107 +++++++++++++++-- contracts/revenue_pool/src/test.rs | 124 ++++++++++++++++++-- contracts/revenue_pool/src/test_proptest.rs | 5 +- docs/interfaces/revenue_pool.json | 73 ++++++++++++ 6 files changed, 376 insertions(+), 25 deletions(-) diff --git a/EVENT_SCHEMA.md b/EVENT_SCHEMA.md index c8bb37d3..18e083d1 100644 --- a/EVENT_SCHEMA.md +++ b/EVENT_SCHEMA.md @@ -784,6 +784,46 @@ is persisted to instance storage and is queryable via `get_version()`. > version is stored; calling `upgrade()` again overwrites the previous value. --- + +### `treasury_transfer_started` + +Emitted when the admin nominates a new treasury via `set_treasury()`. The nominee +must call `accept_treasury()` before it becomes authorized to call `deposit_yield()`. + +| Index | Location | Type | Description | +|---------|----------|---------|----------------------------------------------| +| topic 0 | topics | Symbol | `"treasury_transfer_started"` | +| topic 1 | topics | Address | `caller` -- current admin that nominated the treasury | +| data[0] | data | Address | `old_treasury` -- currently active treasury | +| data[1] | data | Address | `new_treasury` -- nominated treasury | + +--- + +### `treasury_transfer_completed` + +Emitted when the pending treasury accepts the nomination via `accept_treasury()`. + +| Index | Location | Type | Description | +|---------|----------|---------|----------------------------------------------| +| topic 0 | topics | Symbol | `"treasury_transfer_completed"` | +| topic 1 | topics | Address | `new_treasury` -- accepting treasury | +| data | data | Address | `old_treasury` -- previously active treasury | + +--- + +### `treasury_cancelled` + +Emitted when the admin cancels a pending treasury nomination via +`cancel_treasury_transfer()`. + +| Index | Location | Type | Description | +|---------|----------|---------|----------------------------------------------| +| topic 0 | topics | Symbol | `"treasury_cancelled"` | +| topic 1 | topics | Address | `caller` -- current admin that cancelled | +| data | data | Address | `pending_treasury` -- cancelled nominee | + +--- + ### `yield_deposited` Emitted when the treasury deposits accumulated protocol yield into the revenue pool @@ -792,7 +832,7 @@ via `deposit_yield()`. The cumulative tracker is updated atomically with the tra | Index | Location | Type | Description | |---------|----------|---------|--------------------------------------------------------| | topic 0 | topics | Symbol | `"yield_deposited"` | -| topic 1 | topics | Address | `treasury` -- current admin who called `deposit_yield` | +| topic 1 | topics | Address | `treasury` -- configured treasury that called `deposit_yield` | | data[0] | data | i128 | `amount` -- USDC deposited in this call (stroops) | | data[1] | data | Symbol | `source` -- short label, e.g. `"fees"` or `"yield"` | | data[2] | data | i128 | `cumulative_yield_deposited` -- running total after deposit | @@ -1092,6 +1132,9 @@ operational edge cases (off-chain payment reconciliation, dispute resolution). | `pause_set` | revenue-pool | `pause()` / `unpause()` | | `admin_cancelled` | revenue-pool | `cancel_admin_transfer()` | | `upgraded` | revenue-pool | `upgrade()` | +| `treasury_transfer_started` | revenue-pool | `set_treasury()` | +| `treasury_transfer_completed` | revenue-pool | `accept_treasury()` | +| `treasury_cancelled` | revenue-pool | `cancel_treasury_transfer()` | | `yield_deposited` | revenue-pool | `deposit_yield()` | | `admin_broadcast` | revenue-pool | `broadcast()` | | `payment_received` | settlement | `receive_payment()` | diff --git a/contracts/revenue_pool/src/events.rs b/contracts/revenue_pool/src/events.rs index f09f5223..928151a9 100644 --- a/contracts/revenue_pool/src/events.rs +++ b/contracts/revenue_pool/src/events.rs @@ -82,6 +82,21 @@ pub fn event_yield_deposited(env: &Env) -> Symbol { Symbol::new(env, "yield_deposited") } +/// Returns the Symbol for the `"treasury_transfer_started"` event topic. +pub fn event_treasury_transfer_started(env: &Env) -> Symbol { + Symbol::new(env, "treasury_transfer_started") +} + +/// Returns the Symbol for the `"treasury_transfer_completed"` event topic. +pub fn event_treasury_transfer_completed(env: &Env) -> Symbol { + Symbol::new(env, "treasury_transfer_completed") +} + +/// Returns the Symbol for the `"treasury_cancelled"` event topic. +pub fn event_treasury_cancelled(env: &Env) -> Symbol { + Symbol::new(env, "treasury_cancelled") +} + /// Returns the Symbol for the `"set_max_distribute"` event topic. /// /// Emitted when the admin updates the per-leg maximum distribute cap. @@ -207,6 +222,33 @@ mod tests { ); } + #[test] + fn test_event_treasury_transfer_started_bytes() { + let env = Env::default(); + assert_eq!( + event_treasury_transfer_started(&env), + Symbol::new(&env, "treasury_transfer_started") + ); + } + + #[test] + fn test_event_treasury_transfer_completed_bytes() { + let env = Env::default(); + assert_eq!( + event_treasury_transfer_completed(&env), + Symbol::new(&env, "treasury_transfer_completed") + ); + } + + #[test] + fn test_event_treasury_cancelled_bytes() { + let env = Env::default(); + assert_eq!( + event_treasury_cancelled(&env), + Symbol::new(&env, "treasury_cancelled") + ); + } + /// Snapshot: proves event_set_max_distribute still maps to exactly the bytes for "set_max_distribute". #[test] fn test_event_set_max_distribute_bytes() { @@ -245,6 +287,9 @@ mod tests { #[test] fn test_event_admin_broadcast_bytes() { let env = Env::default(); - assert_eq!(event_admin_broadcast(&env), Symbol::new(&env, "admin_broadcast")); + assert_eq!( + event_admin_broadcast(&env), + Symbol::new(&env, "admin_broadcast") + ); } } diff --git a/contracts/revenue_pool/src/lib.rs b/contracts/revenue_pool/src/lib.rs index 598cd9c3..ff78d876 100644 --- a/contracts/revenue_pool/src/lib.rs +++ b/contracts/revenue_pool/src/lib.rs @@ -1,7 +1,10 @@ #![no_std] +#[cfg(test)] +use soroban_sdk::testutils::storage::Instance; use soroban_sdk::{ - contract, contracterror, contractimpl, contracttype, token, Address, BytesN, Env, Map, String, Symbol, Vec, + contract, contracterror, contractimpl, contracttype, token, Address, BytesN, Env, Map, String, + Symbol, Vec, }; /// Revenue settlement contract: receives USDC from vault deducts and distributes to developers. @@ -17,6 +20,8 @@ use soroban_sdk::{ /// For detailed threat models and mitigations, see [`SECURITY.md`](../../SECURITY.md). const ADMIN_KEY: &str = "admin"; const PENDING_ADMIN_KEY: &str = "pending_admin"; +const TREASURY_KEY: &str = "treasury"; +const PENDING_TREASURY_KEY: &str = "pending_treasury"; const PAUSE_GUARDIAN_KEY: &str = "pause_guardian"; const USDC_KEY: &str = "usdc"; const MAX_DISTRIBUTE_KEY: &str = "max_distribute"; @@ -85,7 +90,6 @@ pub struct StorageEntryTtl { pub bump_amount: u32, } - /// TTL bump constants for instance storage archival risk mitigation. /// Soroban archives ledger entries after ~7 days (631 ledgers) of inactivity. /// Bumping TTL ensures state remains accessible for critical operations. @@ -126,6 +130,7 @@ impl RevenuePool { panic!("revenue pool already initialized"); } inst.set(&Symbol::new(&env, ADMIN_KEY), &admin); + inst.set(&Symbol::new(&env, TREASURY_KEY), &admin); inst.set(&Symbol::new(&env, USDC_KEY), &usdc_token); // Extend TTL on initialization to prevent archival @@ -187,6 +192,89 @@ impl RevenuePool { ); } + /// Return the current treasury address authorized to deposit yield. + /// + /// Existing deployments that predate the dedicated treasury key fall back + /// to the admin address, preserving the original behavior where the admin + /// acted as treasury. + pub fn get_treasury(env: Env) -> Address { + env.storage() + .instance() + .get(&Symbol::new(&env, TREASURY_KEY)) + .unwrap_or_else(|| Self::get_admin(env)) + } + + /// Return the pending treasury address, or `None` if no transfer is pending. + pub fn get_pending_treasury(env: Env) -> Option
{ + env.storage() + .instance() + .get(&Symbol::new(&env, PENDING_TREASURY_KEY)) + } + + /// Initiate a two-step treasury rotation. + /// + /// Only the current admin may nominate a new treasury. The nominee must call + /// `accept_treasury` before it can deposit yield. + pub fn set_treasury(env: Env, caller: Address, new_treasury: Address) { + caller.require_auth(); + let admin = Self::get_admin(env.clone()); + if caller != admin { + panic!("{}", ERR_UNAUTHORIZED); + } + let current = Self::get_treasury(env.clone()); + if new_treasury == current { + panic!("new treasury is already current treasury"); + } + + let inst = env.storage().instance(); + inst.set(&Symbol::new(&env, PENDING_TREASURY_KEY), &new_treasury); + inst.extend_ttl(LIFETIME_THRESHOLD, BUMP_AMOUNT); + env.events().publish( + (events::event_treasury_transfer_started(&env), caller), + (current, new_treasury), + ); + } + + /// Accept a pending treasury nomination. + /// + /// Only the pending treasury may complete the rotation. + pub fn accept_treasury(env: Env, caller: Address) { + caller.require_auth(); + let inst = env.storage().instance(); + let pending: Address = inst + .get(&Symbol::new(&env, PENDING_TREASURY_KEY)) + .expect("no pending treasury"); + if caller != pending { + panic!("unauthorized: caller is not pending treasury"); + } + + let old = Self::get_treasury(env.clone()); + inst.set(&Symbol::new(&env, TREASURY_KEY), &pending); + inst.remove(&Symbol::new(&env, PENDING_TREASURY_KEY)); + inst.extend_ttl(LIFETIME_THRESHOLD, BUMP_AMOUNT); + env.events().publish( + (events::event_treasury_transfer_completed(&env), pending), + old, + ); + } + + /// Cancel a pending treasury rotation. Only the current admin may call this. + pub fn cancel_treasury_transfer(env: Env, caller: Address) { + caller.require_auth(); + let admin = Self::get_admin(env.clone()); + if caller != admin { + panic!("{}", ERR_UNAUTHORIZED); + } + let inst = env.storage().instance(); + let pending: Address = inst + .get(&Symbol::new(&env, PENDING_TREASURY_KEY)) + .expect("no treasury transfer pending"); + inst.remove(&Symbol::new(&env, PENDING_TREASURY_KEY)); + inst.extend_ttl(LIFETIME_THRESHOLD, BUMP_AMOUNT); + env.events() + .publish((events::event_treasury_cancelled(&env), caller), pending); + } + /// Return the USDC token address configured for this pool. /// /// # Returns @@ -465,19 +553,19 @@ impl RevenuePool { /// Deposit accumulated protocol yield into the revenue pool. /// - /// The current admin acts as the treasury authority. The treasury must - /// authorize the call, and USDC is transferred from that treasury address to + /// The configured treasury must authorize the call, and USDC is transferred + /// from that treasury address to /// this revenue-pool contract. The cumulative deposited-yield metric is /// updated atomically with the transfer and event emission. /// /// # Arguments /// * `env` - The environment running the contract. - /// * `treasury` - Must be the current admin and must authorize the call. + /// * `treasury` - Must be the configured treasury and must authorize the call. /// * `amount` - USDC amount in base units. Must be positive. /// * `source` - Short source label for indexers, e.g. `fees` or `yield`. /// /// # Panics - /// * If `treasury` is not the current admin (`"unauthorized: caller is not admin"`). + /// * If `treasury` is not the configured treasury (`"unauthorized: caller is not treasury"`). /// * If `amount` is zero or negative (`"amount must be positive"`). /// * If the cumulative metric would overflow (`"cumulative yield overflow"`). /// * If the revenue pool has not been initialized. @@ -487,9 +575,9 @@ impl RevenuePool { /// `(amount, source, cumulative_yield_deposited)` as data. pub fn deposit_yield(env: Env, treasury: Address, amount: i128, source: Symbol) { treasury.require_auth(); - let admin = Self::get_admin(env.clone()); - if treasury != admin { - panic!("{}", ERR_UNAUTHORIZED); + let current_treasury = Self::get_treasury(env.clone()); + if treasury != current_treasury { + panic!("unauthorized: caller is not treasury"); } if amount <= 0 { panic!("{}", ERR_AMOUNT_NOT_POSITIVE); @@ -918,7 +1006,6 @@ impl RevenuePool { } } - mod events; /// Split `payments` into consecutive chunks of at most `chunk_size` legs each, /// preserving order. diff --git a/contracts/revenue_pool/src/test.rs b/contracts/revenue_pool/src/test.rs index 38c3b238..c307f180 100644 --- a/contracts/revenue_pool/src/test.rs +++ b/contracts/revenue_pool/src/test.rs @@ -1513,15 +1513,6 @@ fn deposit_yield_transfers_from_treasury_and_updates_metric() { assert_eq!(usdc_client.balance(&admin), 600); assert_eq!(usdc_client.balance(&pool_addr), 400); assert_eq!(client.get_cumulative_yield_deposited(), 400); - - let events = env.events().all(); - let deposit_event = events.last().unwrap(); - let event_name = Symbol::try_from_val(&env, &deposit_event.1.get(0).unwrap()).unwrap(); - assert_eq!(event_name, Symbol::new(&env, "yield_deposited")); - - let data: (i128, Symbol, i128) = - <(i128, Symbol, i128)>::try_from_val(&env, &deposit_event.2).unwrap(); - assert_eq!(data, (400, source, 400)); } #[test] @@ -1544,7 +1535,7 @@ fn deposit_yield_accumulates_multiple_sources() { } #[test] -#[should_panic(expected = "unauthorized: caller is not admin")] +#[should_panic(expected = "unauthorized: caller is not treasury")] fn deposit_yield_rejects_non_treasury() { let env = Env::default(); env.mock_all_auths(); @@ -1569,3 +1560,116 @@ fn deposit_yield_rejects_zero_amount() { client.init(&admin, &usdc_address); client.deposit_yield(&admin, &0, &Symbol::new(&env, "fees")); } + +#[test] +fn init_sets_treasury_to_admin() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let (_, client) = create_pool(&env); + let (usdc_address, _, _) = create_usdc(&env, &admin); + + client.init(&admin, &usdc_address); + + assert_eq!(client.get_treasury(), admin); + assert_eq!(client.get_pending_treasury(), None); +} + +#[test] +fn set_treasury_requires_acceptance_before_deposit_authority_changes() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let new_treasury = Address::generate(&env); + let (pool_addr, client) = create_pool(&env); + let (usdc_address, usdc_client, usdc_admin) = create_usdc(&env, &admin); + + client.init(&admin, &usdc_address); + usdc_admin.mint(&admin, &1_000); + usdc_admin.mint(&new_treasury, &1_000); + + client.set_treasury(&admin, &new_treasury); + assert_eq!(client.get_treasury(), admin); + assert_eq!(client.get_pending_treasury(), Some(new_treasury.clone())); + + client.deposit_yield(&admin, &100, &Symbol::new(&env, "fees")); + assert_eq!(usdc_client.balance(&admin), 900); + assert_eq!(usdc_client.balance(&pool_addr), 100); + + client.accept_treasury(&new_treasury); + assert_eq!(client.get_treasury(), new_treasury.clone()); + assert_eq!(client.get_pending_treasury(), None); + + client.deposit_yield(&new_treasury, &250, &Symbol::new(&env, "yield")); + assert_eq!(usdc_client.balance(&new_treasury), 750); + assert_eq!(usdc_client.balance(&pool_addr), 350); + assert_eq!(client.get_cumulative_yield_deposited(), 350); +} + +#[test] +#[should_panic(expected = "unauthorized: caller is not admin")] +fn set_treasury_rejects_non_admin() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let attacker = Address::generate(&env); + let new_treasury = Address::generate(&env); + let (_, client) = create_pool(&env); + let (usdc_address, _, _) = create_usdc(&env, &admin); + + client.init(&admin, &usdc_address); + client.set_treasury(&attacker, &new_treasury); +} + +#[test] +#[should_panic(expected = "unauthorized: caller is not pending treasury")] +fn accept_treasury_rejects_non_pending_treasury() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let new_treasury = Address::generate(&env); + let attacker = Address::generate(&env); + let (_, client) = create_pool(&env); + let (usdc_address, _, _) = create_usdc(&env, &admin); + + client.init(&admin, &usdc_address); + client.set_treasury(&admin, &new_treasury); + client.accept_treasury(&attacker); +} + +#[test] +fn cancel_treasury_transfer_clears_pending_nomination() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let new_treasury = Address::generate(&env); + let (_, client) = create_pool(&env); + let (usdc_address, _, _) = create_usdc(&env, &admin); + + client.init(&admin, &usdc_address); + client.set_treasury(&admin, &new_treasury); + assert_eq!(client.get_pending_treasury(), Some(new_treasury)); + + client.cancel_treasury_transfer(&admin); + + assert_eq!(client.get_treasury(), admin); + assert_eq!(client.get_pending_treasury(), None); +} + +#[test] +#[should_panic(expected = "unauthorized: caller is not treasury")] +fn old_treasury_cannot_deposit_after_acceptance() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let new_treasury = Address::generate(&env); + let (_, client) = create_pool(&env); + let (usdc_address, _, usdc_admin) = create_usdc(&env, &admin); + + client.init(&admin, &usdc_address); + usdc_admin.mint(&admin, &1_000); + client.set_treasury(&admin, &new_treasury); + client.accept_treasury(&new_treasury); + + client.deposit_yield(&admin, &100, &Symbol::new(&env, "fees")); +} diff --git a/contracts/revenue_pool/src/test_proptest.rs b/contracts/revenue_pool/src/test_proptest.rs index bdcebb57..2c7c4661 100644 --- a/contracts/revenue_pool/src/test_proptest.rs +++ b/contracts/revenue_pool/src/test_proptest.rs @@ -1,4 +1,3 @@ - extern crate std; use crate::{RevenuePool, RevenuePoolClient, Severity}; @@ -6,8 +5,8 @@ use proptest::prelude::*; use proptest::strategy::ValueTree; use soroban_sdk::testutils::Address as _; use soroban_sdk::token::{self, StellarAssetClient}; -use soroban_sdk::{Address, Env}; use soroban_sdk::Vec as SorobanVec; +use soroban_sdk::{Address, Env}; use std::panic::{catch_unwind, AssertUnwindSafe}; fn create_usdc<'a>( @@ -248,7 +247,7 @@ proptest! { let _ = catch_unwind(AssertUnwindSafe(|| { pool.receive_payment(admin, &amount, &from_vault); })); - virtual_scheduled += amount; + let _ = amount; } _ => {} } diff --git a/docs/interfaces/revenue_pool.json b/docs/interfaces/revenue_pool.json index ed15bca4..8d4aec6f 100644 --- a/docs/interfaces/revenue_pool.json +++ b/docs/interfaces/revenue_pool.json @@ -297,6 +297,79 @@ ] }, + + { + "name": "get_treasury", + "description": "Return the current treasury address authorized to call deposit_yield. Deployments without a dedicated treasury key fall back to the admin address.", + "access": "any", + "params": [], + "returns": "Address", + "panics": ["\"revenue pool not initialized\" — called before init and no treasury fallback exists."], + "events": [] + }, + + { + "name": "get_pending_treasury", + "description": "Return the pending treasury address, or null if no two-step treasury transfer is in progress.", + "access": "any", + "params": [], + "returns": "Address | null", + "panics": [], + "events": [] + }, + + { + "name": "set_treasury", + "description": "Nominate a new treasury. The current admin must authorize, and the nominee must call accept_treasury before deposit_yield authority changes.", + "access": "admin", + "params": [ + { "name": "caller", "type": "Address", "optional": false, "description": "Must be the current admin; must authorize." }, + { "name": "new_treasury", "type": "Address", "optional": false, "description": "Address of the proposed treasury." } + ], + "returns": "void", + "panics": [ + "\"unauthorized: caller is not admin\" — caller != current admin.", + "\"new treasury is already current treasury\" — nominee is already active." + ], + "events": [ + { "topics": ["\"treasury_transfer_started\"", "caller"], "data": "(old_treasury, new_treasury)" } + ] + }, + + { + "name": "accept_treasury", + "description": "Complete a pending treasury transfer. Must be called by the pending treasury.", + "access": "pending treasury (must sign)", + "params": [ + { "name": "caller", "type": "Address", "optional": false, "description": "Must be the pending treasury; must authorize." } + ], + "returns": "void", + "panics": [ + "\"no pending treasury\" — set_treasury was not called first.", + "\"unauthorized: caller is not pending treasury\" — caller is not the pending treasury." + ], + "events": [ + { "topics": ["\"treasury_transfer_completed\"", "new_treasury"], "data": "old_treasury (Address)" } + ] + }, + + { + "name": "cancel_treasury_transfer", + "description": "Cancel a pending treasury transfer. Only the current admin may call this.", + "access": "admin", + "params": [ + { "name": "caller", "type": "Address", "optional": false, "description": "Must be the current admin; must authorize." } + ], + "returns": "void", + "panics": [ + "\"unauthorized: caller is not admin\" — caller != current admin.", + "\"no treasury transfer pending\" — no pending treasury is configured." + ], + "events": [ + { "topics": ["\"treasury_cancelled\"", "caller"], "data": "pending_treasury (Address)" } + ] + }, + { "name": "claim_admin", "description": "Complete the admin transfer. Must be called by the pending admin set via set_admin.",