Disallow custody remove cdd (#1828)

HenriqueNogara · web-flow · commit 971e45663ad2 · 2025-05-30T22:01:26.000+08:00
* Remove cdd and did requirement when nominating

* Block adding custodians to default portfolios

* Move check to accept auth; Remove old benchmark
diff --git a/pallets/identity/src/auth.rs b/pallets/identity/src/auth.rs
@@ -59,6 +59,7 @@ impl<T: Config> Pallet<T> {
             number_of_given_auths < T::MaxGivenAuths::get(),
             Error::<T>::ExceededNumberOfGivenAuths
         );
+
         NumberOfGivenAuths::<T>::insert(from, number_of_given_auths.saturating_add(1));
 
         let new_auth_id = CurrentAuthId::<T>::get().saturating_add(1);
diff --git a/pallets/portfolio/src/lib.rs b/pallets/portfolio/src/lib.rs
@@ -382,6 +382,8 @@ pub mod pallet {
         SelfAdditionNotAllowed,
         /// The extrinsic expected a different `AuthorizationType` than what the `data.auth_type()` is.
         BadAuthorizationType,
+        /// Default portfolios cannot have custodians.
+        DefaultPortfoliosCannotHaveCustodians,
     }
 
     #[pallet::call]
@@ -899,6 +901,11 @@ impl<T: Config> Pallet<T> {
         pallet_identity::Pallet::<T>::accept_auth_with(&to.into(), auth_id, |data, from| {
             let pid = extract_auth!(data, PortfolioCustody(p));
 
+            ensure!(
+                pid.kind != PortfolioKind::Default,
+                Error::<T>::DefaultPortfoliosCannotHaveCustodians
+            );
+
             let curr = Self::custodian(&pid);
             pallet_identity::Pallet::<T>::ensure_auth_by(from, curr)?;
 
diff --git a/pallets/runtime/tests/src/asset_pallet/issue.rs b/pallets/runtime/tests/src/asset_pallet/issue.rs
@@ -130,7 +130,13 @@ fn issue_tokens_assigned_custody() {
     ExtBuilder::default().build().execute_with(|| {
         let bob = User::new(AccountKeyring::Bob);
         let alice = User::new(AccountKeyring::Alice);
-        let portfolio_id = PortfolioId::new(alice.did, PortfolioKind::Default);
+        let portfolio_kind = PortfolioKind::User(PortfolioNumber(1));
+        let portfolio_id = PortfolioId::new(alice.did, portfolio_kind);
+
+        assert_ok!(Portfolio::create_portfolio(
+            alice.origin(),
+            PortfolioName(b"AliceUserPortfolio".to_vec())
+        ));
 
         let asset_id = Asset::generate_asset_id(alice.acc(), false);
         assert_ok!(Asset::create_asset(
@@ -157,7 +163,7 @@ fn issue_tokens_assigned_custody() {
             alice.origin(),
             asset_id,
             1_000,
-            PortfolioKind::Default
+            portfolio_kind
         ));
         assert_eq!(BalanceOf::<TestStorage>::get(asset_id, alice.did), 1_000);
         assert_eq!(
diff --git a/pallets/runtime/tests/src/asset_test.rs b/pallets/runtime/tests/src/asset_test.rs
@@ -1750,7 +1750,13 @@ fn redeem_token_assigned_custody() {
     ExtBuilder::default().build().execute_with(|| {
         let bob = User::new(AccountKeyring::Bob);
         let alice = User::new(AccountKeyring::Alice);
-        let portfolio_id = PortfolioId::new(alice.did, PortfolioKind::Default);
+        let portfolio_kind = PortfolioKind::User(PortfolioNumber(1));
+        let portfolio_id = PortfolioId::new(alice.did, portfolio_kind);
+
+        assert_ok!(Portfolio::create_portfolio(
+            alice.origin(),
+            PortfolioName(b"AliceUserPortfolio".to_vec())
+        ));
 
         let asset_id = create_and_issue_sample_asset(&alice);
         // Change custody of the default portfolio
@@ -1767,12 +1773,7 @@ fn redeem_token_assigned_custody() {
         ));
 
         assert_noop!(
-            Asset::redeem(
-                alice.origin(),
-                asset_id,
-                ISSUE_AMOUNT,
-                PortfolioKind::Default
-            ),
+            Asset::redeem(alice.origin(), asset_id, ISSUE_AMOUNT, portfolio_kind),
             PortfolioError::UnauthorizedCustodian
         );
     })
diff --git a/pallets/runtime/tests/src/nft.rs b/pallets/runtime/tests/src/nft.rs
@@ -16,8 +16,8 @@ use polymesh_primitives::settlement::{InstructionId, Leg, SettlementType};
 use polymesh_primitives::{
     with_transaction, AuthorizationData, Claim, ClaimType, Condition, ConditionType, CountryCode,
     IdentityId, NFTCollectionId, NFTCollectionKeys, NFTId, NFTMetadataAttribute, NFTs, PortfolioId,
-    PortfolioKind, PortfolioNumber, PortfolioUpdateReason, Scope, Signatory, TrustedFor,
-    TrustedIssuer, WeightMeter,
+    PortfolioKind, PortfolioName, PortfolioNumber, PortfolioUpdateReason, Scope, Signatory,
+    TrustedFor, TrustedIssuer, WeightMeter,
 };
 use sp_keyring::AccountKeyring;
 
@@ -506,11 +506,17 @@ fn burn_nft_no_custody() {
 
         let bob: User = User::new(AccountKeyring::Bob);
         let alice: User = User::new(AccountKeyring::Alice);
+        let portfolio_kind = PortfolioKind::User(PortfolioNumber(1));
+        let portfolio_id = PortfolioId::new(alice.did, portfolio_kind);
 
-        let portfolio_id = PortfolioId::new(alice.did, PortfolioKind::Default);
         let collection_keys: NFTCollectionKeys =
             vec![AssetMetadataKey::Local(AssetMetadataLocalKey(1))].into();
 
+        assert_ok!(Portfolio::create_portfolio(
+            alice.origin(),
+            PortfolioName(b"AliceUserPortfolio".to_vec())
+        ));
+
         let asset_id = create_nft_collection(
             alice.clone(),
             AssetType::NonFungible(NonFungibleType::Derivative),
@@ -537,18 +543,12 @@ fn burn_nft_no_custody() {
                 key: AssetMetadataKey::Local(AssetMetadataLocalKey(1)),
                 value: AssetMetadataValue(b"test".to_vec()),
             }],
-            PortfolioKind::Default,
+            portfolio_kind,
         )
         .unwrap();
 
         assert_noop!(
-            NFT::redeem_nft(
-                alice.origin(),
-                asset_id,
-                NFTId(1),
-                PortfolioKind::Default,
-                None
-            ),
+            NFT::redeem_nft(alice.origin(), asset_id, NFTId(1), portfolio_kind, None),
             PortfolioError::UnauthorizedCustodian
         );
     });
diff --git a/pallets/runtime/tests/src/portfolio.rs b/pallets/runtime/tests/src/portfolio.rs
@@ -1153,3 +1153,25 @@ fn allow_identity_to_create_portfolios_not_allowed() {
         );
     });
 }
+
+#[test]
+fn assign_custody_of_default_portfolio() {
+    ExtBuilder::default().build().execute_with(|| {
+        let bob = User::new(AccountKeyring::Bob);
+        let alice = User::new(AccountKeyring::Alice);
+        let portfolio_id = PortfolioId::new(alice.did, PortfolioKind::Default);
+
+        let auth_id = Identity::add_auth(
+            alice.did,
+            Signatory::from(bob.did),
+            AuthorizationData::PortfolioCustody(portfolio_id),
+            None,
+        )
+        .unwrap();
+
+        assert_noop!(
+            Portfolio::accept_portfolio_custody(bob.origin(), auth_id),
+            Error::DefaultPortfoliosCannotHaveCustodians
+        );
+    });
+}
diff --git a/pallets/runtime/tests/src/staking/mod.rs b/pallets/runtime/tests/src/staking/mod.rs
@@ -878,12 +878,7 @@ fn double_staking_should_fail() {
             Error::<Test>::NotController
         );
         // 2 = controller  => nominating should work.
-        assert_noop!(
-            Staking::nominate(RuntimeOrigin::signed(2), vec![1]),
-            Error::<Test>::StashIdentityDoesNotExist
-        );
-        provide_did_to_user(1);
-        assert_ok!(Staking::nominate(RuntimeOrigin::signed(2), vec![1]));
+        assert_ok!(Staking::nominate(RuntimeOrigin::signed(2), vec![1]),);
     });
 }
 
@@ -6586,7 +6581,7 @@ fn create_on_offence_now(offender: u64) {
 }
 
 #[test]
-fn add_nominator_with_invalid_expiry() {
+fn add_nominator_without_cdd() {
     ExtBuilder::default().nominate(true).build_and_execute(|| {
         let alice_acc = 500;
         let alice_controller_acc = 501;
@@ -6606,11 +6601,8 @@ fn add_nominator_with_invalid_expiry() {
         ));
 
         set_timestamp(Utc::now().timestamp() as u64);
-        assert_noop!(
-            Staking::nominate(alice_controller_signed, vec![10, 20, 30]),
-            Error::<Test>::StashIdentityNotCDDed,
-        );
-        assert!(Staking::nominators(&alice_acc).is_none());
+        assert_ok!(Staking::nominate(alice_controller_signed, vec![10, 20, 30]),);
+        assert!(Staking::nominators(&alice_acc).is_some());
     });
 }
 
@@ -6681,18 +6673,6 @@ fn validate_nominators_with_valid_cdd() {
 
             assert_ok!(Staking::nominate(eve_controller_signed, vec![11, 21, 31]));
             assert!(!Staking::nominators(&eve_acc).is_none());
-
-            set_timestamp((Utc::now().timestamp() as u64) + 800_u64);
-            assert_ok!(Staking::validate_cdd_expiry_nominators(
-                RuntimeOrigin::root(),
-                vec![alice_acc.clone(), eve_acc.clone()]
-            ));
-            assert!(Staking::nominators(&alice_acc).is_none());
-            assert!(!Staking::nominators(&eve_acc).is_none());
-
-            let ledger_data = Staking::ledger(&alice_controller_acc).unwrap();
-            assert_eq!(ledger_data.active, 0);
-            assert_eq!(ledger_data.unlocking.len(), 1);
         });
 }
 
diff --git a/pallets/staking/src/benchmarking.rs b/pallets/staking/src/benchmarking.rs
@@ -51,7 +51,6 @@ type MaxNominators<T> = <<T as Config>::BenchmarkingConfig as BenchmarkingConfig
 // -----------------------------------------------------------------
 
 use pallet_identity::benchmarking::{User, UserBuilder};
-use polymesh_primitives::identity_claim::ClaimType;
 use polymesh_primitives::{IdentityId, Permissions};
 
 use crate::types::SlashingSwitch;
@@ -990,39 +989,6 @@ benchmarks! {
         });
     }
 
-    validate_cdd_expiry_nominators {
-        let n in 1 .. T::MaxNominations::get();
-
-        clear_validators_and_nominators::<T>();
-
-        let (validator, nominators) = create_validator_with_nominators::<T>(
-            n,
-            T::MaxNominatorRewardedPerValidator::get() as u32,
-            true,
-            RewardDestination::Controller,
-            Some(10_000_000)
-        )?;
-
-        for nominator in &nominators {
-            let claim_first = pallet_identity::Claim1stKey {
-                target: nominator.0.did(),
-                claim_type: ClaimType::CustomerDueDiligence
-            };
-            let _ = pallet_identity::Claims::<T>::clear_prefix(claim_first, 1, None);
-        }
-
-        let nominators: Vec<T::AccountId> = nominators.iter().map(|x| x.0.account()).collect();
-        for nominator in &nominators {
-            assert!(Nominators::<T>::contains_key(nominator));
-        }
-
-    }: _(RawOrigin::Root, nominators.clone())
-    verify {
-        for nominator in nominators {
-            assert!(!Nominators::<T>::contains_key(nominator));
-        }
-    }
-
     // -----------------------------------------------------------------
 }
 
diff --git a/pallets/staking/src/pallet/impls.rs b/pallets/staking/src/pallet/impls.rs
@@ -890,17 +890,18 @@ impl<T: Config> Pallet<T> {
 
             if let Some(Nominations { targets, .. }) = <Nominators<T>>::get(&voter) {
                 nominators_seen.saturating_inc();
-                if Self::is_nominator_compliant(&voter) {
-                    let voter_weight = weight_of(&voter);
-                    if !targets.is_empty() {
-                        all_voters.push((voter.clone(), voter_weight, targets));
-                    }
-                    min_active_stake = if voter_weight < min_active_stake {
+
+                let voter_weight = weight_of(&voter);
+                if !targets.is_empty() {
+                    all_voters.push((voter.clone(), voter_weight, targets));
+                }
+                min_active_stake = {
+                    if voter_weight < min_active_stake {
                         voter_weight
                     } else {
                         min_active_stake
-                    };
-                }
+                    }
+                };
             } else if Validators::<T>::contains_key(&voter) {
                 validators_seen.saturating_inc();
                 if Self::is_validator_compliant(&voter)
@@ -1117,18 +1118,6 @@ impl<T: Config> Pallet<T> {
         })
     }
 
-    /// Returns `true` if `who` has a valid cdd claim. Otherwise, returns `false`.
-    pub(crate) fn is_nominator_compliant(who: &T::AccountId) -> bool {
-        pallet_identity::Pallet::<T>::get_identity(who)
-            .map_or(false, |id| pallet_identity::Pallet::<T>::has_valid_cdd(id))
-    }
-
-    pub(crate) fn get_bonding_duration_period() -> u64 {
-        (T::SessionsPerEra::get()  * T::BondingDuration::get()) as u64 // total session
-            * T::EpochDuration::get() // session length
-            * T::ExpectedBlockTime::get().saturated_into::<u64>()
-    }
-
     /// Decrease the running count of validators by 1 for the stash identity.
     pub(crate) fn release_running_validator(stash: &T::AccountId) {
         if !<Validators<T>>::contains_key(stash) {
diff --git a/pallets/staking/src/pallet/mod.rs b/pallets/staking/src/pallet/mod.rs
