From d866079b8d1c5e26a28a531895ec48e90a83d2a8 Mon Sep 17 00:00:00 2001 From: Sturdy <91910406+apollo-sturdy@users.noreply.github.com> Date: Mon, 26 Jun 2023 17:53:08 +0200 Subject: [PATCH] fix: Error on too few snapshots and too short window size --- contracts/oracle/base/src/error.rs | 3 + contracts/oracle/wasm/src/price_source.rs | 10 ++++ .../tests/prop_tests.proptest-regressions | 3 + contracts/oracle/wasm/tests/prop_tests.rs | 8 ++- .../oracle/wasm/tests/test_price_source.rs | 60 ++++++++++++++++++- packages/testing/src/wasm_oracle.rs | 14 +++-- 6 files changed, 87 insertions(+), 11 deletions(-) diff --git a/contracts/oracle/base/src/error.rs b/contracts/oracle/base/src/error.rs index c0afe13a..0051112b 100644 --- a/contracts/oracle/base/src/error.rs +++ b/contracts/oracle/base/src/error.rs @@ -62,6 +62,9 @@ pub enum ContractError { #[error("No TWAP snapshots found")] NoSnapshots {}, + + #[error("There needs to be at least two TWAP snapshots")] + NotEnoughSnapshots {}, } pub type ContractResult = Result; diff --git a/contracts/oracle/wasm/src/price_source.rs b/contracts/oracle/wasm/src/price_source.rs index 8682f0fd..42627f9c 100644 --- a/contracts/oracle/wasm/src/price_source.rs +++ b/contracts/oracle/wasm/src/price_source.rs @@ -174,6 +174,12 @@ impl PriceSourceUnchecked for WasmPriceSourceUnch &route_assets, )?; + if window_size <= 1 { + return Err(ContractError::InvalidPriceSource { + reason: "window_size must be greater than 1".to_string(), + }); + } + Ok(WasmPriceSourceChecked::AstroportTwap { pair_address: deps.api.addr_validate(&pair_address)?, window_size, @@ -307,6 +313,10 @@ fn query_astroport_twap_price( .may_load(deps.storage, denom)? .ok_or(ContractError::NoSnapshots {})?; + if snapshots.len() < 2 { + return Err(ContractError::NotEnoughSnapshots {}); + } + // First, query the current TWAP snapshot let current_snapshot = AstroportTwapSnapshot { timestamp: env.block.time.seconds(), diff --git a/contracts/oracle/wasm/tests/prop_tests.proptest-regressions b/contracts/oracle/wasm/tests/prop_tests.proptest-regressions index e408f25b..d9c4eea3 100644 --- a/contracts/oracle/wasm/tests/prop_tests.proptest-regressions +++ b/contracts/oracle/wasm/tests/prop_tests.proptest-regressions @@ -10,3 +10,6 @@ cc 5ca64eeb9f8cb0df4166ea7782ec07d91492eee50da8160b1342f6e8f4f33cfe # shrinks to cc 122d1c7329103011810193f8f25f5908c783eed3271f56b7c00f14e11422aeba # shrinks to pair_type = Xyk, (pair_denoms, route_prices) = (["uosmo", "stake"], [("stake", Decimal(132373101975.866291703926105585)), ("uatom", Decimal(552431738294.751627746132883125))]) cc 6d24c61ba6a263d4d4f0e29f8e2a6c16db3518d2d271202f2a17ac595339cb4f # shrinks to pair_type = Xyk, (pair_denoms, route_prices) = (["stake", "uion"], [("uion", Decimal(7040706654.440918446759799961)), ("uatom", Decimal(1483941554.657027178575457782)), ("uosmo", Decimal(1833715528.085063919419402131))]) cc ba1ad666b045dcc03e1fd28f4d45f10c4f54611eac4b65c7eafbc37507572997 # shrinks to pair_type = Stable, (pair_denoms, route_prices) = (["uosmo", "uatom"], []), initial_liq = [1, 1], (window_size, tolerance) = (2, 0) +cc 9ad2ce2c26a6c085aa252fee5b122324fcfda2fcd470fbe206174da2389f47ab # shrinks to pair_type = Xyk, (pair_denoms, route_prices) = (["uatom", "uosmo"], []), initial_liq = [10822962050445088126871, 10822962036372087568463], (window_size, tolerance) = (6, 0) +cc b4ff0f2eb1a0d677fc1972633f2c2a381e68eb54965501ff58605b81f0093732 # shrinks to pair_type = Xyk, (pair_denoms, route_prices) = (["stake", "uatom"], []), initial_liq = [240051541009477748141810, 6721157557512798311], (window_size, tolerance) = (860495, 38535) +cc 72e53f8b940ea84ad9709479facdc45af95a67f0b6447bbc316ac5ea02152f57 # shrinks to pair_type = Stable, (pair_denoms, route_prices) = (["uosmo", "uatom"], []), initial_liq = [54095484775203625044646, 512553686986456532], (window_size, tolerance) = (82800, 12299) diff --git a/contracts/oracle/wasm/tests/prop_tests.rs b/contracts/oracle/wasm/tests/prop_tests.rs index f0ed6073..490a80b3 100644 --- a/contracts/oracle/wasm/tests/prop_tests.rs +++ b/contracts/oracle/wasm/tests/prop_tests.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::{cmp::min, collections::HashSet}; use astroport::factory::PairType; use cosmwasm_std::Decimal; @@ -34,7 +34,9 @@ pub fn decimal() -> impl Strategy { /// Generates a pair of u128s where the first is greater than the second. This is so we can swap /// without exceeding max spread. pub fn liquidity() -> impl Strategy { - (100000000..MAX_LIQ).prop_flat_map(|v| (v..MAX_LIQ, Just(v))).prop_map(|(x, y)| [x, y]) + (1000000000..MAX_LIQ) + .prop_flat_map(|v| (v..min(v * 10000, MAX_LIQ), Just(v / 10))) + .prop_map(|(x, y)| [x, y]) } /// Generates a vector of (denom, price) tuples that can be used as route assets for the price source. @@ -95,6 +97,6 @@ proptest! { } else { pair_denoms[1] }; - validate_and_query_astroport_twap_price_source(pair_type, &pair_denoms, base_denom, &route_prices, window_size, tolerance,&initial_liq); + validate_and_query_astroport_twap_price_source(pair_type, &pair_denoms, base_denom, &route_prices, tolerance, window_size, &initial_liq); } } diff --git a/contracts/oracle/wasm/tests/test_price_source.rs b/contracts/oracle/wasm/tests/test_price_source.rs index 5a1a7974..ab5efe46 100644 --- a/contracts/oracle/wasm/tests/test_price_source.rs +++ b/contracts/oracle/wasm/tests/test_price_source.rs @@ -3,7 +3,11 @@ use std::str::FromStr; use astroport::factory::PairType; use cosmwasm_std::{from_binary, testing::mock_dependencies, Addr, Decimal, Uint128}; use cw_it::{ - astroport::{robot::AstroportTestRobot, utils::native_asset}, + astroport::{ + robot::AstroportTestRobot, + utils::{native_asset, native_info}, + }, + robot::TestRobot, test_tube::Account, }; use cw_storage_plus::Map; @@ -21,7 +25,8 @@ use mars_testing::{ mock_env_at_block_time, test_runner::get_test_runner, wasm_oracle::{ - fixed_source, get_contracts, setup_test, validate_and_query_astroport_spot_price_source, + astro_init_params, fixed_source, get_contracts, setup_test, + validate_and_query_astroport_spot_price_source, validate_and_query_astroport_twap_price_source, WasmOracleTestRobot, }, }; @@ -191,6 +196,9 @@ pub fn test_validate_and_query_astroport_spot_price_source( #[test_case(PairType::Xyk {}, &["uatom","uion"], "uosmo", &[("uion",TWO)], 5, 100; "XYK, route with non-base existing asset, in pair")] #[test_case(PairType::Stable {}, &["uatom","uosmo"], "uosmo", &[], 5, 100; "Stable, no route, base_denom in pair")] #[test_case(PairType::Stable {}, &["uatom","uion"], "uosmo", &[("uion",TWO)], 5, 100; "Stable, route with non-base existing asset, in pair")] +#[test_case(PairType::Xyk {}, &["uatom","uosmo"], "uosmo", &[], 5,0 => panics; "Zero window size")] +#[test_case(PairType::Xyk {}, &["uatom","uosmo"], "uosmo", &[], 0,5; "Zero tolerance")] +#[test_case(PairType::Xyk {}, &["stake","uatom"], "uatom", &[], 38535, 860495; "idk whats wrong here")] fn test_validate_and_query_astroport_twap_price( pair_type: PairType, pair_denoms: &[&str; 2], @@ -210,6 +218,54 @@ fn test_validate_and_query_astroport_twap_price( ) } +#[test] +fn test_query_astroport_twap_price_with_only_one_snapshot() { + let base_denom = "uosmo"; + let runner = get_test_runner(); + let admin = &runner.init_accounts()[0]; + let robot = WasmOracleTestRobot::new(&runner, get_contracts(&runner), admin, Some(base_denom)); + + let pair_type = PairType::Xyk {}; + let pair_denoms = ["uatom", "uosmo"]; + + let initial_liq: [Uint128; 2] = + DEFAULT_LIQ.iter().map(|x| Uint128::from(*x)).collect::>().try_into().unwrap(); + let (pair_address, _lp_token_addr) = robot.create_astroport_pair( + pair_type.clone(), + [native_info(pair_denoms[0]), native_info(pair_denoms[1])], + astro_init_params(&pair_type), + admin, + Some(initial_liq), + ); + + let price_source = WasmPriceSourceUnchecked::AstroportTwap { + pair_address: pair_address.clone(), + route_assets: vec![], + tolerance: 3, + window_size: 2, + }; + + robot + .add_denom_precision_to_coin_registry(pair_denoms[0], 6, admin) + .add_denom_precision_to_coin_registry(pair_denoms[1], 6, admin) + .add_denom_precision_to_coin_registry(base_denom, 6, admin) + .set_price_source(pair_denoms[0], price_source.clone(), admin) + .assert_price_source(pair_denoms[0], price_source) + .record_twap_snapshots(&[pair_denoms[0]], admin); + + let err = robot + .wasm() + .query::<_, mars_red_bank_types::oracle::PriceResponse>( + &robot.mars_oracle_contract_addr, + &QueryMsg::Price { + denom: "uatom".to_string(), + }, + ) + .unwrap_err(); + + assert!(err.to_string().contains("There needs to be at least two TWAP snapshots")); +} + #[test] #[should_panic] fn record_twap_snapshots_errors_on_non_twap_price_source() { diff --git a/packages/testing/src/wasm_oracle.rs b/packages/testing/src/wasm_oracle.rs index eef22a50..17fc39f8 100644 --- a/packages/testing/src/wasm_oracle.rs +++ b/packages/testing/src/wasm_oracle.rs @@ -241,12 +241,14 @@ impl<'a> WasmOracleTestRobot<'a> { } pub fn query_price_via_simulation(&self, pair_addr: &str, denom: &str) -> Decimal { let decimals = self.query_native_coin_registry(denom).unwrap(); - let one = Uint128::from(10u128.pow(decimals as u32)); + let one: Uint128 = Uint128::from(10u128.pow(decimals as u32)); + let denominator = one * Uint128::from(10u128); - let return_amount = - self.query_simulate_swap(pair_addr, native_asset(denom, one), None).return_amount; + let return_amount = self + .query_simulate_swap(pair_addr, native_asset(denom, denominator), None) + .return_amount; - Decimal::from_ratio(return_amount, one) + Decimal::from_ratio(return_amount, denominator) } // ===== Owner update methods ====== @@ -461,7 +463,7 @@ pub fn validate_and_query_astroport_twap_price_source( .set_price_source(pair_denoms[0], price_source.clone(), admin) .assert_price_source(pair_denoms[0], price_source) .record_twap_snapshots(&[pair_denoms[0]], admin) - .increase_time(window_size / 2) + .increase_time(window_size + tolerance) .swap_on_astroport_pair( &pair_address, native_asset(pair_denoms[1], initial_liq[1].u128() / 1000000), @@ -481,6 +483,6 @@ pub fn validate_and_query_astroport_twap_price_source( robot .record_twap_snapshots(&[pair_denoms[0]], admin) - .increase_time(window_size / 2) + .increase_time(window_size + tolerance) .assert_price_almost_equal(pair_denoms[0], expected_price, Decimal::percent(1)); }