Calculate and refund weight for identity pallet

bin/node/runtime/src/lib.rs

@@ -599,6 +599,7 @@ parameter_types! {
 	pub const SubAccountDeposit: Balance = 2 * DOLLARS;   // 53 bytes on-chain
 	pub const MaxSubAccounts: u32 = 100;
 	pub const MaxAdditionalFields: u32 = 100;
+	pub const MaxRegistrars: u32 = 20;
 }
 
 impl pallet_identity::Trait for Runtime {
@@ -609,6 +610,7 @@ impl pallet_identity::Trait for Runtime {
 	type SubAccountDeposit = SubAccountDeposit;
 	type MaxSubAccounts = MaxSubAccounts;
 	type MaxAdditionalFields = MaxAdditionalFields;
+	type MaxRegistrars = MaxRegistrars;
 	type Slashed = Treasury;
 	type ForceOrigin = pallet_collective::EnsureProportionMoreThan<_1, _2, AccountId, CouncilCollective>;
 	type RegistrarOrigin = pallet_collective::EnsureProportionMoreThan<_1, _2, AccountId, CouncilCollective>;

frame/balances/src/lib.rs

@@ -1099,7 +1099,7 @@ impl<T: Trait<I>, I: Instance> Currency<T::AccountId> for Module<T, I> where
 			// equal and opposite cause (returned as an Imbalance), then in the
 			// instance that there's no other accounts on the system at all, we might
 			// underflow the issuance and our arithmetic will be off.
-			ensure!(value + account.reserved >= ed || !account.total().is_zero(), ());
+			ensure!(value.saturating_add(account.reserved) >= ed || !account.total().is_zero(), ());
 
 			let imbalance = if account.free <= value {
 				SignedImbalance::Positive(PositiveImbalance::new(value - account.free))

frame/benchmarking/src/lib.rs

@@ -69,6 +69,9 @@ pub use paste;
 /// (or not) by each arm. Syntax is available to allow for only the range to be drawn upon if
 /// desired, allowing an alternative instancing expression to be given.
 ///
+/// Note that the ranges are *inclusive* on both sides. This is in contrast to ranges in Rust which
+/// are left-inclusive right-exclusive.
+///
 /// Each arm may also have a block of code which is run prior to any instancing and a block of code
 /// which is run afterwards. All code blocks may draw upon the specific value of each parameter
 /// at any time. Local variables are shared between the two pre- and post- code blocks, but do not
@@ -81,6 +84,7 @@ pub use paste;
 /// ```ignore
 /// benchmarks! {
 ///   // common parameter; just one for this example.
+///   // will be `1`, `MAX_LENGTH` or any value inbetween
 ///   _ {
 ///     let l in 1 .. MAX_LENGTH => initialize_l(l);
 ///   }

frame/identity/src/benchmarking.rs

@@ -27,9 +27,6 @@ use sp_runtime::traits::Bounded;
 
 use crate::Module as Identity;
 
-// The maximum number of identity registrars we will test.
-const MAX_REGISTRARS: u32 = 50;
-
 // Support Functions
 fn account<T: Trait>(name: &'static str, index: u32) -> T::AccountId {
 	let entropy = (name, index).using_encoded(blake2_256);
@@ -53,9 +50,9 @@ fn add_registrars<T: Trait>(r: u32) -> Result<(), &'static str> {
 	Ok(())
 }
 
-// Adds `s` sub-accounts to the identity of `who`. Each wil have 32 bytes of raw data added to it.
-// This additionally returns the vector of sub-accounts to it can be modified if needed.
-fn add_sub_accounts<T: Trait>(who: &T::AccountId, s: u32) -> Result<Vec<(T::AccountId, Data)>, &'static str> {
+// Create `s` sub-accounts for the identity of `who` and return them.
+// Each will have 32 bytes of raw data added to it.
+fn create_sub_accounts<T: Trait>(who: &T::AccountId, s: u32) -> Result<Vec<(T::AccountId, Data)>, &'static str> {
 	let mut subs = Vec::new();
 	let who_origin = RawOrigin::Signed(who.clone());
 	let data = Data::Raw(vec![0; 32]);
@@ -70,9 +67,18 @@ fn add_sub_accounts<T: Trait>(who: &T::AccountId, s: u32) -> Result<Vec<(T::Acco
 	let info = create_identity_info::<T>(1);
 	Identity::<T>::set_identity(who_origin.clone().into(), info)?;
 
+	Ok(subs)
+}
+
+// Adds `s` sub-accounts to the identity of `who`. Each will have 32 bytes of raw data added to it.
+// This additionally returns the vector of sub-accounts so it can be modified if needed.
+fn add_sub_accounts<T: Trait>(who: &T::AccountId, s: u32) -> Result<Vec<(T::AccountId, Data)>, &'static str> {
+	let who_origin = RawOrigin::Signed(who.clone());
+	let subs = create_sub_accounts::<T>(who, s)?;
+
 	Identity::<T>::set_subs(who_origin.into(), subs.clone())?;
 
-	return Ok(subs)
+	Ok(subs)
 }
 
 // This creates an `IdentityInfo` object with `num_fields` extra fields.
@@ -98,7 +104,9 @@ fn create_identity_info<T: Trait>(num_fields: u32) -> IdentityInfo {
 benchmarks! {
 	// These are the common parameters along with their instancing.
 	_ {
-		let r in 1 .. MAX_REGISTRARS => add_registrars::<T>(r)?;
+		let r in 1 .. T::MaxRegistrars::get() => add_registrars::<T>(r)?;
+		// extra parameter for the set_subs bench for previous sub accounts
+		let p in 1 .. T::MaxSubAccounts::get() => ();
 		let s in 1 .. T::MaxSubAccounts::get() => {
 			// Give them s many sub accounts
 			let caller = account::<T>("caller", 0);
@@ -114,7 +122,7 @@ benchmarks! {
 	}
 
 	add_registrar {
-		let r in ...;
+		let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
 	}: _(RawOrigin::Root, account::<T>("registrar", r + 1))
 
 	set_identity {
@@ -153,16 +161,13 @@ benchmarks! {
 	set_subs {
 		let caller = account::<T>("caller", 0);
 
-		// Give them s many sub accounts.
-		let s in 1 .. T::MaxSubAccounts::get() - 1 => {
-			let _ = add_sub_accounts::<T>(&caller, s)?;
+		// Give them p many previous sub accounts.
+		let p in 1 .. T::MaxSubAccounts::get() => {
+			let _ = add_sub_accounts::<T>(&caller, p)?;
 		};
-
-		let mut subs = Module::<T>::subs(&caller);
-
-		// Create an s + 1 sub account.
-		let data = Data::Raw(vec![0; 32]);
-		subs.push((account::<T>("sub", s + 1), data));
+		// Create a new subs vec with s sub accounts
+		let s in 1 .. T::MaxSubAccounts::get() => ();
+		let subs = create_sub_accounts::<T>(&caller, s)?;
 
 	}: _(RawOrigin::Signed(caller), subs)
 
@@ -210,7 +215,7 @@ benchmarks! {
 	set_fee {
 		let caller = account::<T>("caller", 0);
 
-		let r in ...;
+		let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
 
 		Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
 	}: _(RawOrigin::Signed(caller), r, 10.into())
@@ -219,7 +224,7 @@ benchmarks! {
 		let caller = account::<T>("caller", 0);
 		let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
 
-		let r in ...;
+		let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
 
 		Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
 	}: _(RawOrigin::Signed(caller), r, account::<T>("new", 0))
@@ -228,7 +233,7 @@ benchmarks! {
 		let caller = account::<T>("caller", 0);
 		let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
 
-		let r in ...;
+		let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
 
 		Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
 		let fields = IdentityFields(
@@ -247,7 +252,7 @@ benchmarks! {
 		let caller = account::<T>("caller", 0);
 		let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
 
-		let r in ...;
+		let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
 		// For this x, it's the user identity that gts the fields, not the caller.
 		let x in _ .. _ => {
 			let info = create_identity_info::<T>(x);