Oracle: allow multiple feed values from inner calls (#920)

* move HasDispatched check to the extrinsic

* add test

* add option account to DataFeeder trait

* minor comment change

Author: lemunozm

oracle/src/lib.rs

@@ -163,7 +163,16 @@ pub mod module {
 			let feeder = ensure_signed(origin.clone())
 				.map(Some)
 				.or_else(|_| ensure_root(origin).map(|_| None))?;
-			Self::do_feed_values(feeder, values)?;
+
+			let who = Self::ensure_account(feeder)?;
+
+			// ensure account hasn't dispatched an updated yet
+			ensure!(
+				HasDispatched::<T, I>::mutate(|set| set.insert(who.clone())),
+				Error::<T, I>::AlreadyFeeded
+			);
+
+			Self::do_feed_values(who, values)?;
 			Ok(Pays::No.into())
 		}
 	}
@@ -193,21 +202,17 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		T::CombineData::combine_data(key, values, Self::values(key))
 	}
 
-	fn do_feed_values(who: Option<T::AccountId>, values: Vec<(T::OracleKey, T::OracleValue)>) -> DispatchResult {
+	fn ensure_account(who: Option<T::AccountId>) -> Result<T::AccountId, DispatchError> {
 		// ensure feeder is authorized
-		let who = if let Some(who) = who {
+		if let Some(who) = who {
 			ensure!(T::Members::contains(&who), Error::<T, I>::NoPermission);
-			who
+			Ok(who)
 		} else {
-			T::RootOperatorAccountId::get()
-		};
-
-		// ensure account hasn't dispatched an updated yet
-		ensure!(
-			HasDispatched::<T, I>::mutate(|set| set.insert(who.clone())),
-			Error::<T, I>::AlreadyFeeded
-		);
+			Ok(T::RootOperatorAccountId::get())
+		}
+	}
 
+	fn do_feed_values(who: T::AccountId, values: Vec<(T::OracleKey, T::OracleValue)>) -> DispatchResult {
 		let now = T::Time::now();
 		for (key, value) in &values {
 			let timestamped = TimestampedValue {
@@ -258,7 +263,7 @@ impl<T: Config<I>, I: 'static> DataProviderExtended<T::OracleKey, TimestampedVal
 }
 
 impl<T: Config<I>, I: 'static> DataFeeder<T::OracleKey, T::OracleValue, T::AccountId> for Pallet<T, I> {
-	fn feed_value(who: T::AccountId, key: T::OracleKey, value: T::OracleValue) -> DispatchResult {
-		Self::do_feed_values(Some(who), vec![(key, value)])
+	fn feed_value(who: Option<T::AccountId>, key: T::OracleKey, value: T::OracleValue) -> DispatchResult {
+		Self::do_feed_values(Self::ensure_account(who)?, vec![(key, value)])
 	}
 }

oracle/src/tests.rs

@@ -65,6 +65,9 @@ fn should_feed_values_from_root() {
 			vec![(50, 1000), (51, 900), (52, 800)]
 		));
 
+		// Or feed from root using the DataFeeder trait with None
+		assert_ok!(ModuleOracle::feed_value(None, 53, 700));
+
 		assert_eq!(
 			ModuleOracle::raw_values(&root_feeder, &50),
 			Some(TimestampedValue {
@@ -88,6 +91,14 @@ fn should_feed_values_from_root() {
 				timestamp: 12345,
 			})
 		);
+
+		assert_eq!(
+			ModuleOracle::raw_values(&root_feeder, &53),
+			Some(TimestampedValue {
+				value: 700,
+				timestamp: 12345,
+			})
+		);
 	});
 }
 
@@ -167,11 +178,16 @@ fn should_return_none_for_non_exist_key() {
 fn multiple_calls_should_fail() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(ModuleOracle::feed_values(RuntimeOrigin::signed(1), vec![(50, 1300)]));
+
+		// Fails feeding by the the extrinsic
 		assert_noop!(
 			ModuleOracle::feed_values(RuntimeOrigin::signed(1), vec![(50, 1300)]),
 			Error::<Test, _>::AlreadyFeeded,
 		);
 
+		// But not if fed thought the trait internally
+		assert_ok!(ModuleOracle::feed_value(Some(1), 50, 1300));
+
 		ModuleOracle::on_finalize(1);
 
 		assert_ok!(ModuleOracle::feed_values(RuntimeOrigin::signed(1), vec![(50, 1300)]));

traits/src/data_provider.rs

@@ -2,9 +2,9 @@ use sp_runtime::DispatchResult;
 use sp_std::vec::Vec;
 
 /// Data provider with ability to provide data with no-op, and provide all data.
-pub trait DataFeeder<Key, Value, AccountId>: DataProvider<Key, Value> {
+pub trait DataFeeder<Key, Value, AccountId> {
 	/// Provide a new value for a given key from an operator
-	fn feed_value(who: AccountId, key: Key, value: Value) -> DispatchResult;
+	fn feed_value(who: Option<AccountId>, key: Key, value: Value) -> DispatchResult;
 }
 
 /// A simple trait to provide data