Skip to content

try state hook for alliance pallet #10000

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+161 −17
Open

try state hook for alliance pallet #10000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
137 changes: 137 additions & 0 deletions substrate/frame/alliance/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,14 @@ pub mod pallet {
pub phantom: PhantomData<(T, I)>,
}

#[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()
}
}

#[pallet::genesis_build]
impl<T: Config<I>, I: 'static> BuildGenesisConfig for GenesisConfig<T, I> {
fn build(&self) {
Expand Down Expand Up @@ -1123,3 +1131,132 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Ok(info.into())
}
}

#[cfg(any(feature = "try-runtime", test))]
impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Ensure the correctness of the state of this pallet.
///
/// This should be valid before or after each state transition of this pallet.
pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> {
Self::try_state_members_are_disjoint()?;
Self::try_state_members_are_sorted()?;
Self::try_state_retiring_members_are_consistent()?;
Self::try_state_deposit_of_is_consistent()?;
Self::try_state_unscrupulous_items_are_sorted()?;
Self::try_state_announcements_are_sorted()?;
Ok(())
}

/// # Invariants
///
/// * The sets of `Fellows`, and `Allies` members must be mutually exclusive.
/// An account cannot hold more than one role at a time.
fn try_state_members_are_disjoint() -> Result<(), sp_runtime::TryRuntimeError> {
let fellows = Members::<T, I>::get(MemberRole::Fellow);
let allies = Members::<T, I>::get(MemberRole::Ally);

for fellow in fellows.iter() {
ensure!(
allies.binary_search(fellow).is_err(),
"Member is both Fellow and Ally"
);
}


Ok(())
}

/// # Invariants
///
/// * The list of members for each role (`Fellow`, `Ally`, `Retiring`) must be sorted by
/// `AccountId`. This is crucial for efficient lookups using binary search.
fn try_state_members_are_sorted() -> Result<(), sp_runtime::TryRuntimeError> {
let roles = [MemberRole::Fellow, MemberRole::Ally, MemberRole::Retiring];
for role in roles.iter() {
let members = Members::<T, I>::get(role);
let mut sorted_members = members.clone();
sorted_members.sort();
ensure!(
members == sorted_members,
"Members of a role are not sorted"
);
}
Ok(())
}

/// # Invariants
///
/// * The set of accounts in `RetiringMembers` storage must be identical to the set of
/// members with the `Retiring` role.
fn try_state_retiring_members_are_consistent() -> Result<(), sp_runtime::TryRuntimeError> {
let retiring_in_members = Members::<T, I>::get(MemberRole::Retiring);
let retiring_keys_count = RetiringMembers::<T, I>::iter_keys().count();

ensure!(
retiring_in_members.len() == retiring_keys_count,
"Count mismatch between Members<Retiring> and RetiringMembers map"
);

for member in retiring_in_members.iter() {
ensure!(
RetiringMembers::<T, I>::contains_key(member),
"Retiring member not found in RetiringMembers map"
);
}

Ok(())
}

/// # Invariants
///
/// * Every account that has a deposit stored in `DepositOf` must be a member of the
/// alliance (either a `Fellow`, `Ally`, or `Retiring`).
fn try_state_deposit_of_is_consistent() -> Result<(), sp_runtime::TryRuntimeError> {
for (who, _) in DepositOf::<T, I>::iter() {
ensure!(
Self::is_member(&who),
"Account with deposit is not an alliance member"
);
}
Ok(())
}

/// # Invariants
///
/// * The lists of `UnscrupulousAccounts` and `UnscrupulousWebsites` must be sorted. This allows
/// for efficient binary search lookups.
fn try_state_unscrupulous_items_are_sorted() -> Result<(), sp_runtime::TryRuntimeError> {
let accounts = UnscrupulousAccounts::<T, I>::get();
let mut sorted_accounts = accounts.clone();
sorted_accounts.sort();
ensure!(
accounts == sorted_accounts,
"UnscrupulousAccounts is not sorted"
);

let websites = UnscrupulousWebsites::<T, I>::get();
let mut sorted_websites = websites.clone();
sorted_websites.sort();
ensure!(
websites == sorted_websites,
"UnscrupulousWebsites is not sorted"
);

Ok(())
}

/// # Invariants
///
/// * The list of `Announcements` must be sorted. This is necessary because `remove_announcement`
/// uses binary search.
fn try_state_announcements_are_sorted() -> Result<(), sp_runtime::TryRuntimeError> {
let announcements = Announcements::<T, I>::get();
let mut sorted_announcements = announcements.clone();
sorted_announcements.sort();
ensure!(
announcements == sorted_announcements,
"Announcements is not sorted"
);
Ok(())
}
}
7 changes: 7 additions & 0 deletions substrate/frame/alliance/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,13 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
ext
}

pub fn build_and_execute(test: impl FnOnce()) {
new_test_ext().execute_with(|| {
test();
Alliance::do_try_state().expect("All invariants must hold after a test");
})
}

#[cfg(feature = "runtime-benchmarks")]
pub fn new_bench_ext() -> sp_io::TestExternalities {
RuntimeGenesisConfig::default().build_storage().unwrap().into()
Expand Down
34 changes: 17 additions & 17 deletions substrate/frame/alliance/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn assert_powerless(user: RuntimeOrigin, user_is_member: bool) {

#[test]
fn init_members_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
// alliance must be reset first, no witness data
assert_noop!(
Alliance::init_members(RuntimeOrigin::root(), vec![8], vec![],),
Expand Down Expand Up @@ -105,7 +105,7 @@ fn init_members_works() {

#[test]
fn disband_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
let id_deposit = test_identity_info_deposit();
let expected_join_deposit = <Test as Config>::AllyDeposit::get();
assert_eq!(Balances::free_balance(9), 1000 - id_deposit);
Expand Down Expand Up @@ -168,7 +168,7 @@ fn disband_works() {

#[test]
fn propose_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
let (proposal, proposal_len, hash) = make_remark_proposal(42);

// only voting member can propose proposal, 4 is ally not have vote rights
Expand Down Expand Up @@ -208,7 +208,7 @@ fn propose_works() {

#[test]
fn vote_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
let (proposal, proposal_len, hash) = make_remark_proposal(42);
assert_ok!(Alliance::propose(
RuntimeOrigin::signed(1),
Expand Down Expand Up @@ -242,7 +242,7 @@ fn vote_works() {

#[test]
fn close_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
let (proposal, proposal_len, hash) = make_remark_proposal(42);
let proposal_weight = proposal.get_dispatch_info().call_weight;
assert_ok!(Alliance::propose(
Expand Down Expand Up @@ -312,7 +312,7 @@ fn close_works() {

#[test]
fn set_rule_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
let cid = test_cid();
assert_ok!(Alliance::set_rule(RuntimeOrigin::signed(1), cid.clone()));
assert_eq!(alliance::Rule::<Test>::get(), Some(cid.clone()));
Expand All @@ -325,7 +325,7 @@ fn set_rule_works() {

#[test]
fn announce_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
let cid = test_cid();

assert_noop!(Alliance::announce(RuntimeOrigin::signed(2), cid.clone()), BadOrigin);
Expand All @@ -341,7 +341,7 @@ fn announce_works() {

#[test]
fn remove_announcement_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
let cid = test_cid();
assert_ok!(Alliance::announce(RuntimeOrigin::signed(3), cid.clone()));
assert_eq!(alliance::Announcements::<Test>::get(), vec![cid.clone()]);
Expand All @@ -361,7 +361,7 @@ fn remove_announcement_works() {

#[test]
fn join_alliance_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
let id_deposit = test_identity_info_deposit();
let join_deposit = <Test as Config>::AllyDeposit::get();
assert_eq!(Balances::free_balance(9), 1000 - id_deposit);
Expand Down Expand Up @@ -421,7 +421,7 @@ fn join_alliance_works() {

#[test]
fn nominate_ally_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
// check already member
assert_noop!(
Alliance::nominate_ally(RuntimeOrigin::signed(1), 2),
Expand Down Expand Up @@ -476,7 +476,7 @@ fn nominate_ally_works() {

#[test]
fn elevate_ally_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
assert_noop!(
Alliance::elevate_ally(RuntimeOrigin::signed(2), 4),
Error::<Test, ()>::NotAlly
Expand All @@ -494,7 +494,7 @@ fn elevate_ally_works() {

#[test]
fn give_retirement_notice_work() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
assert_noop!(
Alliance::give_retirement_notice(RuntimeOrigin::signed(4)),
Error::<Test, ()>::NotMember
Expand All @@ -517,7 +517,7 @@ fn give_retirement_notice_work() {

#[test]
fn retire_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
assert_noop!(
Alliance::retire(RuntimeOrigin::signed(2)),
Error::<Test, ()>::RetirementNoticeNotGiven
Expand Down Expand Up @@ -551,7 +551,7 @@ fn retire_works() {

#[test]
fn abdicate_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
assert_eq!(alliance::Members::<Test>::get(MemberRole::Fellow), vec![1, 2, 3]);
assert_ok!(Alliance::abdicate_fellow_status(RuntimeOrigin::signed(3)));

Expand All @@ -565,7 +565,7 @@ fn abdicate_works() {

#[test]
fn kick_member_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
assert_noop!(Alliance::kick_member(RuntimeOrigin::signed(4), 4), BadOrigin);

assert_noop!(
Expand All @@ -587,7 +587,7 @@ fn kick_member_works() {

#[test]
fn add_unscrupulous_items_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
assert_noop!(Alliance::add_unscrupulous_items(RuntimeOrigin::signed(2), vec![]), BadOrigin);

assert_ok!(Alliance::add_unscrupulous_items(
Expand Down Expand Up @@ -615,7 +615,7 @@ fn add_unscrupulous_items_works() {

#[test]
fn remove_unscrupulous_items_works() {
new_test_ext().execute_with(|| {
build_and_execute(|| {
assert_noop!(
Alliance::remove_unscrupulous_items(RuntimeOrigin::signed(2), vec![]),
BadOrigin
Expand Down
Loading