diff --git a/src/extensions/env_extensions.rs b/src/extensions/env_extensions.rs index af50275..ba69956 100644 --- a/src/extensions/env_extensions.rs +++ b/src/extensions/env_extensions.rs @@ -50,9 +50,9 @@ pub trait EnvExtensions { fn set_assets(&self, assets: Vec); - fn set_asset_index(&self, asset: Asset, index: u32); + fn set_asset_index(&self, asset: &Asset, index: u32); - fn get_asset_index(&self, asset: Asset) -> Option; + fn get_asset_index(&self, asset: &Asset) -> Option; fn panic_if_not_admin(&self, invoker: &Address); @@ -154,7 +154,7 @@ impl EnvExtensions for Env { get_instance_storage(&self).set(&ASSETS, &assets); } - fn set_asset_index(&self, asset: Asset, index: u32) { + fn set_asset_index(&self, asset: &Asset, index: u32) { match asset { Asset::Stellar(address) => { get_instance_storage(&self).set(&address, &index); @@ -165,7 +165,7 @@ impl EnvExtensions for Env { } } - fn get_asset_index(&self, asset: Asset) -> Option { + fn get_asset_index(&self, asset: &Asset) -> Option { let index: Option; match asset { Asset::Stellar(address) => { diff --git a/src/extensions/i128_extensions.rs b/src/extensions/i128_extensions.rs index aa05c76..564a654 100644 --- a/src/extensions/i128_extensions.rs +++ b/src/extensions/i128_extensions.rs @@ -1,7 +1,39 @@ - pub trait I128Extensions { + + // Divides two i128 numbers, considering decimal places. + // + // Arguments: + // - self: The dividend. + // - y: The divisor. Should not be zero; will cause panic if zero. + // - decimals: Number of decimal places for division. + // + // Behavior: + // - Rounds up towards zero for negative results. + // + // Panic: + // - If dividend (self) or divisor (y) is zero. + // + // Returns: + // - Division result with specified rounding behavior. fn fixed_div_floor(self, y: i128, decimals: u32) -> i128; + + // Encodes a pair of values (u64 and u8) into a single u128 value. + // + // Arguments: + // - val_u64: The first value, a u64, to be encoded. + // - val_u8: The second value, a u8, to be encoded. + // + // Returns: + // - A u128 value combining the u64 and u8 values. fn encode_to_u128(val_u64: u64, val_u8: u8) -> u128; + + // Decodes a u128 value into a tuple of (u64, u8). + // + // Arguments: + // - val: The u128 value to be decoded. + // + // Returns: + // - A tuple consisting of a u64 and u8, extracted from the u128 value. fn decode_from_u128(val: u128) -> (u64, u8); } @@ -23,8 +55,8 @@ impl I128Extensions for i128 { } fn div_floor(dividend: i128, divisor: i128, decimals: u32) -> i128 { - if (dividend == 0) || (divisor == 0) { - 0_i128; + if dividend <= 0 || divisor <= 0 { + panic!("invalid division arguments") } let ashift = core::cmp::min(38 - dividend.ilog10(), decimals); let bshift = core::cmp::max(decimals - ashift, 0); @@ -37,5 +69,5 @@ fn div_floor(dividend: i128, divisor: i128, decimals: u32) -> i128 { if bshift > 0 { vdivisor /= 10_i128.pow(bshift); } - vdividend/vdivisor + vdividend / vdivisor } \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index 9b9aad3..bfd6aa3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -98,7 +98,10 @@ impl PriceOracleContract { // The most recent price for the given asset or None if the asset is not supported pub fn lastprice(e: Env, asset: Asset) -> Option { //get the last timestamp - let timestamp = e.get_last_timestamp(); + let timestamp = obtain_record_timestamp(&e); + if timestamp == 0 { + return None; + } //get the price get_price_data(&e, asset, timestamp) } @@ -114,7 +117,7 @@ impl PriceOracleContract { // // Prices for the given asset or None if the asset is not supported pub fn prices(e: Env, asset: Asset, records: u32) -> Option> { - let asset_index = e.get_asset_index(asset); //get the asset index to avoid multiple calls + let asset_index = e.get_asset_index(&asset); //get the asset index to avoid multiple calls if asset_index.is_none() { return None; } @@ -136,7 +139,10 @@ impl PriceOracleContract { // // The most recent cross price (base_asset_price/quote_asset_price) for the given assets or None if if there were no records found for quoted asset pub fn x_last_price(e: Env, base_asset: Asset, quote_asset: Asset) -> Option { - let timestamp = e.get_last_timestamp(); + let timestamp = obtain_record_timestamp(&e); + if timestamp == 0 { + return None; + } let decimals = e.get_decimals(); get_x_price(&e, base_asset, quote_asset, timestamp, decimals) } @@ -204,7 +210,7 @@ impl PriceOracleContract { // // TWAP for the given asset over N recent records or None if the asset is not supported pub fn twap(e: Env, asset: Asset, records: u32) -> Option { - let asset_index = e.get_asset_index(asset); //get the asset index to avoid multiple calls + let asset_index = e.get_asset_index(&asset); //get the asset index to avoid multiple calls if asset_index.is_none() { return None; } @@ -280,6 +286,9 @@ impl PriceOracleContract { admin.require_auth(); if e.is_initialized() { e.panic_with_error(Error::AlreadyInitialized); + } + if admin != config.admin { + e.panic_with_error(Error::Unauthorized); } e.set_admin(&config.admin); e.set_base_asset(&config.base_asset); @@ -348,6 +357,15 @@ impl PriceOracleContract { // Panics if the caller doesn't match admin address, or if the price snapshot record is invalid pub fn set_price(e: Env, admin: Address, updates: Vec, timestamp: u64) { e.panic_if_not_admin(&admin); + let updates_len = updates.len(); + if updates_len == 0 || updates_len >= 256 { + panic_with_error!(&e, Error::InvalidUpdateLength); + } + let timeframe: u64 = e.get_resolution().into(); + let ledger_timestamp = now(&e); + if timestamp == 0 || !timestamp.is_valid_timestamp(timeframe) || timestamp > ledger_timestamp { + panic_with_error!(&e, Error::InvalidTimestamp); + } let retention_period = e.get_retention_period().unwrap(); @@ -358,6 +376,10 @@ impl PriceOracleContract { //iterate over the updates for (i, price) in updates.iter().enumerate() { + //don't store zero prices + if price == 0 { + continue; + } let asset = i as u8; //store the new price e.set_price(asset, price, timestamp, ledgers_to_live); @@ -383,70 +405,75 @@ impl PriceOracleContract { } fn __add_assets(e: &Env, assets: Vec) { - let mut presented_assets = e.get_assets(); - - let mut assets_indexes: Vec<(Asset, u32)> = Vec::new(&e); + let mut current_assets = e.get_assets(); for asset in assets.iter() { //check if the asset has been already added - if has_asset(&presented_assets, &asset) { - panic_with_error!(&e, Error::AssetAlreadyPresented); + if e.get_asset_index(&asset).is_some() { + panic_with_error!(&e, Error::AssetAlreadyExists); } - presented_assets.push_back(asset.clone()); - assets_indexes.push_back((asset, presented_assets.len() as u32 - 1)); - } - - e.set_assets(presented_assets); - for (asset, index) in assets_indexes.iter() { - e.set_asset_index(asset, index); + e.set_asset_index(&asset, current_assets.len()); + current_assets.push_back(asset); } - } -} - -fn has_asset(assets: &Vec, asset: &Asset) -> bool { - for current_asset in assets.iter() { - if ¤t_asset == asset { - return true; + if current_assets.len() >= 256 { + panic_with_error!(&e, Error::AssetLimitExceeded); } + e.set_assets(current_assets); } - false } fn prices Option>( e: &Env, get_price_fn: F, - records: u32, + mut records: u32, ) -> Option> { - //check if the asset is valid - let mut timestamp = e.get_last_timestamp(); + // Check if the asset is valid + let mut timestamp = obtain_record_timestamp(e); if timestamp == 0 { return None; } - let mut prices = Vec::new(&e); + let mut prices = Vec::new(e); let resolution = e.get_resolution() as u64; - let mut records = records; - if records > 20 { - records = 20; - } + // Limit the number of records to 20 + records = records.min(20); - for _ in 0..records { - let price = get_price_fn(timestamp); - if price.is_none() { - continue; + while records > 0 { + if let Some(price) = get_price_fn(timestamp) { + prices.push_back(price); } - prices.push_back(price.unwrap()); + + // Decrement records counter in every iteration + records -= 1; + if timestamp < resolution { break; } timestamp -= resolution; } - if prices.len() == 0 { - return None; + if prices.is_empty() { + None + } else { + Some(prices) } +} - Some(prices) +fn now(e: &Env) -> u64 { + e.ledger().timestamp() * 1000 //convert to milliseconds +} + +fn obtain_record_timestamp(e: &Env) -> u64 { + let last_timestamp = e.get_last_timestamp(); + let ledger_timestamp = now(&e); + let resolution = e.get_resolution() as u64; + if last_timestamp == 0 //no prices yet + || last_timestamp > ledger_timestamp //last timestamp is in the future + || ledger_timestamp - last_timestamp >= resolution * 2 //last timestamp is too far in the past, so we cannot return the last price + { + return 0; + } + last_timestamp } fn get_twap Option>( @@ -454,19 +481,23 @@ fn get_twap Option>( get_price_fn: F, records: u32, ) -> Option { - let prices_result = prices(&e, get_price_fn, records); - if prices_result.is_none() { + let prices = prices(&e, get_price_fn, records)?; + + if prices.len() != records { return None; } - let prices = prices_result.unwrap(); + let last_price_timestamp = prices.first()?.timestamp; + let timeframe = e.get_resolution() as u64; + let current_time = now(&e); - let mut sum = 0; - for price_data in prices.iter() { - sum += price_data.price; + //check if the last price is too old + if last_price_timestamp + timeframe + 60 * 1000 < current_time { + return None; } - Some(sum / (prices.len() as i128)) + let sum: i128 = prices.iter().map(|price_data| price_data.price).sum(); + Some(sum / prices.len() as i128) } fn get_x_price( @@ -520,12 +551,12 @@ fn get_x_price_by_indexes( } fn get_asset_pair_indexes(e: &Env, base_asset: Asset, quote_asset: Asset) -> Option<(u8, u8)> { - let base_asset = e.get_asset_index(base_asset); + let base_asset = e.get_asset_index(&base_asset); if base_asset.is_none() { return None; } - let quote_asset = e.get_asset_index(quote_asset); + let quote_asset = e.get_asset_index("e_asset); if quote_asset.is_none() { return None; } @@ -534,7 +565,7 @@ fn get_asset_pair_indexes(e: &Env, base_asset: Asset, quote_asset: Asset) -> Opt } fn get_price_data(e: &Env, asset: Asset, timestamp: u64) -> Option { - let asset: Option = e.get_asset_index(asset); + let asset: Option = e.get_asset_index(&asset); if asset.is_none() { return None; } diff --git a/src/test.rs b/src/test.rs index f141ce8..7ec9903 100644 --- a/src/test.rs +++ b/src/test.rs @@ -4,7 +4,8 @@ extern crate alloc; use super::*; use alloc::string::ToString; -use soroban_sdk::{testutils::Address as _, Address, Env, Symbol, String}; +use soroban_sdk::{testutils::{Address as _, Ledger, LedgerInfo}, Address, Env, Symbol, String}; +use std::panic::{self, AssertUnwindSafe}; use {extensions::{u64_extensions::U64Extensions, i128_extensions::I128Extensions}, types::asset::Asset}; @@ -14,6 +15,13 @@ const DECIMALS: u32 = 14; fn init_contract_with_admin<'a>() -> (Env, PriceOracleContractClient<'a>, ConfigData) { let env = Env::default(); + //set timestamp to 900 seconds + let ledger_info = env.ledger().get(); + env.ledger().set(LedgerInfo { + timestamp: 900, + ..ledger_info + }); + let admin = Address::generate(&env); let contract_id = &Address::from_string(&String::from_str(&env, "CDXHQTB7FGRMWTLJJLNI3XPKVC6SZDB5SFGZUYDPEGQQNC4G6CKE4QRC")); @@ -102,6 +110,73 @@ fn bump_test() { client.bump(&6_000_000); } +#[test] +fn set_price_test() { + let (env, client, init_data) = init_contract_with_admin(); + + let admin = &init_data.admin; + let assets = init_data.assets; + + let timestamp = 600_000; + let updates = get_updates(&env, &assets, normalize_price(100)); + + env.mock_all_auths(); + + //set prices for assets + client.set_price(&admin, &updates, ×tamp); +} + +#[test] +#[should_panic] +fn set_price_zero_timestamp_test() { + let (env, client, init_data) = init_contract_with_admin(); + + let admin = &init_data.admin; + let assets = init_data.assets; + + let timestamp = 0; + let updates = get_updates(&env, &assets, normalize_price(100)); + + env.mock_all_auths(); + + //set prices for assets + client.set_price(&admin, &updates, ×tamp); +} + +#[test] +#[should_panic] +fn set_price_invalid_timestamp_test() { + let (env, client, init_data) = init_contract_with_admin(); + + let admin = &init_data.admin; + let assets = init_data.assets; + + let timestamp = 600_001; + let updates = get_updates(&env, &assets, normalize_price(100)); + + env.mock_all_auths(); + + //set prices for assets + client.set_price(&admin, &updates, ×tamp); +} + +#[test] +#[should_panic] +fn set_price_future_timestamp_test() { + let (env, client, init_data) = init_contract_with_admin(); + + let admin = &init_data.admin; + let assets = init_data.assets; + + let timestamp = 1_200_000; + let updates = get_updates(&env, &assets, normalize_price(100)); + + env.mock_all_auths(); + + //set prices for assets + client.set_price(&admin, &updates, ×tamp); +} + #[test] fn last_price_test() { let (env, client, init_data) = init_contract_with_admin(); @@ -181,6 +256,62 @@ fn add_assets_test() { assert_eq!(result, expected_assets); } + + +#[test] +#[should_panic] +fn add_assets_duplicate_test() { + let (env, client, init_data) = init_contract_with_admin(); + + let admin = &init_data.admin; + + let mut assets = Vec::new(&env); + let duplicate_asset = Asset::Other(Symbol::new(&env, &("ASSET_DUPLICATE"))); + assets.push_back(duplicate_asset.clone()); + assets.push_back(duplicate_asset); + + env.mock_all_auths(); + + client.add_assets(&admin, &assets); +} + +#[test] +#[should_panic] +fn assets_update_overflow_test() { + let (env, client, init_data) = init_contract_with_admin(); + + env.mock_all_auths(); + + env.budget().reset_unlimited(); + + let admin = &init_data.admin; + + let mut assets = Vec::new(&env); + for i in 1..=256 { + assets.push_back(Asset::Other(Symbol::new(&env, &("Asset".to_string() + &i.to_string())))); + } + + client.add_assets(&admin, &assets); +} + +#[test] +#[should_panic] +fn prices_update_overflow_test() { + let (env, client, init_data) = init_contract_with_admin(); + + env.mock_all_auths(); + + env.budget().reset_unlimited(); + + let admin = &init_data.admin; + + let mut updates = Vec::new(&env); + for i in 1..=256 { + updates.push_back(normalize_price(i as i128 + 1)); + } + client.set_price(&admin, &updates, &600_000); +} + #[test] fn set_period_test() { let (env, client, init_data) = init_contract_with_admin(); @@ -240,6 +371,25 @@ fn get_price_test() { ); } +#[test] +fn get_lastprice_delayed_update_test() { + let (env, client, init_data) = init_contract_with_admin(); + + let admin = &init_data.admin; + let assets = init_data.assets; + + let timestamp = 300_000; + let updates = get_updates(&env, &assets, normalize_price(100)); + + env.mock_all_auths(); + + client.set_price(&admin, &updates, ×tamp); + + //check last prices + let result = client.lastprice(&assets.get_unchecked(1)); + assert_eq!(result, None); +} + #[test] fn get_x_last_price_test() { let (env, client, init_data) = init_contract_with_admin(); @@ -269,6 +419,32 @@ fn get_x_last_price_test() { ); } + + + + + +#[test] +fn get_x_price_with_zero_test() { + let (env, client, init_data) = init_contract_with_admin(); + + let admin = &init_data.admin; + let assets = init_data.assets; + + let timestamp = 600_000; + let mut updates = get_updates(&env, &assets, normalize_price(100)); + updates.set(1, 0); + + env.mock_all_auths(); + + //set prices for assets + client.set_price(&admin, &updates, ×tamp); + + let result = client.x_price(&assets.get(0).unwrap(), &assets.get(1).unwrap(), ×tamp); + + assert_eq!(result, None); +} + #[test] fn get_x_price_test() { let (env, client, init_data) = init_contract_with_admin(); @@ -354,6 +530,7 @@ fn x_twap_test() { let admin = &init_data.admin; let assets = init_data.assets; + //set prices for assets let timestamp = 600_000; let updates = get_updates(&env, &assets, normalize_price(100)); @@ -378,6 +555,39 @@ fn x_twap_test() { assert_eq!(result.unwrap(), normalize_price(1)); } +#[test] +#[should_panic] +fn x_twap_with_gap_test() { + let (env, client, init_data) = init_contract_with_admin(); + + let admin = &init_data.admin; + let assets = init_data.assets; + + //set prices for assets with gap + let timestamp = 300_000; + let updates = get_updates(&env, &assets, normalize_price(100)); + + env.mock_all_auths(); + + //set prices for assets + client.set_price(&admin, &updates, ×tamp); + + let timestamp = 900_000; + let updates = get_updates(&env, &assets, normalize_price(200)); + + //set prices for assets + client.set_price(&admin, &updates, ×tamp); + + let result = client.x_twap( + &assets.get_unchecked(1), + &assets.get_unchecked(2), + &3, + ); + + assert_ne!(result, None); + assert_eq!(result.unwrap(), normalize_price(1)); +} + #[test] fn get_non_registered_asset_price_test() { @@ -442,11 +652,22 @@ fn div_tests() { let test_cases = [ (154467226919499, 133928752749774, 115335373284703), (i128::MAX/100, 231731687303715884105728, 734216306110962248249052545), - (231731687303715884105728, i128::MAX/100, 13) + (231731687303715884105728, i128::MAX/100, 13), + // -1 expected result for errors + (1, 0, -1), + (0, 1, -1), + (0, 0, -1), + (-1, 0, -1), + (0, -1, -1), + (-1, -1, -1), ]; for (a, b, expected) in test_cases.iter() { - let result = a.fixed_div_floor(*b, 14); - assert_eq!(result, *expected); + let result = panic::catch_unwind(AssertUnwindSafe(|| {a.fixed_div_floor(*b, 14)})); + if expected == &-1 { + assert!(result.is_err()); + } else { + assert_eq!(result.unwrap(), *expected); + } } } \ No newline at end of file diff --git a/src/types/config_data.rs b/src/types/config_data.rs index 22f7184..fab1220 100644 --- a/src/types/config_data.rs +++ b/src/types/config_data.rs @@ -5,18 +5,18 @@ use super::asset::Asset; #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] -/// The configuration parameters for the contract. +// The configuration parameters for the contract. pub struct ConfigData { - /// The admin address. + // The admin address. pub admin: Address, - /// The retention period for the prices. + // The retention period for the prices. pub period: u64, - /// The assets supported by the contract. + // The assets supported by the contract. pub assets: Vec, - /// The base asset for the prices. + // The base asset for the prices. pub base_asset: Asset, - /// The number of decimals for the prices. + // The number of decimals for the prices. pub decimals: u32, - /// The resolution of the prices. + // The resolution of the prices. pub resolution: u32 } diff --git a/src/types/error.rs b/src/types/error.rs index b2b3614..177df59 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -2,22 +2,22 @@ use soroban_sdk::contracterror; #[contracterror] #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] -/// The error codes for the contract. +// The error codes for the contract. pub enum Error { - /// The contract is already initialized. + // The contract is already initialized. AlreadyInitialized = 0, - /// The caller is not authorized to perform the operation. + // The caller is not authorized to perform the operation. Unauthorized = 1, - /// The config assets doen't contain persistent asset. Delete assets is not supported. + // The config assets doen't contain persistent asset. Delete assets is not supported. AssetMissing = 2, - /// The asset is already added to the contract's list of supported assets. - AssetAlreadyPresented = 3, - /// The config version is invalid + // The asset is already added to the contract's list of supported assets. + AssetAlreadyExists = 3, + // The config version is invalid InvalidConfigVersion = 4, - /// Deposit in unsupported asset - InvalidFeeAsset = 11, - /// Deposit amount has negative value - InvalidDepositAmount = 12, - /// Consumer has insufficient balance to pay the fee - InsufficientBalance = 13, + // The prices timestamp is invalid + InvalidTimestamp = 5, + // The assets update length or prices update length is invalid + InvalidUpdateLength = 6, + // The assets storage is full + AssetLimitExceeded = 7 } \ No newline at end of file diff --git a/src/types/price_data.rs b/src/types/price_data.rs index 74ef3ad..2edbabf 100644 --- a/src/types/price_data.rs +++ b/src/types/price_data.rs @@ -2,10 +2,10 @@ use soroban_sdk::contracttype; #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] -/// The price data for an asset at a given timestamp. +// The price data for an asset at a given timestamp. pub struct PriceData { - /// The price in contracts' base asset and decimals. + // The price in contracts' base asset and decimals. pub price: i128, - /// The timestamp of the price. + // The timestamp of the price. pub timestamp: u64, }