MESH-1773 & MESH-1777 condition cost function (#1204)

Neopallium · adamdossa · commit 057d981d9aed · 2022-02-14T12:13:50.000Z
* Improve ComplianceManager benchmarks.

* Prevent a large number of empty asset compliance requirements from being added to an asset.

* Code cleanup.
diff --git a/pallets/common/src/traits/compliance_manager.rs b/pallets/common/src/traits/compliance_manager.rs
@@ -15,7 +15,11 @@
 
 use core::result::Result;
 use frame_support::{dispatch::DispatchError, weights::Weight};
-use polymesh_primitives::{compliance_manager::AssetComplianceResult, Balance, IdentityId, Ticker};
+use polymesh_primitives::{
+    compliance_manager::{AssetComplianceResult, ComplianceRequirement},
+    condition::{conditions_total_counts, Condition},
+    Balance, IdentityId, Ticker,
+};
 
 pub trait Config {
     fn verify_restriction(
@@ -42,4 +46,33 @@ pub trait WeightInfo {
     fn change_compliance_requirement(s: u32, r: u32) -> Weight;
     fn replace_asset_compliance(c: u32) -> Weight;
     fn reset_asset_compliance() -> Weight;
+
+    fn condition_costs(conditions: u32, claims: u32, issuers: u32, claim_types: u32) -> Weight;
+
+    fn add_compliance_requirement_full(sender: &[Condition], receiver: &[Condition]) -> Weight {
+        let (_, claims, issuers, claim_types) =
+            conditions_total_counts(sender.iter().chain(receiver.iter()));
+        Self::add_compliance_requirement(sender.len() as u32, receiver.len() as u32)
+            .saturating_add(Self::condition_costs(0, claims, issuers, claim_types))
+    }
+
+    fn change_compliance_requirement_full(req: &ComplianceRequirement) -> Weight {
+        let (_, claims, issuers, claim_types) = req.counts();
+        Self::change_compliance_requirement(
+            req.sender_conditions.len() as u32,
+            req.receiver_conditions.len() as u32,
+        )
+        .saturating_add(Self::condition_costs(0, claims, issuers, claim_types))
+    }
+
+    fn replace_asset_compliance_full(reqs: &[ComplianceRequirement]) -> Weight {
+        let (conditions, claims, issuers, claim_types) =
+            conditions_total_counts(reqs.iter().flat_map(|req| req.conditions()));
+        Self::replace_asset_compliance(reqs.len() as u32).saturating_add(Self::condition_costs(
+            conditions,
+            claims,
+            issuers,
+            claim_types,
+        ))
+    }
 }
diff --git a/pallets/compliance-manager/src/benchmarking.rs b/pallets/compliance-manager/src/benchmarking.rs
@@ -21,23 +21,55 @@ use polymesh_common_utilities::{
     benchs::{AccountIdOf, User, UserBuilder},
     TestUtilsFn,
 };
-use polymesh_primitives::{asset::AssetType, TrustedFor, TrustedIssuer};
+use polymesh_primitives::{asset::AssetType, ClaimType, Scope, TrustedFor, TrustedIssuer};
+use sp_std::convert::TryFrom;
 
 const MAX_DEFAULT_TRUSTED_CLAIM_ISSUERS: u32 = 3;
 const MAX_TRUSTED_ISSUER_PER_CONDITION: u32 = 3;
 const MAX_SENDER_CONDITIONS_PER_COMPLIANCE: u32 = 3;
 const MAX_RECEIVER_CONDITIONS_PER_COMPLIANCE: u32 = 3;
 const MAX_COMPLIANCE_REQUIREMENTS: u32 = 2;
 
+const MAX_CONDITIONS: u32 = 10;
+const MAX_CONDITION_TYPE_CLAIMS: u32 = 10;
+const MAX_CONDITION_ISSUERS: u32 = 10;
+const MAX_CONDITION_ISSUER_CLAIM_TYPES: u32 = 10;
+
+const CLAIM_TYPES: &[ClaimType] = &[
+    ClaimType::Accredited,
+    ClaimType::Affiliate,
+    ClaimType::BuyLockup,
+    ClaimType::SellLockup,
+    ClaimType::CustomerDueDiligence,
+    ClaimType::KnowYourCustomer,
+    ClaimType::Jurisdiction,
+    ClaimType::Exempted,
+    ClaimType::Blocked,
+    ClaimType::InvestorUniqueness,
+    ClaimType::NoType,
+    ClaimType::InvestorUniquenessV2,
+];
+
 /// Create a token issuer trusted for `Any`.
-pub fn make_issuer<T: IdentityConfig + TestUtilsFn<AccountIdOf<T>>>(id: u32) -> TrustedIssuer {
+pub fn make_issuer<T: IdentityConfig + TestUtilsFn<AccountIdOf<T>>>(
+    id: u32,
+    claim_type_len: Option<usize>,
+) -> TrustedIssuer {
     let u = UserBuilder::<T>::default()
         .generate_did()
         .seed(id)
         .build("ISSUER");
     TrustedIssuer {
         issuer: IdentityId::from(u.did.unwrap()),
-        trusted_for: TrustedFor::Any,
+        trusted_for: match claim_type_len {
+            None => TrustedFor::Any,
+            Some(len) => TrustedFor::Specific(
+                (0..len)
+                    .into_iter()
+                    .map(|idx| CLAIM_TYPES[idx % CLAIM_TYPES.len()])
+                    .collect(),
+            ),
+        },
     }
 }
 
@@ -46,16 +78,29 @@ pub fn make_issuer<T: IdentityConfig + TestUtilsFn<AccountIdOf<T>>>(id: u32) ->
 ///   - It could have more complexity if `TrustedIssuer::trusted_for` is a vector but not on
 ///   benchmarking of add/remove. That could be useful for benchmarking executions/evaluation of
 ///   complience requiriments.
-pub fn make_issuers<T: IdentityConfig + TestUtilsFn<AccountIdOf<T>>>(s: u32) -> Vec<TrustedIssuer> {
-    (0..s).map(|i| make_issuer::<T>(i)).collect::<Vec<_>>()
+pub fn make_issuers<T: IdentityConfig + TestUtilsFn<AccountIdOf<T>>>(
+    s: u32,
+    claim_type_len: Option<usize>,
+) -> Vec<TrustedIssuer> {
+    (0..s)
+        .map(|i| make_issuer::<T>(i, claim_type_len))
+        .collect()
 }
 
 /// Create simple conditions with a variable number of `issuers`.
-pub fn make_conditions(s: u32, issuers: &Vec<TrustedIssuer>) -> Vec<Condition> {
+pub fn make_conditions(s: u32, claims: Option<usize>, issuers: &[TrustedIssuer]) -> Vec<Condition> {
     (0..s)
         .map(|_| Condition {
-            condition_type: ConditionType::IsPresent(Claim::NoData),
-            issuers: issuers.clone(),
+            condition_type: match claims {
+                None => ConditionType::IsPresent(Claim::NoData),
+                Some(len) => ConditionType::IsAnyOf(
+                    (0..len)
+                        .into_iter()
+                        .map(|_| Claim::Blocked(Scope::Custom(vec![0])))
+                        .collect(),
+                ),
+            },
+            issuers: issuers.to_vec(),
         })
         .collect()
 }
@@ -103,7 +148,7 @@ struct ComplianceRequirementInfo<T: Config> {
 
 impl<T: Config + TestUtilsFn<AccountIdOf<T>>> ComplianceRequirementInfo<T> {
     pub fn add_default_trusted_claim_issuer(self: &Self, i: u32) {
-        make_issuers::<T>(i).into_iter().for_each(|issuer| {
+        make_issuers::<T>(i, None).into_iter().for_each(|issuer| {
             Module::<T>::add_default_trusted_claim_issuer(
                 self.owner.origin.clone().into(),
                 self.ticker.clone(),
@@ -130,9 +175,9 @@ impl<T: Config + TestUtilsFn<AccountIdOf<T>>> ComplianceRequirementBuilder<T> {
         let ticker = make_token::<T>(&owner, b"1".to_vec());
 
         // Create issuers (i) and conditions(s & r).
-        let issuers = make_issuers::<T>(trusted_issuer_count);
-        let sender_conditions = make_conditions(sender_conditions_count, &issuers);
-        let receiver_conditions = make_conditions(receiver_conditions_count, &issuers);
+        let issuers = make_issuers::<T>(trusted_issuer_count, None);
+        let sender_conditions = make_conditions(sender_conditions_count, None, &issuers);
+        let receiver_conditions = make_conditions(receiver_conditions_count, None, &issuers);
 
         let info = ComplianceRequirementInfo {
             owner,
@@ -168,9 +213,40 @@ impl<T: Config> ComplianceRequirementBuilder<T> {
     }
 }
 
+fn setup_conditions_bench<T: Config + TestUtilsFn<AccountIdOf<T>>>(
+    conditions: u32,
+    claims: u32,
+    issuers: u32,
+    claim_types: u32,
+) -> Vec<Condition> {
+    let issuers = make_issuers::<T>(issuers, Some(claim_types as usize));
+    let conditions = make_conditions(conditions, Some(claims as usize), &issuers);
+    conditions
+}
+
+fn conditions_bench(conditions: Vec<Condition>) {
+    let encoded = conditions.encode();
+    let decoded = Vec::<Condition>::decode(&mut encoded.as_slice())
+        .expect("This shouldn't fail since we just encoded a `Vec<Condition>` value.");
+    if !conditions.eq(&decoded) {
+        panic!("This shouldn't fail.");
+    }
+}
+
 benchmarks! {
     where_clause { where T: TestUtilsFn<AccountIdOf<T>> }
 
+    condition_costs {
+        let a in 1..MAX_CONDITIONS;
+        let b in 1..MAX_CONDITION_TYPE_CLAIMS;
+        let c in 1..MAX_CONDITION_ISSUERS;
+        let d in 1..MAX_CONDITION_ISSUER_CLAIM_TYPES;
+
+        let conditions = setup_conditions_bench::<T>(a, b, c, d);
+    }: {
+        conditions_bench(conditions);
+    }
+
     add_compliance_requirement {
         // INTERNAL: This benchmark only evaluate the adding operation. Its execution should be measured in another module.
         let s in 1..MAX_SENDER_CONDITIONS_PER_COMPLIANCE;
@@ -237,7 +313,7 @@ benchmarks! {
         d.add_default_trusted_claim_issuer(MAX_DEFAULT_TRUSTED_CLAIM_ISSUERS -1);
 
         // Add one more for benchmarking.
-        let new_issuer = make_issuer::<T>(MAX_DEFAULT_TRUSTED_CLAIM_ISSUERS);
+        let new_issuer = make_issuer::<T>(MAX_DEFAULT_TRUSTED_CLAIM_ISSUERS, None);
     }: _(d.owner.origin, d.ticker, new_issuer.clone())
     verify {
         let trusted_issuers = Module::<T>::trusted_claim_issuer(d.ticker);
@@ -300,9 +376,9 @@ benchmarks! {
             MAX_RECEIVER_CONDITIONS_PER_COMPLIANCE)
             .add_compliance_requirement().build();
 
-        let issuers = make_issuers::<T>(MAX_TRUSTED_ISSUER_PER_CONDITION);
-        let sender_conditions = make_conditions(MAX_SENDER_CONDITIONS_PER_COMPLIANCE, &issuers);
-        let receiver_conditions = make_conditions(MAX_RECEIVER_CONDITIONS_PER_COMPLIANCE, &issuers);
+        let issuers = make_issuers::<T>(MAX_TRUSTED_ISSUER_PER_CONDITION, None);
+        let sender_conditions = make_conditions(MAX_SENDER_CONDITIONS_PER_COMPLIANCE, None, &issuers);
+        let receiver_conditions = make_conditions(MAX_RECEIVER_CONDITIONS_PER_COMPLIANCE, None, &issuers);
 
         // Add more requirements to the asset, if `c > 1`.
         (1..c).for_each( |_i| {
diff --git a/pallets/compliance-manager/src/lib.rs b/pallets/compliance-manager/src/lib.rs
@@ -103,10 +103,7 @@ use polymesh_primitives::{
     proposition, storage_migration_ver, Balance, Claim, Condition, ConditionType, Context,
     IdentityId, Ticker, TrustedFor, TrustedIssuer,
 };
-use sp_std::{
-    convert::{From, TryFrom},
-    prelude::*,
-};
+use sp_std::{convert::From, prelude::*};
 
 type ExternalAgents<T> = pallet_external_agents::Module<T>;
 type Identity<T> = pallet_identity::Module<T>;
@@ -190,7 +187,7 @@ decl_module! {
         ///
         /// # Permissions
         /// * Asset
-        #[weight = <T as Config>::WeightInfo::add_compliance_requirement(sender_conditions.len() as u32, receiver_conditions.len() as u32)]
+        #[weight = <T as Config>::WeightInfo::add_compliance_requirement_full(&sender_conditions, &receiver_conditions)]
         pub fn add_compliance_requirement(origin, ticker: Ticker, sender_conditions: Vec<Condition>, receiver_conditions: Vec<Condition>) {
             let did = <ExternalAgents<T>>::ensure_perms(origin, ticker)?;
 
@@ -257,7 +254,7 @@ decl_module! {
         ///
         /// # Permissions
         /// * Asset
-        #[weight = <T as Config>::WeightInfo::replace_asset_compliance(asset_compliance.len() as u32)]
+        #[weight = <T as Config>::WeightInfo::replace_asset_compliance_full(&asset_compliance)]
         pub fn replace_asset_compliance(origin, ticker: Ticker, asset_compliance: Vec<ComplianceRequirement>) {
             let did = <ExternalAgents<T>>::ensure_perms(origin, ticker)?;
 
@@ -392,10 +389,7 @@ decl_module! {
         ///
         /// # Permissions
         /// * Asset
-        #[weight = <T as Config>::WeightInfo::change_compliance_requirement(
-            new_req.sender_conditions.len() as u32,
-            new_req.receiver_conditions.len() as u32,
-        )]
+        #[weight = <T as Config>::WeightInfo::change_compliance_requirement_full(&new_req)]
         pub fn change_compliance_requirement(origin, ticker: Ticker, new_req: ComplianceRequirement) {
             let did = <ExternalAgents<T>>::ensure_perms(origin, ticker)?;
 
@@ -601,17 +595,15 @@ impl<T: Config> Module<T> {
         let complexity = asset_compliance
             .iter()
             .flat_map(|req| req.conditions())
-            .fold(0usize, |complexity, condition| {
-                let (claims, issuers) = condition.complexity();
-                complexity.saturating_add(claims.saturating_mul(match issuers {
-                    0 => default_issuer_count,
-                    _ => issuers,
-                }))
-            });
-        if let Ok(complexity_u32) = u32::try_from(complexity) {
-            if complexity_u32 <= T::MaxConditionComplexity::get() {
-                return Ok(());
-            }
+            .fold(0u32, |total, condition| {
+                let complexity = condition.complexity(default_issuer_count);
+                total.saturating_add(complexity)
+            })
+            // NB: If the compliance requirements are empty (0 complexity),
+            // then use the count of requirements.
+            .max(asset_compliance.len() as u32);
+        if complexity <= T::MaxConditionComplexity::get() {
+            return Ok(());
         }
         Err(Error::<T>::ComplianceRequirementTooComplex.into())
     }
diff --git a/pallets/weights/src/pallet_compliance_manager.rs b/pallets/weights/src/pallet_compliance_manager.rs
@@ -52,6 +52,17 @@ use polymesh_runtime_common::{RocksDbWeight as DbWeight, Weight};
 /// Weights for pallet_compliance_manager using the Substrate node and recommended hardware.
 pub struct WeightInfo;
 impl pallet_compliance_manager::WeightInfo for WeightInfo {
+    fn condition_costs(a: u32, b: u32, c: u32, d: u32) -> Weight {
+        (0 as Weight)
+            // Standard Error: 154_000
+            .saturating_add((13_470_000 as Weight).saturating_mul(a as Weight))
+            // Standard Error: 154_000
+            .saturating_add((5_972_000 as Weight).saturating_mul(b as Weight))
+            // Standard Error: 154_000
+            .saturating_add((6_144_000 as Weight).saturating_mul(c as Weight))
+            // Standard Error: 154_000
+            .saturating_add((906_000 as Weight).saturating_mul(d as Weight))
+    }
     fn add_compliance_requirement(s: u32, r: u32) -> Weight {
         (98_941_000 as Weight)
             // Standard Error: 740_000
diff --git a/primitives/src/compliance_manager.rs b/primitives/src/compliance_manager.rs
@@ -13,7 +13,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <http://www.gnu.org/licenses/>.
 
-use crate::Condition;
+use crate::condition::{conditions_total_counts, Condition};
 use codec::{Decode, Encode};
 #[cfg(feature = "std")]
 use sp_runtime::{Deserialize, Serialize};
@@ -51,6 +51,13 @@ impl ComplianceRequirement {
             .iter()
             .chain(self.receiver_conditions.iter())
     }
+
+    /// Return the total number of conditions, claims, issuers, and claim_types.
+    ///
+    /// This is used for weight calculation.
+    pub fn counts(&self) -> (u32, u32, u32, u32) {
+        conditions_total_counts(self.conditions())
+    }
 }
 
 /// A compliance requirement along with its evaluation result
diff --git a/primitives/src/condition.rs b/primitives/src/condition.rs
