Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalidate Proposals After Multsig Change #1726

Merged
merged 6 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pallets/common/src/traits/multisig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|| {
Expand Down
5 changes: 5 additions & 0 deletions pallets/multisig/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
74 changes: 73 additions & 1 deletion pallets/multisig/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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> {
Expand Down
Loading