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.

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

Adding try_state hook for Tips pallet #1871

Merged
merged 17 commits into from
Jan 20, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
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_1871.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
title: Adding `try-state` hook to tips pallet

doc:
- audience: Runtime User
description: |
Enforces the following invariants;
1. The number of entries in Tips should be equal to Reasons.
2. If OpenTip.finders_fee is true, then OpenTip.deposit should be greater than zero.
3. Reasons exists for each Tip[OpenTip.reason], implying equal length of storage.
Doordashcon marked this conversation as resolved.
Show resolved Hide resolved

crates:
- name: pallet-tips
57 changes: 57 additions & 0 deletions substrate/frame/tips/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ use sp_std::prelude::*;

use codec::{Decode, Encode};
use frame_support::{
ensure,
traits::{
ContainsLengthBound, Currency, EnsureOrigin, ExistenceRequirement::KeepAlive, Get,
OnUnbalanced, ReservableCurrency, SortedMembers,
Expand All @@ -76,6 +77,9 @@ use frame_support::{
};
use frame_system::pallet_prelude::BlockNumberFor;

#[cfg(any(feature = "try-runtime", test))]
use sp_runtime::TryRuntimeError;

pub use pallet::*;
pub use weights::WeightInfo;

Expand Down Expand Up @@ -465,6 +469,21 @@ pub mod pallet {
Ok(())
}
}

#[pallet::hooks]
impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
fn integrity_test() {
assert!(
!T::TipReportDepositBase::get().is_zero(),
Doordashcon marked this conversation as resolved.
Show resolved Hide resolved
"`TipReportDepositBase` should not be zero",
);
}

#[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> {
Expand Down Expand Up @@ -611,4 +630,42 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Tips::<T, I>::insert(hash, new_tip)
}
}

/// Ensure the correctness of the state of this pallet.
///
/// This should be valid before and after each state transition of this pallet.
///
/// ## Invariants:
/// 1. The number of entries in `Tips` should be equal to `Reasons`.
/// 2. Reasons exists for each Tip[`OpenTip.reason`].
/// 3. If `OpenTip.finders_fee` is true, then OpenTip.deposit should be greater than zero.
#[cfg(any(feature = "try-runtime", test))]
pub fn do_try_state() -> Result<(), TryRuntimeError> {
let reasons = Reasons::<T, I>::iter_keys().collect::<Vec<_>>();
let tips = Tips::<T, I>::iter_keys().collect::<Vec<_>>();

ensure!(
reasons.len() == tips.len(),
TryRuntimeError::Other("Equal length of entries in `Tips` and `Reasons` Storage")
Doordashcon marked this conversation as resolved.
Show resolved Hide resolved
);

for tip in Tips::<T, I>::iter_keys() {
let open_tip = Tips::<T, I>::get(&tip).expect("All map keys are valid; qed");

if open_tip.finders_fee {
ensure!(
!open_tip.deposit.is_zero(),
TryRuntimeError::Other(
"Tips with `finders_fee` should have non-zero `deposit`."
)
)
}

ensure!(
reasons.contains(&open_tip.reason),
TryRuntimeError::Other("no reason for this tip")
);
}
Ok(())
}
}
92 changes: 89 additions & 3 deletions substrate/frame/tips/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use frame_support::{
storage::StoragePrefixedMap,
traits::{
tokens::{PayFromAccount, UnityAssetBalanceConversion},
ConstU32, ConstU64, SortedMembers, StorageVersion,
ConstU32, ConstU64, IntegrityTest, SortedMembers, StorageVersion,
},
PalletId,
};
Expand Down Expand Up @@ -189,13 +189,14 @@ impl pallet_treasury::Config<Instance1> for Test {

parameter_types! {
pub const TipFindersFee: Percent = Percent::from_percent(20);
pub static TipReportDepositBase: u64 = 1;
}
impl Config for Test {
type MaximumReasonLength = ConstU32<16384>;
type Tippers = TenToFourteen;
type TipCountdown = ConstU64<1>;
type TipFindersFee = TipFindersFee;
type TipReportDepositBase = ConstU64<1>;
type TipReportDepositBase = TipReportDepositBase;
type DataDepositPerByte = ConstU64<1>;
type MaxTipAmount = ConstU64<10_000_000>;
type RuntimeEvent = RuntimeEvent;
Expand All @@ -207,7 +208,7 @@ impl Config<Instance1> for Test {
type Tippers = TenToFourteen;
type TipCountdown = ConstU64<1>;
type TipFindersFee = TipFindersFee;
type TipReportDepositBase = ConstU64<1>;
type TipReportDepositBase = TipReportDepositBase;
type DataDepositPerByte = ConstU64<1>;
type MaxTipAmount = ConstU64<10_000_000>;
type RuntimeEvent = RuntimeEvent;
Expand Down Expand Up @@ -657,3 +658,88 @@ fn report_awesome_and_tip_works_second_instance() {
assert_eq!(Balances::free_balance(&Treasury1::account_id()), 191);
});
}

#[test]
fn equal_entries_invariant() {
new_test_ext().execute_with(|| {
use frame_support::pallet_prelude::DispatchError::Other;

Balances::make_free_balance_be(&Treasury::account_id(), 101);

assert_ok!(Tips::report_awesome(RuntimeOrigin::signed(0), b"awesome.dot".to_vec(), 3));

let reason1 = BlakeTwo256::hash(b"reason1");
let hash1 = BlakeTwo256::hash_of(&(reason1, 10u64));

let tip = OpenTip::<u128, u64, u64, H256> {
reason: reason1,
who: 10,
finder: 20,
deposit: 30,
closes: Some(13),
tips: vec![(40, 50), (60, 70)],
finders_fee: true,
};

// Breaks invariant by adding an entry to only `Tips` Storage.
pallet_tips::Tips::<Test>::insert(hash1, tip);

// Invariant violated
assert_eq!(
Tips::do_try_state(),
Err(Other("Equal length of entries in `Tips` and `Reasons` Storage"))
);
})
}

#[test]
fn finders_fee_invariant() {
new_test_ext().execute_with(|| {
use frame_support::pallet_prelude::DispatchError::Other;

// Breaks invariant by having a zero deposit.
TipReportDepositBase::set(0);

Balances::make_free_balance_be(&Treasury::account_id(), 101);

assert_ok!(Tips::report_awesome(RuntimeOrigin::signed(0), b"".to_vec(), 3));

// Invariant violated
assert_eq!(
Tips::do_try_state(),
Err(Other("Tips with `finders_fee` should have non-zero `deposit`."))
);
})
}

#[test]
fn reasons_invariant() {
new_test_ext().execute_with(|| {
use frame_support::pallet_prelude::DispatchError::Other;

Balances::make_free_balance_be(&Treasury::account_id(), 101);

assert_ok!(Tips::report_awesome(RuntimeOrigin::signed(0), b"awesome.dot".to_vec(), 0));

let hash: Vec<_> = pallet_tips::Tips::<Test>::iter_keys().collect();

let mut open_tip = pallet_tips::Tips::<Test>::take(hash[0]).unwrap();

// Breaks invariant by changing value `open_tip.reason` in `Tips` Storage.
open_tip.reason = <Test as frame_system::Config>::Hashing::hash(&b"".to_vec());

pallet_tips::Tips::<Test>::insert(hash[0], open_tip);

// Invariant violated
assert_eq!(Tips::do_try_state(), Err(Other("no reason for this tip")));
})
}

#[test]
#[should_panic = "`TipReportDepositBase` should not be zero"]
fn zero_base_deposit_prohibited() {
new_test_ext().execute_with(|| {
TipReportDepositBase::set(0);
Tips::integrity_test();
Doordashcon marked this conversation as resolved.
Show resolved Hide resolved
});
}