Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

registrar: Avoid freebies in provide_judgement #12465

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions frame/identity/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn add_registrars<T: Config>(r: u32) -> Result<(), &'static str> {
let _ = T::Currency::make_free_balance_be(&registrar, BalanceOf::<T>::max_value());
let registrar_origin = T::RegistrarOrigin::successful_origin();
Identity::<T>::add_registrar(registrar_origin, registrar_lookup)?;
Identity::<T>::set_fee(RawOrigin::Signed(registrar.clone()).into(), i, 10u32.into())?;
Identity::<T>::set_fee(RawOrigin::Signed(registrar.clone()).into(), i, 0u32.into())?;
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved
let fields =
IdentityFields(
IdentityField::Display |
Expand Down Expand Up @@ -144,9 +144,13 @@ benchmarks! {

// User requests judgement from all the registrars, and they approve
for i in 0..r {
Identity::<T>::request_judgement(caller_origin.clone(), i, 10u32.into())?;
let registrar: T::AccountId = account("registrar", i, SEED);
let registrar_lookup = T::Lookup::unlookup(registrar.clone());
let _ = T::Currency::make_free_balance_be(&registrar, 10u32.into());

Identity::<T>::request_judgement(caller_origin.clone(), i, 0u32.into())?;
Identity::<T>::provide_judgement(
RawOrigin::Signed(account("registrar", i, SEED)).into(),
RawOrigin::Signed(registrar).into(),
i,
caller_lookup.clone(),
Judgement::Reasonable,
Expand Down Expand Up @@ -213,9 +217,13 @@ benchmarks! {

// User requests judgement from all the registrars, and they approve
for i in 0..r {
Identity::<T>::request_judgement(caller_origin.clone(), i, 10u32.into())?;
let registrar: T::AccountId = account("registrar", i, SEED);
let registrar_lookup = T::Lookup::unlookup(registrar.clone());
bkchr marked this conversation as resolved.
Show resolved Hide resolved
let _ = T::Currency::make_free_balance_be(&registrar, 10u32.into());

Identity::<T>::request_judgement(caller_origin.clone(), i, 0u32.into())?;
Identity::<T>::provide_judgement(
RawOrigin::Signed(account("registrar", i, SEED)).into(),
RawOrigin::Signed(registrar).into(),
i,
caller_lookup.clone(),
Judgement::Reasonable,
Expand All @@ -240,7 +248,7 @@ benchmarks! {
let caller_origin = <T as frame_system::Config>::RuntimeOrigin::from(RawOrigin::Signed(caller));
Identity::<T>::set_identity(caller_origin, Box::new(info))?;
};
}: _(RawOrigin::Signed(caller.clone()), r - 1, 10u32.into())
}: _(RawOrigin::Signed(caller.clone()), r - 1, 0u32.into())
verify {
assert_last_event::<T>(Event::<T>::JudgementRequested { who: caller, registrar_index: r-1 }.into());
}
Expand All @@ -259,7 +267,7 @@ benchmarks! {
Identity::<T>::set_identity(caller_origin, Box::new(info))?;
};

Identity::<T>::request_judgement(caller_origin, r - 1, 10u32.into())?;
Identity::<T>::request_judgement(caller_origin, r - 1, 0u32.into())?;
}: _(RawOrigin::Signed(caller.clone()), r - 1)
verify {
assert_last_event::<T>(Event::<T>::JudgementUnrequested { who: caller, registrar_index: r-1 }.into());
Expand Down Expand Up @@ -340,7 +348,7 @@ benchmarks! {

let registrar_origin = T::RegistrarOrigin::successful_origin();
Identity::<T>::add_registrar(registrar_origin, caller_lookup)?;
Identity::<T>::request_judgement(user_origin, r, 10u32.into())?;
Identity::<T>::request_judgement(user_origin, r, 0u32.into())?;
}: _(RawOrigin::Signed(caller), r, user_lookup, Judgement::Reasonable, info_hash)
verify {
assert_last_event::<T>(Event::<T>::JudgementGiven { target: user, registrar_index: r }.into())
Expand All @@ -362,9 +370,13 @@ benchmarks! {

// User requests judgement from all the registrars, and they approve
for i in 0..r {
Identity::<T>::request_judgement(target_origin.clone(), i, 10u32.into())?;
let registrar: T::AccountId = account("registrar", i, SEED);
let registrar_lookup = T::Lookup::unlookup(registrar.clone());
bkchr marked this conversation as resolved.
Show resolved Hide resolved
let _ = T::Currency::make_free_balance_be(&registrar, 10u32.into());

Identity::<T>::request_judgement(target_origin.clone(), i, 0u32.into())?;
Identity::<T>::provide_judgement(
RawOrigin::Signed(account("registrar", i, SEED)).into(),
RawOrigin::Signed(registrar).into(),
i,
target_lookup.clone(),
Judgement::Reasonable,
Expand Down
7 changes: 5 additions & 2 deletions frame/identity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ pub mod pallet {
NotOwned,
/// The provided judgement was for a different identity.
JudgementForDifferentIdentity,
/// Error thrown when repatriate_reserved returns an error.
RepatriateReservedError,
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved
}

#[pallet::event]
Expand Down Expand Up @@ -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>::RepatriateReservedError)?;
}
id.judgements[position] = item
},
Expand Down
25 changes: 25 additions & 0 deletions frame/identity/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,31 @@ fn requesting_judgement_should_work() {
});
}

#[test]
fn provide_judgement_should_return_repatriate_reserved_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 repatriate reserved 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>::RepatriateReservedError
);
});
}

#[test]
fn field_deposit_should_work() {
new_test_ext().execute_with(|| {
Expand Down