Skip to content

Commit 293b504

Browse files
dharjeezybkchr
authored andcommitted
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 <[email protected]>
1 parent 9bc7399 commit 293b504

File tree

3 files changed

+46
-5
lines changed

3 files changed

+46
-5
lines changed

frame/identity/src/benchmarking.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,14 @@ benchmarks! {
144144

145145
// User requests judgement from all the registrars, and they approve
146146
for i in 0..r {
147+
let registrar: T::AccountId = account("registrar", i, SEED);
148+
let registrar_lookup = T::Lookup::unlookup(registrar.clone());
149+
let balance_to_use = T::Currency::minimum_balance() * 10u32.into();
150+
let _ = T::Currency::make_free_balance_be(&registrar, balance_to_use);
151+
147152
Identity::<T>::request_judgement(caller_origin.clone(), i, 10u32.into())?;
148153
Identity::<T>::provide_judgement(
149-
RawOrigin::Signed(account("registrar", i, SEED)).into(),
154+
RawOrigin::Signed(registrar).into(),
150155
i,
151156
caller_lookup.clone(),
152157
Judgement::Reasonable,
@@ -213,9 +218,13 @@ benchmarks! {
213218

214219
// User requests judgement from all the registrars, and they approve
215220
for i in 0..r {
221+
let registrar: T::AccountId = account("registrar", i, SEED);
222+
let balance_to_use = T::Currency::minimum_balance() * 10u32.into();
223+
let _ = T::Currency::make_free_balance_be(&registrar, balance_to_use);
224+
216225
Identity::<T>::request_judgement(caller_origin.clone(), i, 10u32.into())?;
217226
Identity::<T>::provide_judgement(
218-
RawOrigin::Signed(account("registrar", i, SEED)).into(),
227+
RawOrigin::Signed(registrar).into(),
219228
i,
220229
caller_lookup.clone(),
221230
Judgement::Reasonable,
@@ -362,9 +371,13 @@ benchmarks! {
362371

363372
// User requests judgement from all the registrars, and they approve
364373
for i in 0..r {
374+
let registrar: T::AccountId = account("registrar", i, SEED);
375+
let balance_to_use = T::Currency::minimum_balance() * 10u32.into();
376+
let _ = T::Currency::make_free_balance_be(&registrar, balance_to_use);
377+
365378
Identity::<T>::request_judgement(target_origin.clone(), i, 10u32.into())?;
366379
Identity::<T>::provide_judgement(
367-
RawOrigin::Signed(account("registrar", i, SEED)).into(),
380+
RawOrigin::Signed(registrar).into(),
368381
i,
369382
target_lookup.clone(),
370383
Judgement::Reasonable,

frame/identity/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,8 @@ pub mod pallet {
238238
NotOwned,
239239
/// The provided judgement was for a different identity.
240240
JudgementForDifferentIdentity,
241+
/// Error that occurs when there is an issue paying for judgement.
242+
JudgementPaymentFailed,
241243
}
242244

243245
#[pallet::event]
@@ -788,12 +790,13 @@ pub mod pallet {
788790
match id.judgements.binary_search_by_key(&reg_index, |x| x.0) {
789791
Ok(position) => {
790792
if let Judgement::FeePaid(fee) = id.judgements[position].1 {
791-
let _ = T::Currency::repatriate_reserved(
793+
T::Currency::repatriate_reserved(
792794
&target,
793795
&sender,
794796
fee,
795797
BalanceStatus::Free,
796-
);
798+
)
799+
.map_err(|_| Error::<T>::JudgementPaymentFailed)?;
797800
}
798801
id.judgements[position] = item
799802
},

frame/identity/src/tests.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,31 @@ fn requesting_judgement_should_work() {
540540
});
541541
}
542542

543+
#[test]
544+
fn provide_judgement_should_return_judgement_payment_failed_error() {
545+
new_test_ext().execute_with(|| {
546+
assert_ok!(Identity::add_registrar(RuntimeOrigin::signed(1), 3));
547+
assert_ok!(Identity::set_fee(RuntimeOrigin::signed(3), 0, 10));
548+
assert_ok!(Identity::set_identity(RuntimeOrigin::signed(10), Box::new(ten())));
549+
assert_ok!(Identity::request_judgement(RuntimeOrigin::signed(10), 0, 10));
550+
// 10 for the judgement request, 10 for the identity.
551+
assert_eq!(Balances::free_balance(10), 80);
552+
553+
// This forces judgement payment failed error
554+
Balances::make_free_balance_be(&3, 0);
555+
assert_noop!(
556+
Identity::provide_judgement(
557+
RuntimeOrigin::signed(3),
558+
0,
559+
10,
560+
Judgement::Erroneous,
561+
BlakeTwo256::hash_of(&ten())
562+
),
563+
Error::<Test>::JudgementPaymentFailed
564+
);
565+
});
566+
}
567+
543568
#[test]
544569
fn field_deposit_should_work() {
545570
new_test_ext().execute_with(|| {

0 commit comments

Comments
 (0)