add set_collection_info and polish aggregation/validation

pallets/oracle-data-collection/src/benchmarking.rs

@@ -4,7 +4,7 @@ use frame_system::RawOrigin;
 
 use crate::{
 	pallet::{Call, Collection, Config, Pallet},
-	types::Change,
+	types::{Change, CollectionInfo},
 };
 
 #[cfg(test)]
@@ -133,10 +133,10 @@ mod benchmarks {
 		}
 
 		// Worst case expect to read the max age
-		Pallet::<T>::set_collection_max_age(
+		Pallet::<T>::set_collection_info(
 			RawOrigin::Signed(admin.clone()).into(),
 			T::CollectionId::default(),
-			T::Timestamp::default(),
+			CollectionInfo::default(),
 		)
 		.unwrap();
 
@@ -154,7 +154,7 @@ mod benchmarks {
 	}
 
 	#[benchmark]
-	fn set_collection_max_age() -> Result<(), BenchmarkError> {
+	fn set_collection_info() -> Result<(), BenchmarkError> {
 		#[cfg(test)]
 		init_mocks();
 
@@ -163,10 +163,10 @@ mod benchmarks {
 		T::ChangeGuard::bench_create_pool(T::CollectionId::default(), &admin);
 
 		#[extrinsic_call]
-		set_collection_max_age(
+		set_collection_info(
 			RawOrigin::Signed(admin),
 			T::CollectionId::default(),
-			T::Timestamp::default(),
+			CollectionInfo::default(),
 		);
 
 		Ok(())

pallets/oracle-data-collection/src/lib.rs

@@ -59,7 +59,7 @@ pub mod pallet {
 
 	use crate::{
 		traits::AggregationProvider,
-		types::{CachedCollection, Change, KeyInfo, OracleValuePair},
+		types::{self, CachedCollection, Change, KeyInfo, OracleValuePair},
 		weights::WeightInfo,
 	};
 
@@ -148,8 +148,8 @@ pub mod pallet {
 
 	/// Store all oracle values indexed by feeder
 	#[pallet::storage]
-	pub(crate) type CollectionMaxAges<T: Config> =
-		StorageMap<_, Blake2_128Concat, T::CollectionId, T::Timestamp, OptionQuery>;
+	pub(crate) type CollectionInfo<T: Config> =
+		StorageMap<_, Blake2_128Concat, T::CollectionId, types::CollectionInfo<T>, ValueQuery>;
 
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
@@ -192,6 +192,9 @@ pub mod pallet {
 
 		/// The oracle value has passed the collection max age.
 		OracleValueOutdated,
+
+		/// The amount of feeders for a key is not enough
+		NotEnoughFeeders,
 	}
 
 	#[pallet::call]
@@ -314,13 +317,13 @@ pub mod pallet {
 			Ok(())
 		}
 
-		/// Sets the minimum
-		#[pallet::weight(T::WeightInfo::set_collection_max_age())]
+		/// Sets a associated information to a collection.
+		#[pallet::weight(T::WeightInfo::set_collection_info())]
 		#[pallet::call_index(3)]
-		pub fn set_collection_max_age(
+		pub fn set_collection_info(
 			origin: OriginFor<T>,
 			collection_id: T::CollectionId,
-			duration: T::Timestamp,
+			info: types::CollectionInfo<T>,
 		) -> DispatchResult {
 			let who = ensure_signed(origin)?;
 
@@ -329,7 +332,7 @@ pub mod pallet {
 				Error::<T>::IsNotAdmin
 			);
 
-			CollectionMaxAges::<T>::insert(collection_id, duration);
+			CollectionInfo::<T>::insert(collection_id, info);
 
 			Ok(())
 		}
@@ -343,7 +346,9 @@ pub mod pallet {
 			key: &T::OracleKey,
 			collection_id: &T::CollectionId,
 		) -> Result<Self::Data, DispatchError> {
+			let min_feeders = CollectionInfo::<T>::get(collection_id).min_feeders;
 			let key_info = Keys::<T>::get(collection_id, key);
+
 			let fed_values = key_info
 				.feeders
 				.into_iter()
@@ -352,10 +357,23 @@ pub mod pallet {
 				})
 				.collect::<Result<Vec<_>, _>>()?;
 
-			let (value, timestamp) = T::AggregationProvider::aggregate(fed_values)
-				.ok_or(Error::<T>::KeyNotInCollection)?;
+			if fed_values.len() < (min_feeders as usize) {
+				Err(Error::<T>::NotEnoughFeeders)?
+			}
+
+			let updated_fed_values = fed_values
+				.into_iter()
+				.filter(|(_, timestamp)| {
+					Self::ensure_valid_timestamp(collection_id, *timestamp).is_ok()
+				})
+				.collect::<Vec<_>>();
 
-			Self::ensure_valid_timestamp(collection_id, timestamp)?;
+			if updated_fed_values.len() < (min_feeders as usize) {
+				Err(Error::<T>::OracleValueOutdated)?
+			}
+
+			let (value, timestamp) = T::AggregationProvider::aggregate(updated_fed_values)
+				.ok_or(Error::<T>::KeyNotInCollection)?;
 
 			Ok((value, timestamp))
 		}
@@ -432,9 +450,9 @@ pub mod pallet {
 			collection_id: &T::CollectionId,
 			timestamp: T::Timestamp,
 		) -> DispatchResult {
-			if let Some(threshold) = CollectionMaxAges::<T>::get(collection_id) {
+			if let Some(duration) = CollectionInfo::<T>::get(collection_id).value_duration {
 				ensure!(
-					T::Time::now().ensure_sub(timestamp)? <= threshold,
+					T::Time::now().ensure_sub(timestamp)? <= duration,
 					Error::<T>::OracleValueOutdated,
 				);
 			}
@@ -491,6 +509,7 @@ pub mod types {
 		pallet_prelude::{Decode, Encode, MaxEncodedLen, TypeInfo},
 		storage::{bounded_btree_map::BoundedBTreeMap, bounded_btree_set::BoundedBTreeSet},
 		traits::Time,
+		RuntimeDebugNoBound,
 	};
 	use sp_runtime::{traits::Zero, RuntimeDebug};
 	use sp_std::vec::Vec;
@@ -522,6 +541,30 @@ pub mod types {
 		}
 	}
 
+	/// Information of a collection
+	#[derive(
+		Encode, Decode, PartialEq, Eq, Clone, TypeInfo, RuntimeDebugNoBound, MaxEncodedLen,
+	)]
+	#[scale_info(skip_type_params(T))]
+	pub struct CollectionInfo<T: Config> {
+		/// Maximum duration to consider an oracle value non-outdated.
+		/// An oracle value is consider updated if its timestam is higher
+		/// than `now() - value_duration`
+		pub value_duration: Option<T::Timestamp>,
+
+		/// Minimun number of feeders to succesfully aggregate a value.
+		pub min_feeders: u32,
+	}
+
+	impl<T: Config> Default for CollectionInfo<T> {
+		fn default() -> Self {
+			Self {
+				value_duration: None,
+				min_feeders: 0,
+			}
+		}
+	}
+
 	/// A collection cached in memory
 	#[derive(Encode, Decode, Clone, TypeInfo, RuntimeDebug, MaxEncodedLen)]
 	#[scale_info(skip_type_params(T))]
@@ -532,7 +575,7 @@ pub mod types {
 
 	impl<T: Config> Default for CachedCollection<T> {
 		fn default() -> Self {
-			CachedCollection {
+			Self {
 				content: Default::default(),
 				older_timestamp: T::Time::now(),
 			}

pallets/oracle-data-collection/src/tests.rs

@@ -5,7 +5,7 @@ use sp_runtime::{testing::H256, traits::Get, DispatchError};
 use crate::{
 	mock::*,
 	pallet::{Config, Error, Event, Keys},
-	types::Change,
+	types::{Change, CollectionInfo},
 };
 
 const ADMIN: AccountId = 1;
@@ -110,13 +110,16 @@ mod util {
 		MockIsAdmin::mock_check(|_| panic!("no check() mock"));
 	}
 
-	pub fn set_max_age(duration: Timestamp) {
+	pub fn set_collection_info(duration: Timestamp, limit: u32) {
 		MockIsAdmin::mock_check(|_| true);
 
-		assert_ok!(OracleCollection::set_collection_max_age(
+		assert_ok!(OracleCollection::set_collection_info(
 			RuntimeOrigin::signed(ADMIN),
 			COLLECTION_ID,
-			duration,
+			CollectionInfo {
+				value_duration: Some(duration),
+				min_feeders: limit,
+			}
 		));
 
 		MockIsAdmin::mock_check(|_| panic!("no check() mock"));
@@ -179,10 +182,10 @@ fn update_collection_max_age() {
 	new_test_ext().execute_with(|| {
 		MockIsAdmin::mock_check(|_| true);
 
-		assert_ok!(OracleCollection::set_collection_max_age(
+		assert_ok!(OracleCollection::set_collection_info(
 			RuntimeOrigin::signed(ADMIN),
 			COLLECTION_ID,
-			50
+			CollectionInfo::default(),
 		));
 	});
 }
@@ -193,10 +196,10 @@ fn update_collection_max_age_wrong_admin() {
 		MockIsAdmin::mock_check(|_| false);
 
 		assert_err!(
-			OracleCollection::set_collection_max_age(
+			OracleCollection::set_collection_info(
 				RuntimeOrigin::signed(ADMIN),
 				COLLECTION_ID,
-				50
+				CollectionInfo::default(),
 			),
 			Error::<Runtime>::IsNotAdmin
 		);
@@ -299,7 +302,7 @@ fn getting_value_not_found() {
 fn getting_value_but_outdated() {
 	new_test_ext().execute_with(|| {
 		util::update_feeders(KEY_A, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
-		util::set_max_age(NOT_ENOUGH_MAX_AGE);
+		util::set_collection_info(NOT_ENOUGH_MAX_AGE, 1);
 
 		mock::prepare_provider();
 		assert_err!(
@@ -343,7 +346,7 @@ fn update_collection_with_max_age() {
 	new_test_ext().execute_with(|| {
 		util::update_feeders(KEY_A, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
 		util::update_feeders(KEY_B, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
-		util::set_max_age(ENOUGH_MAX_AGE);
+		util::set_collection_info(ENOUGH_MAX_AGE, 0);
 
 		mock::prepare_provider();
 		assert_ok!(OracleCollection::update_collection(
@@ -364,7 +367,7 @@ fn update_collection_outdated() {
 	new_test_ext().execute_with(|| {
 		util::update_feeders(KEY_A, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
 		util::update_feeders(KEY_B, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
-		util::set_max_age(NOT_ENOUGH_MAX_AGE);
+		util::set_collection_info(NOT_ENOUGH_MAX_AGE, 1);
 
 		mock::prepare_provider();
 		assert_err!(
@@ -374,20 +377,55 @@ fn update_collection_outdated() {
 	});
 }
 
+#[test]
+fn update_collection_outdated_with_min_feeder() {
+	new_test_ext().execute_with(|| {
+		util::update_feeders(KEY_A, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
+		util::update_feeders(KEY_B, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
+		util::set_collection_info(45, 1);
+
+		mock::prepare_provider();
+		assert_ok!(OracleCollection::update_collection(
+			RuntimeOrigin::signed(ANY),
+			COLLECTION_ID
+		));
+
+		let collection = OracleCollection::collection(&COLLECTION_ID).unwrap();
+		assert_eq!(
+			collection.as_vec(),
+			vec![(KEY_A, (102, NOW - 45)), (KEY_B, (1000, NOW))]
+		);
+	});
+}
+
+#[test]
+fn update_collection_without_min_feeder() {
+	new_test_ext().execute_with(|| {
+		util::update_feeders(KEY_B, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
+		util::set_collection_info(ENOUGH_MAX_AGE, 2);
+
+		mock::prepare_provider();
+		assert_err!(
+			OracleCollection::update_collection(RuntimeOrigin::signed(ANY), COLLECTION_ID),
+			Error::<Runtime>::NotEnoughFeeders
+		);
+	});
+}
+
 #[test]
 fn update_collection_but_getting_elements_out_of_time() {
 	new_test_ext().execute_with(|| {
 		util::update_feeders(KEY_A, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
 		util::update_feeders(KEY_B, vec![FEEDER_1, FEEDER_2, FEEDER_3]);
-		util::set_max_age(ENOUGH_MAX_AGE);
+		util::set_collection_info(ENOUGH_MAX_AGE, 0);
 
 		mock::prepare_provider();
 		assert_ok!(OracleCollection::update_collection(
 			RuntimeOrigin::signed(ANY),
 			COLLECTION_ID
 		));
 
-		util::set_max_age(NOT_ENOUGH_MAX_AGE);
+		util::set_collection_info(NOT_ENOUGH_MAX_AGE, 0);
 
 		assert_err!(
 			OracleCollection::collection(&COLLECTION_ID),

pallets/oracle-data-collection/src/weights.rs

@@ -17,7 +17,7 @@ pub trait WeightInfo {
 	fn propose_update_feeders(feeders: u32) -> Weight;
 	fn apply_update_feeders(feeders: u32) -> Weight;
 	fn update_collection(feeders: u32, keys: u32) -> Weight;
-	fn set_collection_max_age() -> Weight;
+	fn set_collection_info() -> Weight;
 }
 
 impl WeightInfo for () {
@@ -33,7 +33,7 @@ impl WeightInfo for () {
 		Weight::zero()
 	}
 
-	fn set_collection_max_age() -> Weight {
+	fn set_collection_info() -> Weight {
 		Weight::zero()
 	}
 }

runtime/altair/src/weights/pallet_oracle_data_collection.rs

@@ -18,7 +18,7 @@ impl<T: frame_system::Config> pallet_oracle_data_collection::WeightInfo for Weig
 		Weight::zero()
 	}
 
-	fn set_collection_max_age() -> Weight {
+	fn set_collection_info() -> Weight {
 		Weight::zero()
 	}
 }

runtime/centrifuge/src/weights/pallet_oracle_data_collection.rs

@@ -18,7 +18,7 @@ impl<T: frame_system::Config> pallet_oracle_data_collection::WeightInfo for Weig
 		Weight::zero()
 	}
 
-	fn set_collection_max_age() -> Weight {
+	fn set_collection_info() -> Weight {
 		Weight::zero()
 	}
 }

runtime/development/src/weights/pallet_oracle_data_collection.rs

@@ -18,7 +18,7 @@ impl<T: frame_system::Config> pallet_oracle_data_collection::WeightInfo for Weig
 		Weight::zero()
 	}
 
-	fn set_collection_max_age() -> Weight {
+	fn set_collection_info() -> Weight {
 		Weight::zero()
 	}
 }