Skip to content

Commit

Permalink
remove unwrap or defaults from asset_info
Browse files Browse the repository at this point in the history
  • Loading branch information
@JesseAbram
JesseAbram committed Jan 17, 2022
1 parent 43524e6 commit 9483d0d
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 26 deletions.
39 changes: 21 additions & 18 deletions frame/oracle/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,8 +614,8 @@ pub mod pallet {
pre_prices: &[PrePrice<T::PriceValue, T::BlockNumber, T::AccountId>],
price: T::PriceValue,
asset_id: T::AssetId,
asset_info: &AssetInfo<Percent, T::BlockNumber, BalanceOf<T>>,
) {
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)
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -661,7 +661,7 @@ pub mod pallet {
for (asset_id, asset_info) in AssetsInfo::<T>::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);
Expand All @@ -673,7 +673,7 @@ pub mod pallet {
#[allow(clippy::type_complexity)]
pub fn update_pre_prices(
asset_id: T::AssetId,
asset_info: AssetInfo<Percent, T::BlockNumber, BalanceOf<T>>,
asset_info: &AssetInfo<Percent, T::BlockNumber, BalanceOf<T>>,
block: T::BlockNumber,
) -> (usize, Vec<PrePrice<T::PriceValue, T::BlockNumber, T::AccountId>>) {
// TODO maybe add a check if price is requested, is less operations?
Expand All @@ -685,8 +685,7 @@ pub mod pallet {
// because pre_pruned_prices.len() limited by u32
// (type of AssetsInfo::<T>::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 {
Expand Down Expand Up @@ -725,17 +724,16 @@ pub mod pallet {
}
PrePrices::<T>::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<Percent, T::BlockNumber, BalanceOf<T>>,
asset_info: &AssetInfo<Percent, T::BlockNumber, BalanceOf<T>>,
mut pre_prices: Vec<PrePrice<T::PriceValue, T::BlockNumber, T::AccountId>>,
block: T::BlockNumber,
asset_id: &T::AssetId,
) -> (
Vec<PrePrice<T::PriceValue, T::BlockNumber, T::AccountId>>,
Vec<PrePrice<T::PriceValue, T::BlockNumber, T::AccountId>>,
Expand All @@ -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)
},
Expand All @@ -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
Expand Down Expand Up @@ -792,9 +790,9 @@ pub mod pallet {
}

pub fn check_requests() {
for (i, _) in AssetsInfo::<T>::iter() {
for (i, asset_info) in AssetsInfo::<T>::iter() {
if Self::is_requested(&i) {
let _ = Self::fetch_price_and_send_signed(&i);
let _ = Self::fetch_price_and_send_signed(&i, asset_info);
}
}
}
Expand All @@ -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<Percent, T::BlockNumber, BalanceOf<T>>,
) {
AnswerInTransit::<T>::mutate(&who, |transit| {
*transit = Some(transit.unwrap_or_else(Zero::zero).saturating_sub(asset_info.slash))
});
Expand Down Expand Up @@ -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<Percent, T::BlockNumber, BalanceOf<T>>,
) -> Result<(), &'static str> {
let signer = Signer::<T, T::AuthorityId>::all_accounts();
if !signer.can_sign() {
log::info!("no signer");
Expand All @@ -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")
}
Expand Down
42 changes: 34 additions & 8 deletions frame/oracle/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -852,17 +871,24 @@ 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();
});
}

#[test]
#[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();
});
}

Expand Down

0 comments on commit 9483d0d

Please sign in to comment.