[BUG] Fix unwrap store_oracle and add test case (#185)

Signed-off-by: dung5ire <[email protected]>
5ire-tech · Aug 26, 2024 · b533746 · b533746
1 parent 97432cf
commit b533746
Show file tree

Hide file tree

Showing 9 changed files with 125 additions and 34 deletions.
diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs
@@ -275,6 +275,8 @@ impl pallet_esg::Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type MaxFileSize = ConstU32<1024000>;
 	type WeightInfo = ();
+	type MaxNumOfSudoOracles =  ConstU32<5>;
+	type MaxNumOfNonSudoOracles = ConstU32<5>;
 }
 
 impl pallet_offences::Config for Test {

diff --git a/frame/esg/src/lib.rs b/frame/esg/src/lib.rs
@@ -132,8 +132,11 @@ use frame_support::{
 					if max_num_of_sudo_oracles > num_of_sudo_oracles_stored {
 						<SudoOraclesStore<T>>::mutate(fn_mutate);
 						return Ok(());
+					}
+					else {
+						return Err(Error::<T>::MaxNumOfSudoOraclesReached.into());
 					} 
-					Err(Error::<T>::MaxNumOfSudoOraclesReached.into())
+
 				},
 
 				false => {
@@ -144,7 +147,10 @@ use frame_support::{
 						<NonSudoOraclesStore<T>>::mutate(fn_mutate);
 						return Ok(());
 					} 
-					Err(Error::<T>::MaxNumOfNonSudoOraclesReached.into())
+					else {
+						return Err(Error::<T>::MaxNumOfNonSudoOraclesReached.into());
+					}
+
 				},
 			}
 		}
@@ -263,7 +269,7 @@ use frame_support::{
 			}
 
 			if is_root || Self::is_sudo_oracle(&acc_id) {
-				let _ = Self::store_oracle(&oracle, is_sudo_oracle);
+				let _ = Self::store_oracle(&oracle, is_sudo_oracle)?;
 			} else {
 				return Err(Error::<T>::CallerNotRootOrSudoOracle.into())
 			}

diff --git a/frame/esg/src/mock.rs b/frame/esg/src/mock.rs
@@ -27,10 +27,7 @@ type Block = frame_system::mocking::MockBlock<Test>;
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
 
 frame_support::construct_runtime!(
-	pub enum Test where
-		Block = Block,
-		NodeBlock = Block,
-		UncheckedExtrinsic = UncheckedExtrinsic,
+	pub enum Test
 	{
 		System: frame_system,
 		Esg: pallet_esg,
@@ -65,11 +62,11 @@ impl system::Config for Test {
 
 parameter_types! {
 	pub const MaxFileSize: u32 = 1024000;
-	pub const MaxNumOfSudoOracles: u32 = 5;
-	pub const MaxNumOfNonSudoOracles: u32 = 100;
+	pub const MaxNumOfSudoOracles: u32 = 3;
+	pub const MaxNumOfNonSudoOracles: u32 = 2;
 }
 
-impl pallet_esg::Config for Runtime {
+impl pallet_esg::Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type MaxFileSize = MaxFileSize;
 	type MaxNumOfSudoOracles = MaxNumOfSudoOracles;

diff --git a/frame/esg/src/tests.rs b/frame/esg/src/tests.rs
@@ -1,7 +1,7 @@
 use fp_account::AccountId20;
 use sp_runtime::DispatchError;
 pub use crate::{mock::*, Error};
-use frame_support::{assert_noop, assert_ok, WeakBoundedVec};
+use frame_support::{assert_err, assert_noop, assert_ok, WeakBoundedVec};
 
 const MAX_ESG_SCORE: u16 = 100;
 
@@ -11,14 +11,13 @@ struct Addr {
 	ALICE: AccountId20,
 	SUDO_ORACLE: AccountId20,
 	SUDO_ORACLE_2: AccountId20,
+	SUDO_ORACLE_3: AccountId20,
+	SUDO_ORACLE_4: AccountId20,
 	NON_SUDO_ORACLE: AccountId20,
 	DUMMY_SUDO_ORACLE: AccountId20,
 	NON_SUDO_ORACLE_2: AccountId20,
 	NON_SUDO_ORACLE_6: AccountId20,
 	NON_SUDO_ORACLE_7: AccountId20,
-	NON_SUDO_ORACLE_8: AccountId20,
-	NON_SUDO_ORACLE_9: AccountId20,
-	NON_SUDO_ORACLE_10: AccountId20,
 }
 
 impl Default for Addr {
@@ -28,14 +27,13 @@ impl Default for Addr {
 			ALICE: AccountId20::from([4u8; 20]),
 			SUDO_ORACLE: AccountId20::from([0u8; 20]),
 			SUDO_ORACLE_2: AccountId20::from([1u8; 20]),
+			SUDO_ORACLE_3: AccountId20::from([11u8; 20]),
+			SUDO_ORACLE_4: AccountId20::from([12u8; 20]),
 			NON_SUDO_ORACLE: AccountId20::from([2u8; 20]),
 			DUMMY_SUDO_ORACLE: AccountId20::from([5u8; 20]),
 			NON_SUDO_ORACLE_2: AccountId20::from([3u8; 20]),
 			NON_SUDO_ORACLE_6: AccountId20::from([6u8; 20]),
 			NON_SUDO_ORACLE_7: AccountId20::from([7u8; 20]),
-			NON_SUDO_ORACLE_8: AccountId20::from([8u8; 20]),
-			NON_SUDO_ORACLE_9: AccountId20::from([9u8; 20]),
-			NON_SUDO_ORACLE_10: AccountId20::from([10u8; 20]),
 		}
 	}
 }
@@ -52,28 +50,14 @@ fn it_must_register_oracles_new() {
 			addr.NON_SUDO_ORACLE_6,
 			false
 		));
-		assert_ok!(Esg::register_an_oracle(
-			RuntimeOrigin::signed(addr.SUDO_ORACLE),
-			addr.NON_SUDO_ORACLE_8,
-			false
-		));
-		assert_ok!(Esg::register_an_oracle(
-			RuntimeOrigin::signed(addr.SUDO_ORACLE),
-			addr.NON_SUDO_ORACLE_9,
-			false
-		));
-		assert_ok!(Esg::register_an_oracle(
-			RuntimeOrigin::signed(addr.SUDO_ORACLE),
-			addr.NON_SUDO_ORACLE_10,
-			false
-		));
 		assert_ok!(Esg::register_an_oracle(
 			RuntimeOrigin::signed(addr.SUDO_ORACLE),
 			addr.NON_SUDO_ORACLE_7,
 			false
 		));
-
 		assert_ok!(Esg::deregister_an_oracle(RuntimeOrigin::root(), addr.NON_SUDO_ORACLE_7, false));
+
+
 	});
 }
 
@@ -663,3 +647,52 @@ fn it_must_handle_scores_exceeding_set_maximum() {
 		}));
 	});
 }
+
+
+#[test]
+fn it_must_fail_to_register_oracle_non_oracle_exceeding_maximum(){
+	let addr = Addr::default();
+
+	new_test_ext().execute_with(|| {
+		System::set_block_number(1);
+
+
+		// register sudo as an oracle
+		assert_ok!(Esg::register_an_oracle(RuntimeOrigin::root(), addr.SUDO_ORACLE, true));
+		let one_sudo = Esg::get_oracle_sudo();
+		assert_eq!(one_sudo, vec![addr.SUDO_ORACLE]);
+		assert_ok!(Esg::register_an_oracle(RuntimeOrigin::root(), addr.SUDO_ORACLE_2, true));
+		let two_sudoes = Esg::get_oracle_sudo();
+		assert_eq!(two_sudoes, vec![addr.SUDO_ORACLE, addr.SUDO_ORACLE_2]);
+
+		assert_ok!(Esg::register_an_oracle(RuntimeOrigin::root(), addr.SUDO_ORACLE_3, true));
+		let three_sudoes = Esg::get_oracle_sudo();
+		assert_eq!(three_sudoes, vec![addr.SUDO_ORACLE, addr.SUDO_ORACLE_2, addr.SUDO_ORACLE_3]);
+
+		// Return Error due to maximum oracle sudo 
+		assert_noop!(Esg::register_an_oracle(RuntimeOrigin::root(), addr.SUDO_ORACLE_4, true), Error::<Test>::MaxNumOfSudoOraclesReached);
+
+		// Deregister oracle sudo
+		assert_ok!(Esg::deregister_an_oracle(RuntimeOrigin::root(), addr.SUDO_ORACLE_3, true));
+		let sudoes_after_deregister = Esg::get_oracle_sudo();
+		assert_eq!(sudoes_after_deregister, vec![addr.SUDO_ORACLE, addr.SUDO_ORACLE_2]);	
+		// Add oracle sudo again 
+		assert_ok!(Esg::register_an_oracle(RuntimeOrigin::root(), addr.SUDO_ORACLE_4, true));	
+		let three_sudoes_after_registering = Esg::get_oracle_sudo();
+		assert_eq!(three_sudoes_after_registering, vec![addr.SUDO_ORACLE, addr.SUDO_ORACLE_2, addr.SUDO_ORACLE_4]);
+
+
+
+		// Register sudo as an non-sudo oracle 
+		assert_ok!(Esg::register_an_oracle(RuntimeOrigin::root(), addr.NON_SUDO_ORACLE, false));
+		let one_non_sudo = Esg::get_oracle_nsudo();
+		assert_eq!(one_non_sudo, vec![addr.NON_SUDO_ORACLE]);
+		assert_ok!(Esg::register_an_oracle(RuntimeOrigin::root(), addr.NON_SUDO_ORACLE_2, false));
+		let two_non_sudoes = Esg::get_oracle_nsudo();
+		assert_eq!(two_non_sudoes, vec![addr.NON_SUDO_ORACLE, addr.NON_SUDO_ORACLE_2]);
+
+		// Return Error due to maximum non oracle sudo 
+		assert_noop!(Esg::register_an_oracle(RuntimeOrigin::root(), addr.NON_SUDO_ORACLE_6, false), Error::<Test>::MaxNumOfNonSudoOraclesReached);
+
+	});
+}
diff --git a/frame/esg/src/weights.rs b/frame/esg/src/weights.rs
@@ -29,7 +29,7 @@
 #![allow(unused_imports)]
 #![allow(missing_docs)]
 
-use frame_support::{traits::Get, weights::Weight};
+use frame_support::{traits::Get, weights::{constants::RocksDbWeight, Weight}};
 use core::marker::PhantomData;
 
 pub trait WeightInfo {
@@ -82,3 +82,48 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeightInfo<T> {
 			.saturating_add(T::DbWeight::get().writes(2))
 	}
 }
+
+
+
+impl WeightInfo for () {
+	/// Storage: `EsgScore::SudoOraclesStore` (r:1 w:1)
+	/// Proof: `EsgScore::SudoOraclesStore` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
+	/// Storage: `EsgScore::NonSudoOraclesStore` (r:1 w:0)
+	/// Proof: `EsgScore::NonSudoOraclesStore` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
+	fn register_an_oracle() -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `2109`
+		//  Estimated: `3594`
+		// Minimum execution time: 17_000_000 picoseconds.
+		Weight::from_parts(18_000_000, 0)
+			.saturating_add(Weight::from_parts(0, 3594))
+			.saturating_add(RocksDbWeight::get().reads(2))
+			.saturating_add(RocksDbWeight::get().writes(1))
+	}
+	/// Storage: `EsgScore::SudoOraclesStore` (r:1 w:1)
+	/// Proof: `EsgScore::SudoOraclesStore` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
+	fn deregister_an_oracle() -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `238`
+		//  Estimated: `1723`
+		// Minimum execution time: 14_000_000 picoseconds.
+		Weight::from_parts(14_000_000, 0)
+			.saturating_add(Weight::from_parts(0, 1723))
+			.saturating_add(RocksDbWeight::get().reads(1))
+			.saturating_add(RocksDbWeight::get().writes(1))
+	}
+	/// Storage: `EsgScore::SudoOraclesStore` (r:1 w:0)
+	/// Proof: `EsgScore::SudoOraclesStore` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
+	/// Storage: `EsgScore::ESGScoresMap` (r:2 w:2)
+	/// Proof: `EsgScore::ESGScoresMap` (`max_values`: None, `max_size`: None, mode: `Measured`)
+	fn upsert_esg_scores() -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `238`
+		//  Estimated: `6178`
+		// Minimum execution time: 1_028_000_000 picoseconds.
+		Weight::from_parts(1_048_000_000, 0)
+			.saturating_add(Weight::from_parts(0, 6178))
+			.saturating_add(RocksDbWeight::get().reads(3))
+			.saturating_add(RocksDbWeight::get().writes(2))
+	}
+}
diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs
@@ -220,6 +220,8 @@ impl pallet_esg::Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type MaxFileSize = ConstU32<1024000>;
 	type WeightInfo = ();
+	type MaxNumOfSudoOracles =  ConstU32<5>;
+	type MaxNumOfNonSudoOracles = ConstU32<5>;
 }
 
 pub struct OnChainSeqPhragmen;

diff --git a/frame/im-online/src/mock.rs b/frame/im-online/src/mock.rs
@@ -243,6 +243,8 @@ impl pallet_esg::Config for Test {
 	type WeightInfo = ();
 	type MaxFileSize = ConstU32<102400>;
 	type RuntimeEvent = RuntimeEvent;
+	type MaxNumOfSudoOracles =  ConstU32<5>;
+	type MaxNumOfNonSudoOracles = ConstU32<5>;
 }
 
 type VoterBagsListInstance = pallet_bags_list::Instance1;

diff --git a/frame/reward/src/mock.rs b/frame/reward/src/mock.rs
@@ -277,6 +277,8 @@ impl pallet_esg::Config for Test {
 	type WeightInfo = ();
 	type RuntimeEvent = RuntimeEvent;
 	type MaxFileSize = ConstU32<1024000>;
+	type MaxNumOfSudoOracles =  ConstU32<5>;
+	type MaxNumOfNonSudoOracles = ConstU32<5>;
 }
 
 impl pallet_offences::Config for Test {

diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs
@@ -177,6 +177,8 @@ impl pallet_esg::Config for Test {
 	type WeightInfo = ();
 	type RuntimeEvent = RuntimeEvent;
 	type MaxFileSize = ConstU32<1024000>;
+	type MaxNumOfSudoOracles =  ConstU32<5>;
+	type MaxNumOfNonSudoOracles = ConstU32<5>;
 }
 
 pub type Extrinsic = sp_runtime::testing::TestXt<RuntimeCall, ()>;