Invalidate Proposals After Multsig Change (#1726)

* Set invalid proposals; Check proposal expiration date

* Add unit tests

* Add remove_admin extrinsic

* Cargo fmt

* Return error if mutlsig has no admin; Invalidate proposal only when accepting multisig signer

* Fix typo

pallets/common/src/traits/multisig.rs

@@ -40,6 +40,7 @@ pub trait WeightInfo {
     fn create_join_identity() -> Weight;
     fn approve_join_identity() -> Weight;
     fn join_identity() -> Weight;
+    fn remove_admin() -> Weight;
 
     fn default_max_weight(max_weight: &Option<Weight>) -> Weight {
         max_weight.unwrap_or_else(|| {

pallets/multisig/src/benchmarking.rs

@@ -369,4 +369,9 @@ benchmarks! {
         MultiSig::<T>::approve_join_identity(users[0].origin().into(), multisig.clone(), auth_id).unwrap();
         // The second approval call just approves.
     }: approve_join_identity(users[1].origin(), multisig, auth_id)
+
+    remove_admin {
+        let (alice, multisig, _, _, multisig_origin) = generate_multisig_for_alice::<T>(2, 2).unwrap();
+        init_admin(&multisig, &alice);
+    }: _(multisig_origin)
 }

pallets/multisig/src/lib.rs

@@ -533,6 +533,21 @@ pub mod pallet {
             pallet_identity::Module::<T>::join_identity(origin, auth_id)?;
             Ok(().into())
         }
+
+        /// Removes the admin identity from the `multisig`.  This must be called by the multisig itself.
+        #[pallet::call_index(17)]
+        #[pallet::weight(<T as Config>::WeightInfo::remove_admin())]
+        pub fn remove_admin(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
+            let multisig = ensure_signed(origin)?;
+            let caller_did = Self::ensure_ms_has_did(&multisig)?;
+            let admin_did = AdminDid::<T>::take(&multisig).ok_or(Error::<T>::AdminNotFound)?;
+            Self::deposit_event(Event::MultiSigRemovedAdmin {
+                caller_did,
+                multisig,
+                admin_did,
+            });
+            Ok(().into())
+        }
     }
 
     #[pallet::event]
@@ -677,6 +692,12 @@ pub mod pallet {
         TooManySigners,
         /// Multisig doesn't have a paying DID.
         NoPayingDid,
+        /// Expiry must be in the future.
+        InvalidExpiryDate,
+        /// The proposal has been invalidated after a multisg update.
+        InvalidatedProposal,
+        /// Multisig has no admin.
+        AdminNotFound,
     }
 
     /// Nonce to ensure unique MultiSig addresses are generated; starts from 1.
@@ -783,6 +804,14 @@ pub mod pallet {
     #[pallet::getter(fn transaction_version)]
     pub(super) type TransactionVersion<T: Config> = StorageValue<_, u32, ValueQuery>;
 
+    /// The last proposal id before the multisig changed signers or signatures required.
+    ///
+    /// multisig => Option<proposal id>
+    #[pallet::storage]
+    #[pallet::getter(fn last_invalid_proposal)]
+    pub type LastInvalidProposal<T: Config> =
+        StorageMap<_, Identity, T::AccountId, u64, OptionQuery>;
+
     /// Storage version.
     #[pallet::storage]
     #[pallet::getter(fn storage_version)]
@@ -876,8 +905,12 @@ impl<T: Config> Pallet<T> {
             Some(ProposalState::ExecutionSuccessful | ProposalState::ExecutionFailed) => {
                 Err(Error::<T>::ProposalAlreadyExecuted.into())
             }
-            Some(ProposalState::Active { until: None }) => Ok(()),
+            Some(ProposalState::Active { until: None }) => {
+                Self::ensure_valid_proposal(multisig, proposal_id)?;
+                Ok(())
+            }
             Some(ProposalState::Active { until: Some(until) }) => {
+                Self::ensure_valid_proposal(multisig, proposal_id)?;
                 // Ensure proposal is not expired
                 ensure!(
                     until > pallet_timestamp::Pallet::<T>::get(),
@@ -949,6 +982,7 @@ impl<T: Config> Pallet<T> {
         }
 
         NumberOfSigners::<T>::insert(&multisig, pending_num_of_signers);
+        Self::set_invalid_proposals(&multisig);
         Self::deposit_event(Event::MultiSigSignersRemoved {
             caller_did: caller_did.unwrap_or(ms_did),
             multisig: multisig.clone(),
@@ -1004,6 +1038,7 @@ impl<T: Config> Pallet<T> {
         let max_weight = proposal.get_dispatch_info().weight;
         let caller_did = Self::ensure_ms_get_did(multisig)?;
         let proposal_id = Self::next_proposal_id(multisig);
+        Self::ensure_valid_expiry(&expiry)?;
 
         Proposals::<T>::insert(multisig, proposal_id, &*proposal);
         ProposalVoteCounts::<T>::insert(multisig, proposal_id, ProposalVoteCount::default());
@@ -1211,6 +1246,8 @@ impl<T: Config> Pallet<T> {
 
                 IdentityPallet::<T>::ensure_auth_by(ms_identity, auth_by)?;
 
+                Self::set_invalid_proposals(&multisig);
+
                 // Update number of signers for this multisig.
                 let pending_num_of_signers = Self::ensure_max_signers(&multisig, 1)?;
                 NumberOfSigners::<T>::insert(&multisig, pending_num_of_signers);
@@ -1280,13 +1317,48 @@ impl<T: Config> Pallet<T> {
             Error::<T>::ChangeNotAllowed
         );
         MultiSigSignsRequired::<T>::insert(multisig, &signatures_required);
+        Self::set_invalid_proposals(&multisig);
         Self::deposit_event(Event::MultiSigSignersRequiredChanged {
             caller_did: caller_did.or(ms_did),
             multisig: multisig.clone(),
             sigs_required: signatures_required,
         });
         Ok(())
     }
+
+    /// Returns `Ok` if `expiry` is in the future. Otherwise, returns [`Error::InvalidExpiryDate`].
+    fn ensure_valid_expiry(expiry: &Option<T::Moment>) -> DispatchResult {
+        if let Some(expiry) = expiry {
+            ensure!(
+                expiry > &pallet_timestamp::Pallet::<T>::get(),
+                Error::<T>::InvalidExpiryDate
+            );
+        }
+        Ok(())
+    }
+
+    /// Returns `Ok` if `proposal_id` is valid. Otherwise, returns [`Error::InvalidatedProposal`].
+    fn ensure_valid_proposal(multisig: &T::AccountId, proposal_id: u64) -> DispatchResult {
+        if let Some(last_invalid_proposal) = Self::last_invalid_proposal(multisig) {
+            ensure!(
+                proposal_id > last_invalid_proposal,
+                Error::<T>::InvalidatedProposal
+            );
+        }
+        Ok(())
+    }
+
+    /// Sets [`LastInvalidProposal`] with the proposal id of the last proposal.
+    fn set_invalid_proposals(multisig: &T::AccountId) {
+        let next_proposal_id = Self::next_proposal_id(multisig);
+
+        // There are no proposals for the multisig
+        if next_proposal_id == 0 {
+            return;
+        }
+
+        LastInvalidProposal::<T>::insert(multisig, next_proposal_id.saturating_sub(1));
+    }
 }
 
 impl<T: Config> MultiSigSubTrait<T::AccountId> for Pallet<T> {