Skip to content

Commit

Permalink
Try State Hook for Ranked Collective (paritytech#3007)
Browse files Browse the repository at this point in the history 
Part of: paritytech#239

Polkadot address: 12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW

---------

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
  • Loading branch information
@dharjeezy @liamaharon
dharjeezy and liamaharon authored Feb 8, 2024
1 parent 3ea83f6 commit 5b22c03
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 19 deletions.
2 changes: 1 addition & 1 deletion substrate/frame/ranked-collective/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,5 +181,5 @@ benchmarks_instance_pallet! {
assert_has_event::<T, I>(Event::MemberExchanged { who, new_who }.into());
}

impl_benchmark_test_suite!(RankedCollective, crate::tests::new_test_ext(), crate::tests::Test);
impl_benchmark_test_suite!(RankedCollective, crate::tests::ExtBuilder::default().build(), crate::tests::Test);
}
134 changes: 134 additions & 0 deletions substrate/frame/ranked-collective/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,14 @@ pub mod pallet {
}
}

#[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<(), sp_runtime::TryRuntimeError> {
Self::do_try_state()
}
}

impl<T: Config<I>, I: 'static> Pallet<T, I> {
fn ensure_member(who: &T::AccountId) -> Result<MemberRecord, DispatchError> {
Members::<T, I>::get(who).ok_or(Error::<T, I>::NotMember.into())
Expand Down Expand Up @@ -847,6 +855,132 @@ pub mod pallet {
}
}

#[cfg(any(feature = "try-runtime", test))]
impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Ensure the correctness of the state of this pallet.
pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> {
Self::try_state_members()?;
Self::try_state_index()?;

Ok(())
}

/// ### Invariants of Member storage items
///
/// Total number of [`Members`] in storage should be >= [`MemberIndex`] of a [`Rank`] in
/// [`MemberCount`].
/// [`Rank`] in Members should be in [`MemberCount`]
/// [`Sum`] of [`MemberCount`] index should be the same as the sum of all the index attained
/// for rank possessed by [`Members`]
fn try_state_members() -> Result<(), sp_runtime::TryRuntimeError> {
MemberCount::<T, I>::iter().try_for_each(|(_, member_index)| -> DispatchResult {
let total_members = Members::<T, I>::iter().count();
ensure!(
total_members as u32 >= member_index,
"Total count of `Members` should be greater than or equal to the number of `MemberIndex` of a particular `Rank` in `MemberCount`."
);

Ok(())
})?;

let mut sum_of_member_rank_indexes = 0;
Members::<T, I>::iter().try_for_each(|(_, member_record)| -> DispatchResult {
ensure!(
Self::is_rank_in_member_count(member_record.rank.into()),
"`Rank` in Members should be in `MemberCount`"
);

sum_of_member_rank_indexes += Self::determine_index_of_a_rank(member_record.rank);

Ok(())
})?;

let sum_of_all_member_count_indexes =
MemberCount::<T, I>::iter_values().fold(0, |sum, index| sum + index);
ensure!(
sum_of_all_member_count_indexes == sum_of_member_rank_indexes as u32,
"Sum of `MemberCount` index should be the same as the sum of all the index attained for rank possessed by `Members`"
);
Ok(())
}

/// ### Invariants of Index storage items
/// [`Member`] in storage of [`IdToIndex`] should be the same as [`Member`] in [`IndexToId`]
/// [`Rank`] in [`IdToIndex`] should be the same as the the [`Rank`] in [`IndexToId`]
/// [`Rank`] of the member [`who`] in [`IdToIndex`] should be the same as the [`Rank`] of
/// the member [`who`] in [`Members`]
fn try_state_index() -> Result<(), sp_runtime::TryRuntimeError> {
IdToIndex::<T, I>::iter().try_for_each(
|(rank, who, member_index)| -> DispatchResult {
let who_from_index = IndexToId::<T, I>::get(rank, member_index).unwrap();
ensure!(
who == who_from_index,
"`Member` in storage of `IdToIndex` should be the same as `Member` in `IndexToId`."
);

ensure!(
Self::is_rank_in_index_to_id_storage(rank.into()),
"`Rank` in `IdToIndex` should be the same as the `Rank` in `IndexToId`"
);
Ok(())
},
)?;

Members::<T, I>::iter().try_for_each(|(who, member_record)| -> DispatchResult {
ensure!(
Self::is_who_rank_in_id_to_index_storage(who, member_record.rank),
"`Rank` of the member `who` in `IdToIndex` should be the same as the `Rank` of the member `who` in `Members`"
);

Ok(())
})?;

Ok(())
}

/// Checks if a rank is part of the `MemberCount`
fn is_rank_in_member_count(rank: u32) -> bool {
for (r, _) in MemberCount::<T, I>::iter() {
if r as u32 == rank {
return true;
}
}

return false;
}

/// Checks if a rank is the same as the rank `IndexToId`
fn is_rank_in_index_to_id_storage(rank: u32) -> bool {
for (r, _, _) in IndexToId::<T, I>::iter() {
if r as u32 == rank {
return true;
}
}

return false;
}

/// Checks if a member(who) rank is the same as the rank of a member(who) in `IdToIndex`
fn is_who_rank_in_id_to_index_storage(who: T::AccountId, rank: u16) -> bool {
for (rank_, who_, _) in IdToIndex::<T, I>::iter() {
if who == who_ && rank == rank_ {
return true;
}
}

return false;
}

/// Determines the total index for a rank
fn determine_index_of_a_rank(rank: u16) -> u16 {
let mut sum = 0;
for _ in 0..rank + 1 {
sum += 1;
}
sum
}
}

impl<T: Config<I>, I: 'static> RankedMembers for Pallet<T, I> {
type AccountId = T::AccountId;
type Rank = Rank;
Expand Down
53 changes: 35 additions & 18 deletions substrate/frame/ranked-collective/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,28 @@ impl Config for Test {
type BenchmarkSetup = ();
}

pub fn new_test_ext() -> sp_io::TestExternalities {
let t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
let mut ext = sp_io::TestExternalities::new(t);
ext.execute_with(|| System::set_block_number(1));
ext
pub struct ExtBuilder {}

impl Default for ExtBuilder {
fn default() -> Self {
Self {}
}
}

impl ExtBuilder {
pub fn build(self) -> sp_io::TestExternalities {
let t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
let mut ext = sp_io::TestExternalities::new(t);
ext.execute_with(|| System::set_block_number(1));
ext
}

pub fn build_and_execute(self, test: impl FnOnce() -> ()) {
self.build().execute_with(|| {
test();
Club::do_try_state().expect("All invariants must hold after a test");
})
}
}

fn next_block() {
Expand Down Expand Up @@ -225,14 +242,14 @@ fn completed_poll_should_panic() {

#[test]
fn basic_stuff() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_eq!(tally(3), Tally::from_parts(0, 0, 0));
});
}

#[test]
fn member_lifecycle_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::demote_member(RuntimeOrigin::root(), 1));
Expand All @@ -244,7 +261,7 @@ fn member_lifecycle_works() {

#[test]
fn add_remove_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_noop!(Club::add_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin);
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_eq!(member_count(0), 1);
Expand Down Expand Up @@ -274,7 +291,7 @@ fn add_remove_works() {

#[test]
fn promote_demote_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_noop!(Club::add_member(RuntimeOrigin::signed(1), 1), DispatchError::BadOrigin);
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_eq!(member_count(0), 1);
Expand Down Expand Up @@ -305,7 +322,7 @@ fn promote_demote_works() {

#[test]
fn promote_demote_by_rank_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
for _ in 0..7 {
assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1));
Expand Down Expand Up @@ -372,7 +389,7 @@ fn promote_demote_by_rank_works() {

#[test]
fn voting_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_ok!(Club::add_member(RuntimeOrigin::root(), 0));
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1));
Expand Down Expand Up @@ -406,7 +423,7 @@ fn voting_works() {

#[test]
fn cleanup_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::add_member(RuntimeOrigin::root(), 2));
Expand All @@ -433,7 +450,7 @@ fn cleanup_works() {

#[test]
fn remove_member_cleanup_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::add_member(RuntimeOrigin::root(), 2));
Expand All @@ -459,7 +476,7 @@ fn remove_member_cleanup_works() {

#[test]
fn ensure_ranked_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::add_member(RuntimeOrigin::root(), 2));
Expand Down Expand Up @@ -528,7 +545,7 @@ fn ensure_ranked_works() {

#[test]
fn do_add_member_to_rank_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
let max_rank = 9u16;
assert_ok!(Club::do_add_member_to_rank(69, max_rank / 2, true));
assert_ok!(Club::do_add_member_to_rank(1337, max_rank, true));
Expand All @@ -545,7 +562,7 @@ fn do_add_member_to_rank_works() {

#[test]
fn tally_support_correct() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
// add members,
// rank 1: accounts 1, 2, 3
// rank 2: accounts 2, 3
Expand Down Expand Up @@ -585,7 +602,7 @@ fn tally_support_correct() {

#[test]
fn exchange_member_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_eq!(member_count(0), 1);

Expand Down Expand Up @@ -613,7 +630,7 @@ fn exchange_member_works() {

#[test]
fn exchange_member_same_noops() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1));
assert_ok!(Club::add_member(RuntimeOrigin::root(), 2));
Expand Down

0 comments on commit 5b22c03

Please sign in to comment.