Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Try-state for Collective pallet #13645

Merged
merged 16 commits into from
Apr 17, 2023
2 changes: 1 addition & 1 deletion frame/collective/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,5 +646,5 @@ benchmarks_instance_pallet! {
assert_last_event::<T, I>(Event::Disapproved { proposal_hash: last_hash }.into());
}

impl_benchmark_test_suite!(Collective, crate::tests::new_test_ext(), crate::tests::Test);
impl_benchmark_test_suite!(Collective, crate::tests::ExtBuilder::default().build(), crate::tests::Test);
}
118 changes: 116 additions & 2 deletions frame/collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ use sp_std::{marker::PhantomData, prelude::*, result};
use frame_support::{
codec::{Decode, Encode, MaxEncodedLen},
dispatch::{
DispatchError, DispatchResultWithPostInfo, Dispatchable, GetDispatchInfo, Pays,
PostDispatchInfo,
DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable, GetDispatchInfo,
Pays, PostDispatchInfo,
},
ensure,
traits::{
Expand Down Expand Up @@ -341,6 +341,15 @@ pub mod pallet {
WrongProposalLength,
}

#[pallet::hooks]
impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
#[cfg(feature = "try-runtime")]
fn try_state(_n: BlockNumberFor<T>) -> Result<(), &'static str> {
Self::do_try_state()?;
Ok(())
}
}

// Note that councillor operations are assigned to the operational class.
#[pallet::call]
impl<T: Config<I>, I: 'static> Pallet<T, I> {
Expand Down Expand Up @@ -916,6 +925,111 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
});
num_proposals as u32
}

/// Ensure the correctness of the state of this pallet.
///
/// The following expectation must always apply.
///
/// ## Expectations:
///
/// Looking at proposals:
///
/// * Each hash of a proposal that is stored inside `Proposals` must have a
/// call mapped to it inside the `ProposalOf` storage map.
/// * `ProposalCount` must always be more or equal to the number of
/// proposals inside the `Proposals` storage value. The reason why
/// `ProposalCount` can be more is because when a proposal is removed the
/// count is not deducted.
/// * Count of `ProposalOf` should match the count of `Proposals`
///
/// Looking at votes:
/// * The sum of aye and nay votes for a proposal can never exceed
/// `MaxMembers`.
/// * The proposal index inside the `Voting` storage map must be unique.
/// * All proposal hashes inside `Voting` must exist in `Proposals`.
///
/// Looking at members:
/// * The members count must never exceed `MaxMembers`.
/// * All the members must be sorted by value.
///
/// Looking at prime account:
/// * The prime account must be a member of the collective.
#[cfg(any(feature = "try-runtime", test))]
fn do_try_state() -> DispatchResult {
Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult {
ensure!(
Self::proposal_of(proposal).is_some(),
DispatchError::Other(
"Proposal hash from `Proposals` is not found inside the `ProposalOf` mapping."
)
);
Ok(())
})?;

ensure!(
Self::proposals().into_iter().count() <= Self::proposal_count() as usize,
DispatchError::Other("The actual number of proposals is greater than `ProposalCount`")
);
ensure!(
Self::proposals().into_iter().count() == <ProposalOf<T, I>>::iter_keys().count(),
DispatchError::Other("Proposal count inside `Proposals` is not equal to the proposal count in `ProposalOf`")
);

Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult {
if let Some(votes) = Self::voting(proposal) {
let ayes = votes.ayes.len();
let nays = votes.nays.len();

ensure!(
ayes.saturating_add(nays) <= T::MaxMembers::get() as usize,
DispatchError::Other("The sum of ayes and nays is greater than `MaxMembers`")
);
}
Ok(())
})?;

let mut proposal_indices = vec![];
Self::proposals().into_iter().try_for_each(|proposal| -> DispatchResult {
if let Some(votes) = Self::voting(proposal) {
let proposal_index = votes.index;
ensure!(
!proposal_indices.contains(&proposal_index),
DispatchError::Other("The proposal index is not unique.")
);
proposal_indices.push(proposal_index);
}
Ok(())
})?;

<Voting<T, I>>::iter_keys().try_for_each(|proposal_hash| -> DispatchResult {
ensure!(
Self::proposals().contains(&proposal_hash),
DispatchError::Other(
"`Proposals` doesn't contain the proposal hash from the `Voting` storage map."
)
);
Ok(())
})?;

ensure!(
Self::members().len() <= T::MaxMembers::get() as usize,
DispatchError::Other("The member count is greater than `MaxMembers`.")
);

ensure!(
Self::members().windows(2).all(|members| members[0] <= members[1]),
DispatchError::Other("The members are not sorted by value.")
);

if let Some(prime) = Self::prime() {
ensure!(
Self::members().contains(&prime),
DispatchError::Other("Prime account is not a member.")
);
}

Ok(())
}
}

impl<T: Config<I>, I: 'static> ChangeMembers<T::AccountId> for Pallet<T, I> {
Expand Down
Loading