From 5463f5af8877bc829faa1f9ee73b59604dfb9b3e Mon Sep 17 00:00:00 2001
From: Xiliang Chen <xlchen1291@gmail.com>
Date: Fri, 21 May 2021 22:26:59 +1200
Subject: [PATCH] refactor oracle to remove members (#502)
* no longer need Members
* fix warning
* fix tests
---
oracle/src/lib.rs | 61 ++++++++-------------------------------------
oracle/src/mock.rs | 22 +++++++++-------
oracle/src/tests.rs | 1 +
3 files changed, 24 insertions(+), 60 deletions(-)
diff --git a/oracle/src/lib.rs b/oracle/src/lib.rs
index e17708f..b061633 100644
--- a/oracle/src/lib.rs
+++ b/oracle/src/lib.rs
@@ -11,10 +11,8 @@
//! offchain data. The raw values can be combined to provide an aggregated
//! value.
//!
-//! The data is valid only if feeded by an authorized operator. This module
-//! implements `frame_support::traits::InitializeMembers` and `frame_support::
-//! traits::ChangeMembers`, to provide a way to manage operators membership.
-//! Typically it could be leveraged to `pallet_membership` in FRAME.
+//! The data is valid only if feeded by an authorized operator.
+//! `pallet_membership` in FRAME can be used to as source of `T::Members`.
#![cfg_attr(not(feature = "std"), no_std)]
// Disable the following two lints since they originate from an external macro (namely decl_storage)
@@ -29,7 +27,7 @@ use serde::{Deserialize, Serialize};
use frame_support::{
ensure,
pallet_prelude::*,
- traits::{ChangeMembers, Get, InitializeMembers, Time},
+ traits::{ChangeMembers, Get, SortedMembers, Time},
weights::{Pays, Weight},
Parameter,
};
@@ -86,6 +84,9 @@ pub mod module {
/// The root operator account id, record all sudo feeds on this account.
type RootOperatorAccountId: Get<Self::AccountId>;
+ /// Oracle operators.
+ type Members: SortedMembers<Self::AccountId>;
+
/// Weight information for extrinsics in this module.
type WeightInfo: WeightInfo;
}
@@ -128,36 +129,6 @@ pub mod module {
pub(crate) type HasDispatched<T: Config<I>, I: 'static = ()> =
StorageValue<_, OrderedSet<T::AccountId>, ValueQuery>;
- // TODO: this shouldn't be required https://github.com/paritytech/substrate/issues/6041
- /// The current members of the collective. This is stored sorted (just by
- /// value).
- #[pallet::storage]
- #[pallet::getter(fn members)]
- pub type Members<T: Config<I>, I: 'static = ()> = StorageValue<_, OrderedSet<T::AccountId>, ValueQuery>;
-
- #[pallet::genesis_config]
- pub struct GenesisConfig<T: Config<I>, I: 'static = ()> {
- pub members: OrderedSet<T::AccountId>,
- pub phantom: sp_std::marker::PhantomData<I>,
- }
-
- #[cfg(feature = "std")]
- impl<T: Config<I>, I: 'static> Default for GenesisConfig<T, I> {
- fn default() -> Self {
- GenesisConfig {
- members: Default::default(),
- phantom: Default::default(),
- }
- }
- }
-
- #[pallet::genesis_build]
- impl<T: Config<I>, I: 'static> GenesisBuild<T, I> for GenesisConfig<T, I> {
- fn build(&self) {
- <Members<T, I>>::put(self.members.clone());
- }
- }
-
#[pallet::pallet]
pub struct Pallet<T, I = ()>(PhantomData<(T, I)>);
@@ -194,8 +165,7 @@ pub mod module {
impl<T: Config<I>, I: 'static> Pallet<T, I> {
pub fn read_raw_values(key: &T::OracleKey) -> Vec<TimestampedValueOf<T, I>> {
- Self::members()
- .0
+ T::Members::sorted_members()
.iter()
.chain(vec![T::RootOperatorAccountId::get()].iter())
.filter_map(|x| Self::raw_values(x, key))
@@ -248,7 +218,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
fn do_feed_values(who: T::AccountId, values: Vec<(T::OracleKey, T::OracleValue)>) -> DispatchResult {
// ensure feeder is authorized
ensure!(
- Self::members().contains(&who) || who == T::RootOperatorAccountId::get(),
+ T::Members::contains(&who) || who == T::RootOperatorAccountId::get(),
Error::<T, I>::NoPermission
);
@@ -274,24 +244,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}
}
-impl<T: Config<I>, I: 'static> InitializeMembers<T::AccountId> for Pallet<T, I> {
- fn initialize_members(members: &[T::AccountId]) {
- if !members.is_empty() {
- assert!(Members::<T, I>::get().0.is_empty(), "Members are already initialized!");
- Members::<T, I>::put(OrderedSet::from_sorted_set(members.into()));
- }
- }
-}
-
impl<T: Config<I>, I: 'static> ChangeMembers<T::AccountId> for Pallet<T, I> {
- fn change_members_sorted(_incoming: &[T::AccountId], outgoing: &[T::AccountId], new: &[T::AccountId]) {
- // remove session keys and its values
+ fn change_members_sorted(_incoming: &[T::AccountId], outgoing: &[T::AccountId], _new: &[T::AccountId]) {
+ // remove values
for removed in outgoing {
RawValues::<T, I>::remove_prefix(removed);
}
- Members::<T, I>::put(OrderedSet::from_sorted_set(new.into()));
-
// not bothering to track which key needs recompute, just update all
IsUpdated::<T, I>::remove_all();
}
diff --git a/oracle/src/mock.rs b/oracle/src/mock.rs
index 569dcdc..6ab658c 100644
--- a/oracle/src/mock.rs
+++ b/oracle/src/mock.rs
@@ -2,7 +2,7 @@
use super::*;
-use frame_support::{construct_runtime, parameter_types};
+use frame_support::{construct_runtime, parameter_types, traits::SortedMembers};
use sp_core::H256;
use sp_runtime::{
testing::Header,
@@ -71,6 +71,15 @@ parameter_types! {
pub const MinimumCount: u32 = 3;
pub const ExpiresIn: u32 = 600;
pub const RootOperatorAccountId: AccountId = 4;
+ pub static OracleMembers: Vec<AccountId> = vec![1, 2, 3];
+}
+
+pub struct Members;
+
+impl SortedMembers<AccountId> for Members {
+ fn sorted_members() -> Vec<AccountId> {
+ OracleMembers::get()
+ }
}
impl Config for Test {
@@ -81,6 +90,7 @@ impl Config for Test {
type OracleKey = Key;
type OracleValue = Value;
type RootOperatorAccountId = RootOperatorAccountId;
+ type Members = Members;
type WeightInfo = ();
}
@@ -94,20 +104,14 @@ construct_runtime!(
UncheckedExtrinsic = UncheckedExtrinsic,
{
System: frame_system::{Pallet, Call, Storage, Config, Event<T>},
- ModuleOracle: oracle::{Pallet, Storage, Call, Config<T>, Event<T>},
+ ModuleOracle: oracle::{Pallet, Storage, Call, Event<T>},
}
);
// This function basically just builds a genesis storage key/value store
// according to our desired mockup.
pub fn new_test_ext() -> sp_io::TestExternalities {
- let mut storage = frame_system::GenesisConfig::default().build_storage::<Test>().unwrap();
-
- let _ = oracle::GenesisConfig::<Test> {
- members: vec![1, 2, 3].into(),
- phantom: Default::default(),
- }
- .assimilate_storage(&mut storage);
+ let storage = frame_system::GenesisConfig::default().build_storage::<Test>().unwrap();
let mut t: sp_io::TestExternalities = storage.into();
diff --git a/oracle/src/tests.rs b/oracle/src/tests.rs
index 79f0fcc..d5a8fae 100644
--- a/oracle/src/tests.rs
+++ b/oracle/src/tests.rs
@@ -237,6 +237,7 @@ fn get_all_values_should_work() {
#[test]
fn change_member_should_work() {
new_test_ext().execute_with(|| {
+ OracleMembers::set(vec![2, 3, 4]);
<ModuleOracle as ChangeMembers<AccountId>>::change_members_sorted(&[4], &[1], &[2, 3, 4]);
assert_noop!(
ModuleOracle::feed_values(Origin::signed(1), vec![(50, 1000)]),
--
GitLab