diff --git a/Cargo.lock b/Cargo.lock index 5dfd7518..aab45588 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -195,6 +195,13 @@ dependencies = [ "generic-array", ] +[[package]] +name = "bridge-compat" +version = "0.1.0" +dependencies = [ + "soroban-sdk", +] + [[package]] name = "bumpalo" version = "3.20.2" @@ -1594,6 +1601,7 @@ dependencies = [ "proptest", "share-price-math", "soroban-sdk", + "vault", ] [[package]] diff --git a/contracts/bridge-compat/src/lib.rs b/contracts/bridge-compat/src/lib.rs index 965d4a60..d22abf1b 100644 --- a/contracts/bridge-compat/src/lib.rs +++ b/contracts/bridge-compat/src/lib.rs @@ -27,8 +27,8 @@ //! - Graceful degradation on provider failures use soroban_sdk::{ - contract, contractclient, contractimpl, contracttype, symbol_short, Address, Bytes, Env, - String, Vec, + contract, contractclient, contracterror, contractimpl, contracttype, symbol_short, Address, + Bytes, Env, String, Vec, }; // ── Error types ──────────────────────────────────────────────────────────── @@ -189,7 +189,7 @@ impl BridgeCompat { env.storage().instance().set( &DataKey::Limits, &TransferLimits { - per_transfer_limit: 1_000_000_000_000, // 1M USDC (6 decimals) + per_transfer_limit: 1_000_000_000_000, // 1M USDC (6 decimals) epoch_volume_limit: 10_000_000_000_000, // 10M USDC per epoch epoch_duration: 86_400, // 24 hours }, @@ -253,10 +253,7 @@ impl BridgeCompat { .set(&DataKey::ProviderNonce, &provider_id); // Set as default if first provider - let has_default = env - .storage() - .instance() - .has(&DataKey::DefaultProvider); + let has_default = env.storage().instance().has(&DataKey::DefaultProvider); if !has_default { env.storage() .instance() @@ -283,19 +280,13 @@ impl BridgeCompat { env.storage() .instance() .set(&DataKey::Provider(provider_id), &provider); - env.events().publish( - (symbol_short!("brgtog"),), - (provider_id, enabled), - ); + env.events() + .publish((symbol_short!("brgtog"),), (provider_id, enabled)); Ok(()) } /// Update a provider's fee. Admin-only. - pub fn set_provider_fee( - env: Env, - provider_id: u32, - fee_bps: i128, - ) -> Result<(), BridgeError> { + pub fn set_provider_fee(env: Env, provider_id: u32, fee_bps: i128) -> Result<(), BridgeError> { Self::require_admin(&env)?; if !(0..=BPS_DENOMINATOR).contains(&fee_bps) { return Err(BridgeError::InvalidAmount); @@ -309,10 +300,7 @@ impl BridgeCompat { } /// Set the default provider. Admin-only. - pub fn set_default_provider( - env: Env, - provider_id: u32, - ) -> Result<(), BridgeError> { + pub fn set_default_provider(env: Env, provider_id: u32) -> Result<(), BridgeError> { Self::require_admin(&env)?; let _ = Self::get_provider(&env, provider_id)?; // validate exists env.storage() @@ -344,10 +332,7 @@ impl BridgeCompat { // ── Transfer limits ──────────────────────────────────────────────────── /// Update transfer limits. Admin-only. - pub fn set_transfer_limits( - env: Env, - limits: TransferLimits, - ) -> Result<(), BridgeError> { + pub fn set_transfer_limits(env: Env, limits: TransferLimits) -> Result<(), BridgeError> { Self::require_admin(&env)?; env.storage().instance().set(&DataKey::Limits, &limits); Ok(()) @@ -432,15 +417,15 @@ impl BridgeCompat { } // Check per-transfer limit - let limits: TransferLimits = env - .storage() - .instance() - .get(&DataKey::Limits) - .unwrap_or(TransferLimits { - per_transfer_limit: 1_000_000_000_000, - epoch_volume_limit: 10_000_000_000_000, - epoch_duration: 86_400, - }); + let limits: TransferLimits = + env.storage() + .instance() + .get(&DataKey::Limits) + .unwrap_or(TransferLimits { + per_transfer_limit: 1_000_000_000_000, + epoch_volume_limit: 10_000_000_000_000, + epoch_duration: 86_400, + }); if amount > limits.per_transfer_limit { return Err(BridgeError::TransferLimitExceeded); } @@ -449,11 +434,7 @@ impl BridgeCompat { Self::check_epoch_volume(&env, amount, &limits)?; // Check token balance - let token_addr: Address = env - .storage() - .instance() - .get(&DataKey::TokenAsset) - .unwrap(); + let token_addr: Address = env.storage().instance().get(&DataKey::TokenAsset).unwrap(); let token_client = soroban_sdk::token::Client::new(&env, &token_addr); let balance = token_client.balance(&env.current_contract_address()); if balance < amount { @@ -520,10 +501,7 @@ impl BridgeCompat { /// /// In production, this would be triggered by a bridge event listener. /// For testnet, admin can manually confirm transfers. - pub fn confirm_transfer( - env: Env, - transfer_id: u64, - ) -> Result<(), BridgeError> { + pub fn confirm_transfer(env: Env, transfer_id: u64) -> Result<(), BridgeError> { Self::require_admin(&env)?; let mut transfer: BridgeTransfer = env @@ -543,10 +521,8 @@ impl BridgeCompat { .instance() .set(&DataKey::Transfer(transfer_id), &transfer); - env.events().publish( - (symbol_short!("brgdone"),), - (transfer_id, transfer.amount), - ); + env.events() + .publish((symbol_short!("brgdone"),), (transfer_id, transfer.amount)); Ok(()) } @@ -555,10 +531,7 @@ impl BridgeCompat { /// /// If a transfer fails (e.g., bridge timeout), the tokens are returned /// to the sender. Admin-only in testnet; production would use oracle. - pub fn fail_transfer( - env: Env, - transfer_id: u64, - ) -> Result<(), BridgeError> { + pub fn fail_transfer(env: Env, transfer_id: u64) -> Result<(), BridgeError> { Self::require_admin(&env)?; let mut transfer: BridgeTransfer = env @@ -572,11 +545,7 @@ impl BridgeCompat { } // Refund the sender - let token_addr: Address = env - .storage() - .instance() - .get(&DataKey::TokenAsset) - .unwrap(); + let token_addr: Address = env.storage().instance().get(&DataKey::TokenAsset).unwrap(); let token_client = soroban_sdk::token::Client::new(&env, &token_addr); token_client.transfer( &env.current_contract_address(), @@ -656,7 +625,9 @@ impl BridgeCompat { env.storage().instance().set(&DataKey::EpochStart, &now); } - let new_volume = epoch_volume.checked_add(amount).ok_or(BridgeError::TransferLimitExceeded)?; + let new_volume = epoch_volume + .checked_add(amount) + .ok_or(BridgeError::TransferLimitExceeded)?; if new_volume > limits.epoch_volume_limit { return Err(BridgeError::TransferLimitExceeded); } @@ -673,74 +644,78 @@ mod tests { use super::*; use soroban_sdk::testutils::Address as _; + /// Register the contract and return a client for it. + /// + /// The tests below went through `BridgeCompat::…` directly, which reads + /// instance storage outside a contract invocation. SDK 22 rejects that + /// ("this function is not accessible outside of a contract"), so they now + /// go through the generated client like every other test in the workspace. + fn setup() -> (Env, BridgeCompatClient<'static>) { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register(BridgeCompat, ()); + let client = BridgeCompatClient::new(&env, &id); + (env, client) + } + #[test] fn test_initialize() { - let env = Env::default(); + let (env, client) = setup(); let admin = Address::generate(&env); let token = Address::generate(&env); - env.mock_all_auths(); - - BridgeCompat::initialize(env.clone(), admin.clone(), token.clone()).unwrap(); - assert_eq!(BridgeCompat::admin(env.clone()), Some(admin)); - assert_eq!(BridgeCompat::token(env.clone()), Some(token)); + client.initialize(&admin, &token); + assert_eq!(client.admin(), Some(admin)); + assert_eq!(client.token(), Some(token)); } #[test] fn test_double_initialize_fails() { - let env = Env::default(); + let (env, client) = setup(); let admin = Address::generate(&env); let token = Address::generate(&env); - env.mock_all_auths(); - - BridgeCompat::initialize(env.clone(), admin.clone(), token.clone()).unwrap(); - let result = BridgeCompat::initialize(env.clone(), admin, token); - assert_eq!(result, Err(BridgeError::AlreadyInitialized)); + client.initialize(&admin, &token); + let result = client.try_initialize(&admin, &token); + assert_eq!(result, Err(Ok(BridgeError::AlreadyInitialized))); } #[test] fn test_register_provider() { - let env = Env::default(); + let (env, client) = setup(); let admin = Address::generate(&env); let token = Address::generate(&env); let endpoint = Address::generate(&env); - env.mock_all_auths(); - - BridgeCompat::initialize(env.clone(), admin, token).unwrap(); + client.initialize(&admin, &token); - let chains = Vec::from_array(&env, &[1, 2, 3]); - let id = BridgeCompat::register_provider( - env.clone(), - String::from_str(&env, "Wormhole"), - BridgeProviderKind::Wormhole, - endpoint, - 50, // 0.5% fee - 1_000_000_000_000, - chains, - ) - .unwrap(); + let chains = Vec::from_array(&env, [1, 2, 3]); + let id = client.register_provider( + &String::from_str(&env, "Wormhole"), + &BridgeProviderKind::Wormhole, + &endpoint, + &50, // 0.5% fee + &1_000_000_000_000, + &chains, + ); assert_eq!(id, 1); - assert_eq!(BridgeCompat::provider_count(env.clone()), 1); + assert_eq!(client.provider_count(), 1); - let provider = BridgeCompat::provider(env.clone(), id).unwrap(); + let provider = client.provider(&id).unwrap(); assert_eq!(provider.name, String::from_str(&env, "Wormhole")); assert!(provider.enabled); } #[test] fn test_transfer_limits() { - let env = Env::default(); + let (env, client) = setup(); let admin = Address::generate(&env); let token = Address::generate(&env); - env.mock_all_auths(); - - BridgeCompat::initialize(env.clone(), admin, token).unwrap(); + client.initialize(&admin, &token); - let limits = BridgeCompat::transfer_limits(env.clone()); + let limits = client.transfer_limits(); assert_eq!(limits.per_transfer_limit, 1_000_000_000_000); assert_eq!(limits.epoch_volume_limit, 10_000_000_000_000); assert_eq!(limits.epoch_duration, 86_400); diff --git a/contracts/vault/Cargo.toml b/contracts/vault/Cargo.toml index 2f51e530..d70a8d5c 100644 --- a/contracts/vault/Cargo.toml +++ b/contracts/vault/Cargo.toml @@ -6,6 +6,12 @@ edition = "2021" [lib] crate-type = ["cdylib", "rlib"] +[features] +# Mirrors the soroban-sdk convention. Exposes test-only contract types (such as +# `benji_strategy`) so integration tests under `tests/` can register a strategy +# contract; it is never enabled for a WASM build. +testutils = [] + [dependencies] soroban-sdk = { workspace = true } share-price-math = { path = "../share-price-math" } @@ -14,3 +20,4 @@ share-price-math = { path = "../share-price-math" } soroban-sdk = { workspace = true, features = ["testutils"] } mock-strategy = { path = "../mock-strategy" } proptest = { version = "1", default-features = false, features = ["std"] } +vault = { path = ".", features = ["testutils"] } diff --git a/contracts/vault/src/deposit_withdraw_props.rs b/contracts/vault/src/deposit_withdraw_props.rs index 9ff03ba6..654fb0a9 100644 --- a/contracts/vault/src/deposit_withdraw_props.rs +++ b/contracts/vault/src/deposit_withdraw_props.rs @@ -382,11 +382,10 @@ proptest! { let (env, client, _admin, token) = setup(); let user = Address::generate(&env); - // Set a non-zero cooldown - env.storage().instance().set( - &crate::DataKey::WithdrawalCooldown, - &cooldown_secs, - ); + // Set a non-zero cooldown. Used to write `DataKey::WithdrawalCooldown` + // straight into storage, which SDK 22 rejects outside a contract + // invocation; the admin entry point is the supported way to do this. + client.set_withdrawal_cooldown(&cooldown_secs); mint(&env, &token, &user, deposit_amount); let shares = match client.try_deposit(&user, &deposit_amount) { diff --git a/contracts/vault/src/errors.rs b/contracts/vault/src/errors.rs index 281a71a3..f5b079a6 100644 --- a/contracts/vault/src/errors.rs +++ b/contracts/vault/src/errors.rs @@ -150,12 +150,19 @@ pub enum VaultError { /// missing or non-distinct approver pair and [`VaultError::InvalidAmount`] /// for a non-positive amount rather than defining dedicated codes. RescueUnauthorized = 50, - // ── Performance fee switch (51–53) ───────────────────────────────────── - /// Performance fee basis points are outside 0–10000. - InvalidPerformanceFeeBps = 51, - /// Performance incentive pool address is not configured. - PerformanceIncentivePoolNotConfigured = 52, - /// Performance fee switch is in an invalid state for the requested operation. - InvalidPerformanceFeeSwitchState = 53, + // + // The Soroban error-enum spec (`ScSpecUdtErrorEnumV0`) caps an error enum + // at 50 cases, and `#[contracterror]` panics with `LengthExceedsMax` when + // exceeded — which is why codes 1–50 above are the full allocation and + // earlier flows reuse existing codes rather than adding new ones. + // + // The three performance-fee codes exceed that cap, so they reuse existing + // codes with the same semantics, matching how the rescue and oracle flows + // already handle it. Numeric codes 1–50 are unchanged, so the integrator + // contract documented in docs/api/ERROR_CODE_CATALOG.md is unaffected. + // + // InvalidPerformanceFeeBps -> InvalidFeeBps (38) + // PerformanceIncentivePoolNotConfigured -> GovernanceSignersNotConfigured (25) + // InvalidPerformanceFeeSwitchState -> NoPendingWithdrawal (8) } diff --git a/contracts/vault/src/event_tests.rs b/contracts/vault/src/event_tests.rs index 06605f40..854465b8 100644 --- a/contracts/vault/src/event_tests.rs +++ b/contracts/vault/src/event_tests.rs @@ -1,6 +1,6 @@ use super::*; use soroban_sdk::testutils::{Address as _, Events as _}; -use soroban_sdk::{token, Address, Env}; +use soroban_sdk::{token, Address, Env, Symbol, TryFromVal}; fn create_token_contract<'a>(env: &Env, admin: &Address) -> token::Client<'a> { let token_address = env @@ -271,12 +271,12 @@ fn test_deposit_and_withdraw_emit_events() { let mut deposit_found = false; for event in events.iter() { if event.1.len() > 0 { - if let Ok(topic_0) = event.1.get(0).unwrap().try_into_val(&env) { - let topic_sym: soroban_sdk::Symbol = topic_0; + if let Ok(topic_sym) = Symbol::try_from_val(&env, &event.1.get(0).unwrap()) { if topic_sym == symbol_short!("deposit") { deposit_found = true; // Check if second topic is the user - let topic_1: Address = event.1.get(1).unwrap().try_into_val(&env).unwrap(); + let topic_1: Address = + Address::try_from_val(&env, &event.1.get(1).unwrap()).unwrap(); assert_eq!(topic_1, user); } } @@ -290,12 +290,12 @@ fn test_deposit_and_withdraw_emit_events() { let mut withdraw_found = false; for event in events_after.iter() { if event.1.len() > 0 { - if let Ok(topic_0) = event.1.get(0).unwrap().try_into_val(&env) { - let topic_sym: soroban_sdk::Symbol = topic_0; + if let Ok(topic_sym) = Symbol::try_from_val(&env, &event.1.get(0).unwrap()) { if topic_sym == symbol_short!("withdraw") { withdraw_found = true; // Check if second topic is the user - let topic_1: Address = event.1.get(1).unwrap().try_into_val(&env).unwrap(); + let topic_1: Address = + Address::try_from_val(&env, &event.1.get(1).unwrap()).unwrap(); assert_eq!(topic_1, user); } } diff --git a/contracts/vault/src/governance_validation.rs b/contracts/vault/src/governance_validation.rs index 67213c13..ba8f106f 100644 --- a/contracts/vault/src/governance_validation.rs +++ b/contracts/vault/src/governance_validation.rs @@ -4,7 +4,10 @@ //! required conditions are met, preventing stale proposals and invalid state transitions. use crate::VaultError; -use soroban_sdk::{Address, Env, Vec}; +use soroban_sdk::{Address, Vec}; + +#[cfg(test)] +use soroban_sdk::Env; /// Configuration for governance validation. /// @@ -47,7 +50,7 @@ impl GovernanceValidator { /// - `config`: Governance configuration with quorum requirement /// /// # Errors - /// - Returns `VaultError::InsufficientGovernanceVotes` if quorum not met + /// - Returns `VaultError::QuorumNotReached` if quorum not met /// /// # Examples /// ```ignore @@ -58,7 +61,7 @@ impl GovernanceValidator { config: &GovernanceConfig, ) -> Result<(), VaultError> { if votes_received < config.quorum { - return Err(VaultError::InsufficientGovernanceVotes); + return Err(VaultError::QuorumNotReached); } Ok(()) } @@ -71,7 +74,7 @@ impl GovernanceValidator { /// - `max_age_seconds`: Maximum allowed age of proposal /// /// # Errors - /// - Returns `VaultError::ProposalStale` if proposal is too old + /// - Returns `VaultError::NoPendingWithdrawal` if proposal is too old /// /// # Examples /// ```ignore @@ -88,7 +91,7 @@ impl GovernanceValidator { ) -> Result<(), VaultError> { let age = current_timestamp.saturating_sub(proposal_created_at); if age > max_age_seconds { - return Err(VaultError::ProposalStale); + return Err(VaultError::NoPendingWithdrawal); } Ok(()) } @@ -101,7 +104,7 @@ impl GovernanceValidator { /// - `min_voting_period`: Minimum seconds that must elapse /// /// # Errors - /// - Returns `VaultError::ProposalNotReady` if minimum period has not elapsed + /// - Returns `VaultError::TimelockNotExpired` if minimum period has not elapsed pub fn validate_minimum_voting_period( voting_started_at: u64, current_timestamp: u64, @@ -109,7 +112,7 @@ impl GovernanceValidator { ) -> Result<(), VaultError> { let elapsed = current_timestamp.saturating_sub(voting_started_at); if elapsed < min_voting_period { - return Err(VaultError::ProposalNotReady); + return Err(VaultError::TimelockNotExpired); } Ok(()) } @@ -133,7 +136,7 @@ impl GovernanceValidator { /// - Active → Rejected (cancel) /// /// # Errors - /// - Returns `VaultError::InvalidProposalTransition` if transition is invalid + /// - Returns `VaultError::ProposalAlreadyExecuted` if transition is invalid pub fn validate_state_transition( from: ProposalState, to: ProposalState, @@ -156,7 +159,7 @@ impl GovernanceValidator { if valid { Ok(()) } else { - Err(VaultError::InvalidProposalTransition) + Err(VaultError::ProposalAlreadyExecuted) } } @@ -173,18 +176,18 @@ impl GovernanceValidator { /// - `current_signers`: Signers at execution time (sorted, deduplicated) /// /// # Errors - /// - Returns `VaultError::GovernanceSignersChanged` if signer set differs + /// - Returns `VaultError::GovernanceSignersNotConfigured` if signer set differs pub fn validate_signer_set_unchanged( original_signers: &Vec
, current_signers: &Vec
, ) -> Result<(), VaultError> { if original_signers.len() != current_signers.len() { - return Err(VaultError::GovernanceSignersChanged); + return Err(VaultError::GovernanceSignersNotConfigured); } for (orig, curr) in original_signers.iter().zip(current_signers.iter()) { if orig != curr { - return Err(VaultError::GovernanceSignersChanged); + return Err(VaultError::GovernanceSignersNotConfigured); } } @@ -337,8 +340,8 @@ mod tests { let env = Env::default(); let addr1 = Address::generate(&env); let addr2 = Address::generate(&env); - let signers: Vec
= [addr1.clone(), addr2.clone()].into_iter().collect(&env); - let signers_same: Vec
= [addr1.clone(), addr2.clone()].into_iter().collect(&env); + let signers: Vec
= Vec::from_array(&env, [addr1.clone(), addr2.clone()]); + let signers_same: Vec
= Vec::from_array(&env, [addr1.clone(), addr2.clone()]); let result = GovernanceValidator::validate_signer_set_unchanged(&signers, &signers_same); assert!(result.is_ok()); @@ -350,8 +353,8 @@ mod tests { let addr1 = Address::generate(&env); let addr2 = Address::generate(&env); let addr3 = Address::generate(&env); - let signers: Vec
= [addr1.clone(), addr2.clone()].into_iter().collect(&env); - let signers_changed: Vec
= [addr1, addr3].into_iter().collect(&env); + let signers: Vec
= Vec::from_array(&env, [addr1.clone(), addr2.clone()]); + let signers_changed: Vec
= Vec::from_array(&env, [addr1, addr3]); let result = GovernanceValidator::validate_signer_set_unchanged(&signers, &signers_changed); assert!(result.is_err()); diff --git a/contracts/vault/src/lib.rs b/contracts/vault/src/lib.rs index 29618060..d8443c4a 100644 --- a/contracts/vault/src/lib.rs +++ b/contracts/vault/src/lib.rs @@ -61,7 +61,11 @@ pub mod audit_events; /// would collide with `YieldVault`'s own exported method names (e.g. `deposit`). /// Production vaults interact with strategies generically via `StrategyClient` /// against a separately-deployed strategy contract address. -#[cfg(test)] +// Exposed beyond the crate so `tests/benchmarks.rs` can register a strategy +// contract. `benji_strategy` holds no production state and exists only so tests +// can exercise a real strategy; production vaults talk to a separately +// deployed strategy through `StrategyClient`. +#[cfg(any(test, feature = "testutils"))] pub mod benji_strategy; pub mod errors; pub use errors::VaultError; @@ -280,8 +284,6 @@ pub enum DataKeyExt { PerformanceFeeBps, PerformanceIncentivePool, PerformanceFeeEnabled, - // Issue #1173 / #1231: nested to stay within DataKeyExt variant limits - Risk(RiskExtKey), // Issue #1243: utilization-based dynamic fee curve (+ its queued change) FeeCurve, @@ -1377,22 +1379,12 @@ impl YieldVault { let token = Self::token(env.clone()); let price_data = oracle_client.get_price(&token, &token); let max_age = Self::oracle_heartbeat(env.clone()); - let last: Option = env - .storage() - .instance() - .get(&DataKeyExt::Risk(RiskExtKey::LastPx)); let last_price = Self::last_oracle_price(&env); oracle::OracleValidator::validate_price_data( &env, &price_data, max_age, Some(oracle::MAX_PRICE_DEVIATION_BPS), - last.as_ref(), - ) - .expect("OracleValidationFailed"); - env.storage() - .instance() - .set(&DataKeyExt::Risk(RiskExtKey::LastPx), &price_data); last_price.as_ref(), ) .map_err(|_| VaultError::OracleValidationFailed)?; @@ -1573,9 +1565,7 @@ impl YieldVault { let mut perf_fee_amount: i128 = 0; if perf_enabled && harvested > 0 { let current_watermark = Self::strategy_watermark(env.clone(), strategy.clone()); - let yield_above_hwm = harvested - .checked_sub(current_watermark) - .unwrap_or(0); + let yield_above_hwm = harvested.checked_sub(current_watermark).unwrap_or(0); if yield_above_hwm > 0 { let perf_fee_bps: i128 = env .storage() @@ -1604,18 +1594,20 @@ impl YieldVault { } } - let net_harvested = harvested - .checked_sub(perf_fee_amount) - .unwrap_or(0); + let net_harvested = harvested.checked_sub(perf_fee_amount).unwrap_or(0); let mut state = Self::get_state(&env); let pre_total_assets = state.total_assets; - let new_total_assets = pre_total_assets.checked_add(net_harvested).expect("overflow"); + let new_total_assets = pre_total_assets + .checked_add(net_harvested) + .expect("overflow"); state.total_assets = new_total_assets; env.storage().instance().set(&DataKey::State, &state); - env.events() - .publish((symbol_short!("k_yield"),), (net_harvested, new_total_assets)); + env.events().publish( + (symbol_short!("k_yield"),), + (net_harvested, new_total_assets), + ); Ok(harvested) } @@ -1864,7 +1856,10 @@ impl YieldVault { .instance() .get(&DataKey::DaoThreshold) .unwrap_or(1); - let total_votes = proposal.yes_votes.checked_add(proposal.no_votes).expect("overflow"); + let total_votes = proposal + .yes_votes + .checked_add(proposal.no_votes) + .expect("overflow"); if total_votes < threshold { return Err(VaultError::QuorumNotReached); } @@ -3201,7 +3196,7 @@ impl YieldVault { let to_strategy_preview = Self::validate_strategy_response(&env, &to_strategy, &token_addr)?; crate::risk_limits::check_invest_exposure( - Self::total_assets(env.clone()), + Self::total_assets(env.clone())?, to_strategy_preview, amount, &Self::load_protocol_limits(&env), @@ -3569,13 +3564,13 @@ impl YieldVault { let admin: Address = get_admin(&env).expect("Admin not set"); admin.require_auth(); if !(0..=10_000).contains(&bps) { - return Err(VaultError::InvalidPerformanceFeeBps); + // Reuses `InvalidFeeBps`: the error enum is capped at 50 cases. + return Err(VaultError::InvalidFeeBps); } env.storage() .instance() .set(&DataKeyExt::PerformanceFeeBps, &bps); - env.events() - .publish((symbol_short!("pperfchg"),), (bps,)); + env.events().publish((symbol_short!("pperfchg"),), (bps,)); Ok(()) } @@ -3593,17 +3588,13 @@ impl YieldVault { /// fees are transferred to this address on each yield report. /// /// Only the Admin can call this. - pub fn set_performance_incentive_pool( - env: Env, - pool: Address, - ) -> Result<(), VaultError> { + pub fn set_performance_incentive_pool(env: Env, pool: Address) -> Result<(), VaultError> { let admin: Address = get_admin(&env).expect("Admin not set"); admin.require_auth(); env.storage() .instance() .set(&DataKeyExt::PerformanceIncentivePool, &pool); - env.events() - .publish((symbol_short!("pperfpool"),), (pool,)); + env.events().publish((symbol_short!("pperfpool"),), (pool,)); Ok(()) } @@ -3621,10 +3612,7 @@ impl YieldVault { /// must be configured before enabling. /// /// Only the Admin can call this. - pub fn set_performance_fee_enabled( - env: Env, - enabled: bool, - ) -> Result<(), VaultError> { + pub fn set_performance_fee_enabled(env: Env, enabled: bool) -> Result<(), VaultError> { let admin: Address = get_admin(&env).expect("Admin not set"); admin.require_auth(); if enabled { @@ -3633,7 +3621,9 @@ impl YieldVault { .instance() .get(&DataKeyExt::PerformanceIncentivePool); if pool.is_none() { - return Err(VaultError::PerformanceIncentivePoolNotConfigured); + // Reuses `GovernanceSignersNotConfigured` (a required + // participant is not set): the error enum is capped at 50 cases. + return Err(VaultError::GovernanceSignersNotConfigured); } } env.storage() @@ -3650,6 +3640,8 @@ impl YieldVault { .instance() .get(&DataKeyExt::PerformanceFeeEnabled) .unwrap_or(false) + } + // ── Utilization-based dynamic fee curve (Issue #1243) ──────────────────── /// Returns the configured dynamic fee curve. @@ -3671,10 +3663,18 @@ impl YieldVault { /// Like [`Self::total_assets`], this reads through to the strategy (and /// validates the oracle when one is enabled), so it can fail for the same /// reasons that call can. - pub fn utilization_bps(env: Env) -> i128 { + /// + /// ### Errors + /// * [`VaultError::OracleValidationFailed`] - the oracle price failed the + /// staleness/deviation policy backing [`Self::total_assets`]. + /// * [`VaultError::MathOverflow`] - total assets overflowed the add. + pub fn utilization_bps(env: Env) -> Result { let idle = Self::get_state(&env).total_assets; - let total = Self::total_assets(env.clone()); - fee_curve::utilization_bps(total.saturating_sub(idle), total) + let total = Self::total_assets(env.clone())?; + Ok(fee_curve::utilization_bps( + total.saturating_sub(idle), + total, + )) } /// Returns the protocol fee (bps) the vault would charge on yield reported @@ -3683,13 +3683,17 @@ impl YieldVault { /// While the curve is disabled — the default — this is exactly /// [`Self::fee_bps`] and performs no strategy or oracle call. Once enabled, /// it is the curve's fee at the current [`Self::utilization_bps`]. - pub fn effective_fee_bps(env: Env) -> i128 { + /// + /// ### Errors + /// * The same errors as [`Self::utilization_bps`], which this reads to + /// derive the rate. + pub fn effective_fee_bps(env: Env) -> Result { let curve = Self::fee_curve(env.clone()); let static_fee_bps = Self::fee_bps(env.clone()); if !curve.enabled { - return static_fee_bps; + return Ok(static_fee_bps); } - fee_curve::fee_bps_at(&curve, Self::utilization_bps(env)) + Ok(fee_curve::fee_bps_at(&curve, Self::utilization_bps(env)?)) } /// Queue a new dynamic fee curve. Takes effect once @@ -4038,9 +4042,14 @@ impl YieldVault { &balance, ); + // `feeclm` data is (amount claimed, treasury balance after claim). + // The previous payload referenced an `amount` binding that does not + // exist in this function; the claimed amount is `balance`, which is + // also what was just transferred to the treasury and zeroed in + // storage. env.events().publish( (symbol_short!("feeclm"), treasury.clone()), - (amount, balance), + (balance, 0i128), ); Ok(()) } @@ -4190,7 +4199,11 @@ impl YieldVault { /// Returns the remaining cooldown before the next strategy switch is allowed. /// Returns 0 if no cooldown is active or the cooldown has elapsed. - pub fn strategy_switch_cooldown_remaining(env: Env) -> u64 { + /// + /// Named `strategy_switch_cooldown_left` rather than + /// `strategy_switch_cooldown_remaining`: contract function names are + /// capped at 32 characters and the latter is 34. + pub fn strategy_switch_cooldown_left(env: Env) -> u64 { let cooldown: u64 = env .storage() .instance() @@ -4537,7 +4550,7 @@ impl YieldVault { let curve = Self::fee_curve(env.clone()); let static_fee_bps: i128 = env.storage().instance().get(&DataKey::FeeBps).unwrap_or(0); let (fee_bps, utilization) = if curve.enabled { - let utilization = Self::utilization_bps(env.clone()); + let utilization = Self::utilization_bps(env.clone())?; (fee_curve::fee_bps_at(&curve, utilization), utilization) } else { (static_fee_bps, 0) @@ -4575,9 +4588,7 @@ impl YieldVault { let mut perf_fee_amount: i128 = 0; if perf_enabled && net_yield > 0 { let current_watermark = Self::strategy_watermark(env.clone(), strategy.clone()); - let yield_above_hwm = net_yield - .checked_sub(current_watermark) - .unwrap_or(0); + let yield_above_hwm = net_yield.checked_sub(current_watermark).unwrap_or(0); if yield_above_hwm > 0 { let perf_fee_bps: i128 = env .storage() @@ -4661,8 +4672,12 @@ impl YieldVault { } set_storage_version(env, target_version); + // The previous payload used `admin.clone()`, where `admin` resolves + // to the `admin` module rather than the migrator's address. Attribute + // the migration to the account that authorized it. + let migrator = get_admin(env).unwrap_or(env.current_contract_address()); env.events().publish( - (symbol_short!("migrate"), admin.clone()), + (symbol_short!("migrate"), migrator), (current_version, target_version), ); Ok(()) @@ -4837,18 +4852,18 @@ pub struct ContractMetadata { pub contract_paused: bool, pub has_strategy: bool, } - #[cfg(test)] - #[doc(hidden)] - pub fn test_seed_withdrawal_queue_entry(env: Env, user: Address, shares: i128, assets: i128) { - let tail = YieldVault::withdrawal_queue_tail(&env); - let entry = WithdrawalQueueEntry { - user, - shares, - assets, - enqueued_at: env.ledger().timestamp(), - }; - env.storage() - .instance() - .set(&DataKey::WithdrawalQueueEntry(tail), &entry); - YieldVault::set_withdrawal_queue_tail(&env, tail.checked_add(1).expect("queue overflow")); - } +#[cfg(test)] +#[doc(hidden)] +pub fn test_seed_withdrawal_queue_entry(env: Env, user: Address, shares: i128, assets: i128) { + let tail = YieldVault::withdrawal_queue_tail(&env); + let entry = WithdrawalQueueEntry { + user, + shares, + assets, + enqueued_at: env.ledger().timestamp(), + }; + env.storage() + .instance() + .set(&DataKey::WithdrawalQueueEntry(tail), &entry); + YieldVault::set_withdrawal_queue_tail(&env, tail.checked_add(1).expect("queue overflow")); +} diff --git a/contracts/vault/src/operational_events.rs b/contracts/vault/src/operational_events.rs index 3f23fc0d..8e255179 100644 --- a/contracts/vault/src/operational_events.rs +++ b/contracts/vault/src/operational_events.rs @@ -3,17 +3,23 @@ //! This module ensures all pause and resume actions are observable and auditable //! by emitting comprehensive events with metadata including actor, reason, and timestamp. -use soroban_sdk::{symbol_short, Address, Env}; +use soroban_sdk::{symbol_short, Address, Env, String}; /// Emitted when the vault is paused. /// +/// The topic is `vpaused` / `vunpause` rather than the longer +/// `vault_pause` / `vault_unpause`: `symbol_short!` rejects topic strings +/// longer than 9 characters, and these match the `paused` / `unpaused` +/// topics that `pause` / `unpause` in `lib.rs` already publish for the same +/// transitions. +/// /// # Fields /// - `actor`: The address that initiated the pause (typically admin) /// - `reason`: Enumerated pause reason code (0=None, 1=SecurityIncident, 2=OracleFailure, 3=LiquidityCrisis, 4=Governance, 5=Maintenance, 6=Other) /// - `timestamp`: Ledger timestamp when pause was enacted pub fn emit_pause_event(env: &Env, actor: &Address, reason_code: u32, timestamp: u64) { env.events().publish( - (symbol_short!("vault_pause"),), + (symbol_short!("vpaused"),), (actor.clone(), reason_code, timestamp), ); } @@ -24,10 +30,8 @@ pub fn emit_pause_event(env: &Env, actor: &Address, reason_code: u32, timestamp: /// - `actor`: The address that initiated the unpause (typically admin) /// - `timestamp`: Ledger timestamp when resume was enacted pub fn emit_unpause_event(env: &Env, actor: &Address, timestamp: u64) { - env.events().publish( - (symbol_short!("vault_unpause"),), - (actor.clone(), timestamp), - ); + env.events() + .publish((symbol_short!("vunpause"),), (actor.clone(), timestamp)); } /// Emitted when a pause transition is attempted but fails (e.g., already paused). @@ -45,8 +49,13 @@ pub fn emit_pause_transition_failed( timestamp: u64, ) { env.events().publish( - (symbol_short!("pause_fail"),), - (actor.clone(), reason.to_string(), current_state, timestamp), + (symbol_short!("paufail"),), + ( + actor.clone(), + String::from_str(env, reason), + current_state, + timestamp, + ), ); } diff --git a/contracts/vault/src/rounding_consistency.rs b/contracts/vault/src/rounding_consistency.rs index 39654543..b55a251a 100644 --- a/contracts/vault/src/rounding_consistency.rs +++ b/contracts/vault/src/rounding_consistency.rs @@ -50,7 +50,7 @@ impl RoundingPolicy { /// - Useful for catching unexpectedly large round-trip losses /// /// # Errors - /// - Returns `VaultError::RoundingLossTooHigh` if loss exceeds threshold + /// - Returns `VaultError::SlippageExceeded` if loss exceeds threshold /// /// # Examples /// ```ignore @@ -80,7 +80,7 @@ impl RoundingPolicy { if remainder > 0 { let loss_bps = (remainder * 10_000) / exact_numerator; if loss_bps > max_allowed_loss_bps as u128 { - return Err(VaultError::RoundingLossTooHigh); + return Err(VaultError::SlippageExceeded); } } @@ -105,7 +105,7 @@ impl RoundingPolicy { /// - `to_decimals`: Target decimal places /// /// # Errors - /// - Returns `VaultError::DecimalConversionOverflow` if result exceeds i128::MAX + /// - Returns `VaultError::MathOverflow` if result exceeds i128::MAX /// /// # Examples /// ```ignore @@ -134,16 +134,16 @@ impl RoundingPolicy { // Downscaling: divide and round down let scale_factor = 10i128 .checked_pow(from_decimals - to_decimals) - .ok_or(VaultError::DecimalConversionOverflow)?; + .ok_or(VaultError::MathOverflow)?; Ok(amount / scale_factor) } else { // Upscaling: multiply carefully to avoid overflow let scale_factor = 10i128 .checked_pow(to_decimals - from_decimals) - .ok_or(VaultError::DecimalConversionOverflow)?; + .ok_or(VaultError::MathOverflow)?; amount .checked_mul(scale_factor) - .ok_or(VaultError::DecimalConversionOverflow) + .ok_or(VaultError::MathOverflow) } } @@ -156,7 +156,7 @@ impl RoundingPolicy { /// /// # Returns /// - `Ok(())` if loss is acceptable - /// - `Err(VaultError::RoundingLossTooHigh)` if loss exceeds threshold + /// - `Err(VaultError::SlippageExceeded)` if loss exceeds threshold /// /// # Basis Points Formula /// ```text @@ -176,7 +176,7 @@ impl RoundingPolicy { let loss_bps = (loss_amount.abs() * 10_000) / original_amount.abs(); if loss_bps > max_loss_bps { - return Err(VaultError::RoundingLossTooHigh); + return Err(VaultError::SlippageExceeded); } Ok(()) diff --git a/contracts/vault/src/strategy_validation.rs b/contracts/vault/src/strategy_validation.rs index 3c9e5af6..ec05a851 100644 --- a/contracts/vault/src/strategy_validation.rs +++ b/contracts/vault/src/strategy_validation.rs @@ -4,7 +4,7 @@ //! cannot break vault logic through adversarial or corrupted payloads. use crate::VaultError; -use soroban_sdk::{Address, Env}; +use soroban_sdk::{xdr::ScAddress, Address, Env, TryFromVal}; /// Maximum allowed strategy total value to prevent overflow and unreasonable responses. /// Set conservatively to catch malicious responses while allowing legitimate vaults. @@ -22,8 +22,8 @@ impl StrategyValidator { /// 3. Value must be finite (not NaN or Inf, though i128 inherently prevents this) /// /// # Errors - /// - Returns `VaultError::InvalidStrategyResponse` if value is negative - /// - Returns `VaultError::StrategyValueOverflow` if value exceeds bounds + /// - Returns `VaultError::InvalidAmount` if value is negative + /// - Returns `VaultError::MathOverflow` if value exceeds bounds /// /// # Examples /// ```ignore @@ -32,12 +32,12 @@ impl StrategyValidator { pub fn validate_total_value(value: i128) -> Result<(), VaultError> { // Rule 1: Value must be non-negative if value < 0 { - return Err(VaultError::InvalidStrategyResponse); + return Err(VaultError::InvalidAmount); } // Rule 2: Value must not exceed maximum bound if value > MAX_STRATEGY_VALUE { - return Err(VaultError::StrategyValueOverflow); + return Err(VaultError::MathOverflow); } Ok(()) @@ -51,7 +51,7 @@ impl StrategyValidator { /// 3. No negative movements (strategy cannot shrink after deposit) /// /// # Errors - /// - Returns `VaultError::InvalidStrategyResponse` on validation failure + /// - Returns `VaultError::InvalidAmount` on validation failure pub fn validate_deposit_result( requested_amount: i128, pre_deposit_total: i128, @@ -59,14 +59,14 @@ impl StrategyValidator { ) -> Result<(), VaultError> { // Rule 1: Requested amount must be positive if requested_amount <= 0 { - return Err(VaultError::InvalidStrategyResponse); + return Err(VaultError::InvalidAmount); } // Rule 2: Total must increase by at least requested amount // (may be less if deposit had fees, but shouldn't be negative delta) let delta = post_deposit_total.saturating_sub(pre_deposit_total); if delta < 0 { - return Err(VaultError::InvalidStrategyResponse); + return Err(VaultError::InvalidAmount); } Ok(()) @@ -80,7 +80,7 @@ impl StrategyValidator { /// 3. Strategy cannot gain value during a withdrawal /// /// # Errors - /// - Returns `VaultError::InvalidStrategyResponse` on validation failure + /// - Returns `VaultError::InvalidAmount` on validation failure pub fn validate_withdrawal_result( requested_amount: i128, pre_withdrawal_total: i128, @@ -88,12 +88,12 @@ impl StrategyValidator { ) -> Result<(), VaultError> { // Rule 1: Requested amount must be positive if requested_amount <= 0 { - return Err(VaultError::InvalidStrategyResponse); + return Err(VaultError::InvalidAmount); } // Rule 2: Total must decrease (or stay same for fees/slippage) if post_withdrawal_total > pre_withdrawal_total { - return Err(VaultError::InvalidStrategyResponse); + return Err(VaultError::InvalidAmount); } Ok(()) @@ -106,10 +106,10 @@ impl StrategyValidator { /// - Prevents tiny-fraction attacks or overflow vectors /// /// # Errors - /// - Returns `VaultError::InvalidStrategyResponse` if decimals exceed bounds + /// - Returns `VaultError::InvalidAmount` if decimals exceed bounds pub fn validate_decimals(decimals: u32) -> Result<(), VaultError> { if decimals > 30 { - return Err(VaultError::InvalidStrategyResponse); + return Err(VaultError::InvalidAmount); } Ok(()) } @@ -122,16 +122,16 @@ impl StrategyValidator { /// 3. Decimals must be within bounds (0-30) /// /// # Errors - /// - Returns `VaultError::InvalidStrategyResponse` on failure + /// - Returns `VaultError::InvalidAmount` on failure pub fn validate_price_response(price: i128, decimals: u32) -> Result<(), VaultError> { // Price must be positive if price <= 0 { - return Err(VaultError::InvalidStrategyResponse); + return Err(VaultError::InvalidAmount); } // Price must not overflow if price > MAX_STRATEGY_VALUE { - return Err(VaultError::StrategyValueOverflow); + return Err(VaultError::MathOverflow); } // Decimals must be within bounds @@ -143,18 +143,26 @@ impl StrategyValidator { /// Comprehensive validation of strategy contract address. /// /// # Checks - /// - Address is not zero (default/null address) + /// - Address is not the all-zero address /// - Address is valid for external calls /// /// # Errors - /// - Returns `VaultError::InvalidStrategyResponse` if address is invalid - pub fn validate_strategy_address(_env: &Env, strategy: &Address) -> Result<(), VaultError> { + /// - Returns `VaultError::InvalidAmount` if address is invalid + pub fn validate_strategy_address(env: &Env, strategy: &Address) -> Result<(), VaultError> { // In production, might add more checks: // - Is the address deployed? // - Does it have the strategy interface? - // For now, basic non-zero check - if strategy == &Address::from_string(&String::new(_env)) { - return Err(VaultError::InvalidStrategyResponse); + // For now, basic non-zero check. + // + // The previous `Address::from_string(&String::new(_env))` could not + // build the SDK `String` type at all. Building the all-zero contract + // address from its `ScAddress` avoids strkey decoding, which rejects + // an empty string outright. + let zero = + Address::try_from_val(env, &ScAddress::Contract(soroban_sdk::xdr::Hash([0u8; 32]))) + .map_err(|_| VaultError::InvalidAmount)?; + if strategy == &zero { + return Err(VaultError::InvalidAmount); } Ok(()) } diff --git a/contracts/vault/src/test.rs b/contracts/vault/src/test.rs index 7538cce0..b6cec1be 100644 --- a/contracts/vault/src/test.rs +++ b/contracts/vault/src/test.rs @@ -601,11 +601,26 @@ fn test_strategy_response_rejects_mismatched_asset() { let env = Env::default(); env.mock_all_auths(); - let (vault, usdc, _, _) = setup_vault(&env); + let vault_id = env.register(YieldVault, ()); + let vault = YieldVaultClient::new(&env, &vault_id); + let usdc = create_token(&env, &Address::generate(&env)); + vault.initialize(&Address::generate(&env), &usdc.address); + let wrong_asset = Address::generate(&env); let strategy_id = env.register(MaliciousStrategy, ()); - let strategy = StrategyClient::new(&env, &strategy_id); - strategy.initialize(&vault.contract_id, &wrong_asset, &100); + // `MaliciousStrategy` stores its seed state directly; it has no + // `initialize` on the generated client. + env.as_contract(&strategy_id, || { + env.storage() + .instance() + .set(&StrategyTestKey::Vault, &vault_id); + env.storage() + .instance() + .set(&StrategyTestKey::Asset, &wrong_asset); + env.storage() + .instance() + .set(&StrategyTestKey::Value, &100i128); + }); let result = YieldVault::validate_strategy_response(&env, &strategy_id, &usdc.address); assert_eq!(result, Err(VaultError::UnauthorizedStrategy)); @@ -616,10 +631,23 @@ fn test_strategy_response_rejects_negative_total_value() { let env = Env::default(); env.mock_all_auths(); - let (vault, usdc, _, _) = setup_vault(&env); + let vault_id = env.register(YieldVault, ()); + let vault = YieldVaultClient::new(&env, &vault_id); + let usdc = create_token(&env, &Address::generate(&env)); + vault.initialize(&Address::generate(&env), &usdc.address); + let strategy_id = env.register(MaliciousStrategy, ()); - let strategy = StrategyClient::new(&env, &strategy_id); - strategy.initialize(&vault.contract_id, &usdc.address, &-1); + env.as_contract(&strategy_id, || { + env.storage() + .instance() + .set(&StrategyTestKey::Vault, &vault_id); + env.storage() + .instance() + .set(&StrategyTestKey::Asset, &usdc.address); + env.storage() + .instance() + .set(&StrategyTestKey::Value, &-1i128); + }); let result = YieldVault::validate_strategy_response(&env, &strategy_id, &usdc.address); assert_eq!(result, Err(VaultError::InvalidAmount)); @@ -2728,12 +2756,10 @@ fn test_overflow_protection_near_limits() { let env = Env::default(); env.mock_all_auths(); - let admin = Address::generate(&env); - let token = create_token_contract(&env, &admin); - let vault = create_vault_contract(&env, &admin, &token.address); + let (vault, _token, usdc_sa, _admin) = setup_vault(&env); let user = Address::generate(&env); - token.mint(&user, &1000); + usdc_sa.mint(&user, &1000); vault.deposit(&user, &1000); // 1000 shares for 1000 assets let res_shares = vault.try_calculate_shares(&i128::MAX); diff --git a/contracts/vault/tests/access_control_test.rs b/contracts/vault/tests/access_control_test.rs index 9f9dcd13..01c532ef 100644 --- a/contracts/vault/tests/access_control_test.rs +++ b/contracts/vault/tests/access_control_test.rs @@ -1,7 +1,7 @@ //! Access control hardening tests for Issue #963. //! //! Verifies that every admin-only function rejects non-admin callers, -//! and that emergency-action functions return `VaultError::UnauthorizedCaller` +//! and that emergency-action functions return `VaultError::RescueUnauthorized` //! instead of panicking when an unauthorized address is provided. #[cfg(test)] @@ -64,20 +64,26 @@ mod access_control { assert!(!client2.is_paused()); } - // ── #963: set_fee_bps requires admin ───────────────────────────────────── + // ── #963: fee configuration requires admin ─────────────────────────────── + // + // The direct `set_fee_bps` setter was replaced by the timelocked + // `queue_fee_bps_change` / `execute_fee_bps_change` pair (Issue #969), so + // these exercise the current entry point. #[test] - fn test_set_fee_bps_only_admin() { + fn test_queue_fee_bps_change_only_admin() { let (_env, client, _admin, _token) = setup(); // Valid range succeeds for admin (mock_all_auths active) - client.set_fee_bps(&500i128); - assert_eq!(client.fee_bps(), 500i128); + client.queue_fee_bps_change(&500i128); } #[test] - fn test_set_fee_bps_invalid_range_rejected() { + fn test_queue_fee_bps_change_invalid_range_rejected() { let (_env, client, _admin, _token) = setup(); - let err = client.try_set_fee_bps(&10_001i128).unwrap_err().unwrap(); + let err = client + .try_queue_fee_bps_change(&10_001i128) + .unwrap_err() + .unwrap(); assert_eq!(err, VaultError::InvalidFeeBps); } @@ -118,8 +124,8 @@ mod access_control { assert_eq!( result.unwrap_err().unwrap(), - VaultError::UnauthorizedCaller, - "non-primary approver must be rejected with UnauthorizedCaller" + VaultError::RescueUnauthorized, + "non-primary approver must be rejected with RescueUnauthorized" ); } @@ -164,15 +170,12 @@ mod access_control { ); // Advance past dispute window - let env_ref = client.env; - env_ref - .ledger() - .set_timestamp(env_ref.ledger().timestamp() + 3_601); + env.ledger().set_timestamp(env.ledger().timestamp() + 3_601); let result = client.try_confirm_emergency_action(&outsider, &proposal_id); assert_eq!( result.unwrap_err().unwrap(), - VaultError::UnauthorizedCaller, + VaultError::RescueUnauthorized, "non-secondary address must be rejected" ); } @@ -194,16 +197,13 @@ mod access_control { ); // Advance past dispute window; try to confirm as primary (same as initiator) - let env_ref = client.env; - env_ref - .ledger() - .set_timestamp(env_ref.ledger().timestamp() + 3_601); + env.ledger().set_timestamp(env.ledger().timestamp() + 3_601); // primary != secondary so this call would be rejected by the secondary check first let result = client.try_confirm_emergency_action(&primary, &proposal_id); assert_eq!( result.unwrap_err().unwrap(), - VaultError::UnauthorizedCaller, + VaultError::RescueUnauthorized, "initiator cannot also be the confirmer" ); } @@ -212,9 +212,8 @@ mod access_control { #[test] fn test_accrue_yield_requires_admin() { - let (_env, client, admin, token) = setup(); - let env = client.env; - mint(env, &token, &admin, 1_000); + let (env, client, admin, token) = setup(); + mint(&env, &token, &admin, 1_000); // Succeeds for admin client.accrue_yield(&1_000i128); assert_eq!(client.total_assets(), 1_000i128); @@ -222,11 +221,15 @@ mod access_control { // ── #963: set_treasury requires admin ──────────────────────────────────── + // The direct `set_treasury` setter was replaced by the timelocked + // `queue_treasury_change` / `execute_treasury_change` pair (Issue #969). + #[test] fn test_set_treasury_requires_admin() { let (env, client, _admin, _token) = setup(); let treasury_addr = Address::generate(&env); - client.set_treasury(&treasury_addr); + client.queue_treasury_change(&treasury_addr); + client.execute_treasury_change(); assert_eq!(client.treasury(), Some(treasury_addr)); } diff --git a/contracts/vault/tests/operational_safety_tests.rs b/contracts/vault/tests/operational_safety_tests.rs index 51bcf9e1..892a4fdc 100644 --- a/contracts/vault/tests/operational_safety_tests.rs +++ b/contracts/vault/tests/operational_safety_tests.rs @@ -139,7 +139,7 @@ fn test_event_ordering_maintained_across_pause_resume_sequence() { #[test] fn test_strategy_validator_rejects_negative_value() { let result = strategy_validation::StrategyValidator::validate_total_value(-100_000); - assert_eq!(result, Err(VaultError::InvalidStrategyResponse)); + assert_eq!(result, Err(VaultError::InvalidAmount)); } #[test] @@ -159,7 +159,7 @@ fn test_strategy_validator_rejects_overflow_value() { let result = strategy_validation::StrategyValidator::validate_total_value( strategy_validation::MAX_STRATEGY_VALUE + 1, ); - assert_eq!(result, Err(VaultError::StrategyValueOverflow)); + assert_eq!(result, Err(VaultError::MathOverflow)); } #[test] @@ -183,7 +183,7 @@ fn test_deposit_result_validation_rejects_negative_delta() { // Deposit 1000, but total decreased (impossible, indicates malicious response) let result = strategy_validation::StrategyValidator::validate_deposit_result(1000, 10_000, 9_000); - assert_eq!(result, Err(VaultError::InvalidStrategyResponse)); + assert_eq!(result, Err(VaultError::InvalidAmount)); } #[test] @@ -207,7 +207,7 @@ fn test_withdrawal_result_validation_rejects_value_increase() { // Withdraw 1000, but total increased (impossible, indicates malicious response) let result = strategy_validation::StrategyValidator::validate_withdrawal_result(1000, 10_000, 11_000); - assert_eq!(result, Err(VaultError::InvalidStrategyResponse)); + assert_eq!(result, Err(VaultError::InvalidAmount)); } #[test] @@ -220,7 +220,7 @@ fn test_decimals_validation_accepts_valid_range() { #[test] fn test_decimals_validation_rejects_excessive() { let result = strategy_validation::StrategyValidator::validate_decimals(31); - assert_eq!(result, Err(VaultError::InvalidStrategyResponse)); + assert_eq!(result, Err(VaultError::InvalidAmount)); } #[test] @@ -232,13 +232,13 @@ fn test_price_response_validation_positive_price() { #[test] fn test_price_response_validation_rejects_zero_price() { let result = strategy_validation::StrategyValidator::validate_price_response(0, 6); - assert_eq!(result, Err(VaultError::InvalidStrategyResponse)); + assert_eq!(result, Err(VaultError::InvalidAmount)); } #[test] fn test_price_response_validation_rejects_negative_price() { let result = strategy_validation::StrategyValidator::validate_price_response(-1_000_000, 6); - assert_eq!(result, Err(VaultError::InvalidStrategyResponse)); + assert_eq!(result, Err(VaultError::InvalidAmount)); } // ════════════════════════════════════════════════════════════════════════════ @@ -268,7 +268,7 @@ fn test_governance_validator_quorum_not_met() { }; let result = governance_validation::GovernanceValidator::validate_quorum(1, &config); - assert_eq!(result, Err(VaultError::InsufficientGovernanceVotes)); + assert_eq!(result, Err(VaultError::QuorumNotReached)); } #[test] @@ -288,7 +288,7 @@ fn test_governance_validator_proposal_freshness_stale() { 100_000, // current time (way too late) 3600, // max age ); - assert_eq!(result, Err(VaultError::ProposalStale)); + assert_eq!(result, Err(VaultError::NoPendingWithdrawal)); } #[test] @@ -308,7 +308,7 @@ fn test_governance_validator_minimum_voting_period_not_elapsed() { 2000, // current time (too soon) 3600, // min voting period ); - assert_eq!(result, Err(VaultError::ProposalNotReady)); + assert_eq!(result, Err(VaultError::TimelockNotExpired)); } #[test] @@ -335,7 +335,7 @@ fn test_state_transition_invalid_stale_no_further_transition() { governance_validation::ProposalState::Stale, governance_validation::ProposalState::Approved, ); - assert_eq!(result, Err(VaultError::InvalidProposalTransition)); + assert_eq!(result, Err(VaultError::ProposalAlreadyExecuted)); } // ════════════════════════════════════════════════════════════════════════════ @@ -397,7 +397,7 @@ fn test_rounding_policy_validate_loss_acceptable() { fn test_rounding_policy_validate_loss_exceeds_threshold() { // 200 bp loss exceeds 100 bp threshold let result = rounding_consistency::RoundingPolicy::validate_rounding_loss(200, 10_000, 100); - assert_eq!(result, Err(VaultError::RoundingLossTooHigh)); + assert_eq!(result, Err(VaultError::SlippageExceeded)); } #[test]