Soften CDD requirement

pallets/identity/src/keys.rs

@@ -891,23 +891,33 @@ impl<T: Config> Pallet<T> {
         Ok(())
     }
 
-    /// Ensures that `origin`'s key is the primary key of a DID.
+    /// Ensures that `origin`'s key is the primary key of a DID and has a valid CDD claim.
+    /// Returns the caller's account and DID.
     pub fn ensure_primary_key(
         origin: T::RuntimeOrigin,
     ) -> Result<(T::AccountId, IdentityId), DispatchError> {
         let sender = ensure_signed(origin)?;
         let key_rec = KeyRecords::<T>::get(&sender)
             .ok_or(pallet_permissions::Error::<T>::UnauthorizedCaller)?;
         let did = key_rec.is_primary_key().ok_or(Error::<T>::KeyNotAllowed)?;
+        ensure!(
+            Self::has_valid_cdd(did),
+            Error::<T>::UnauthorizedCallerDidMissingCdd
+        );
         Ok((sender, did))
     }
 
-    /// Ensures that `origin`'s key is linked to a DID and returns both.
+    /// Ensures that `origin`'s key is linked to a DID and has a valid CDD claim.
+    /// Returns the caller's account and DID.
     pub fn ensure_did(
         origin: T::RuntimeOrigin,
     ) -> Result<(T::AccountId, IdentityId), DispatchError> {
         let sender = ensure_signed(origin)?;
         let did = Self::get_identity(&sender).ok_or(Error::<T>::MissingIdentity)?;
+        ensure!(
+            Self::has_valid_cdd(did),
+            Error::<T>::UnauthorizedCallerDidMissingCdd
+        );
         Ok((sender, did))
     }
 
@@ -969,32 +979,51 @@ impl<T: Config> Pallet<T> {
 }
 
 impl<T: Config> CheckAccountCallPermissions<T::AccountId> for Pallet<T> {
-    // For weighting purposes, the function reads 4 storage values.
     fn check_account_call_permissions(
         who: &T::AccountId,
         pallet_name: impl FnOnce() -> PalletName,
         function_name: impl FnOnce() -> ExtrinsicName,
-    ) -> Option<AccountCallPermissionsData<T::AccountId>> {
+    ) -> Result<AccountCallPermissionsData<T::AccountId>, DispatchError> {
         let data = |did, secondary_key| AccountCallPermissionsData {
             primary_did: did,
             secondary_key,
         };
 
-        match KeyRecords::<T>::get(who)? {
-            // Primary keys do not have / require further permission checks.
-            KeyRecord::PrimaryKey(did) => Some(data(did, None)),
-            // Secondary Key. Ensure DID isn't frozen + key has sufficient permissions.
-            KeyRecord::SecondaryKey(did) if !IsDidFrozen::<T>::get(&did) => {
+        let key_record = KeyRecords::<T>::get(who).ok_or(Error::<T>::MissingIdentity)?;
+
+        match key_record {
+            // Primary keys only require a valid CDD claim.
+            KeyRecord::PrimaryKey(did) => {
+                ensure!(
+                    Self::has_valid_cdd(did),
+                    Error::<T>::UnauthorizedCallerDidMissingCdd
+                );
+                Ok(data(did, None))
+            }
+            // Secondary Key. Ensure DID isn't frozen + has a valid CDD claim + key has sufficient permissions.
+            KeyRecord::SecondaryKey(did) => {
+                ensure!(
+                    !IsDidFrozen::<T>::get(&did),
+                    Error::<T>::UnauthorizedCallerFrozenDid
+                );
+
+                ensure!(
+                    Self::has_valid_cdd(did),
+                    Error::<T>::UnauthorizedCallerDidMissingCdd
+                );
+
                 let permissions = Self::get_key_permissions(who);
-                let sk = SecondaryKey {
-                    key: who.clone(),
-                    permissions,
-                };
-                sk.has_extrinsic_permission(&pallet_name(), &function_name())
-                    .then(|| data(did, Some(sk)))
+                let sk = SecondaryKey::new(who.clone(), permissions);
+                ensure!(
+                    sk.has_extrinsic_permission(&pallet_name(), &function_name()),
+                    Error::<T>::UnauthorizedCallerMissingPermissions
+                );
+
+                Ok(data(did, Some(sk)))
+            }
+            KeyRecord::MultiSigSignerKey(_) => {
+                Err(Error::<T>::UnauthorizedCallerMultisigKey.into())
             }
-            // DIDs with frozen secondary keys, AKA frozen DIDs, are not permitted to call extrinsics.
-            _ => None,
         }
     }
 }

pallets/identity/src/lib.rs

@@ -1088,6 +1088,14 @@ pub mod pallet {
         /// Auth identified by an `auth_id` for a given `target` does not exist.
         /// The `target` might be wrong or the `auth_id` was never created at all.
         InvalidAuthorization,
+        /// Frozen secondary key.
+        UnauthorizedCallerFrozenDid,
+        /// The DID is missing a CDD claim.
+        UnauthorizedCallerDidMissingCdd,
+        /// The key does not have permissions to execute the extrinsic.
+        UnauthorizedCallerMissingPermissions,
+        /// Multisig keys are not allowed for some extrinsics.
+        UnauthorizedCallerMultisigKey,
     }
 }
 

pallets/permissions/src/lib.rs

@@ -77,7 +77,7 @@ pub mod pallet {
             who: &AccountId,
             pallet_name: impl FnOnce() -> PalletName,
             function_name: impl FnOnce() -> ExtrinsicName,
-        ) -> Option<AccountCallPermissionsData<AccountId>>;
+        ) -> Result<AccountCallPermissionsData<AccountId>, DispatchError>;
     }
 
     #[pallet::pallet]
@@ -110,7 +110,6 @@ pub mod pallet {
                 || CurrentPalletName::<T>::get(),
                 || CurrentDispatchableName::<T>::get(),
             )
-            .ok_or_else(|| Error::<T>::UnauthorizedCaller.into())
         }
     }
 }

pallets/runtime/common/src/fee_details.rs

@@ -164,10 +164,8 @@ where
                 | pallet_multisig::Call::approve { multisig, .. }
                 | pallet_multisig::Call::reject { multisig, .. },
             )) => handle_multisig(multisig, caller),
-            // All other calls.
-            //
-            // The external account must directly be linked to an identity with valid CDD.
-            _ => caller_pays(caller),
+            // All other calls
+            _ => Ok(Some(caller.clone())),
         }
     }
 

pallets/runtime/tests/src/contracts_test.rs

@@ -175,7 +175,7 @@ fn chain_extension_calls() {
                 None,
                 register_ticker_input.clone()
             ),
-            pallet_permissions::Error::<TestStorage>::UnauthorizedCaller,
+            pallet_identity::Error::<TestStorage>::UnauthorizedCallerMissingPermissions,
         ));
         // Successfull call
         assert_ok!(Identity::set_secondary_key_permissions(

pallets/runtime/tests/src/fee_details.rs

@@ -1,16 +1,17 @@
-use super::{
-    multisig::{create_multisig_default_perms, create_signers},
-    storage::{get_last_auth_id, make_account_without_cdd, register_keyring_account, TestStorage},
-    ExtBuilder,
-};
 use frame_support::assert_noop;
+use sp_keyring::AccountKeyring;
+use sp_runtime::transaction_validity::InvalidTransaction;
+
 use pallet_balances as balances;
 use pallet_identity as identity;
 use pallet_multisig as multisig;
 use polymesh_primitives::{traits::CddAndFeeDetails, Signatory, TransactionError};
 use polymesh_runtime_develop::runtime::{CddHandler, RuntimeCall};
-use sp_keyring::AccountKeyring;
-use sp_runtime::transaction_validity::InvalidTransaction;
+
+use super::multisig::{create_multisig_default_perms, create_signers};
+use super::storage::{get_last_auth_id, make_account_without_cdd};
+use super::storage::{register_keyring_account, TestStorage};
+use super::ExtBuilder;
 
 type MultiSig = multisig::Pallet<TestStorage>;
 type Balances = balances::Pallet<TestStorage>;
@@ -34,15 +35,14 @@ fn cdd_checks() {
             let charlie_account = AccountKeyring::Charlie.to_account_id();
             let charlie_signatory = Signatory::Account(charlie_account.clone());
 
-            // normal tx without cdd should fail
-            assert_noop!(
+            assert_eq!(
                 CddHandler::get_valid_payer(
                     &RuntimeCall::MultiSig(multisig::Call::change_sigs_required {
                         sigs_required: 1
                     }),
                     &alice_account
                 ),
-                InvalidTransaction::Custom(TransactionError::CddRequired as u8)
+                Ok(Some(AccountKeyring::Alice.to_account_id()))
             );
 
             // call to accept being a multisig signer should fail when invalid auth
@@ -54,22 +54,15 @@ fn cdd_checks() {
                 InvalidTransaction::Custom(TransactionError::InvalidAuthorization as u8)
             );
 
-            // call to accept being a multisig signer should fail when authorizer does not have a valid cdd (expired)
-            create_multisig_default_perms(
-                alice_account.clone(),
-                create_signers(vec![alice_account.clone()]),
-                1,
-            );
-
-            let alice_auth_id = get_last_auth_id(&alice_signatory);
+            let alice_auth_id = 0;
             assert_noop!(
                 CddHandler::get_valid_payer(
                     &RuntimeCall::MultiSig(multisig::Call::accept_multisig_signer {
                         auth_id: alice_auth_id
                     }),
                     &alice_account
                 ),
-                InvalidTransaction::Custom(TransactionError::CddRequired as u8)
+                InvalidTransaction::Custom(TransactionError::InvalidAuthorization as u8)
             );
 
             // call to remove authorisation with issuer paying should fail if issuer does not have a valid cdd
@@ -82,11 +75,10 @@ fn cdd_checks() {
                     }),
                     &alice_account
                 ),
-                InvalidTransaction::Custom(TransactionError::CddRequired as u8)
+                InvalidTransaction::Custom(TransactionError::InvalidAuthorization as u8)
             );
 
-            // call to remove authorisation with caller paying should fail if caller does not have a valid cdd
-            assert_noop!(
+            assert_eq!(
                 CddHandler::get_valid_payer(
                     &RuntimeCall::Identity(identity::Call::remove_authorization {
                         target: alice_signatory.clone(),
@@ -95,7 +87,7 @@ fn cdd_checks() {
                     }),
                     &alice_account
                 ),
-                InvalidTransaction::Custom(TransactionError::CddRequired as u8)
+                Ok(Some(AccountKeyring::Alice.to_account_id()))
             );
 
             // check that authorisation can be removed correctly
@@ -106,8 +98,7 @@ fn cdd_checks() {
             );
             let alice_auth_id = get_last_auth_id(&alice_signatory);
 
-            // call to remove authorisation with caller paying should fail if caller does not have a valid cdd
-            assert_noop!(
+            assert_eq!(
                 CddHandler::get_valid_payer(
                     &RuntimeCall::Identity(identity::Call::remove_authorization {
                         target: alice_signatory.clone(),
@@ -116,7 +107,7 @@ fn cdd_checks() {
                     }),
                     &alice_account
                 ),
-                InvalidTransaction::Custom(TransactionError::CddRequired as u8)
+                Ok(Some(AccountKeyring::Alice.to_account_id()))
             );
 
             // call to remove authorisation with issuer paying should succeed as issuer has CDD
@@ -132,13 +123,7 @@ fn cdd_checks() {
                 Ok(Some(AccountKeyring::Charlie.to_account_id()))
             );
 
-            // create an authorisation where the target has a CDD claim and the issuer does not
-            create_multisig_default_perms(
-                alice_account.clone(),
-                create_signers(vec![charlie_account.clone()]),
-                1,
-            );
-            let charlie_auth_id = get_last_auth_id(&charlie_signatory);
+            let charlie_auth_id = 0;
 
             // call to remove authorisation with issuer paying should fail if issuer does not have a valid cdd
             assert_noop!(
@@ -150,7 +135,7 @@ fn cdd_checks() {
                     }),
                     &charlie_account
                 ),
-                InvalidTransaction::Custom(TransactionError::CddRequired as u8)
+                InvalidTransaction::Custom(TransactionError::InvalidAuthorization as u8)
             );
 
             // call to remove authorisation with caller paying should succeed as caller has CDD

pallets/runtime/tests/src/identity_test.rs

@@ -29,12 +29,11 @@ use polymesh_primitives::{
     AssetPermissions, AuthorizationData, AuthorizationType, Claim, ClaimType, CustomClaimTypeId,
     ExtrinsicName, ExtrinsicPermissions, IdentityClaim, IdentityId, KeyRecord, PalletName,
     PalletPermissions, Permissions, PortfolioId, PortfolioNumber, Scope, SecondaryKey, Signatory,
-    SubsetRestriction, SystematicIssuers, Ticker, TransactionError, GC_DID,
+    SubsetRestriction, SystematicIssuers, Ticker, GC_DID,
 };
 use polymesh_runtime_develop::runtime::{CddHandler, RuntimeCall};
 use sp_core::H512;
 use sp_keyring::AccountKeyring;
-use sp_runtime::transaction_validity::InvalidTransaction;
 use sp_std::iter;
 use std::convert::From;
 
@@ -454,7 +453,7 @@ fn frozen_secondary_keys_cdd_verification_test_we() {
         alice.clone()
     )));
 
-    // 4. Bob should NOT transfer any amount. SE is simulated.
+    // 4. Bob should be able to transfer any amount. SE is simulated.
     // Balances::transfer_with_memo(Origin::signed(bob), charlie, 1_000, None),
     let payer = CddHandler::get_valid_payer(
         &RuntimeCall::Balances(balances::Call::transfer_with_memo {
@@ -464,10 +463,7 @@ fn frozen_secondary_keys_cdd_verification_test_we() {
         }),
         &AccountKeyring::Bob.to_account_id(),
     );
-    assert_noop!(
-        payer,
-        InvalidTransaction::Custom(TransactionError::MissingIdentity as u8)
-    );
+    assert!(payer.is_ok());
 
     assert_eq!(Balances::free_balance(charlie.clone()), 1100);
 
@@ -1689,15 +1685,15 @@ fn invalidate_cdd_claims_we() {
     assert_eq!(Identity::has_valid_cdd(alice_id), true);
     assert_noop!(
         Identity::cdd_register_did(Origin::signed(cdd.clone()), bob_acc.clone(), vec![]),
-        Error::UnAuthorizedCddProvider
+        Error::UnauthorizedCallerDidMissingCdd
     );
 
     // Move to time 11 ... CDD_1 is expired: Its claims are invalid.
     set_timestamp(11);
     assert_eq!(Identity::has_valid_cdd(alice_id), false);
     assert_noop!(
         Identity::cdd_register_did(Origin::signed(cdd), bob_acc, vec![]),
-        Error::UnAuthorizedCddProvider
+        Error::UnauthorizedCallerDidMissingCdd
     );
 }
 

pallets/runtime/tests/src/relayer_test.rs

@@ -30,6 +30,7 @@ type ProtocolFee = pallet_protocol_fee::Pallet<TestStorage>;
 type TransactionPayment = pallet_transaction_payment::Pallet<TestStorage>;
 type ChargeTransactionPayment = pallet_transaction_payment::ChargeTransactionPayment<TestStorage>;
 type Error = pallet_relayer::Error<TestStorage>;
+type IdentityError = pallet_identity::Error<TestStorage>;
 
 // Relayer Test Helper functions
 // =======================================
@@ -395,14 +396,9 @@ fn do_relayer_paying_key_missing_cdd_test() {
     let (bob_sign, _) = make_account_without_cdd(bob_acc.clone()).unwrap();
 
     // Add authorization for using Bob as the paying key for Alice.
-    assert_ok!(Relayer::set_paying_key(bob_sign, alice.acc(), 10u128));
-
-    // Alice tries to accept the paying key, but the paying key
-    // is without a CDD.
-    let auth_id = get_last_auth_id(&Signatory::Account(alice.acc()));
-    assert_eq!(
-        Relayer::accept_paying_key(alice.origin(), auth_id),
-        Err(Error::PayingKeyCddMissing.into()),
+    assert_noop!(
+        Relayer::set_paying_key(bob_sign, alice.acc(), 10u128),
+        IdentityError::UnauthorizedCallerDidMissingCdd
     );
 }
 

pallets/runtime/tests/src/settlement_pallet/lock_instruction.rs

@@ -25,6 +25,7 @@ type System = frame_system::Pallet<TestStorage>;
 type Timestamp = pallet_timestamp::Pallet<TestStorage>;
 
 type AssetError = pallet_asset::Error<TestStorage>;
+type IdentityError = pallet_identity::Error<TestStorage>;
 type PortfolioError = pallet_portfolio::Error<TestStorage>;
 type NFTError = pallet_nft::Error<TestStorage>;
 
@@ -271,7 +272,7 @@ fn missing_cdd_claim() {
 
             assert_noop!(
                 Settlement::lock_instruction(dave.origin(), InstructionId(0), Weight::MAX),
-                Error::<TestStorage>::FailedAssetTransferringConditions
+                IdentityError::UnauthorizedCallerDidMissingCdd
             );
         });
 }