fix: oracle price calculation

ema-oracle/src/benchmarking.rs

@@ -30,9 +30,6 @@ use pretty_assertions::assert_eq;
 
 use crate::Pallet as EmaOracle;
 
-/// Maximum parameter to run the benchmark for.
-pub const MAX_TOKEN_PAIRS: u32 = MAX_UNIQUE_ENTRIES - 1;
-
 /// Default oracle source.
 const SOURCE: Source = *b"dummysrc";
 
@@ -106,7 +103,7 @@ benchmarks! {
     }
 
     on_finalize_multiple_tokens {
-        let b in 1 .. MAX_TOKEN_PAIRS;
+        let b in 1 .. (T::MaxUniqueEntries::get() - 1);
 
         let initial_data_block: T::BlockNumber = 5u32.into();
         let block_num = initial_data_block.saturating_add(1_000_000u32.into());
@@ -146,7 +143,7 @@ benchmarks! {
     }
 
     on_trade_multiple_tokens {
-        let b in 1 .. MAX_TOKEN_PAIRS;
+        let b in 1 .. (T::MaxUniqueEntries::get() - 1);
 
         let initial_data_block: T::BlockNumber = 5u32.into();
         let block_num = initial_data_block.saturating_add(1_000_000u32.into());
@@ -196,7 +193,7 @@ benchmarks! {
     }
 
     on_liquidity_changed_multiple_tokens {
-        let b in 1 .. MAX_TOKEN_PAIRS;
+        let b in 1 .. (T::MaxUniqueEntries::get() - 1);
 
         let initial_data_block: T::BlockNumber = 5u32.into();
         let block_num = initial_data_block.saturating_add(1_000_000u32.into());
@@ -205,47 +202,45 @@ benchmarks! {
 
         frame_system::Pallet::<T>::set_block_number(initial_data_block);
         EmaOracle::<T>::on_initialize(initial_data_block);
-        let (amount_in, amount_out) = (1_000_000_000_000, 2_000_000_000_000);
-        let (liquidity_asset_in, liquidity_asset_out) = (1_000_000_000_000_000, 2_000_000_000_000_000);
+        let (amount_a, amount_b) = (1_000_000_000_000, 2_000_000_000_000);
+        let (liquidity_asset_a, liquidity_asset_b) = (1_000_000_000_000_000, 2_000_000_000_000_000);
         for i in 0 .. b {
             let asset_a = i * 1_000;
             let asset_b = asset_a + 500;
-            assert_ok!(OnActivityHandler::<T>::on_trade(SOURCE, asset_a, asset_b, amount_in, amount_out, liquidity_asset_in, liquidity_asset_out));
+            assert_ok!(OnActivityHandler::<T>::on_trade(SOURCE, asset_a, asset_b, amount_a, amount_b, liquidity_asset_a, liquidity_asset_b));
         }
         EmaOracle::<T>::on_finalize(initial_data_block);
 
         frame_system::Pallet::<T>::set_block_number(block_num);
         EmaOracle::<T>::on_initialize(block_num);
         let entry = OracleEntry {
-            price: Price::from((amount_in, amount_out)),
-            volume: Volume::from_a_in_b_out(amount_in, amount_out),
-            liquidity: Liquidity::new(liquidity_asset_in, liquidity_asset_out),
+            price: Price::from((amount_a, amount_b)),
+            volume: Volume::from_a_in_b_out(amount_a, amount_b),
+            liquidity: Liquidity::new(liquidity_asset_a, liquidity_asset_b),
             timestamp: block_num,
         };
         for i in 0 .. b {
             let asset_a = i * 1_000;
             let asset_b = asset_a + 500;
-            assert_ok!(OnActivityHandler::<T>::on_trade(SOURCE, asset_a, asset_b, amount_in, amount_out, liquidity_asset_in, liquidity_asset_out));
+            assert_ok!(OnActivityHandler::<T>::on_trade(SOURCE, asset_a, asset_b, amount_a, amount_b, liquidity_asset_a, liquidity_asset_b));
             entries.push(((SOURCE, ordered_pair(asset_a, asset_b)), entry.clone()));
         }
         let asset_a = b * 1_000;
         let asset_b = asset_a + 500;
-        let amount_a = amount_in;
-        let amount_b = amount_out;
 
         let res = core::cell::RefCell::new(Err(DispatchError::Other("Not initialized")));
     }: {
         let _ = res.replace(
-            OnActivityHandler::<T>::on_liquidity_changed(SOURCE, asset_a, asset_b, amount_a, amount_b, liquidity_asset_in, liquidity_asset_out)
+            OnActivityHandler::<T>::on_liquidity_changed(SOURCE, asset_a, asset_b, amount_a, amount_b, liquidity_asset_a, liquidity_asset_b)
                 .map_err(|(_w, e)| e)
         );
     }
     verify {
         assert_ok!(*res.borrow());
         let liquidity_entry = OracleEntry {
-            price: Price::from((amount_a, amount_b)),
+            price: Price::from((liquidity_asset_a, liquidity_asset_b)),
             volume: Volume::default(),
-            liquidity: Liquidity::new(liquidity_asset_in, liquidity_asset_out),
+            liquidity: Liquidity::new(liquidity_asset_a, liquidity_asset_b),
             timestamp: block_num,
         };
         entries.push(((SOURCE, ordered_pair(asset_a, asset_b)), liquidity_entry));
@@ -272,7 +267,10 @@ benchmarks! {
 
         let res = core::cell::RefCell::new(Err(OracleError::NotPresent));
 
-    }: { let _ = res.replace(EmaOracle::<T>::get_entry(asset_a, asset_b, TenMinutes, SOURCE)); }
+        // aim to find a period that is not `LastBlock`, falling back to `LastBlock` if none is found.
+        let period = T::SupportedPeriods::get().into_iter().find(|p| p != &LastBlock).unwrap_or(LastBlock);
+
+    }: { let _ = res.replace(EmaOracle::<T>::get_entry(asset_a, asset_b, period, SOURCE)); }
     verify {
         assert_eq!(*res.borrow(), Ok(AggregatedEntry {
             price: Price::from((amount_in, amount_out)),

ema-oracle/src/lib.rs

@@ -384,21 +384,21 @@ pub(crate) fn fractional_on_finalize_weight<T: Config>(max_entries: u32) -> Weig
 impl<T: Config> OnTradeHandler<AssetId, Balance> for OnActivityHandler<T> {
     fn on_trade(
         source: Source,
-        asset_in: AssetId,
-        asset_out: AssetId,
-        amount_in: Balance,
-        amount_out: Balance,
+        asset_a: AssetId,
+        asset_b: AssetId,
+        amount_a: Balance,
+        amount_b: Balance,
         liquidity_a: Balance,
         liquidity_b: Balance,
     ) -> Result<Weight, (Weight, DispatchError)> {
         // We assume that zero values are not valid and can be ignored.
-        if liquidity_a.is_zero() || liquidity_b.is_zero() || amount_in.is_zero() || amount_out.is_zero() {
-            log::warn!(target: LOG_TARGET, "Neither liquidity nor amounts should be zero. Ignoring. Source: {source:?}, liquidity: ({liquidity_a},{liquidity_a}) , amount_in/_out: {amount_in}/{amount_out}");
+        if liquidity_a.is_zero() || liquidity_b.is_zero() || amount_a.is_zero() || amount_b.is_zero() {
+            log::warn!(target: LOG_TARGET, "Neither liquidity nor amounts should be zero. Ignoring. Source: {source:?}, liquidity: ({liquidity_a},{liquidity_a}) , amount_a/_out: {amount_a}/{amount_b}");
             return Err((Self::on_trade_weight(), Error::<T>::OnTradeValueZero.into()));
         }
-        let price = determine_normalized_price(asset_in, asset_out, amount_in, amount_out);
-        let volume = determine_normalized_volume(asset_in, asset_out, amount_in, amount_out);
-        let liquidity = determine_normalized_liquidity(asset_in, asset_out, liquidity_a, liquidity_b);
+        let price = determine_normalized_price(asset_a, asset_b, amount_a, amount_b);
+        let volume = determine_normalized_volume(asset_a, asset_b, amount_a, amount_b);
+        let liquidity = determine_normalized_liquidity(asset_a, asset_b, liquidity_a, liquidity_b);
 
         let timestamp = T::BlockNumberProvider::current_block_number();
         let entry = OracleEntry {
@@ -407,7 +407,7 @@ impl<T: Config> OnTradeHandler<AssetId, Balance> for OnActivityHandler<T> {
             liquidity,
             timestamp,
         };
-        Pallet::<T>::on_trade(source, ordered_pair(asset_in, asset_out), entry)
+        Pallet::<T>::on_trade(source, ordered_pair(asset_a, asset_b), entry)
     }
 
     fn on_trade_weight() -> Weight {
@@ -423,18 +423,21 @@ impl<T: Config> OnLiquidityChangedHandler<AssetId, Balance> for OnActivityHandle
         source: Source,
         asset_a: AssetId,
         asset_b: AssetId,
-        amount_a: Balance,
-        amount_b: Balance,
+        _amount_a: Balance,
+        _amount_b: Balance,
         liquidity_a: Balance,
         liquidity_b: Balance,
     ) -> Result<Weight, (Weight, DispatchError)> {
-        if liquidity_a.is_zero() || liquidity_b.is_zero() || amount_a.is_zero() || amount_b.is_zero() {
-            log::trace!(target: LOG_TARGET, "Liquidity or amounts are zero. Source: {source:?}, liquidity: ({liquidity_a},{liquidity_a}) , amount_a: {amount_a}, amount_b: {amount_b}");
+        if liquidity_a.is_zero() || liquidity_b.is_zero() {
+            log::trace!(
+                target: LOG_TARGET,
+                "Liquidity is zero. Source: {source:?}, liquidity: ({liquidity_a},{liquidity_a})"
+            );
         }
-        let price = if amount_a.is_zero() || amount_b.is_zero() {
+        let price = if liquidity_a.is_zero() || liquidity_b.is_zero() {
             Price::zero()
         } else {
-            determine_normalized_price(asset_a, asset_b, amount_a, amount_b)
+            determine_normalized_price(asset_a, asset_b, liquidity_a, liquidity_b)
         };
         let liquidity = determine_normalized_liquidity(asset_a, asset_b, liquidity_a, liquidity_b);
         let timestamp = T::BlockNumberProvider::current_block_number();

ema-oracle/src/tests/mock.rs

@@ -40,7 +40,7 @@ type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
 type Block = frame_system::mocking::MockBlock<Test>;
 
 use crate::MAX_PERIODS;
-use crate::MAX_UNIQUE_ENTRIES;
+
 pub const HDX: AssetId = 1_000;
 pub const DOT: AssetId = 2_000;
 pub const ACA: AssetId = 3_000;
@@ -133,7 +133,7 @@ impl Config for Test {
     type WeightInfo = ();
     type BlockNumberProvider = System;
     type SupportedPeriods = SupportedPeriods;
-    type MaxUniqueEntries = ConstU32<MAX_UNIQUE_ENTRIES>;
+    type MaxUniqueEntries = ConstU32<45>;
 }
 
 pub type InitialDataEntry = (Source, (AssetId, AssetId), Price, Liquidity<Balance>);

ema-oracle/src/tests/mod.rs

@@ -146,7 +146,7 @@ fn on_liquidity_changed_handler_should_work() {
         let timestamp = 5;
         System::set_block_number(timestamp);
         let no_volume_entry = OracleEntry {
-            price: Price::new(1_000, 500),
+            price: Price::new(2_000, 1_000),
             volume: Volume::default(),
             liquidity: Liquidity::new(2_000, 1_000),
             timestamp,
@@ -177,7 +177,7 @@ fn on_liquidity_changed_should_allow_zero_values() {
             liquidity_b,
         ));
         let only_liquidity_entry = OracleEntry {
-            price: Price::zero(),
+            price: Price::new(liquidity_a, liquidity_b),
             volume: Volume::default(),
             liquidity: (liquidity_a, liquidity_b).into(),
             timestamp,
@@ -197,7 +197,7 @@ fn on_liquidity_changed_should_allow_zero_values() {
             liquidity_b,
         ));
         let only_liquidity_entry = OracleEntry {
-            price: Price::zero(),
+            price: Price::new(liquidity_a, liquidity_b),
             volume: Volume::default(),
             liquidity: (liquidity_a, liquidity_b).into(),
             timestamp,
@@ -217,7 +217,7 @@ fn on_liquidity_changed_should_allow_zero_values() {
             Balance::zero(),
         ));
         let only_price_entry = OracleEntry {
-            price: Price::new(amount, amount),
+            price: Price::zero(),
             volume: Volume::default(),
             liquidity: (Balance::zero(), Balance::zero()).into(),
             timestamp,
@@ -258,7 +258,7 @@ fn on_trade_should_exclude_zero_values() {
 #[test]
 fn on_entry_should_error_on_accumulator_overflow() {
     new_test_ext().execute_with(|| {
-        let max_entries = MAX_UNIQUE_ENTRIES;
+        let max_entries = <<Test as crate::Config>::MaxUniqueEntries as Get<u32>>::get();
         // let's fill the accumulator
         for i in 0..max_entries {
             assert_ok!(OnActivityHandler::<Test>::on_trade(
@@ -715,22 +715,26 @@ fn check_period_smoothing_factors() {
     let days = 24 * hours;
 
     let last_block = smoothing_from_period(1);
-    println!("Last Block: {}", last_block.to_bits());
+    println!("Last Block: {} (bits: {})", last_block, last_block.to_bits());
     assert_eq!(into_smoothing(LastBlock), last_block);
 
+    let short = smoothing_from_period(9);
+    println!("Short: {} (bits: {})", short, short.to_bits());
+    assert_eq!(into_smoothing(Short), short);
+
     let ten_minutes = smoothing_from_period(10 * minutes);
-    println!("Ten Minutes: {}", ten_minutes.to_bits());
+    println!("Ten Minutes: {} (bits: {})", ten_minutes, ten_minutes.to_bits());
     assert_eq!(into_smoothing(TenMinutes), ten_minutes);
 
     let hour = smoothing_from_period(hours);
-    println!("Hour: {}", hour.to_bits());
+    println!("Hour: {} (bits: {})", hour, hour.to_bits());
     assert_eq!(into_smoothing(Hour), hour);
 
     let day = smoothing_from_period(days);
-    println!("Day: {}", day.to_bits());
+    println!("Day: {} (bits: {})", day, day.to_bits());
     assert_eq!(into_smoothing(Day), day);
 
     let week = smoothing_from_period(7 * days);
-    println!("Week: {}", week.to_bits());
+    println!("Week: {} (bits: {})", week, week.to_bits());
     assert_eq!(into_smoothing(Week), week);
 }

ema-oracle/src/types.rs

@@ -198,6 +198,7 @@ where
 pub fn into_smoothing(period: OraclePeriod) -> Fraction {
     match period {
         OraclePeriod::LastBlock => Fraction::from_bits(170141183460469231731687303715884105728),
+        OraclePeriod::Short => Fraction::from_bits(34028236692093846346337460743176821146),
         OraclePeriod::TenMinutes => Fraction::from_bits(3369132345751865974884897103284833777),
         OraclePeriod::Hour => Fraction::from_bits(566193622164623067326746434994622648),
         OraclePeriod::Day => Fraction::from_bits(23629079016800115510268356880200556),

traits/src/lib.rs

@@ -46,7 +46,7 @@ pub struct AMMTransfer<AccountId, AssetId, AssetPair, Balance> {
     pub origin: AccountId,
     pub assets: AssetPair,
     pub amount: Balance,
-    pub amount_out: Balance,
+    pub amount_b: Balance,
     pub discount: bool,
     pub discount_amount: Balance,
     pub fee: (AssetId, Balance),
@@ -161,10 +161,10 @@ pub trait OnTradeHandler<AssetId, Balance> {
         source: Source,
         asset_a: AssetId,
         asset_b: AssetId,
-        amount_in: Balance,
-        amount_out: Balance,
-        liq_amount_a: Balance,
-        liq_amount_b: Balance,
+        amount_a: Balance,
+        amount_b: Balance,
+        liquidity_a: Balance,
+        liquidity_b: Balance,
     ) -> Result<Weight, (Weight, DispatchError)>;
     /// Known overhead for a trade in `on_initialize/on_finalize`.
     /// Needs to be specified here if we don't want to make AMM pools tightly coupled with the price oracle pallet, otherwise we can't access the weight.
@@ -177,10 +177,10 @@ impl<AssetId, Balance> OnTradeHandler<AssetId, Balance> for () {
         _source: Source,
         _asset_a: AssetId,
         _asset_b: AssetId,
-        _amount_in: Balance,
-        _amount_out: Balance,
-        _liq_amount_a: Balance,
-        _liq_amount_b: Balance,
+        _amount_a: Balance,
+        _amount_b: Balance,
+        _liquidity_a: Balance,
+        _liquidity_b: Balance,
     ) -> Result<Weight, (Weight, DispatchError)> {
         Ok(Weight::zero())
     }

traits/src/oracle.rs

@@ -34,6 +34,8 @@ where
 pub enum OraclePeriod {
     /// The oracle data is from the last block, thus unaggregated.
     LastBlock,
+    /// The oracle data was aggregated over the last few blocks.
+    Short,
     /// The oracle data was aggregated over the blocks of the last ten minutes.
     TenMinutes,
     /// The oracle data was aggregated over the blocks of the last hour.