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 Bounties #4563

Merged
merged 12 commits into from
Jul 12, 2024
Merged

Try State Hook for Bounties #4563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ebbd603
invariants for bounties, bounties description and bounty count
dharjeezy May 24, 2024
428f0b3
fmt
dharjeezy May 24, 2024
78d9727
nit
dharjeezy May 24, 2024
126904e
pr doc
dharjeezy May 24, 2024
01257fe
nit
dharjeezy May 24, 2024
90b8917
Merge remote-tracking branch 'origin/master' into dami/try-state-hook…
dharjeezy Jun 29, 2024
5335477
Merge remote-tracking branch 'origin/master' into dami/try-state-hook…
dharjeezy Jun 29, 2024
9bf71e0
Update substrate/frame/bounties/src/lib.rs
ggwpez Jul 12, 2024
c899e2d
Update substrate/frame/bounties/src/lib.rs
ggwpez Jul 12, 2024
a964908
Merge branch 'master' into dami/try-state-hook-pallet-bounties
ggwpez Jul 12, 2024
2e6e975
fix
ggwpez Jul 12, 2024
120de38
fix prdoc
ggwpez Jul 12, 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
12 changes: 12 additions & 0 deletions prdoc/pr_4563.prdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
title: Try State Hook for Bounties.

doc:
- audience: Runtime User
description: |
Invariants for storage items in the bounties pallet. Enforces the following Invariants:
1.`BountyCount` should be greater or equals to the length of the number of items in `Bounties`.
2.`BountyCount` should be greater or equals to the length of the number of items in `BountyDescriptions`.
3. Number of items in `Bounties` should be the same as `BountyDescriptions` length.
crates:
- name: pallet-bounties
bump: minor
2 changes: 1 addition & 1 deletion substrate/frame/bounties/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,5 +231,5 @@ benchmarks_instance_pallet! {
}
}

impl_benchmark_test_suite!(Bounties, crate::tests::new_test_ext(), crate::tests::Test)
impl_benchmark_test_suite!(Bounties, crate::tests::ExtBuilder::default().build(), crate::tests::Test)
}
48 changes: 48 additions & 0 deletions substrate/frame/bounties/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,54 @@ pub mod pallet {
Ok(())
}
}

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

#[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_bounties_count()?;

Ok(())
}

/// # Invariants
///
/// * `BountyCount` should be greater or equals to the length of the number of items in
/// `Bounties`.
/// * `BountyCount` should be greater or equals to the length of the number of items in
/// `BountyDescriptions`.
/// * Number of items in `Bounties` should be the same as `BountyDescriptions` length.
fn try_state_bounties_count() -> Result<(), sp_runtime::TryRuntimeError> {
let bounties_length = Bounties::<T, I>::iter().count() as u32;

ensure!(
<BountyCount<T, I>>::get() >= bounties_length,
"`BountyCount` must be grater or equals the number of `Bounties` in storage"
);

let bounties_description_length = BountyDescriptions::<T, I>::iter().count() as u32;
ensure!(
<BountyCount<T, I>>::get() >= bounties_description_length,
"`BountyCount` must be grater or equals the number of `BountiesDescriptions` in storage."
);

ensure!(
bounties_length == bounties_description_length,
"Number of `Bounties` in storage must be the same as the Number of `BountiesDescription` in storage."
);
Ok(())
}
}

impl<T: Config<I>, I: 'static> Pallet<T, I> {
Expand Down
92 changes: 55 additions & 37 deletions substrate/frame/bounties/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,18 +167,36 @@ impl Config<Instance1> for Test {
type TreasuryError = pallet_treasury::Error<Test>;
type TreasuryError1 = pallet_treasury::Error<Test, Instance1>;

pub fn new_test_ext() -> sp_io::TestExternalities {
let mut ext: sp_io::TestExternalities = RuntimeGenesisConfig {
system: frame_system::GenesisConfig::default(),
balances: pallet_balances::GenesisConfig { balances: vec![(0, 100), (1, 98), (2, 1)] },
treasury: Default::default(),
treasury_1: Default::default(),
pub struct ExtBuilder {}

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

impl ExtBuilder {
pub fn build(self) -> sp_io::TestExternalities {
let mut ext: sp_io::TestExternalities = RuntimeGenesisConfig {
system: frame_system::GenesisConfig::default(),
balances: pallet_balances::GenesisConfig { balances: vec![(0, 100), (1, 98), (2, 1)] },
treasury: Default::default(),
treasury_1: Default::default(),
}
.build_storage()
.unwrap()
.into();
ext.execute_with(|| System::set_block_number(1));
ext
}

pub fn build_and_execute(self, test: impl FnOnce() -> ()) {
self.build().execute_with(|| {
test();
Bounties::do_try_state().expect("All invariants must hold after a test");
Bounties1::do_try_state().expect("All invariants must hold after a test");
})
}
.build_storage()
.unwrap()
.into();
ext.execute_with(|| System::set_block_number(1));
ext
}

fn last_event() -> BountiesEvent<Test> {
Expand All @@ -192,15 +210,15 @@ fn last_event() -> BountiesEvent<Test> {

#[test]
fn genesis_config_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
assert_eq!(Treasury::pot(), 0);
assert_eq!(Treasury::proposal_count(), 0);
});
}

#[test]
fn minting_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
// Check that accumulate works when we have Some value in Dummy already.
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Treasury::pot(), 100);
Expand All @@ -209,7 +227,7 @@ fn minting_works() {

#[test]
fn accepted_spend_proposal_ignored_outside_spend_period() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);

assert_ok!({ Treasury::spend_local(RuntimeOrigin::root(), 100, 3) });
Expand All @@ -222,7 +240,7 @@ fn accepted_spend_proposal_ignored_outside_spend_period() {

#[test]
fn unused_pot_should_diminish() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
let init_total_issuance = Balances::total_issuance();
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::total_issuance(), init_total_issuance + 100);
Expand All @@ -235,7 +253,7 @@ fn unused_pot_should_diminish() {

#[test]
fn accepted_spend_proposal_enacted_on_spend_period() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Treasury::pot(), 100);

Expand All @@ -249,7 +267,7 @@ fn accepted_spend_proposal_enacted_on_spend_period() {

#[test]
fn pot_underflow_should_not_diminish() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Treasury::pot(), 100);

Expand All @@ -269,7 +287,7 @@ fn pot_underflow_should_not_diminish() {
// i.e. pot should not include existential deposit needed for account survival.
#[test]
fn treasury_account_doesnt_get_deleted() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Treasury::pot(), 100);
let treasury_balance = Balances::free_balance(&Treasury::account_id());
Expand Down Expand Up @@ -321,7 +339,7 @@ fn inexistent_account_works() {

#[test]
fn propose_bounty_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);

Balances::make_free_balance_be(&Treasury::account_id(), 101);
Expand Down Expand Up @@ -358,7 +376,7 @@ fn propose_bounty_works() {

#[test]
fn propose_bounty_validation_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);

Balances::make_free_balance_be(&Treasury::account_id(), 101);
Expand Down Expand Up @@ -387,7 +405,7 @@ fn propose_bounty_validation_works() {

#[test]
fn close_bounty_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_noop!(Bounties::close_bounty(RuntimeOrigin::root(), 0), Error::<Test>::InvalidIndex);
Expand All @@ -412,7 +430,7 @@ fn close_bounty_works() {

#[test]
fn approve_bounty_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_noop!(
Expand Down Expand Up @@ -473,7 +491,7 @@ fn approve_bounty_works() {

#[test]
fn assign_curator_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);

Expand Down Expand Up @@ -543,7 +561,7 @@ fn assign_curator_works() {

#[test]
fn unassign_curator_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
Expand Down Expand Up @@ -596,7 +614,7 @@ fn unassign_curator_works() {

#[test]
fn award_and_claim_bounty_works() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Balances::make_free_balance_be(&4, 10);
Expand Down Expand Up @@ -663,7 +681,7 @@ fn award_and_claim_bounty_works() {

#[test]
fn claim_handles_high_fee() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Balances::make_free_balance_be(&4, 30);
Expand Down Expand Up @@ -704,7 +722,7 @@ fn claim_handles_high_fee() {

#[test]
fn cancel_and_refund() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);

Balances::make_free_balance_be(&Treasury::account_id(), 101);
Expand Down Expand Up @@ -747,7 +765,7 @@ fn cancel_and_refund() {

#[test]
fn award_and_cancel() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
Expand Down Expand Up @@ -790,7 +808,7 @@ fn award_and_cancel() {

#[test]
fn expire_and_unassign() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
Expand Down Expand Up @@ -838,7 +856,7 @@ fn expire_and_unassign() {

#[test]
fn extend_expiry() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Balances::make_free_balance_be(&4, 10);
Expand Down Expand Up @@ -974,7 +992,7 @@ fn genesis_funding_works() {

#[test]
fn unassign_curator_self() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
Expand Down Expand Up @@ -1015,7 +1033,7 @@ fn unassign_curator_self() {
fn accept_curator_handles_different_deposit_calculations() {
// This test will verify that a bounty with and without a fee results
// in a different curator deposit: one using the value, and one using the fee.
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
// Case 1: With a fee
let user = 1;
let bounty_index = 0;
Expand Down Expand Up @@ -1092,7 +1110,7 @@ fn accept_curator_handles_different_deposit_calculations() {

#[test]
fn approve_bounty_works_second_instance() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
// Set burn to 0 to make tracking funds easier.
Burn::set(Permill::from_percent(0));

Expand All @@ -1118,7 +1136,7 @@ fn approve_bounty_works_second_instance() {

#[test]
fn approve_bounty_insufficient_spend_limit_errors() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);

Balances::make_free_balance_be(&Treasury::account_id(), 101);
Expand All @@ -1136,7 +1154,7 @@ fn approve_bounty_insufficient_spend_limit_errors() {

#[test]
fn approve_bounty_instance1_insufficient_spend_limit_errors() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);

Balances::make_free_balance_be(&Treasury1::account_id(), 101);
Expand All @@ -1154,7 +1172,7 @@ fn approve_bounty_instance1_insufficient_spend_limit_errors() {

#[test]
fn propose_curator_insufficient_spend_limit_errors() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);

Expand All @@ -1177,7 +1195,7 @@ fn propose_curator_insufficient_spend_limit_errors() {

#[test]
fn propose_curator_instance1_insufficient_spend_limit_errors() {
new_test_ext().execute_with(|| {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);

Expand Down
Loading