From 9483d0d796ea6b43d1d6b8d39104c98d18cbc435 Mon Sep 17 00:00:00 2001 From: JesseAbram Date: Mon, 17 Jan 2022 14:12:34 -0500 Subject: [PATCH] remove unwrap or defaults from asset_info --- frame/oracle/src/lib.rs | 39 +++++++++++++++++++----------------- frame/oracle/src/tests.rs | 42 +++++++++++++++++++++++++++++++-------- 2 files changed, 55 insertions(+), 26 deletions(-) diff --git a/frame/oracle/src/lib.rs b/frame/oracle/src/lib.rs index b2363369ded..1147610bdc0 100644 --- a/frame/oracle/src/lib.rs +++ b/frame/oracle/src/lib.rs @@ -614,8 +614,8 @@ pub mod pallet { pre_prices: &[PrePrice], price: T::PriceValue, asset_id: T::AssetId, + asset_info: &AssetInfo>, ) { - let asset_info = Self::asset_info(asset_id).unwrap_or_default(); for answer in pre_prices { let accuracy: Percent = if answer.price < price { PerThing::from_rational(answer.price, price) @@ -650,8 +650,8 @@ pub mod pallet { asset_id, reward_amount, )); - } - Self::remove_price_in_transit(&asset_id, &answer.who); + }; + Self::remove_price_in_transit(&answer.who, asset_info) } } @@ -661,7 +661,7 @@ pub mod pallet { 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.clone(), block); + 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); @@ -673,7 +673,7 @@ pub mod pallet { #[allow(clippy::type_complexity)] pub fn update_pre_prices( asset_id: T::AssetId, - asset_info: AssetInfo>, + asset_info: &AssetInfo>, block: T::BlockNumber, ) -> (usize, Vec>) { // TODO maybe add a check if price is requested, is less operations? @@ -685,8 +685,7 @@ 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, block, &asset_id); + let res = Self::prune_old_pre_prices(asset_info, pre_pruned_prices, block); let staled_prices = res.0; pre_prices = res.1; for p in staled_prices { @@ -725,17 +724,16 @@ pub mod pallet { } PrePrices::::remove(asset_id); - Self::handle_payout(&pre_prices, price, asset_id); + Self::handle_payout(&pre_prices, price, asset_id, &asset_info); } } } #[allow(clippy::type_complexity)] pub fn prune_old_pre_prices( - asset_info: AssetInfo>, + asset_info: &AssetInfo>, mut pre_prices: Vec>, block: T::BlockNumber, - asset_id: &T::AssetId, ) -> ( Vec>, Vec>, @@ -744,7 +742,7 @@ pub mod pallet { let (staled_prices, mut fresh_prices) = match pre_prices.iter().enumerate().find(|(_, p)| p.block >= stale_block) { Some((index, pre_price)) => { - Self::remove_price_in_transit(asset_id, &pre_price.who); + Self::remove_price_in_transit(&pre_price.who, asset_info); let fresh_prices = pre_prices.split_off(index); (pre_prices, fresh_prices) }, @@ -756,7 +754,7 @@ pub mod pallet { if fresh_prices.len() as u32 > max_answers { let pruned = fresh_prices.len() - max_answers as usize; for price in fresh_prices.iter().skip(pruned) { - Self::remove_price_in_transit(asset_id, &price.who); + Self::remove_price_in_transit(&price.who, asset_info); } #[allow(clippy::indexing_slicing)] // max_answers is confirmed to be less than the len in the condition of the if @@ -792,9 +790,9 @@ pub mod pallet { } pub fn check_requests() { - for (i, _) in AssetsInfo::::iter() { + for (i, asset_info) in AssetsInfo::::iter() { if Self::is_requested(&i) { - let _ = Self::fetch_price_and_send_signed(&i); + let _ = Self::fetch_price_and_send_signed(&i, asset_info); } } } @@ -810,8 +808,10 @@ pub mod pallet { } } - pub fn remove_price_in_transit(asset_id: &T::AssetId, who: &T::AccountId) { - let asset_info = Self::asset_info(asset_id).unwrap_or_default(); + pub fn remove_price_in_transit( + who: &T::AccountId, + asset_info: &AssetInfo>, + ) { AnswerInTransit::::mutate(&who, |transit| { *transit = Some(transit.unwrap_or_else(Zero::zero).saturating_sub(asset_info.slash)) }); @@ -866,7 +866,10 @@ pub mod pallet { } // REVIEW: indexing - pub fn fetch_price_and_send_signed(price_id: &T::AssetId) -> Result<(), &'static str> { + pub fn fetch_price_and_send_signed( + price_id: &T::AssetId, + asset_info: AssetInfo>, + ) -> Result<(), &'static str> { let signer = Signer::::all_accounts(); if !signer.can_sign() { log::info!("no signer"); @@ -884,7 +887,7 @@ pub mod pallet { let mut to32 = AccountId32::as_ref(&account); let address: T::AccountId = T::AccountId::decode(&mut to32).unwrap_or_default(); - if prices.len() as u32 >= Self::asset_info(price_id).unwrap_or_default().max_answers { + if prices.len() as u32 >= asset_info.max_answers { log::info!("Max answers reached"); return Err("Max answers reached") } diff --git a/frame/oracle/src/tests.rs b/frame/oracle/src/tests.rs index a21928d802d..4312652d198 100644 --- a/frame/oracle/src/tests.rs +++ b/frame/oracle/src/tests.rs @@ -449,8 +449,17 @@ fn test_payout_slash() { let four = PrePrice { price: 400, block: 0, who: account_4 }; let five = PrePrice { price: 100, block: 0, who: account_5 }; + + let asset_info = AssetInfo { + threshold: Percent::from_percent(0), + min_answers: 0, + max_answers: 0, + block_interval: 0, + reward: 0, + slash: 0, + }; // doesn't panic when percent not set - Oracle::handle_payout(&vec![one, two, three, four, five], 100, 0); + Oracle::handle_payout(&vec![one, two, three, four, five], 100, 0, &asset_info); assert_eq!(Balances::free_balance(account_1), 100); assert_ok!(Oracle::add_asset_and_info( @@ -470,7 +479,12 @@ fn test_payout_slash() { assert_eq!(Oracle::answer_in_transit(account_1), Some(5)); assert_eq!(Oracle::answer_in_transit(account_2), Some(5)); - Oracle::handle_payout(&vec![one, two, three, four, five], 100, 0); + Oracle::handle_payout( + &vec![one, two, three, four, five], + 100, + 0, + &Oracle::asset_info(0).unwrap(), + ); assert_eq!(Oracle::answer_in_transit(account_1), Some(0)); assert_eq!(Oracle::answer_in_transit(account_2), Some(0)); @@ -493,7 +507,12 @@ fn test_payout_slash() { 4, 5 )); - Oracle::handle_payout(&vec![one, two, three, four, five], 100, 0); + Oracle::handle_payout( + &vec![one, two, three, four, five], + 100, + 0, + &Oracle::asset_info(0).unwrap(), + ); // account 4 gets slashed 2 5 and 1 gets rewarded assert_eq!(Balances::free_balance(account_1), 90); @@ -748,7 +767,7 @@ fn prune_old_pre_prices_edgecase() { reward: 5, slash: 5, }; - Oracle::prune_old_pre_prices(asset_info, vec![], 0, &0); + Oracle::prune_old_pre_prices(&asset_info, vec![], 0); }); } @@ -821,7 +840,7 @@ fn should_submit_signed_transaction_on_chain() { )); // when - Oracle::fetch_price_and_send_signed(&0).unwrap(); + Oracle::fetch_price_and_send_signed(&0, Oracle::asset_info(0).unwrap()).unwrap(); // then let tx = pool_state.write().transactions.pop().unwrap(); assert!(pool_state.read().transactions.is_empty()); @@ -852,7 +871,7 @@ fn should_check_oracles_submitted_price() { add_price_storage(100_u128, 0, account_2, 0); // when - Oracle::fetch_price_and_send_signed(&0).unwrap(); + Oracle::fetch_price_and_send_signed(&0, Oracle::asset_info(0).unwrap()).unwrap(); }); } @@ -860,9 +879,16 @@ fn should_check_oracles_submitted_price() { #[should_panic = "Max answers reached"] fn should_check_oracles_max_answer() { let (mut t, _) = offchain_worker_env(|state| price_oracle_response(state, "0")); - + let asset_info = AssetInfo { + threshold: Percent::from_percent(0), + min_answers: 0, + max_answers: 0, + block_interval: 0, + reward: 0, + slash: 0, + }; t.execute_with(|| { - Oracle::fetch_price_and_send_signed(&0).unwrap(); + Oracle::fetch_price_and_send_signed(&0, asset_info).unwrap(); }); }