Mesh 1992/add-max-auths (#1649)

* Add MaxGivenAuths; Add NextAuthId; Add OutdatedAuthorizations * Fix tests * Update NumberOfGivenAuths when removing an auth * Fix benchmarks; Fix tests; Add consts * Initializes NumberOfGivenAuths; Remove duplicate const * Fix integration tests * Improve naming * Fix integration tests * Remove dead code; Update NumberOfGivenAuths when accepting an auth --------- Co-authored-by: Robert G. Jakabosky <[email protected]> Co-authored-by: Adam Dossa <[email protected]>
PolymeshAssociation · May 8, 2024 · 78f298c · 78f298c
1 parent 9e5fa26
commit 78f298c
Show file tree

Hide file tree

Showing 39 changed files with 548 additions and 490 deletions.
diff --git a/pallets/asset/src/benchmarking.rs b/pallets/asset/src/benchmarking.rs
@@ -281,7 +281,8 @@ benchmarks! {
             Signatory::from(did),
             AuthorizationData::TransferTicker(ticker),
             None
-        );
+        )
+        .unwrap();
     }: _(new_owner.origin, new_owner_auth_id)
     verify {
         verify_ownership::<T>(ticker, owner.did(), did, AssetOwnershipRelation::TickerOwned);
@@ -297,7 +298,8 @@ benchmarks! {
             Signatory::from(did),
             AuthorizationData::TransferAssetOwnership(ticker),
             None,
-        );
+        )
+        .unwrap();
     }: _(new_owner.origin, new_owner_auth_id)
     verify {
         assert_eq!(token_details::<T>(ticker).owner_did, did);
@@ -458,7 +460,8 @@ benchmarks! {
             Signatory::from(bob.did()),
             AuthorizationData::BecomeAgent(ticker, AgentGroup::Full),
             None,
-        );
+        )
+        .unwrap();
         pallet_external_agents::Module::<T>::accept_become_agent(bob.origin().into(), auth_id)?;
     }: _(bob.origin.clone(), ticker, 1_000,  PortfolioId::default_portfolio(alice.did()))
     verify {

diff --git a/pallets/common/src/traits/identity.rs b/pallets/common/src/traits/identity.rs
@@ -213,6 +213,9 @@ pub trait Config: CommonConfig + pallet_timestamp::Config + crate::traits::base:
     /// Only allow MultiSig primary/secondary keys to be removed from an identity
     /// if its POLYX balance is below this limit.
     type MultiSigBalanceLimit: Get<<Self::Balances as Currency<Self::AccountId>>::Balance>;
+
+    /// Maximum number of authorizations an identity can give.
+    type MaxGivenAuths: Get<u32>;
 }
 
 decl_event!(

diff --git a/pallets/compliance-manager/src/benchmarking.rs b/pallets/compliance-manager/src/benchmarking.rs
@@ -270,7 +270,8 @@ fn add_external_agent<T>(
         external_agent_id.into(),
         AuthorizationData::BecomeAgent(ticker, AgentGroup::Full),
         None,
-    );
+    )
+    .unwrap();
     pallet_external_agents::Module::<T>::accept_become_agent(external_agent_origin, auth_id)
         .unwrap();
 }

diff --git a/pallets/external-agents/src/benchmarking.rs b/pallets/external-agents/src/benchmarking.rs
@@ -46,7 +46,8 @@ fn add_auth<T: Asset + TestUtilsFn<AccountIdOf<T>>>(
         other.did().into(),
         AuthorizationData::BecomeAgent(ticker, AgentGroup::Full),
         None,
-    );
+    )
+    .unwrap();
     (other, auth_id)
 }
 

diff --git a/pallets/external-agents/src/lib.rs b/pallets/external-agents/src/lib.rs
@@ -324,7 +324,7 @@ impl<T: Config> Module<T> {
             Signatory::Identity(target),
             AuthorizationData::BecomeAgent(ticker, AgentGroup::Custom(ag_id)),
             expiry,
-        );
+        )?;
         Ok(())
     }
 

diff --git a/pallets/identity/src/auth.rs b/pallets/identity/src/auth.rs
@@ -14,8 +14,8 @@
 // along with this program. If not, see <http://www.gnu.org/licenses/>.
 
 use crate::{
-    AuthorizationType, Authorizations, AuthorizationsGiven, Config, Error, KeyRecords, Module,
-    MultiPurposeNonce, RawEvent,
+    AuthorizationType, Authorizations, AuthorizationsGiven, Config, CurrentAuthId, Error,
+    KeyRecords, Module, NumberOfGivenAuths, RawEvent,
 };
 use frame_support::dispatch::DispatchResult;
 use frame_support::{ensure, StorageDoubleMap, StorageMap, StorageValue};
@@ -24,6 +24,7 @@ use polymesh_common_utilities::Context;
 use polymesh_primitives::{
     Authorization, AuthorizationData, AuthorizationError, IdentityId, Signatory,
 };
+use sp_core::Get;
 use sp_runtime::DispatchError;
 use sp_std::vec::Vec;
 
@@ -41,7 +42,12 @@ impl<T: Config> Module<T> {
         {
             Self::ensure_perms_length_limited(perms)?;
         }
-        Ok(Self::add_auth(from_did, target, authorization_data, expiry))
+        Ok(Self::add_auth(
+            from_did,
+            target,
+            authorization_data,
+            expiry,
+        )?)
     }
 
     /// Adds an authorization.
@@ -50,32 +56,39 @@ impl<T: Config> Module<T> {
         target: Signatory<T::AccountId>,
         authorization_data: AuthorizationData<T::AccountId>,
         expiry: Option<T::Moment>,
-    ) -> u64 {
-        let new_nonce = Self::multi_purpose_nonce() + 1u64;
-        MultiPurposeNonce::put(&new_nonce);
+    ) -> Result<u64, DispatchError> {
+        let number_of_given_auths = NumberOfGivenAuths::get(from);
+        ensure!(
+            number_of_given_auths < T::MaxGivenAuths::get(),
+            Error::<T>::ExceededNumberOfGivenAuths
+        );
+        NumberOfGivenAuths::insert(from, number_of_given_auths.saturating_add(1));
+
+        let new_auth_id = Self::current_auth_id().saturating_add(1);
+        CurrentAuthId::put(new_auth_id);
 
         let auth = Authorization {
             authorization_data: authorization_data.clone(),
             authorized_by: from,
             expiry,
-            auth_id: new_nonce,
+            auth_id: new_auth_id,
             count: 50,
         };
 
-        <Authorizations<T>>::insert(target.clone(), new_nonce, auth);
-        <AuthorizationsGiven<T>>::insert(from, new_nonce, target.clone());
+        <Authorizations<T>>::insert(target.clone(), new_auth_id, auth);
+        <AuthorizationsGiven<T>>::insert(from, new_auth_id, target.clone());
 
         // This event is split in order to help the event harvesters.
         Self::deposit_event(RawEvent::AuthorizationAdded(
             from,
             target.as_identity().cloned(),
             target.as_account().cloned(),
-            new_nonce,
+            new_auth_id,
             authorization_data,
             expiry,
         ));
 
-        new_nonce
+        Ok(new_auth_id)
     }
 
     /// Removes an authorization.
@@ -112,6 +125,9 @@ impl<T: Config> Module<T> {
     ) {
         <Authorizations<T>>::remove(target, auth_id);
         <AuthorizationsGiven<T>>::remove(authorizer, auth_id);
+        NumberOfGivenAuths::mutate(authorizer, |number_of_given_auths| {
+            *number_of_given_auths = number_of_given_auths.saturating_sub(1);
+        });
         let id = target.as_identity().cloned();
         let acc = target.as_account().cloned();
         let event = if revoked {
@@ -170,7 +186,7 @@ impl<T: Config> Module<T> {
         accepter: impl FnOnce(AuthorizationData<T::AccountId>, IdentityId) -> DispatchResult,
     ) -> DispatchResult {
         // Extract authorization.
-        let mut auth = Self::ensure_authorization(target, auth_id)?;
+        let auth = Self::ensure_authorization(target, auth_id)?;
 
         // Ensure that `auth.expiry`, if provided, is in the future.
         if let Some(expiry) = auth.expiry {
@@ -179,33 +195,14 @@ impl<T: Config> Module<T> {
         }
 
         // Run custom per-type validation and updates.
-        let res = accepter(auth.authorization_data.clone(), auth.authorized_by);
-
-        if res.is_err() {
-            // decrement
-            auth.count = auth.count.saturating_sub(1);
-
-            // check if count is zero
-            if auth.count == 0 {
-                <Authorizations<T>>::remove(&target, auth_id);
-                <AuthorizationsGiven<T>>::remove(auth.authorized_by, auth_id);
-                Self::deposit_event(RawEvent::AuthorizationRetryLimitReached(
-                    target.as_identity().cloned(),
-                    target.as_account().cloned(),
-                    auth_id,
-                ));
-            } else {
-                // update authorization
-                <Authorizations<T>>::insert(&target, auth_id, auth);
-            }
-
-            // return error
-            return res;
-        }
+        accepter(auth.authorization_data.clone(), auth.authorized_by)?;
 
         // Remove authorization from storage and emit event.
         <Authorizations<T>>::remove(&target, auth_id);
         <AuthorizationsGiven<T>>::remove(auth.authorized_by, auth_id);
+        NumberOfGivenAuths::mutate(auth.authorized_by, |number_of_given_auths| {
+            *number_of_given_auths = number_of_given_auths.saturating_sub(1);
+        });
         Self::deposit_event(RawEvent::AuthorizationConsumed(
             target.as_identity().cloned(),
             target.as_account().cloned(),
@@ -214,11 +211,19 @@ impl<T: Config> Module<T> {
         Ok(())
     }
 
-    /// Return and ensure that there's an authorization `auth_id` for `target`.
+    /// Return and ensure that there's a valid authorization `auth_id` for `target`.
     fn ensure_authorization(
         target: &Signatory<T::AccountId>,
         auth_id: u64,
     ) -> Result<Authorization<T::AccountId, T::Moment>, DispatchError> {
-        Self::authorizations(target, auth_id).ok_or_else(|| AuthorizationError::Invalid.into())
+        let auth =
+            Self::authorizations(target, auth_id).ok_or_else(|| AuthorizationError::Invalid)?;
+        // Ensures the authorization is not outdated
+        if let Some(outdated_id) = Self::outdated_authorizations(target) {
+            if auth_id <= outdated_id {
+                return Err(AuthorizationError::Invalid.into());
+            }
+        }
+        Ok(auth)
     }
 }
diff --git a/pallets/identity/src/benchmarking.rs b/pallets/identity/src/benchmarking.rs
@@ -165,7 +165,8 @@ benchmarks! {
             cdd.did(), signatory.clone(),
             AuthorizationData::AttestPrimaryKeyRotation(target.did()),
             None,
-        );
+        )
+        .unwrap();
         Module::<T>::change_cdd_requirement_for_mk_rotation(
             RawOrigin::Root.into(),
             true
@@ -175,7 +176,8 @@ benchmarks! {
             target.did(), signatory,
             AuthorizationData::RotatePrimaryKey,
             None,
-        );
+        ).
+        unwrap();
     }: _(new_key.origin, owner_auth_id, Some(cdd_auth_id))
 
     rotate_primary_key_to_secondary {
@@ -188,12 +190,14 @@ benchmarks! {
             cdd.did(), signatory.clone(),
             AuthorizationData::AttestPrimaryKeyRotation(target.did()),
             None,
-        );
+        )
+        .unwrap();
         let rotate_auth_id =  Module::<T>::add_auth(
             target.did(), signatory.clone(),
             AuthorizationData::RotatePrimaryKeyToSecondary(Permissions::default()),
             None,
-        );
+        )
+        .unwrap();
         Module::<T>::change_cdd_requirement_for_mk_rotation(
             RawOrigin::Root.into(),
             true
@@ -223,7 +227,8 @@ benchmarks! {
             Signatory::Account(new_key.account()),
             AuthorizationData::JoinIdentity(Permissions::default()),
             None,
-        );
+        )
+        .unwrap();
     }: _(new_key.origin, auth_id)
 
     leave_identity_as_key {
@@ -236,7 +241,8 @@ benchmarks! {
             signatory,
             AuthorizationData::JoinIdentity(Permissions::default()),
             None,
-        );
+        )
+        .unwrap();
         Module::<T>::join_identity_as_key(key.origin().into(), auth_id)
             .expect("Key cannot be joined to identity");
 
@@ -360,7 +366,8 @@ benchmarks! {
             signatory.clone(),
             AuthorizationData::JoinIdentity(Permissions::default()),
             Some(666u32.into()),
-        );
+        )
+        .unwrap();
     }: _(caller.origin, signatory, auth_id, true)
 
     add_secondary_keys_with_authorization {

diff --git a/pallets/identity/src/keys.rs b/pallets/identity/src/keys.rs
@@ -14,9 +14,9 @@
 // along with this program. If not, see <http://www.gnu.org/licenses/>.
 
 use crate::{
-    types, AccountKeyRefCount, ChildDid, Config, DidKeys, DidRecords, Error, IsDidFrozen,
-    KeyRecords, Module, MultiPurposeNonce, OffChainAuthorizationNonce, ParentDid,
-    PermissionedCallOriginData, RawEvent, RpcDidRecords,
+    types, AccountKeyRefCount, ChildDid, Config, CurrentAuthId, DidKeys, DidRecords, Error,
+    IsDidFrozen, KeyRecords, Module, MultiPurposeNonce, OffChainAuthorizationNonce,
+    OutdatedAuthorizations, ParentDid, PermissionedCallOriginData, RawEvent, RpcDidRecords,
 };
 use codec::{Decode, Encode as _};
 use core::mem;
@@ -596,12 +596,21 @@ impl<T: Config> Module<T> {
         for key in &keys {
             // Unlink the secondary account key.
             Self::remove_key_record(key, Some(did));
+            // Sets all authorizations for key as outdated (these will be deleted on_intialize)
+            Self::set_outdated_autorizations(Signatory::Account(key.clone()));
         }
 
         Self::deposit_event(RawEvent::SecondaryKeysRemoved(did, keys));
         Ok(())
     }
 
+    /// Sets all authorizations with auth_id less or equal to the current id as invalid for the
+    /// `signatory_account`.
+    fn set_outdated_autorizations(signatory_account: Signatory<T::AccountId>) {
+        let current_auth_id = CurrentAuthId::get();
+        OutdatedAuthorizations::<T>::insert(signatory_account, current_auth_id);
+    }
+
     /// Adds secondary keys to target identity `id`.
     /// Keys are directly added to identity because each of them has an authorization.
     pub(crate) fn base_add_secondary_keys_with_authorization(
@@ -795,11 +804,11 @@ impl<T: Config> Module<T> {
         Self::deposit_event(RawEvent::DidCreated(did, sender, secondary_keys.clone()));
 
         // 2.3. add pre-authorized secondary keys.
-        secondary_keys.iter().for_each(|sk| {
+        for sk in secondary_keys {
             let signer = Signatory::Account(sk.key.clone());
             let data = AuthorizationData::JoinIdentity(sk.permissions.clone());
-            Self::add_auth(did, signer, data, None);
-        });
+            Self::add_auth(did, signer, data, None)?;
+        }
         Ok(did)
     }