From 94d1005858e3ee9c05dd4238eba476490c322a03 Mon Sep 17 00:00:00 2001 From: Andrey Orlov Date: Fri, 4 Feb 2022 15:25:56 +0400 Subject: [PATCH] Add support of polkadot-v0.9.16 in pallet-oracle --- frame/oracle/src/lib.rs | 76 ++++++++++++++++++++++----------------- frame/oracle/src/mock.rs | 21 +++++++++-- frame/oracle/src/tests.rs | 27 +++++++------- runtime/dali/src/lib.rs | 2 ++ 4 files changed, 78 insertions(+), 48 deletions(-) diff --git a/frame/oracle/src/lib.rs b/frame/oracle/src/lib.rs index a31b071b188..5dfb21f3036 100644 --- a/frame/oracle/src/lib.rs +++ b/frame/oracle/src/lib.rs @@ -43,9 +43,7 @@ pub mod pallet { ExistenceRequirement::{AllowDeath, KeepAlive}, ReservableCurrency, }, - transactional, weights::{DispatchClass::Operational, Pays}, - PalletId, }; use frame_system::{ offchain::{ @@ -365,6 +363,7 @@ pub mod pallet { BlockIntervalLength, /// There was an error transferring TransferError, + MaxHistory, MaxPrePrices, } @@ -606,11 +605,6 @@ pub mod pallet { Error::::NotEnoughStake ); - let set_price = PrePrice { - price, - block: frame_system::Pallet::::block_number(), - who: who.clone(), - }; let asset_info = Self::asset_info(asset_id).ok_or(Error::::InvalidAssetId)?; PrePrices::::try_mutate(asset_id, |current_prices| -> Result<(), DispatchError> { // There can convert current_prices.len() to u32 safely @@ -622,7 +616,14 @@ pub mod pallet { if current_prices.iter().any(|candidate| candidate.who == who) { return Err(Error::::AlreadySubmitted.into()) } - current_prices.push(set_price); + current_prices + .try_push(PrePrice { + price, + block: frame_system::Pallet::::block_number(), + who: who.clone(), + }) + .map_err(|_| Error::::MaxPrePrices)?; + Ok(()) })?; @@ -713,18 +714,22 @@ pub mod pallet { } } - #[transactional] pub fn update_prices(block: T::BlockNumber) -> Weight { let mut total_weight: Weight = Zero::zero(); let one_read = T::DbWeight::get().reads(1); for (asset_id, asset_info) in AssetsInfo::::iter() { total_weight += one_read; - let (removed_pre_prices_len, pre_prices) = - Self::update_pre_prices(asset_id, &asset_info, block); - total_weight += T::WeightInfo::update_pre_prices(removed_pre_prices_len as u32); - let pre_prices_len = pre_prices.len(); - Self::update_price(asset_id, asset_info.clone(), block, pre_prices); - total_weight += T::WeightInfo::update_price(pre_prices_len as u32); + if let Ok((removed_pre_prices_len, pre_prices)) = + Self::update_pre_prices(asset_id, &asset_info, block) + { + total_weight += T::WeightInfo::update_pre_prices(removed_pre_prices_len as u32); + let pre_prices_len = pre_prices.len(); + + // We can ignore `Error::::MaxHistory` because it can not be + // because we control the length of items of `PriceHistory`. + let _ = Self::update_price(asset_id, asset_info.clone(), block, pre_prices); + total_weight += T::WeightInfo::update_price(pre_prices_len as u32); + }; } total_weight } @@ -734,7 +739,10 @@ pub mod pallet { asset_id: T::AssetId, asset_info: &AssetInfo>, block: T::BlockNumber, - ) -> (usize, Vec>) { + ) -> Result< + (usize, Vec>), + DispatchError, + > { // TODO maybe add a check if price is requested, is less operations? let pre_pruned_prices = PrePrices::::get(asset_id); let prev_pre_prices_len = pre_pruned_prices.len(); @@ -744,7 +752,8 @@ pub mod pallet { // because pre_pruned_prices.len() limited by u32 // (type of AssetsInfo::::get(asset_id).max_answers). if pre_pruned_prices.len() as u32 >= asset_info.min_answers { - let res = Self::prune_old_pre_prices(asset_info, pre_pruned_prices.to_vec(), block); + let res = + Self::prune_old_pre_prices(asset_info, pre_pruned_prices.into_inner(), block); let staled_prices = res.0; pre_prices = res.1; for p in staled_prices { @@ -752,11 +761,12 @@ pub mod pallet { } PrePrices::::insert( asset_id, - pre_prices.try_into().map_err(|_| Error::::MaxPrePrices)?, + BoundedVec::try_from(pre_prices.clone()) + .map_err(|_| Error::::MaxPrePrices)?, ); } - (prev_pre_prices_len - pre_prices.len(), pre_prices) + Ok((prev_pre_prices_len - pre_prices.len(), pre_prices)) } pub fn update_price( @@ -764,31 +774,30 @@ pub mod pallet { asset_info: AssetInfo>, block: T::BlockNumber, pre_prices: Vec>, - ) { + ) -> DispatchResult { // There can convert pre_prices.len() to u32 safely // because pre_prices.len() limited by u32 // (type of AssetsInfo::::get(asset_id).max_answers). if pre_prices.len() as u32 >= asset_info.min_answers { if let Some(price) = Self::get_median_price(&pre_prices) { Prices::::insert(asset_id, Price { price, block }); - let historical = Self::price_history(asset_id); - if (historical.len() as u32) < T::MaxHistory::get() { - PriceHistory::::mutate(asset_id, |prices| { - if Self::prices(asset_id).block != 0_u32.into() { - prices.push(Price { price, block }); - } - }) - } else { - PriceHistory::::mutate(asset_id, |prices| { + PriceHistory::::try_mutate(asset_id, |prices| -> DispatchResult { + if prices.len() as u32 >= T::MaxHistory::get() { prices.remove(0); - prices.push(Price { price, block }); - }) - } + } + if block != 0_u32.into() { + prices + .try_push(Price { price, block }) + .map_err(|_| Error::::MaxHistory)?; + } + Ok(()) + })?; PrePrices::::remove(asset_id); Self::handle_payout(&pre_prices, price, asset_id, &asset_info); } } + Ok(()) } #[allow(clippy::type_complexity)] @@ -946,7 +955,8 @@ pub mod pallet { public_keys.first().ok_or("No public keys for crypto key type `orac`")?.0, ); let mut to32 = AccountId32::as_ref(&account); - let address: T::AccountId = T::AccountId::decode(&mut to32).unwrap_or_default(); + let address: T::AccountId = + T::AccountId::decode(&mut to32).map_err(|_| "Could not decode account")?; if prices.len() as u32 >= asset_info.max_answers { log::info!("Max answers reached"); diff --git a/frame/oracle/src/mock.rs b/frame/oracle/src/mock.rs index 33e0518832e..bab2132f1de 100644 --- a/frame/oracle/src/mock.rs +++ b/frame/oracle/src/mock.rs @@ -1,6 +1,8 @@ use crate as pallet_oracle; use crate::*; -use frame_support::{ord_parameter_types, parameter_types, traits::Everything}; +use frame_support::{ + ord_parameter_types, pallet_prelude::ConstU32, parameter_types, traits::Everything, +}; use frame_system as system; use frame_system::EnsureSignedBy; use sp_core::{sr25519::Signature, H256}; @@ -56,6 +58,7 @@ impl system::Config for Test { type SystemWeightInfo = (); type SS58Prefix = SS58Prefix; type OnSetCode = (); + type MaxConsumers = ConstU32<16>; } parameter_types! { @@ -81,6 +84,7 @@ parameter_types! { pub const MaxAnswerBound: u32 = 5; pub const MaxAssetsCount: u32 = 2; pub const MaxHistory: u32 = 3; + pub const MaxPrePrices: u32 = 12; } ord_parameter_types! { @@ -130,6 +134,7 @@ impl pallet_oracle::Config for Test { type MaxAnswerBound = MaxAnswerBound; type MaxAssetsCount = MaxAssetsCount; type MaxHistory = MaxHistory; + type MaxPrePrices = MaxPrePrices; type WeightInfo = (); type LocalAssets = (); } @@ -139,7 +144,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = system::GenesisConfig::default().build_storage::().unwrap(); let genesis = pallet_balances::GenesisConfig:: { balances: vec![ - (Default::default(), 100), + (get_account_1(), 100), (get_account_2(), 100), (get_account_4(), 100), (get_account_5(), 100), @@ -149,6 +154,18 @@ pub fn new_test_ext() -> sp_io::TestExternalities { t.into() } +pub fn get_account_1() -> AccountId { + const PHRASE: &str = + "elite reward rabbit exotic course desk miracle involve old surface educate direct"; + let keystore = KeyStore::new(); + SyncCryptoStore::sr25519_generate_new( + &keystore, + crate::crypto::Public::ID, + Some(&format!("{}/hunter1", PHRASE)), + ) + .unwrap() +} + pub fn get_account_2() -> AccountId { const PHRASE: &str = "news slush supreme milk chapter athlete soap sausage put clutch what kitten"; diff --git a/frame/oracle/src/tests.rs b/frame/oracle/src/tests.rs index 332b11bd930..93c3a4b565c 100644 --- a/frame/oracle/src/tests.rs +++ b/frame/oracle/src/tests.rs @@ -7,6 +7,7 @@ use composable_traits::defi::CurrencyPair; use frame_support::{ assert_noop, assert_ok, traits::{Currency, OnInitialize}, + BoundedVec, }; use pallet_balances::Error as BalancesError; use parking_lot::RwLock; @@ -80,7 +81,7 @@ fn add_asset_and_info() { assert_eq!(Oracle::assets_count(), 2); // fails with non permission - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); assert_noop!( Oracle::add_asset_and_info( Origin::signed(account_1), @@ -184,7 +185,7 @@ fn add_asset_and_info() { #[test] fn set_signer() { new_test_ext().execute_with(|| { - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); let account_2 = get_account_2(); let account_3 = get_account_3(); let account_4 = get_account_4(); @@ -216,7 +217,7 @@ fn set_signer() { #[test] fn add_stake() { new_test_ext().execute_with(|| { - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); let account_2 = get_account_2(); // fails no controller set assert_noop!(Oracle::add_stake(Origin::signed(account_1), 50), Error::::UnsetSigner); @@ -254,7 +255,7 @@ fn add_stake() { #[test] fn remove_and_reclaim_stake() { new_test_ext().execute_with(|| { - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); let account_2 = get_account_2(); let account_3 = get_account_3(); @@ -291,7 +292,7 @@ fn remove_and_reclaim_stake() { #[test] fn add_price() { new_test_ext().execute_with(|| { - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); let account_2 = get_account_2(); let account_4 = get_account_4(); let account_5 = get_account_5(); @@ -368,7 +369,7 @@ fn add_price() { #[test] fn medianize_price() { new_test_ext().execute_with(|| { - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); // should not panic Oracle::get_median_price(&Oracle::pre_prices(0)); for i in 0..3 { @@ -437,7 +438,7 @@ fn is_requested() { #[test] fn test_payout_slash() { new_test_ext().execute_with(|| { - let account_1 = Default::default(); + let account_1 = get_account_1(); let account_2 = get_account_2(); let account_3 = get_account_3(); let account_4 = get_account_4(); @@ -548,7 +549,7 @@ fn on_init() { 5 )); // set prices into storage - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); for i in 0..3 { let price = i as u128 + 100_u128; add_price_storage(price, 0, account_1, 2); @@ -743,7 +744,7 @@ fn on_init_prune_scenerios() { 5 )); // set prices into storage - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); for i in 0..3 { let price = i as u128 + 100_u128; add_price_storage(price, 0, account_1, 0); @@ -803,7 +804,7 @@ fn on_init_over_max_answers() { 5 )); // set prices into storage - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); for i in 0..5 { let price = i as u128 + 100_u128; add_price_storage(price, 0, account_1, 0); @@ -974,14 +975,14 @@ fn parse_price_works() { fn add_price_storage(price: u128, asset_id: u128, who: AccountId, block: u64) { let price = PrePrice { price, block, who }; - PrePrices::::mutate(asset_id, |current_prices| current_prices.push(price)); + PrePrices::::mutate(asset_id, |current_prices| current_prices.try_push(price).unwrap()); AnswerInTransit::::mutate(who, |transit| { *transit = Some(transit.unwrap_or_else(Zero::zero) + 5) }); } fn do_price_update(asset_id: u128, block: u64) { - let account_1: AccountId = Default::default(); + let account_1 = get_account_1(); for i in 0..3 { let price = i as u128 + 100_u128; add_price_storage(price, asset_id, account_1, block); @@ -994,7 +995,7 @@ fn do_price_update(asset_id: u128, block: u64) { } fn set_historic_prices(asset_id: u128, historic_prices: Vec>) { - PriceHistory::::insert(asset_id, historic_prices); + PriceHistory::::insert(asset_id, BoundedVec::try_from(historic_prices).unwrap()); } fn price_oracle_response(state: &mut testing::OffchainState, price_id: &str) { diff --git a/runtime/dali/src/lib.rs b/runtime/dali/src/lib.rs index 364857c1f4d..20c9cc4ff74 100644 --- a/runtime/dali/src/lib.rs +++ b/runtime/dali/src/lib.rs @@ -421,6 +421,7 @@ parameter_types! { pub const MaxAnswerBound: u32 = 25; pub const MaxAssetsCount: u32 = 100_000; pub const MaxHistory: u32 = 20; + pub const MaxPrePrices: u32 = 40; } impl oracle::Config for Runtime { @@ -436,6 +437,7 @@ impl oracle::Config for Runtime { type MaxAnswerBound = MaxAnswerBound; type MaxAssetsCount = MaxAssetsCount; type MaxHistory = MaxHistory; + type MaxPrePrices = MaxPrePrices; type WeightInfo = weights::oracle::WeightInfo; type LocalAssets = Factory; }