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

Remove pallet::getter usage from the bounties and child-bounties pallets #4392

Merged
16 changes: 16 additions & 0 deletions prdoc/pr_4392.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Remove `pallet::getter` usage from both bounties and child bounties pallet

doc:
- audience: Runtime Dev
description: |
This PR removes `pallet::getter`s from `pallet-bounties` and `pallet-child-bounties`.
The syntax `StorageItem::<T, I>::get()` should be used instead.

crates:
- name: pallet-bounties
bump: major
- name: pallet-child-bounties
bump: major
6 changes: 1 addition & 5 deletions substrate/frame/bounties/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,12 +303,10 @@ pub mod pallet {

/// Number of bounty proposals that have been made.
#[pallet::storage]
#[pallet::getter(fn bounty_count)]
pub type BountyCount<T: Config<I>, I: 'static = ()> = StorageValue<_, BountyIndex, ValueQuery>;

/// Bounties that have been made.
#[pallet::storage]
#[pallet::getter(fn bounties)]
pub type Bounties<T: Config<I>, I: 'static = ()> = StorageMap<
_,
Twox64Concat,
Expand All @@ -318,13 +316,11 @@ pub mod pallet {

/// The description of each bounty.
#[pallet::storage]
#[pallet::getter(fn bounty_descriptions)]
pub type BountyDescriptions<T: Config<I>, I: 'static = ()> =
StorageMap<_, Twox64Concat, BountyIndex, BoundedVec<u8, T::MaximumReasonLength>>;

/// Bounty indices that have been approved but not yet funded.
#[pallet::storage]
#[pallet::getter(fn bounty_approvals)]
pub type BountyApprovals<T: Config<I>, I: 'static = ()> =
StorageValue<_, BoundedVec<BountyIndex, T::MaxApprovals>, ValueQuery>;

Expand Down Expand Up @@ -849,7 +845,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
description.try_into().map_err(|_| Error::<T, I>::ReasonTooBig)?;
ensure!(value >= T::BountyValueMinimum::get(), Error::<T, I>::InvalidValue);

let index = Self::bounty_count();
let index = BountyCount::<T, I>::get();

// reserve deposit for new bounty
let bond = T::BountyDepositBase::get() +
Expand Down
51 changes: 27 additions & 24 deletions substrate/frame/bounties/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ fn propose_bounty_works() {
assert_eq!(Balances::free_balance(0), 100 - deposit);

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee: 0,
Expand All @@ -545,9 +545,12 @@ fn propose_bounty_works() {
}
);

assert_eq!(Bounties::bounty_descriptions(0).unwrap(), b"1234567890".to_vec());
assert_eq!(
pallet_bounties::BountyDescriptions::<Test>::get(0).unwrap(),
b"1234567890".to_vec()
);

assert_eq!(Bounties::bounty_count(), 1);
assert_eq!(pallet_bounties::BountyCount::<Test>::get(), 1);
});
}

Expand Down Expand Up @@ -598,10 +601,10 @@ fn close_bounty_works() {
assert_eq!(Balances::reserved_balance(0), 0);
assert_eq!(Balances::free_balance(0), 100 - deposit);

assert_eq!(Bounties::bounties(0), None);
assert_eq!(pallet_bounties::Bounties::<Test>::get(0), None);
assert!(!pallet_treasury::Proposals::<Test>::contains_key(0));

assert_eq!(Bounties::bounty_descriptions(0), None);
assert_eq!(pallet_bounties::BountyDescriptions::<Test>::get(0), None);
});
}

Expand All @@ -622,7 +625,7 @@ fn approve_bounty_works() {
let deposit: u64 = 80 + 5;

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee: 0,
Expand All @@ -632,7 +635,7 @@ fn approve_bounty_works() {
status: BountyStatus::Approved,
}
);
assert_eq!(Bounties::bounty_approvals(), vec![0]);
assert_eq!(pallet_bounties::BountyApprovals::<Test>::get(), vec![0]);

assert_noop!(
Bounties::close_bounty(RuntimeOrigin::root(), 0),
Expand All @@ -650,7 +653,7 @@ fn approve_bounty_works() {
assert_eq!(Balances::free_balance(0), 100);

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee: 0,
Expand Down Expand Up @@ -693,7 +696,7 @@ fn assign_curator_works() {
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, fee));

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee,
Expand All @@ -720,7 +723,7 @@ fn assign_curator_works() {
let expected_deposit = Bounties::calculate_curator_deposit(&fee);

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee,
Expand Down Expand Up @@ -755,7 +758,7 @@ fn unassign_curator_works() {
assert_ok!(Bounties::unassign_curator(RuntimeOrigin::signed(4), 0));

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee,
Expand All @@ -773,7 +776,7 @@ fn unassign_curator_works() {
assert_ok!(Bounties::unassign_curator(RuntimeOrigin::root(), 0));

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee,
Expand Down Expand Up @@ -817,7 +820,7 @@ fn award_and_claim_bounty_works() {
assert_ok!(Bounties::award_bounty(RuntimeOrigin::signed(4), 0, 3));

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee,
Expand Down Expand Up @@ -851,8 +854,8 @@ fn award_and_claim_bounty_works() {
assert_eq!(Balances::free_balance(3), 56);
assert_eq!(Balances::free_balance(Bounties::bounty_account_id(0)), 0);

assert_eq!(Bounties::bounties(0), None);
assert_eq!(Bounties::bounty_descriptions(0), None);
assert_eq!(pallet_bounties::Bounties::<Test>::get(0), None);
assert_eq!(pallet_bounties::BountyDescriptions::<Test>::get(0), None);
});
}

Expand Down Expand Up @@ -892,8 +895,8 @@ fn claim_handles_high_fee() {
assert_eq!(Balances::free_balance(3), 0);
assert_eq!(Balances::free_balance(Bounties::bounty_account_id(0)), 0);

assert_eq!(Bounties::bounties(0), None);
assert_eq!(Bounties::bounty_descriptions(0), None);
assert_eq!(pallet_bounties::Bounties::<Test>::get(0), None);
assert_eq!(pallet_bounties::BountyDescriptions::<Test>::get(0), None);
});
}

Expand All @@ -918,7 +921,7 @@ fn cancel_and_refund() {
));

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee: 0,
Expand Down Expand Up @@ -978,8 +981,8 @@ fn award_and_cancel() {
assert_eq!(Balances::free_balance(0), 95);
assert_eq!(Balances::reserved_balance(0), 0);

assert_eq!(Bounties::bounties(0), None);
assert_eq!(Bounties::bounty_descriptions(0), None);
assert_eq!(pallet_bounties::Bounties::<Test>::get(0), None);
assert_eq!(pallet_bounties::BountyDescriptions::<Test>::get(0), None);
});
}

Expand Down Expand Up @@ -1015,7 +1018,7 @@ fn expire_and_unassign() {
assert_ok!(Bounties::unassign_curator(RuntimeOrigin::signed(0), 0));

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee: 10,
Expand Down Expand Up @@ -1065,7 +1068,7 @@ fn extend_expiry() {
assert_ok!(Bounties::extend_bounty_expiry(RuntimeOrigin::signed(4), 0, Vec::new()));

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee: 10,
Expand All @@ -1079,7 +1082,7 @@ fn extend_expiry() {
assert_ok!(Bounties::extend_bounty_expiry(RuntimeOrigin::signed(4), 0, Vec::new()));

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee: 10,
Expand Down Expand Up @@ -1190,7 +1193,7 @@ fn unassign_curator_self() {
assert_ok!(Bounties::unassign_curator(RuntimeOrigin::signed(1), 0));

assert_eq!(
Bounties::bounties(0).unwrap(),
pallet_bounties::Bounties::<Test>::get(0).unwrap(),
Bounty {
proposer: 0,
fee: 10,
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/child-bounties/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ fn activate_bounty<T: Config>(
child_bounty_setup.reason.clone(),
)?;

child_bounty_setup.bounty_id = Bounties::<T>::bounty_count() - 1;
child_bounty_setup.bounty_id = pallet_bounties::BountyCount::<T>::get() - 1;

let approve_origin =
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Expand Down
29 changes: 12 additions & 17 deletions substrate/frame/child-bounties/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,19 +181,16 @@ pub mod pallet {

/// Number of total child bounties.
#[pallet::storage]
#[pallet::getter(fn child_bounty_count)]
pub type ChildBountyCount<T: Config> = StorageValue<_, BountyIndex, ValueQuery>;

/// Number of child bounties per parent bounty.
/// Map of parent bounty index to number of child bounties.
#[pallet::storage]
#[pallet::getter(fn parent_child_bounties)]
pub type ParentChildBounties<T: Config> =
StorageMap<_, Twox64Concat, BountyIndex, u32, ValueQuery>;

/// Child bounties that have been added.
#[pallet::storage]
#[pallet::getter(fn child_bounties)]
pub type ChildBounties<T: Config> = StorageDoubleMap<
_,
Twox64Concat,
Expand All @@ -205,13 +202,11 @@ pub mod pallet {

/// The description of each child-bounty.
#[pallet::storage]
#[pallet::getter(fn child_bounty_descriptions)]
pub type ChildBountyDescriptions<T: Config> =
StorageMap<_, Twox64Concat, BountyIndex, BoundedVec<u8, T::MaximumReasonLength>>;

/// The cumulative child-bounty curator fee for each parent bounty.
#[pallet::storage]
#[pallet::getter(fn children_curator_fees)]
pub type ChildrenCuratorFees<T: Config> =
StorageMap<_, Twox64Concat, BountyIndex, BalanceOf<T>, ValueQuery>;

Expand Down Expand Up @@ -251,7 +246,7 @@ pub mod pallet {
description.try_into().map_err(|_| BountiesError::<T>::ReasonTooBig)?;
ensure!(value >= T::ChildBountyValueMinimum::get(), BountiesError::<T>::InvalidValue);
ensure!(
Self::parent_child_bounties(parent_bounty_id) <=
ParentChildBounties::<T>::get(parent_bounty_id) <=
T::MaxActiveChildBountyCount::get() as u32,
Error::<T>::TooManyChildBounties,
);
Expand All @@ -276,15 +271,15 @@ pub mod pallet {
)?;

// Get child-bounty ID.
let child_bounty_id = Self::child_bounty_count();
let child_bounty_id = ChildBountyCount::<T>::get();
let child_bounty_account = Self::child_bounty_account_id(child_bounty_id);

// Transfer funds from parent bounty to child-bounty.
T::Currency::transfer(&parent_bounty_account, &child_bounty_account, value, KeepAlive)?;

// Increment the active child-bounty count.
<ParentChildBounties<T>>::mutate(parent_bounty_id, |count| count.saturating_inc());
<ChildBountyCount<T>>::put(child_bounty_id.saturating_add(1));
ParentChildBounties::<T>::mutate(parent_bounty_id, |count| count.saturating_inc());
ChildBountyCount::<T>::put(child_bounty_id.saturating_add(1));

// Create child-bounty instance.
Self::create_child_bounty(
Expand Down Expand Up @@ -710,12 +705,12 @@ pub mod pallet {
});

// Update the active child-bounty tracking count.
<ParentChildBounties<T>>::mutate(parent_bounty_id, |count| {
ParentChildBounties::<T>::mutate(parent_bounty_id, |count| {
count.saturating_dec()
});

// Remove the child-bounty description.
<ChildBountyDescriptions<T>>::remove(child_bounty_id);
ChildBountyDescriptions::<T>::remove(child_bounty_id);

// Remove the child-bounty instance from the state.
*maybe_child_bounty = None;
Expand Down Expand Up @@ -817,7 +812,7 @@ impl<T: Config> Pallet<T> {
fn ensure_bounty_active(
bounty_id: BountyIndex,
) -> Result<(T::AccountId, BlockNumberFor<T>), DispatchError> {
let parent_bounty = pallet_bounties::Pallet::<T>::bounties(bounty_id)
let parent_bounty = pallet_bounties::Bounties::<T>::get(bounty_id)
.ok_or(BountiesError::<T>::InvalidIndex)?;
if let BountyStatus::Active { curator, update_due } = parent_bounty.get_status() {
Ok((curator, update_due))
Expand Down Expand Up @@ -862,7 +857,7 @@ impl<T: Config> Pallet<T> {
ChildrenCuratorFees::<T>::mutate(parent_bounty_id, |value| {
*value = value.saturating_sub(child_bounty.fee)
});
<ParentChildBounties<T>>::mutate(parent_bounty_id, |count| {
ParentChildBounties::<T>::mutate(parent_bounty_id, |count| {
*count = count.saturating_sub(1)
});

Expand All @@ -880,7 +875,7 @@ impl<T: Config> Pallet<T> {
debug_assert!(transfer_result.is_ok());

// Remove the child-bounty description.
<ChildBountyDescriptions<T>>::remove(child_bounty_id);
ChildBountyDescriptions::<T>::remove(child_bounty_id);

*maybe_child_bounty = None;

Expand All @@ -901,14 +896,14 @@ impl<T: Config> pallet_bounties::ChildBountyManager<BalanceOf<T>> for Pallet<T>
fn child_bounties_count(
bounty_id: pallet_bounties::BountyIndex,
) -> pallet_bounties::BountyIndex {
Self::parent_child_bounties(bounty_id)
ParentChildBounties::<T>::get(bounty_id)
}

fn children_curator_fees(bounty_id: pallet_bounties::BountyIndex) -> BalanceOf<T> {
// This is asked for when the parent bounty is being claimed. No use of
// keeping it in state after that. Hence removing.
let children_fee_total = Self::children_curator_fees(bounty_id);
<ChildrenCuratorFees<T>>::remove(bounty_id);
let children_fee_total = ChildrenCuratorFees::<T>::get(bounty_id);
ChildrenCuratorFees::<T>::remove(bounty_id);
children_fee_total
}
}
Loading
Loading