fix asset count to change an existing asset

frame/oracle/src/benchmarking.rs

@@ -154,7 +154,7 @@ benchmarks! {
 		.collect::<Vec<_>>();
 		PrePrices::<T>::insert(asset_id, pre_prices);
 	}: {
-		Oracle::<T>::update_pre_prices(asset_id, asset_info, block)
+		Oracle::<T>::update_pre_prices(asset_id, &asset_info, block)
 	}
 
 	update_price {

frame/oracle/src/lib.rs

@@ -135,8 +135,6 @@ pub mod pallet {
 		type StalePrice: Get<Self::BlockNumber>;
 		/// Origin to add new price types
 		type AddOracle: EnsureOrigin<Self::Origin>;
-		/// Slash for an incorrect answer
-		type SlashAmount: Get<BalanceOf<Self>>;
 		/// Upper bound for max answers for a price
 		type MaxAnswerBound: Get<u32>;
 		/// Upper bound for total assets available for the oracle
@@ -269,7 +267,7 @@ pub mod pallet {
 		Blake2_128Concat,
 		T::AssetId,
 		AssetInfo<Percent, T::BlockNumber, BalanceOf<T>>,
-		ValueQuery,
+		OptionQuery,
 	>;
 
 	#[pallet::event]
@@ -427,8 +425,13 @@ pub mod pallet {
 			);
 			let asset_info =
 				AssetInfo { threshold, min_answers, max_answers, block_interval, reward, slash };
+
+			let current_asset_info = Self::asset_info(asset_id);
+			if current_asset_info.is_none() {
+				AssetsCount::<T>::mutate(|a| *a += 1);
+			}
+
 			AssetsInfo::<T>::insert(asset_id, asset_info);
-			AssetsCount::<T>::mutate(|a| *a += 1);
 			Self::deposit_event(Event::AssetInfoChange(
 				asset_id,
 				threshold,
@@ -550,7 +553,7 @@ pub mod pallet {
 				block: frame_system::Pallet::<T>::block_number(),
 				who: who.clone(),
 			};
-			let asset_info = AssetsInfo::<T>::get(asset_id);
+			let asset_info = Self::asset_info(asset_id).ok_or(Error::<T>::InvalidAssetId)?;
 			PrePrices::<T>::try_mutate(asset_id, |current_prices| -> Result<(), DispatchError> {
 				// There can convert current_prices.len() to u32 safely
 				// because current_prices.len() limited by u32
@@ -611,16 +614,16 @@ 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);
 			for answer in pre_prices {
 				let accuracy: Percent = if answer.price < price {
 					PerThing::from_rational(answer.price, price)
 				} else {
 					let adjusted_number = price.saturating_sub(answer.price - price);
 					PerThing::from_rational(adjusted_number, price)
 				};
-				let min_accuracy = AssetsInfo::<T>::get(asset_id).threshold;
+				let min_accuracy = asset_info.threshold;
 				if accuracy < min_accuracy {
 					let slash_amount = asset_info.slash;
 					let try_slash = T::Currency::can_slash(&answer.who, slash_amount);
@@ -647,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)
 			}
 		}
 
@@ -658,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);
@@ -670,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?
@@ -682,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 {
@@ -722,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>>,
@@ -741,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)
 					},
@@ -753,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
@@ -789,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);
 				}
 			}
 		}
@@ -800,11 +801,17 @@ pub mod pallet {
 			let last_update = Self::prices(price_id);
 			let current_block = frame_system::Pallet::<T>::block_number();
 			let asset_info = Self::asset_info(price_id);
-			last_update.block + asset_info.block_interval < current_block
+			if asset_info.is_none() {
+				false
+			} else {
+				last_update.block + asset_info.unwrap_or_default().block_interval < current_block
+			}
 		}
 
-		pub fn remove_price_in_transit(asset_id: &T::AssetId, who: &T::AccountId) {
-			let asset_info = AssetsInfo::<T>::get(asset_id);
+		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))
 			});
@@ -859,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");
@@ -877,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).max_answers {
+			if prices.len() as u32 >= asset_info.max_answers {
 				log::info!("Max answers reached");
 				return Err("Max answers reached")
 			}

frame/oracle/src/mock.rs

@@ -78,7 +78,6 @@ parameter_types! {
 	pub const StakeLock: u64 = 1;
 	pub const MinStake: u64 = 1;
 	pub const StalePrice: u64 = 2;
-	pub const SlashAmount: u64 = 5;
 	pub const MaxAnswerBound: u32 = 5;
 	pub const MaxAssetsCount: u32 = 2;
 	pub const MaxHistory: u32 = 3;
@@ -128,7 +127,6 @@ impl pallet_oracle::Config for Test {
 	type StalePrice = StalePrice;
 	type MinStake = MinStake;
 	type AddOracle = EnsureSignedBy<RootAccount, sp_core::sr25519::Public>;
-	type SlashAmount = SlashAmount;
 	type MaxAnswerBound = MaxAnswerBound;
 	type MaxAssetsCount = MaxAssetsCount;
 	type MaxHistory = MaxHistory;

frame/oracle/src/tests.rs

@@ -42,6 +42,19 @@ fn add_asset_and_info() {
 			SLASH
 		));
 
+		// does not increment if exists
+		assert_ok!(Oracle::add_asset_and_info(
+			Origin::signed(account_2),
+			ASSET_ID,
+			THRESHOLD,
+			MIN_ANSWERS,
+			MAX_ANSWERS,
+			BLOCK_INTERVAL,
+			REWARD,
+			SLASH
+		));
+		assert_eq!(Oracle::assets_count(), 1);
+
 		assert_ok!(Oracle::add_asset_and_info(
 			Origin::signed(account_2),
 			ASSET_ID + 1,
@@ -62,8 +75,9 @@ fn add_asset_and_info() {
 			slash: SLASH,
 		};
 		// id now activated and count incremented
-		assert_eq!(Oracle::asset_info(1), asset_info);
+		assert_eq!(Oracle::asset_info(1), Some(asset_info));
 		assert_eq!(Oracle::assets_count(), 2);
+
 		// fails with non permission
 		let account_1: AccountId = Default::default();
 		assert_noop!(
@@ -345,7 +359,7 @@ fn add_price() {
 		// non existent asset_id
 		assert_noop!(
 			Oracle::submit_price(Origin::signed(account_1), 100_u128, 10_u128),
-			Error::<Test>::MaxPrices
+			Error::<Test>::PriceNotRequested
 		);
 	});
 }
@@ -435,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(
@@ -456,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));
@@ -479,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);
@@ -734,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);
 	});
 }
 
@@ -807,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());
@@ -838,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();
 	});
 }
 

runtime/dali/src/lib.rs

@@ -419,8 +419,6 @@ parameter_types! {
 
 	/// TODO: discuss with omar/cosmin
 	pub MinStake: Balance = 1000 * CurrencyId::PICA.unit::<Balance>();
-	// Shouldn't this be a ratio based on locked amount?
-	pub const SlashAmount: Balance = 5;
 	pub const MaxAnswerBound: u32 = 25;
 	pub const MaxAssetsCount: u32 = 100_000;
 	pub const MaxHistory: u32 = 20;
@@ -436,7 +434,6 @@ impl oracle::Config for Runtime {
 	type MinStake = MinStake;
 	type StalePrice = StalePrice;
 	type AddOracle = EnsureRootOrHalfCouncil;
-	type SlashAmount = SlashAmount;
 	type MaxAnswerBound = MaxAnswerBound;
 	type MaxAssetsCount = MaxAssetsCount;
 	type MaxHistory = MaxHistory;

runtime/picasso/src/lib.rs

@@ -418,8 +418,6 @@ parameter_types! {
 
 	/// TODO: discuss with omar/cosmin
 	pub MinStake: Balance = 1000 * CurrencyId::PICA.unit::<Balance>();
-	// Shouldn't this be a ratio based on locked amount?
-	pub const SlashAmount: Balance = 5;
 	pub const MaxAnswerBound: u32 = 25;
 	pub const MaxAssetsCount: u32 = 100_000;
 	pub const MaxHistory: u32 = 20;