From e16b8b3bbf6db66f1ecbb2077165c5a33043e71e Mon Sep 17 00:00:00 2001 From: carrion256 Date: Fri, 15 May 2026 10:39:35 +0200 Subject: [PATCH] fix: validate adapter asset observations (Nexus bc013b77-b8a9-4b9a-867a-228152a95c3b) Move adapter-reported external asset validation out of the generic kernel sync path and into the Soroban runtime policy boundary. Supply callbacks now validate the active market step against the supplied amount, refresh callbacks validate observed market principals against policy caps, and legitimate refresh decreases are allowed instead of rejected by aggregate no-decrease bounds. Stage policy principal updates before kernel sync and operation completion so failed validation or sync cannot leave in-memory policy state mutated. Remove the old aggregate sync heuristic and keep kernel SyncExternalAssets focused on op-state and total-assets consistency. Verification: cargo fmt --all; git diff --check; cargo test -p templar-soroban-runtime --features testutils complete_supply_allocation -- --nocapture; cargo test -p templar-soroban-runtime --features testutils complete_refresh -- --nocapture; cargo test -p templar-vault-kernel sync_external_assets -- --nocapture; cargo test -p templar-vault-kernel --lib -- --nocapture; cargo test -p templar-soroban-runtime --features testutils --lib -- --nocapture; cargo test -p templar-soroban-runtime --features testutils --test property_tests -- --nocapture; just -f contract/vault/soroban/justfile size-budget-check. --- contract/vault/kernel/src/actions/mod.rs | 17 --- contract/vault/kernel/src/actions/tests.rs | 94 ++++++++++++---- contract/vault/kernel/src/error.rs | 4 - contract/vault/kernel/tests/property_tests.rs | 15 +-- .../soroban/src/contract/curator_vault.rs | 102 +++++++++++++++--- contract/vault/soroban/src/tests.rs | 92 ++++++++++++++++ 6 files changed, 265 insertions(+), 59 deletions(-) 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();