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.

Sign up for GitHub

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

Jump to bottom

Try State Hook for Ranked Collective #3007

Merged
merged 21 commits into from
Feb 8, 2024
Merged

Try State Hook for Ranked Collective #3007

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5cdc28e
try state for members
dharjeezy Jan 21, 2024
efce0b5
test changes
dharjeezy Jan 21, 2024
7b0ac7e
try_state_index
dharjeezy Jan 24, 2024
5d76104
Merge branch 'master' into dami/try-state-ranked-collective
dharjeezy Jan 24, 2024
deac1d2
more invariants added
dharjeezy Feb 5, 2024
c3c3bfb
Merge remote-tracking branch 'origin/dami/try-state-ranked-collective…
dharjeezy Feb 5, 2024
76edeaf
Merge branch 'master' of https://github.com/dharjeezy/polkadot-sdk in…
dharjeezy Feb 5, 2024
71ae86f
fix test
dharjeezy Feb 5, 2024
993857a
try runtime impl
dharjeezy Feb 5, 2024
93d57e4
pr doc
dharjeezy Feb 5, 2024
8a5b776
invariant: `Rank` of the member `who` in `IdToIndex` should be the sa…
dharjeezy Feb 6, 2024
74c1746
Merge branch 'master' into dami/try-state-ranked-collective
dharjeezy Feb 6, 2024
d105b35
Merge branch 'master' into dami/try-state-ranked-collective
dharjeezy Feb 6, 2024
c01ad61
invariant: `Sum` of `MemberCount` index should be the same as the sum…
dharjeezy Feb 7, 2024
7f1d133
Merge remote-tracking branch 'origin/dami/try-state-ranked-collective…
dharjeezy Feb 7, 2024
43a4d65
Merge branch 'master' into dami/try-state-ranked-collective
liamaharon Feb 8, 2024
06111ba
fix error
dharjeezy Feb 8, 2024
ab56102
Merge remote-tracking branch 'origin/dami/try-state-ranked-collective…
dharjeezy Feb 8, 2024
13fe351
fix clippy
dharjeezy Feb 8, 2024
ba4503e
fix error
dharjeezy Feb 8, 2024
6b8e350
Merge branch 'master' into dami/try-state-ranked-collective
liamaharon Feb 8, 2024
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
54 changes: 54 additions & 0 deletions substrate/frame/ranked-collective/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,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<(), 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 @@ -763,6 +771,52 @@ pub mod pallet {
use frame_support::traits::CallerTrait;
o.as_signed().and_then(Self::rank_of)
}

/// Ensure the correctness of the state of this pallet.
#[cfg(any(feature = "try-runtime", test))]
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved
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`].
#[cfg(any(feature = "try-runtime", test))]
fn try_state_members() -> Result<(), sp_runtime::TryRuntimeError> {
MemberCount::<T, I>::iter().try_for_each(|(_rank, member_index)| -> DispatchResult {
let total_members = Members::<T, I>::iter().count();
ensure!(
total_members as u32 >= member_index,
liamaharon marked this conversation as resolved.
Show resolved Hide resolved
"Total count of `Members` should be greater than or equal to the number of `MemberIndex` of a particular `Rank` in `MemberCount`."
);

Ok(())
})?;

Ok(())
}
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved

/// ### Invariants of Index storage items
/// [`Member`] in storage of [`IdToIndex`] should be the same as [`Member`] in [`IndexToId`]
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`."
);
liamaharon marked this conversation as resolved.
Show resolved Hide resolved

Ok(())
},
)?;

Ok(())
}
}

impl<T: Config<I>, I: 'static> RankedMembers for Pallet<T, I> {
Expand Down
49 changes: 33 additions & 16 deletions substrate/frame/ranked-collective/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,28 @@ impl Config for Test {
type VoteWeight = Geometric;
}

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 @@ -211,14 +228,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 @@ -230,7 +247,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 @@ -260,7 +277,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 @@ -291,7 +308,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 @@ -358,7 +375,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 @@ -392,7 +409,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 @@ -419,7 +436,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 @@ -445,7 +462,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 @@ -514,7 +531,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));
assert_ok!(Club::do_add_member_to_rank(1337, max_rank));
Expand All @@ -531,7 +548,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
Loading