registrar: Avoid freebies in provide_judgement (paritytech#12465)

* evaluate repatriate reserved error in pallet identity * fix benchmarks * add repatriate reserved error test * benchmark fix * undo lock * include balance to use for benchmarks * rename test * Update frame/identity/src/benchmarking.rs * Update frame/identity/src/benchmarking.rs Co-authored-by: Bastian Köcher <git@kchr.de>
gear-tech · Feb 27, 2023 · 293b504 · 293b504
1 parent 9bc7399
commit 293b504
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 5 deletions.
diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs
@@ -144,9 +144,14 @@ benchmarks! {
 
 			// User requests judgement from all the registrars, and they approve
 			for i in 0..r {
+				let registrar: T::AccountId = account("registrar", i, SEED);
+				let registrar_lookup = T::Lookup::unlookup(registrar.clone());
+				let balance_to_use =  T::Currency::minimum_balance() * 10u32.into();
+				let _ = T::Currency::make_free_balance_be(&registrar, balance_to_use);
+
 				Identity::<T>::request_judgement(caller_origin.clone(), i, 10u32.into())?;
 				Identity::<T>::provide_judgement(
-					RawOrigin::Signed(account("registrar", i, SEED)).into(),
+					RawOrigin::Signed(registrar).into(),
 					i,
 					caller_lookup.clone(),
 					Judgement::Reasonable,
@@ -213,9 +218,13 @@ benchmarks! {
 
 		// User requests judgement from all the registrars, and they approve
 		for i in 0..r {
+			let registrar: T::AccountId = account("registrar", i, SEED);
+			let balance_to_use =  T::Currency::minimum_balance() * 10u32.into();
+			let _ = T::Currency::make_free_balance_be(&registrar, balance_to_use);
+
 			Identity::<T>::request_judgement(caller_origin.clone(), i, 10u32.into())?;
 			Identity::<T>::provide_judgement(
-				RawOrigin::Signed(account("registrar", i, SEED)).into(),
+				RawOrigin::Signed(registrar).into(),
 				i,
 				caller_lookup.clone(),
 				Judgement::Reasonable,
@@ -362,9 +371,13 @@ benchmarks! {
 
 		// User requests judgement from all the registrars, and they approve
 		for i in 0..r {
+			let registrar: T::AccountId = account("registrar", i, SEED);
+			let balance_to_use =  T::Currency::minimum_balance() * 10u32.into();
+			let _ = T::Currency::make_free_balance_be(&registrar, balance_to_use);
+
 			Identity::<T>::request_judgement(target_origin.clone(), i, 10u32.into())?;
 			Identity::<T>::provide_judgement(
-				RawOrigin::Signed(account("registrar", i, SEED)).into(),
+				RawOrigin::Signed(registrar).into(),
 				i,
 				target_lookup.clone(),
 				Judgement::Reasonable,

diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs
@@ -238,6 +238,8 @@ pub mod pallet {
 		NotOwned,
 		/// The provided judgement was for a different identity.
 		JudgementForDifferentIdentity,
+		/// Error that occurs when there is an issue paying for judgement.
+		JudgementPaymentFailed,
 	}
 
 	#[pallet::event]
@@ -788,12 +790,13 @@ pub mod pallet {
 			match id.judgements.binary_search_by_key(&reg_index, |x| x.0) {
 				Ok(position) => {
 					if let Judgement::FeePaid(fee) = id.judgements[position].1 {
-						let _ = T::Currency::repatriate_reserved(
+						T::Currency::repatriate_reserved(
 							&target,
 							&sender,
 							fee,
 							BalanceStatus::Free,
-						);
+						)
+						.map_err(|_| Error::<T>::JudgementPaymentFailed)?;
 					}
 					id.judgements[position] = item
 				},

diff --git a/frame/identity/src/tests.rs b/frame/identity/src/tests.rs
@@ -540,6 +540,31 @@ fn requesting_judgement_should_work() {
 	});
 }
 
+#[test]
+fn provide_judgement_should_return_judgement_payment_failed_error() {
+	new_test_ext().execute_with(|| {
+		assert_ok!(Identity::add_registrar(RuntimeOrigin::signed(1), 3));
+		assert_ok!(Identity::set_fee(RuntimeOrigin::signed(3), 0, 10));
+		assert_ok!(Identity::set_identity(RuntimeOrigin::signed(10), Box::new(ten())));
+		assert_ok!(Identity::request_judgement(RuntimeOrigin::signed(10), 0, 10));
+		// 10 for the judgement request, 10 for the identity.
+		assert_eq!(Balances::free_balance(10), 80);
+
+		// This forces judgement payment failed error
+		Balances::make_free_balance_be(&3, 0);
+		assert_noop!(
+			Identity::provide_judgement(
+				RuntimeOrigin::signed(3),
+				0,
+				10,
+				Judgement::Erroneous,
+				BlakeTwo256::hash_of(&ten())
+			),
+			Error::<Test>::JudgementPaymentFailed
+		);
+	});
+}
+
 #[test]
 fn field_deposit_should_work() {
 	new_test_ext().execute_with(|| {