From d78978091731cbd78c8516e84ff3980f1f333f99 Mon Sep 17 00:00:00 2001 From: obedojukwu50-stack Date: Thu, 23 Jul 2026 20:57:53 +0000 Subject: [PATCH] fix: respect force_refresh in ReflectorOracleClient::twap cache The twap method accepted a force_refresh parameter but never used it, causing the intra-transaction cache to be returned even when a fresh oracle call was requested. Changes: - Skip cache read when force_refresh is true in twap method - Still cache the freshly-fetched result for subsequent calls - Update NatSpec doc comment documenting cache behavior - Add cross-transaction cache reset test to verify cache isolation - Remove disabled test file (replaced by active test) Closes: TWAP cache force_refresh bug --- contracts/predictify-hybrid/src/oracles.rs | 20 +++++++-- .../src/tests/reflector_twap_cache_tests.rs | 27 +++++++++++- .../reflector_twap_cache_tests.rs.disabled | 43 ------------------- 3 files changed, 42 insertions(+), 48 deletions(-) delete mode 100644 contracts/predictify-hybrid/tests/reflector_twap_cache_tests.rs.disabled diff --git a/contracts/predictify-hybrid/src/oracles.rs b/contracts/predictify-hybrid/src/oracles.rs index 3cef0de9..9b95168a 100644 --- a/contracts/predictify-hybrid/src/oracles.rs +++ b/contracts/predictify-hybrid/src/oracles.rs @@ -643,7 +643,16 @@ impl<'a> ReflectorOracleClient<'a> { res } - /// Get TWAP (Time-Weighted Average Price) for an asset + /// Get TWAP (Time-Weighted Average Price) for an asset. + /// + /// Uses an intra-transaction cache keyed by (asset, records) to avoid + /// duplicate oracle calls within the same transaction. The cache lives + /// in temporary storage and is automatically discarded when the + /// transaction ends. + /// + /// When `force_refresh` is `true` the cache is bypassed and a fresh + /// oracle call is made; the result still updates the cache so subsequent + /// calls in the same transaction benefit from it. pub fn twap(&self, asset: ReflectorAsset, records: u32, force_refresh: bool) -> Option { // Build a cache key unique to this transaction let cache_key: (Symbol, Val, Val) = ( @@ -652,10 +661,13 @@ impl<'a> ReflectorOracleClient<'a> { records.into_val(self.env), ); // Attempt to read from temporary storage (per-transaction cache) - if let Some(cached) = self.env.storage().temporary().get::<_, Option>(&cache_key) { - return cached; + // only when the caller hasn't requested a forced refresh. + if !force_refresh { + if let Some(cached) = self.env.storage().temporary().get::<_, Option>(&cache_key) { + return cached; + } } - // Not cached; perform contract call + // Not cached (or force_refresh requested); perform contract call let args = vec![ self.env, asset.into_val(self.env), diff --git a/contracts/predictify-hybrid/src/tests/reflector_twap_cache_tests.rs b/contracts/predictify-hybrid/src/tests/reflector_twap_cache_tests.rs index 0e900590..299c097a 100644 --- a/contracts/predictify-hybrid/src/tests/reflector_twap_cache_tests.rs +++ b/contracts/predictify-hybrid/src/tests/reflector_twap_cache_tests.rs @@ -21,7 +21,7 @@ impl MockReflectorOracle { } #[test] -fn test_reflector_twap_cache() { +fn test_reflector_twap_cache_within_transaction() { let env = Env::default(); // Register the mock oracle contract @@ -53,3 +53,28 @@ fn test_reflector_twap_cache() { assert_eq!(res4, Some(100_000_000)); assert_eq!(mock_client.get_calls(), 2); } + +#[test] +fn test_twap_cache_resets_between_transactions() { + // Transaction 1 + let env1 = Env::default(); + let mock_id1 = env1.register_contract(None, MockReflectorOracle); + let mock_client1 = MockReflectorOracleClient::new(&env1, &mock_id1); + let client1 = ReflectorOracleClient::new(&env1, mock_id1.clone()); + let asset = ReflectorAsset::Other(Symbol::new(&env1, "BTC")); + + let val1 = client1.twap(asset.clone(), 5, false); + assert_eq!(val1, Some(100_000_000)); + assert_eq!(mock_client1.get_calls(), 1); + + // Transaction 2 — fresh Env, cache must not persist + let env2 = Env::default(); + let mock_id2 = env2.register_contract(None, MockReflectorOracle); + let mock_client2 = MockReflectorOracleClient::new(&env2, &mock_id2); + let client2 = ReflectorOracleClient::new(&env2, mock_id2.clone()); + + let val2 = client2.twap(asset, 5, false); + assert_eq!(val2, Some(100_000_000)); + // In a new transaction the cache is empty, so the mock must be called again. + assert_eq!(mock_client2.get_calls(), 1); +} diff --git a/contracts/predictify-hybrid/tests/reflector_twap_cache_tests.rs.disabled b/contracts/predictify-hybrid/tests/reflector_twap_cache_tests.rs.disabled deleted file mode 100644 index b48b9f54..00000000 --- a/contracts/predictify-hybrid/tests/reflector_twap_cache_tests.rs.disabled +++ /dev/null @@ -1,43 +0,0 @@ -//! Tests for intra‑transaction TWAP cache in ReflectorOracleClient - -#[cfg(test)] -mod tests { - use super::*; - use soroban_sdk::{Env, Address}; - use crate::mocks::oracle_mock::MockReflectorOracleClient; - use crate::oracles::ReflectorAsset; - - #[test] - fn test_twap_caches_within_transaction() { - let env = Env::default(); - let contract_id = Address::generate(&env); - let client = MockReflectorOracleClient::new(&env, contract_id.clone()); - - // First call should compute and cache the result - let first = client.twap(ReflectorAsset::BTC, 5); - assert_eq!(first, Some(5000)); // Mock returns records * 1000 - - // Second call in the same transaction should hit the cache - let second = client.twap(ReflectorAsset::BTC, 5); - assert_eq!(second, first); - } - - #[test] - fn test_twap_cache_resets_between_transactions() { - // Transaction 1 - let env1 = Env::default(); - let contract_id1 = Address::generate(&env1); - let client1 = MockReflectorOracleClient::new(&env1, contract_id1.clone()); - let val1 = client1.twap(ReflectorAsset::ETH, 3); - assert_eq!(val1, Some(3000)); - - // Simulate a new transaction by creating a new Env and client - let env2 = Env::default(); - let contract_id2 = Address::generate(&env2); - let client2 = MockReflectorOracleClient::new(&env2, contract_id2.clone()); - let val2 = client2.twap(ReflectorAsset::ETH, 3); - assert_eq!(val2, Some(3000)); - // The cached value from the first transaction should not affect the second - assert_eq!(val2, val1); - } -}