Permissions benchmarks cleanup (#1193)

* Some comments and cleanup.

* Add docs to helper functions.

* Add more comments to benchmark.

Author: Neopallium

pallets/common/src/traits/identity.rs

@@ -102,13 +102,13 @@ pub trait WeightInfo {
     fn add_investor_uniqueness_claim_v2() -> Weight;
     fn revoke_claim_by_index() -> Weight;
 
-    // Helpers for extrinsics with Permissions.
+    /// Add complexity cost of Permissions to `add_secondary_keys_with_authorization` extrinsic.
     fn add_secondary_keys_full<AccountId>(
         additional_keys: &[SecondaryKeyWithAuth<AccountId>],
     ) -> Weight {
         let perm_cost = additional_keys.iter().fold(0u64, |cost, key_with_auth| {
             let (assets, portfolios, pallets, extrinsics) =
-                key_with_auth.secondary_key.permissions.get_weights();
+                key_with_auth.secondary_key.permissions.counts();
             let perm_cost = Self::permissions_cost(assets, portfolios, pallets, extrinsics);
             cost.saturating_add(perm_cost)
         });
@@ -117,10 +117,11 @@ pub trait WeightInfo {
         ))
     }
 
+    /// Add complexity cost of Permissions to `add_authorization` extrinsic.
     fn add_authorization_full<AccountId>(data: &AuthorizationData<AccountId>) -> Weight {
         let perm_cost = match data {
             AuthorizationData::JoinIdentity(perms) => {
-                let (assets, portfolios, pallets, extrinsics) = perms.get_weights();
+                let (assets, portfolios, pallets, extrinsics) = perms.counts();
                 Self::permissions_cost(assets, portfolios, pallets, extrinsics)
             }
             _ => 0,
@@ -129,13 +130,16 @@ pub trait WeightInfo {
         perm_cost.saturating_add(Self::add_authorization())
     }
 
+    /// Add complexity cost of Permissions to `set_permission_to_signer` extrinsic.
     fn set_permission_to_signer_full(perms: &Permissions) -> Weight {
-        let (assets, portfolios, pallets, extrinsics) = perms.get_weights();
+        let (assets, portfolios, pallets, extrinsics) = perms.counts();
         Self::permissions_cost(assets, portfolios, pallets, extrinsics)
             .saturating_add(Self::set_permission_to_signer())
     }
+
+    /// Add complexity cost of Permissions to `legacy_set_permission_to_signer` extrinsic.
     fn legacy_set_permission_to_signer_full(perms: &LegacyPermissions) -> Weight {
-        let (assets, portfolios, pallets, extrinsics) = perms.get_weights();
+        let (assets, portfolios, pallets, extrinsics) = perms.counts();
         Self::permissions_cost(assets, portfolios, pallets, extrinsics)
             .saturating_add(Self::set_permission_to_signer())
     }

pallets/identity/src/benchmarking.rs

@@ -305,12 +305,22 @@ benchmarks! {
         Module::<T>::unsafe_join_identity(target.did(), Permissions::empty(), key.account());
     }: _(target.origin, signatory, Permissions::default().into())
 
+    // Benchmark the memory/cpu complexity of Permissions.
     permissions_cost {
         // Number of assets/portfolios/pallets/extrinsics.
         let a in 0 .. MAX_ASSETS; // a=(A)ssets
         let p in 0 .. MAX_PORTFOLIOS; // p=(P)ortfolios
         let l in 0 .. MAX_PALLETS; // l=pa(L)lets
         let e in 0 .. MAX_EXTRINSICS; // e=(E)xtrinsics
+        // When the benchmarks run for parameter `e` (number of extrinsics)
+        // it will use `l == MAX_PALLETS`.  `e` will be the number of
+        // extrinsics per pallet.  So the total number of extrinsics in
+        // the `Permissions` will be `MAX_PALLETS * e`.
+        //
+        // When calculating the weight of a `Permissions` value in a
+        // transaction, we use the total number of extrinsics in the
+        // permissions.  This is to make sure that the worst-case cost
+        // is covered.
 
         let asset = AssetPermissions::elems(
             (0..a as u64).map(Ticker::generate_into)
@@ -339,14 +349,19 @@ benchmarks! {
             extrinsic,
             portfolio
         };
-
-        let orig = permissions.clone();
     }: {
-        let enc = permissions.encode();
-        // Panic the benchmarks if decoding fails.
-        let perm = Permissions::decode(&mut enc.as_slice())
-            .expect("Permissions should decode correctly.");
-        assert_eq!(orig, perm);
+        // For this benchmark we need to do some "work" based on
+        // how complex the permissions object is.
+
+        // 1. Encode the Permissions value.
+        let encoded = permissions.encode();
+        // 2. Decode the Permissions value.
+        let decoded = Permissions::decode(&mut encoded.as_slice())
+            .expect("This shouldn't fail since we just encoded a Permissions value.");
+        // 3. Compare the original and decoded values.  This will touch the full value.
+        if !permissions.eq(&decoded) {
+            panic!("This shouldn't fail.");
+        }
     }
 
     freeze_secondary_keys {

primitives/src/secondary_key.rs

@@ -190,7 +190,7 @@ impl Permissions {
     /// Return number of assets, portfolios, pallets, and extrinsics.
     ///
     /// This is used for weight calculation.
-    pub fn get_weights(&self) -> (u32, u32, u32, u32) {
+    pub fn counts(&self) -> (u32, u32, u32, u32) {
         // Count the number of assets.
         let assets = self.asset.complexity().try_into().unwrap_or(u32::MAX);
         // Count the number of portfolios.
@@ -452,7 +452,7 @@ pub mod api {
         /// Return number of assets, portfolios, pallets, and extrinsics.
         ///
         /// This is used for weight calculation.
-        pub fn get_weights(&self) -> (u32, u32, u32, u32) {
+        pub fn counts(&self) -> (u32, u32, u32, u32) {
             // Count the number of assets.
             let assets = self.asset.complexity().try_into().unwrap_or(u32::MAX);
             // Count the number of portfolios.