diff --git a/contract/vault/kernel/src/actions/mod.rs b/contract/vault/kernel/src/actions/mod.rs index 5846a7fc3..7b42e58af 100644 --- a/contract/vault/kernel/src/actions/mod.rs +++ b/contract/vault/kernel/src/actions/mod.rs @@ -914,14 +914,6 @@ fn plan_withdrawal_request( }) } -#[inline] -fn sync_external_in_flight_assets(op_state: &OpState) -> u128 { - match op_state { - OpState::Allocating(state) => state.remaining, - _ => 0, - } -} - #[inline] fn ensure_sync_external_state_allowed(op_state: &OpState) -> Result<(), KernelError> { match op_state { @@ -942,15 +934,6 @@ fn plan_external_asset_sync( .checked_add(new_external_assets) .ok_or_else(|| KernelError::from(InvalidStateCode::SyncExternalOverflowIdlePlusExternal))?; - let reference_total = state - .total_assets - .saturating_add(sync_external_in_flight_assets(&state.op_state)); - if reference_total > 0 && new_total_assets > reference_total.saturating_mul(2) { - return Err(KernelError::from( - InvalidStateCode::SyncExternalWouldMoreThanDoubleTotalAssets, - )); - } - Ok(ExternalAssetSyncPlan { new_external_assets, new_total_assets, diff --git a/contract/vault/kernel/src/actions/tests.rs b/contract/vault/kernel/src/actions/tests.rs index cfda48344..fda699fa9 100644 --- a/contract/vault/kernel/src/actions/tests.rs +++ b/contract/vault/kernel/src/actions/tests.rs @@ -1239,15 +1239,15 @@ fn sync_external_assets_withdrawing() { None, &addr(0xFF), KernelAction::SyncExternalAssets { - new_external_assets: 400, + new_external_assets: 500, op_id: 4, now_ns: TimestampNs(0), }, ) .unwrap(); - assert_eq!(result.state.external_assets, 400); - assert_eq!(result.state.total_assets, 900); + assert_eq!(result.state.external_assets, 500); + assert_eq!(result.state.total_assets, 1_000); } #[test] @@ -1268,14 +1268,14 @@ fn sync_external_assets_refreshing() { None, &addr(0xFF), KernelAction::SyncExternalAssets { - new_external_assets: 600, + new_external_assets: 500, op_id: 5, now_ns: TimestampNs(0), }, ) .unwrap(); - assert_eq!(result.state.external_assets, 600); + assert_eq!(result.state.external_assets, 500); } #[test] @@ -1374,9 +1374,9 @@ fn sync_external_assets_payout_fails() { } #[test] -fn sync_external_assets_rejects_doubling() { +fn sync_external_assets_no_longer_applies_in_flight_allocation_bound() { use crate::state::op_state::AllocatingState; - // total_assets = 1000; trying to set external to 2001 would make new total > 2x + let mut state = idle_state(1_000, 1_000); state.op_state = OpState::Allocating(AllocatingState { op_id: 1, @@ -1392,24 +1392,21 @@ fn sync_external_assets_rejects_doubling() { None, &addr(0xFF), KernelAction::SyncExternalAssets { - new_external_assets: 2_001, + new_external_assets: 501, op_id: 1, now_ns: TimestampNs(0), }, - ); + ) + .unwrap(); - assert!(matches!( - result, - Err(KernelError::InvalidState( - InvalidStateCode::SyncExternalWouldMoreThanDoubleTotalAssets - )) - )); + assert_eq!(result.state.external_assets, 501); + assert_eq!(result.state.total_assets, 1_501); } #[test] -fn sync_external_assets_allows_up_to_double() { +fn sync_external_assets_allows_up_to_in_flight_allocation() { use crate::state::op_state::AllocatingState; - // total_assets = 1000; setting external to 1000 with idle=1000 => new total=2000 = 2x, OK + let mut state = idle_state(1_000, 1_000); state.op_state = OpState::Allocating(AllocatingState { op_id: 1, @@ -1425,7 +1422,7 @@ fn sync_external_assets_allows_up_to_double() { None, &addr(0xFF), KernelAction::SyncExternalAssets { - new_external_assets: 1_000, + new_external_assets: 500, op_id: 1, now_ns: TimestampNs(0), }, @@ -1433,7 +1430,66 @@ fn sync_external_assets_allows_up_to_double() { assert!(result.is_ok()); let result = result.unwrap(); - assert_eq!(result.state.total_assets, 2_000); + assert_eq!(result.state.external_assets, 500); + assert_eq!(result.state.total_assets, 1_500); +} + +#[test] +fn sync_external_assets_accepts_runtime_validated_refresh_total() { + use crate::state::op_state::RefreshingState; + + let mut state = VaultState::with_initial(1_999, 1_000, 1_000, 999, TimestampNs(0)); + state.op_state = OpState::Refreshing(RefreshingState { + op_id: 8, + index: 0, + plan: vec![0], + }); + let config = test_config(); + + let result = apply_action( + state, + &config, + None, + &addr(0xFF), + KernelAction::SyncExternalAssets { + new_external_assets: 2_997, + op_id: 8, + now_ns: TimestampNs(0), + }, + ) + .unwrap(); + + assert_eq!(result.state.external_assets, 2_997); + assert_eq!(result.state.total_assets, 3_997); +} + +#[test] +fn sync_external_assets_allows_decrease() { + use crate::state::op_state::RefreshingState; + + let mut state = VaultState::with_initial(11_000, 1_000, 1_000, 10_000, TimestampNs(0)); + state.op_state = OpState::Refreshing(RefreshingState { + op_id: 9, + index: 0, + plan: vec![0], + }); + let config = test_config(); + + let result = apply_action( + state, + &config, + None, + &addr(0xFF), + KernelAction::SyncExternalAssets { + new_external_assets: 0, + op_id: 9, + now_ns: TimestampNs(0), + }, + ) + .unwrap(); + + assert_eq!(result.state.external_assets, 0); + assert_eq!(result.state.total_assets, 1_000); } #[test] diff --git a/contract/vault/kernel/src/error.rs b/contract/vault/kernel/src/error.rs index f510982f2..0cc62bb9c 100644 --- a/contract/vault/kernel/src/error.rs +++ b/contract/vault/kernel/src/error.rs @@ -27,7 +27,6 @@ pub enum InvalidStateCode { SyncExternalRequiresActiveOp = 16, SyncExternalRequiresAllowedStates = 17, SyncExternalOverflowIdlePlusExternal = 18, - SyncExternalWouldMoreThanDoubleTotalAssets = 19, AbortRefreshingRequiresActiveOp = 20, AbortRefreshingRequiresRefreshing = 21, AbortAllocatingRequiresAllocating = 22, @@ -87,9 +86,6 @@ impl InvalidStateCode { Self::SyncExternalOverflowIdlePlusExternal => { "sync_external_assets overflow: idle + external exceeds u128" } - Self::SyncExternalWouldMoreThanDoubleTotalAssets => { - "sync_external_assets would more than double total_assets" - } Self::AbortRefreshingRequiresActiveOp => "abort_refreshing requires active op", Self::AbortRefreshingRequiresRefreshing => "abort_refreshing requires Refreshing", Self::AbortAllocatingRequiresAllocating => "abort_allocating requires Allocating", diff --git a/contract/vault/kernel/tests/property_tests.rs b/contract/vault/kernel/tests/property_tests.rs index 2c3d872d9..109ae1002 100644 --- a/contract/vault/kernel/tests/property_tests.rs +++ b/contract/vault/kernel/tests/property_tests.rs @@ -3815,18 +3815,21 @@ proptest! { #[test] fn prop_spec_sync_external_assets_updates_total( idle in 0u64..1_000_000, - external in 0u64..1_000_000, + existing_external in 0u64..1_000_000, + in_flight in 0u64..1_000_000, + delta in 0u64..1_000_000, ) { - prop_assume!(external <= idle); + let delta = delta.min(in_flight); + let external = existing_external + delta; let mut state = VaultState::new(); state.idle_assets = idle as u128; - state.external_assets = 0; - state.total_assets = idle as u128; + state.external_assets = existing_external as u128; + state.total_assets = idle as u128 + existing_external as u128; state.op_state = OpState::Allocating(AllocatingState { op_id: 7, index: 0, - remaining: 0, - plan: vec![alloc_step(0, 0)], + remaining: in_flight as u128, + plan: vec![alloc_step(0, in_flight as u128)], }); let config = default_config(); diff --git a/contract/vault/soroban/src/contract/curator_vault.rs b/contract/vault/soroban/src/contract/curator_vault.rs index 04e24686c..5df7f897b 100644 --- a/contract/vault/soroban/src/contract/curator_vault.rs +++ b/contract/vault/soroban/src/contract/curator_vault.rs @@ -749,6 +749,52 @@ where .unwrap_or_else(|_| abort!("market principal failed")); } + fn set_policy_principal( + policy: &mut PolicyState, + market: TargetId, + principal: u128, + message: &'static str, + ) -> Result<(), RuntimeError> { + policy + .set_principal(market, principal) + .map_err(|_| invalid_state_error(message)) + } + + fn validate_supply_observation( + policy: &PolicyState, + market: TargetId, + observed_total_assets: u128, + supply_amount: u128, + ) -> Result<(), RuntimeError> { + let previous_principal = policy + .principal_for(market) + .ok_or_else(|| invalid_state_error("unknown market principal on supply"))?; + let max_principal = previous_principal + .checked_add(supply_amount) + .ok_or_else(|| invalid_state_error("principal overflow on supply"))?; + if observed_total_assets < previous_principal || observed_total_assets > max_principal { + return Err(invalid_state_error("supply observation out of bounds")); + } + Ok(()) + } + + fn validate_refresh_observation( + policy: &PolicyState, + market: TargetId, + observed_total_assets: u128, + ) -> Result<(), RuntimeError> { + let cap = policy + .market_config(market) + .ok_or_else(|| invalid_state_error("unknown refreshed market"))? + .cap; + if observed_total_assets > cap { + return Err(invalid_state_error( + "refresh observation exceeds market cap", + )); + } + Ok(()) + } + pub(crate) fn complete_supply_allocation( &mut self, caller: Address, @@ -757,14 +803,35 @@ where op_id: u64, now_ns: u64, ) -> Result { - self.update_market_principal(market, observed_total_assets); - let new_external = self.sync_external_assets( - caller, - op_id, - self.policy_state().external_assets()?, - now_ns, + let allocation = self + .state()? + .op_state + .as_allocating() + .ok_or_else(|| invalid_state_error(""))?; + let current_step = allocation + .plan + .get(allocation.index as usize) + .ok_or_else(|| invalid_state_error("allocation step missing"))?; + if current_step.target_id != market { + return Err(RuntimeError::invalid_input("")); + } + Self::validate_supply_observation( + self.policy_state(), + market, + observed_total_assets, + current_step.amount, + )?; + let mut staged_policy = self.policy_state.clone(); + Self::set_policy_principal( + &mut staged_policy, + market, + observed_total_assets, + "market principal failed", )?; + let new_external_assets = staged_policy.external_assets()?; + let new_external = self.sync_external_assets(caller, op_id, new_external_assets, now_ns)?; self.finish_allocation_internal(caller, op_id, now_ns)?; + self.policy_state = staged_policy; self.storage.save_policy_state(&self.policy_state)?; Ok(new_external) } @@ -816,13 +883,21 @@ where Ok(()) } - fn apply_refreshed_positions(&mut self, refreshed_positions: &[(TargetId, u128)]) { - let policy = self.policy_state_mut(); + fn stage_refreshed_positions( + &self, + refreshed_positions: &[(TargetId, u128)], + ) -> Result { + let mut policy = self.policy_state.clone(); for &(market, total_assets) in refreshed_positions { - policy - .set_principal(market, total_assets) - .unwrap_or_else(|_| abort!("refresh principal failed")); + Self::validate_refresh_observation(&policy, market, total_assets)?; + Self::set_policy_principal( + &mut policy, + market, + total_assets, + "refresh principal failed", + )?; } + Ok(policy) } pub(crate) fn complete_refresh_with_positions( @@ -834,10 +909,11 @@ where ) -> Result { let refreshed_positions = Self::classify_refreshed_positions(refreshed_positions); self.validate_refreshed_positions_against_plan(&refreshed_positions)?; - self.apply_refreshed_positions(&refreshed_positions); - let new_external_assets = self.policy_state().external_assets()?; + let staged_policy = self.stage_refreshed_positions(&refreshed_positions)?; + let new_external_assets = staged_policy.external_assets()?; self.sync_external_assets(caller, op_id, new_external_assets, now_ns)?; let result = self.finish_refreshing(caller, op_id, now_ns)?; + self.policy_state = staged_policy; self.storage.save_policy_state(&self.policy_state)?; Ok(result) } diff --git a/contract/vault/soroban/src/tests.rs b/contract/vault/soroban/src/tests.rs index e15a8b8db..582a5db4a 100644 --- a/contract/vault/soroban/src/tests.rs +++ b/contract/vault/soroban/src/tests.rs @@ -296,6 +296,7 @@ mod contract_tests { use alloc::vec; use alloc::vec::Vec; use soroban_sdk::{Address as SdkAddress, Bytes, Env}; + use templar_curator_primitives::policy::state::MarketConfig; use templar_curator_primitives::PolicyState; use templar_soroban_shared_types::{ GovernanceCommand, VaultCommand, VaultCommandResult, GOVERNANCE_CONFIG_KIND_VIRTUAL_OFFSETS, @@ -586,6 +587,36 @@ mod contract_tests { assert!(vault.state().unwrap().op_state.is_idle()); } + #[test] + fn test_complete_supply_allocation_rejects_observed_assets_above_supplied_step() { + let mut vault = create_test_vault(); + let caller = templar_vault_kernel::Address([3u8; 32]); + + { + let policy = vault.policy_state_mut(); + policy + .set_market_config(0, MarketConfig::new(true, 10_000, None)) + .unwrap(); + policy.set_principal(0, 1_000).unwrap(); + } + { + let state = vault.state_mut().unwrap(); + state.idle_assets = 1_000; + state.external_assets = 1_000; + state.total_assets = 2_000; + } + let op_id = begin_allocating(&mut vault, caller, vec![(0, 500)], 1_000).unwrap(); + + let error = vault + .complete_supply_allocation(caller, 0, 1_501, op_id, 1_000) + .expect_err("supply callback must not over-report the active step"); + + assert_eq!(error, RuntimeError::InvalidState); + assert!(vault.state().unwrap().op_state.is_allocating()); + assert_eq!(vault.state().unwrap().external_assets, 1_000); + assert_eq!(vault.policy_state().principal_for(0), Some(1_000)); + } + #[test] fn test_sync_external_assets_requires_active_allocation_op_id() { let mut vault = create_test_vault(); @@ -668,6 +699,67 @@ mod contract_tests { assert!(vault.state().unwrap().op_state.is_refreshing()); } + #[test] + fn test_complete_refresh_rejects_cumulative_adapter_inflation() { + let mut vault = create_test_vault(); + let caller = templar_vault_kernel::Address([3u8; 32]); + + { + let policy = vault.policy_state_mut(); + policy + .set_market_config(0, MarketConfig::new(true, 1_998, None)) + .unwrap(); + policy.set_principal(0, 999).unwrap(); + } + { + let state = vault.state_mut().unwrap(); + state.idle_assets = 1_000; + state.external_assets = 999; + state.total_assets = 1_999; + } + let op_id = vault.begin_refreshing(caller, vec![0], 1_500).unwrap(); + + let error = vault + .complete_refresh_with_positions(caller, &[(0, 2_997)], op_id, 1_500) + .expect_err("refresh must not accept cumulative adapter inflation"); + + assert_eq!(error, RuntimeError::InvalidState); + assert!(vault.state().unwrap().op_state.is_refreshing()); + assert_eq!(vault.state().unwrap().external_assets, 999); + assert_eq!(vault.policy_state().principal_for(0), Some(999)); + } + + #[test] + fn test_complete_refresh_allows_adapter_reported_decrease_within_cap() { + let mut vault = create_test_vault(); + let caller = templar_vault_kernel::Address([3u8; 32]); + + { + let policy = vault.policy_state_mut(); + policy + .set_market_config(0, MarketConfig::new(true, 10_000, None)) + .unwrap(); + policy.set_principal(0, 10_000).unwrap(); + } + { + let state = vault.state_mut().unwrap(); + state.idle_assets = 1_000; + state.external_assets = 10_000; + state.total_assets = 11_000; + } + let op_id = vault.begin_refreshing(caller, vec![0], 1_500).unwrap(); + + let result = vault + .complete_refresh_with_positions(caller, &[(0, 0)], op_id, 1_500) + .expect("refresh may report a legitimate market decrease"); + + assert_eq!(result.markets_refreshed, 1); + assert!(vault.state().unwrap().op_state.is_idle()); + assert_eq!(vault.state().unwrap().external_assets, 0); + assert_eq!(vault.state().unwrap().total_assets, 1_000); + assert_eq!(vault.policy_state().principal_for(0), Some(0)); + } + #[test] fn test_execute_withdraw_respects_min_withdrawal_assets() { let mut vault = create_test_vault();