Small fixes (#1739)

* Improve weight functions; Add SelfAdditionNotAllowed restriction to allow_identity_to_create_portfolios

* Remove deprecated benchmarks

* Change as_derivative call_index

---------

Co-authored-by: Adam Dossa <[email protected]>

pallets/asset/src/lib.rs

@@ -565,8 +565,11 @@ decl_module! {
         #[weight = <T as Config>::WeightInfo::create_asset(
             asset_name.len() as u32,
             asset_identifiers.len() as u32,
-            funding_round_name.as_ref().map_or(0, |name| name.len()) as u32
-        ) + <T as Config>::WeightInfo::register_custom_asset_type(custom_asset_type.len() as u32)]
+            funding_round_name.as_ref().map_or(0, |name| name.len()) as u32,
+        )
+        .saturating_add(<T as Config>::WeightInfo::register_custom_asset_type(
+            custom_asset_type.len() as u32,
+        ))]
         pub fn create_asset_with_custom_type(
             origin,
             asset_name: AssetName,

pallets/common/src/traits/portfolio.rs

@@ -100,7 +100,7 @@ pub trait PortfolioSubTrait<AccountId> {
 }
 
 pub trait WeightInfo {
-    fn create_portfolio() -> Weight;
+    fn create_portfolio(l: u32) -> Weight;
     fn delete_portfolio() -> Weight;
     fn rename_portfolio(i: u32) -> Weight;
     fn quit_portfolio_custody() -> Weight;

pallets/portfolio/src/benchmarking.rs

@@ -67,13 +67,14 @@ benchmarks! {
     where_clause { where T: TestUtilsFn<AccountIdOf<T>> + AssetConfig }
 
     create_portfolio {
-        let target = user::<T>("target", 0);
-        let did = target.did();
-        let portfolio_name = PortfolioName(vec![65u8; PORTFOLIO_NAME_LEN]);
-        let next_portfolio_num = NextPortfolioNumber::get(&did);
-    }: _(target.origin, portfolio_name.clone())
+        let l in 1..PORTFOLIO_NAME_LEN.try_into().unwrap();
+
+        let alice = UserBuilder::<T>::default().generate_did().build("Alice");
+        let next_portfolio_num = NextPortfolioNumber::get(alice.did());
+        let portfolio_name = PortfolioName(vec![65; l as usize]);
+    }: _(alice.origin.clone(), portfolio_name.clone())
     verify {
-        assert_eq!(Portfolios::get(&did, &next_portfolio_num), Some(portfolio_name));
+        assert_eq!(Portfolios::get(alice.did(), next_portfolio_num), Some(portfolio_name));
     }
 
     delete_portfolio {

pallets/portfolio/src/lib.rs

@@ -171,7 +171,9 @@ decl_error! {
         /// The caller doesn't have permission to create portfolios on the owner's behalf.
         MissingOwnersPermission,
         /// The sender identity can't be the same as the receiver identity.
-        InvalidTransferSenderIdMatchesReceiverId
+        InvalidTransferSenderIdMatchesReceiverId,
+        /// Adding itself as an AllowedCustodian is not permitted.
+        SelfAdditionNotAllowed
     }
 }
 
@@ -190,7 +192,7 @@ decl_module! {
         }
 
         /// Creates a portfolio with the given `name`.
-        #[weight = <T as Config>::WeightInfo::create_portfolio()]
+        #[weight = <T as Config>::WeightInfo::create_portfolio(name.len() as u32)]
         pub fn create_portfolio(origin, name: PortfolioName) -> DispatchResult {
             let callers_did = Identity::<T>::ensure_perms(origin)?;
             Self::base_create_portfolio(callers_did, name)
@@ -857,6 +859,10 @@ impl<T: Config> Module<T> {
         trusted_identity: IdentityId,
     ) -> DispatchResult {
         let callers_did = Identity::<T>::ensure_perms(origin)?;
+        ensure!(
+            callers_did != trusted_identity,
+            Error::<T>::SelfAdditionNotAllowed
+        );
         AllowedCustodians::insert(callers_did, trusted_identity, true);
         Ok(())
     }

pallets/runtime/tests/src/portfolio.rs

@@ -1118,3 +1118,15 @@ fn create_custody_portfolio_revoke_permission() {
         );
     });
 }
+
+#[test]
+fn allow_identity_to_create_portfolios_not_allowed() {
+    ExtBuilder::default().build().execute_with(|| {
+        let alice = User::new(AccountKeyring::Alice);
+
+        assert_noop!(
+            Portfolio::allow_identity_to_create_portfolios(alice.origin(), alice.did),
+            Error::SelfAdditionNotAllowed
+        );
+    });
+}

pallets/utility/src/benchmarking.rs

@@ -29,9 +29,6 @@ use polymesh_common_utilities::traits::TestUtilsFn;
 
 use super::*;
 
-// POLYMESH:
-const MAX_CALLS: u32 = 30;
-
 const SEED: u32 = 0;
 
 fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
@@ -151,45 +148,6 @@ benchmarks! {
         assert!(Pallet::<T>::ensure_root(u.origin.into()).is_err());
     }
 
-    // POLYMESH:
-    batch_old {
-        let c in 0..MAX_CALLS;
-
-        let u = UserBuilder::<T>::default().generate_did().build("ALICE");
-        let calls = make_calls::<T>(c);
-
-    }: _(u.origin, calls)
-    verify {
-        // NB In this case we are using `frame_system::Call::<T>::remark` which makes *no DB
-        // operations*. This helps us to fetch the DB read/write ops only from `batch` instead of
-        // its batched calls.
-        // So there is no way to verify it.
-        // The following cases use `balances::transfer` to be able to verify their outputs.
-    }
-
-    // POLYMESH:
-    batch_atomic {
-        let c in 0..MAX_CALLS;
-
-        let alice = UserBuilder::<T>::default().generate_did().build("ALICE");
-        let calls = make_calls::<T>(c);
-    }: _(alice.origin, calls)
-    verify {
-        // NB see comment at `batch` verify section.
-    }
-
-    // POLYMESH:
-    batch_optimistic {
-        let c in 0..MAX_CALLS;
-
-        let alice = UserBuilder::<T>::default().generate_did().build("ALICE");
-        let calls = make_calls::<T>(c);
-
-    }: _(alice.origin, calls)
-    verify {
-        // NB see comment at `batch` verify section.
-    }
-
     // POLYMESH:
     as_derivative {
         let index = 1;