From 293b504fb4a0c3ed310b8f6e1de116b8a9cecd9b Mon Sep 17 00:00:00 2001
From: dharjeezy <dharjeezy@gmail.com>
Date: Wed, 19 Oct 2022 14:29:20 +0100
Subject: [PATCH] registrar: Avoid freebies in provide_judgement (#12465)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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>
---
 frame/identity/src/benchmarking.rs | 19 ++++++++++++++++---
 frame/identity/src/lib.rs          |  7 +++++--
 frame/identity/src/tests.rs        | 25 +++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs
index c628387a4d22e..3584eb954b399 100644
--- 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
index 8b22ecedaec19..95f5a84d8abb7 100644
--- 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
index 20628da50937a..6b0006971f0e6 100644
--- 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(|| {