From 4f0c6163634d2de138a6678c5970bd12387221b0 Mon Sep 17 00:00:00 2001 From: piobab Date: Thu, 17 Aug 2023 17:24:24 +0200 Subject: [PATCH] Validate if denom_decimals <= 18. Validate if scaled pyth price != 0. --- contracts/oracle/base/Cargo.toml | 3 +- contracts/oracle/base/src/lib.rs | 1 - contracts/oracle/base/src/pyth.rs | 33 +++++++++- contracts/oracle/osmosis/Cargo.toml | 3 +- contracts/oracle/osmosis/src/price_source.rs | 6 +- .../osmosis/tests/test_set_price_source.rs | 54 +++++++++------- contracts/oracle/wasm/Cargo.toml | 2 +- contracts/oracle/wasm/src/price_source.rs | 2 +- .../oracle/wasm/tests/test_price_source.rs | 63 +++++++++++++++++++ 9 files changed, 136 insertions(+), 31 deletions(-) diff --git a/contracts/oracle/base/Cargo.toml b/contracts/oracle/base/Cargo.toml index d7c357996..535dcc096 100644 --- a/contracts/oracle/base/Cargo.toml +++ b/contracts/oracle/base/Cargo.toml @@ -15,7 +15,6 @@ doctest = false [features] # for more explicit tests, cargo test --features=backtraces backtraces = ["cosmwasm-std/backtraces"] -pyth = ["pyth-sdk-cw"] [dependencies] cosmwasm-std = { workspace = true } @@ -24,7 +23,7 @@ cw-storage-plus = { workspace = true } mars-owner = { workspace = true } mars-utils = { workspace = true } mars-red-bank-types = { workspace = true } -pyth-sdk-cw = { workspace = true, optional = true } +pyth-sdk-cw = { workspace = true } schemars = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } diff --git a/contracts/oracle/base/src/lib.rs b/contracts/oracle/base/src/lib.rs index 0756db31a..450ac1ac1 100644 --- a/contracts/oracle/base/src/lib.rs +++ b/contracts/oracle/base/src/lib.rs @@ -2,7 +2,6 @@ mod contract; mod error; mod traits; -#[cfg(feature = "pyth")] pub mod pyth; pub use contract::*; diff --git a/contracts/oracle/base/src/pyth.rs b/contracts/oracle/base/src/pyth.rs index 751937e0b..f0d25ed86 100644 --- a/contracts/oracle/base/src/pyth.rs +++ b/contracts/oracle/base/src/pyth.rs @@ -6,6 +6,9 @@ use pyth_sdk_cw::{query_price_feed, Price, PriceFeed, PriceFeedResponse, PriceId use super::*; use crate::error::ContractError::InvalidPrice; +// We don't support any denom with more than 18 decimals +const MAX_DENOM_DECIMALS: u8 = 18; + /// We want to discriminate which actions should trigger a circuit breaker check. /// The objective is to allow liquidations to happen without requiring too many checks (always be open for liquidations) /// while not allowing other actions to be taken in cases of extreme volatility (which could indicate price manipulation attacks). @@ -121,7 +124,11 @@ fn query_pyth_price_for_liquidation( } /// Assert Pyth configuration -pub fn assert_pyth(max_confidence: Decimal, max_deviation: Decimal) -> ContractResult<()> { +pub fn assert_pyth( + max_confidence: Decimal, + max_deviation: Decimal, + denom_decimals: u8, +) -> ContractResult<()> { if !max_confidence.le(&Decimal::percent(20u64)) { return Err(ContractError::InvalidPriceSource { reason: "max_confidence must be in the range of <0;0.2>".to_string(), @@ -134,6 +141,12 @@ pub fn assert_pyth(max_confidence: Decimal, max_deviation: Decimal) -> ContractR }); } + if denom_decimals > MAX_DENOM_DECIMALS { + return Err(ContractError::InvalidPriceSource { + reason: format!("denom_decimals must be <= {}", MAX_DENOM_DECIMALS), + }); + } + Ok(()) } @@ -255,6 +268,12 @@ pub fn scale_pyth_price( // 26 decimals used (overflow) !!! let price = usd_price.checked_mul(denom_scaled)?.checked_mul(pyth_price)?; + if price.is_zero() { + return Err(InvalidPrice { + reason: "price is zero".to_string(), + }); + } + Ok(price) } @@ -327,4 +346,16 @@ mod tests { .unwrap(); assert_eq!(ueth_price_in_uusd, Decimal::from_atomics(100000098000001u128, 20u32).unwrap()); } + + #[test] + fn return_error_if_scaled_pyth_price_is_zero() { + let price_err = + scale_pyth_price(1u128, -18, 18u8, Decimal::from_str("1000000").unwrap()).unwrap_err(); + assert_eq!( + price_err, + ContractError::InvalidPrice { + reason: "price is zero".to_string() + } + ); + } } diff --git a/contracts/oracle/osmosis/Cargo.toml b/contracts/oracle/osmosis/Cargo.toml index 2a6e48688..57e951f7b 100644 --- a/contracts/oracle/osmosis/Cargo.toml +++ b/contracts/oracle/osmosis/Cargo.toml @@ -23,7 +23,7 @@ cosmwasm-schema = { workspace = true } cosmwasm-std = { workspace = true } cw2 = { workspace = true } cw-storage-plus = { workspace = true } -mars-oracle-base = { workspace = true, features = ["pyth"] } +mars-oracle-base = { workspace = true } mars-osmosis = { workspace = true } mars-owner = { workspace = true } mars-utils = { workspace = true } @@ -38,4 +38,3 @@ cosmwasm-schema = { workspace = true } mars-owner = { workspace = true } mars-testing = { workspace = true } mars-utils = { workspace = true } -pyth-sdk-cw = { workspace = true } diff --git a/contracts/oracle/osmosis/src/price_source.rs b/contracts/oracle/osmosis/src/price_source.rs index adb229297..0b2e5b4f9 100644 --- a/contracts/oracle/osmosis/src/price_source.rs +++ b/contracts/oracle/osmosis/src/price_source.rs @@ -403,7 +403,11 @@ impl PriceSourceUnchecked for OsmosisPriceSour max_deviation, denom_decimals, } => { - mars_oracle_base::pyth::assert_pyth(*max_confidence, *max_deviation)?; + mars_oracle_base::pyth::assert_pyth( + *max_confidence, + *max_deviation, + *denom_decimals, + )?; mars_oracle_base::pyth::assert_usd_price_source(deps, price_sources)?; Ok(OsmosisPriceSourceChecked::Pyth { contract_addr: deps.api.addr_validate(contract_addr)?, diff --git a/contracts/oracle/osmosis/tests/test_set_price_source.rs b/contracts/oracle/osmosis/tests/test_set_price_source.rs index 4cd0c60f2..9048dbab6 100644 --- a/contracts/oracle/osmosis/tests/test_set_price_source.rs +++ b/contracts/oracle/osmosis/tests/test_set_price_source.rs @@ -1021,30 +1021,31 @@ fn setting_price_source_xyk_lp() { fn setting_price_source_pyth_with_invalid_params() { let mut deps = helpers::setup_test(); - let mut set_price_source_pyth = |max_confidence: Decimal, max_deviation: Decimal| { - execute( - deps.as_mut(), - mock_env(), - mock_info("owner"), - ExecuteMsg::SetPriceSource { - denom: "uatom".to_string(), - price_source: OsmosisPriceSourceUnchecked::Pyth { - contract_addr: "pyth_contract_addr".to_string(), - price_feed_id: PriceIdentifier::from_hex( - "61226d39beea19d334f17c2febce27e12646d84675924ebb02b9cdaea68727e3", - ) - .unwrap(), - max_staleness: 30, - max_confidence, - max_deviation, - denom_decimals: 6u8, + let mut set_price_source_pyth = + |max_confidence: Decimal, max_deviation: Decimal, denom_decimals: u8| { + execute( + deps.as_mut(), + mock_env(), + mock_info("owner"), + ExecuteMsg::SetPriceSource { + denom: "uatom".to_string(), + price_source: OsmosisPriceSourceUnchecked::Pyth { + contract_addr: "pyth_contract_addr".to_string(), + price_feed_id: PriceIdentifier::from_hex( + "61226d39beea19d334f17c2febce27e12646d84675924ebb02b9cdaea68727e3", + ) + .unwrap(), + max_staleness: 30, + max_confidence, + max_deviation, + denom_decimals, + }, }, - }, - ) - }; + ) + }; // attempting to set max_confidence > 20%; should fail - let err = set_price_source_pyth(Decimal::percent(21), Decimal::percent(6)).unwrap_err(); + let err = set_price_source_pyth(Decimal::percent(21), Decimal::percent(6), 6).unwrap_err(); assert_eq!( err, ContractError::InvalidPriceSource { @@ -1053,13 +1054,22 @@ fn setting_price_source_pyth_with_invalid_params() { ); // attempting to set max_deviation > 20%; should fail - let err = set_price_source_pyth(Decimal::percent(5), Decimal::percent(21)).unwrap_err(); + let err = set_price_source_pyth(Decimal::percent(5), Decimal::percent(21), 18).unwrap_err(); assert_eq!( err, ContractError::InvalidPriceSource { reason: "max_deviation must be in the range of <0;0.2>".to_string() } ); + + // attempting to set denom_decimals > 18; should fail + let err = set_price_source_pyth(Decimal::percent(5), Decimal::percent(20), 19).unwrap_err(); + assert_eq!( + err, + ContractError::InvalidPriceSource { + reason: "denom_decimals must be <= 18".to_string() + } + ); } #[test] diff --git a/contracts/oracle/wasm/Cargo.toml b/contracts/oracle/wasm/Cargo.toml index 1b293cb07..502618919 100644 --- a/contracts/oracle/wasm/Cargo.toml +++ b/contracts/oracle/wasm/Cargo.toml @@ -26,7 +26,7 @@ cosmwasm-schema = { workspace = true } cosmwasm-std = { workspace = true } cw2 = { workspace = true } cw-storage-plus = { workspace = true } -mars-oracle-base = { workspace = true, features = ["pyth"] } +mars-oracle-base = { workspace = true } mars-red-bank-types = { workspace = true } pyth-sdk-cw = { workspace = true } diff --git a/contracts/oracle/wasm/src/price_source.rs b/contracts/oracle/wasm/src/price_source.rs index f0c26bf7b..4df7140a7 100644 --- a/contracts/oracle/wasm/src/price_source.rs +++ b/contracts/oracle/wasm/src/price_source.rs @@ -201,7 +201,7 @@ impl PriceSourceUnchecked for WasmPriceSourceUnch max_deviation, denom_decimals, } => { - mars_oracle_base::pyth::assert_pyth(max_confidence, max_deviation)?; + mars_oracle_base::pyth::assert_pyth(max_confidence, max_deviation, denom_decimals)?; mars_oracle_base::pyth::assert_usd_price_source(deps, price_sources)?; Ok(WasmPriceSourceChecked::Pyth { contract_addr: deps.api.addr_validate(&contract_addr)?, diff --git a/contracts/oracle/wasm/tests/test_price_source.rs b/contracts/oracle/wasm/tests/test_price_source.rs index ca119203c..4c5b4f5f6 100644 --- a/contracts/oracle/wasm/tests/test_price_source.rs +++ b/contracts/oracle/wasm/tests/test_price_source.rs @@ -654,6 +654,69 @@ fn setting_price_source_pyth_if_missing_usd() { ); } +#[test] +fn setting_price_source_pyth_with_invalid_params() { + let runner = get_test_runner(); + let robot = WasmOracleTestRobot::new( + &runner, + get_contracts(&get_test_runner()), + &get_test_runner().init_default_accounts().unwrap()[0], + None, + ); + + let mut deps = helpers::setup_test(&robot.astroport_contracts.factory.address); + + let mut set_price_source_pyth = + |max_confidence: Decimal, max_deviation: Decimal, denom_decimals: u8| { + execute( + deps.as_mut(), + mock_env(), + mock_info("owner"), + ExecuteMsg::SetPriceSource { + denom: "uatom".to_string(), + price_source: WasmPriceSourceUnchecked::Pyth { + contract_addr: "pyth_contract_addr".to_string(), + price_feed_id: PriceIdentifier::from_hex( + "61226d39beea19d334f17c2febce27e12646d84675924ebb02b9cdaea68727e3", + ) + .unwrap(), + max_staleness: 30, + max_confidence, + max_deviation, + denom_decimals, + }, + }, + ) + }; + + // attempting to set max_confidence > 20%; should fail + let err = set_price_source_pyth(Decimal::percent(21), Decimal::percent(6), 6).unwrap_err(); + assert_eq!( + err, + ContractError::InvalidPriceSource { + reason: "max_confidence must be in the range of <0;0.2>".to_string() + } + ); + + // attempting to set max_deviation > 20%; should fail + let err = set_price_source_pyth(Decimal::percent(5), Decimal::percent(21), 18).unwrap_err(); + assert_eq!( + err, + ContractError::InvalidPriceSource { + reason: "max_deviation must be in the range of <0;0.2>".to_string() + } + ); + + // attempting to set denom_decimals > 18; should fail + let err = set_price_source_pyth(Decimal::percent(5), Decimal::percent(20), 19).unwrap_err(); + assert_eq!( + err, + ContractError::InvalidPriceSource { + reason: "denom_decimals must be <= 18".to_string() + } + ); +} + #[test] fn twap_window_size_not_gt_tolerance() { let runner = get_test_runner();