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 treasury #4962

Merged
14 changes: 14 additions & 0 deletions prdoc/pr_4962.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# 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: Removed `pallet::getter` usage from the pallet-treasury

doc:
- audience: Runtime Dev
description: |
This PR removed `pallet::getter`s from `pallet-treasury`s storage items.
When accessed inside the pallet, use the syntax `StorageItem::<T, I>::get()`.
Polkaverse marked this conversation as resolved.
Show resolved Hide resolved

crates:
- name: pallet-treasury
bump: minor
Polkaverse marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions substrate/frame/treasury/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ fn create_approved_proposals<T: Config<I>, I: 'static>(n: u32) -> Result<(), &'s
let (_, value, lookup) = setup_proposal::<T, I>(i);
Treasury::<T, I>::spend_local(origin.clone(), value, lookup)?;
}
ensure!(<Approvals<T, I>>::get().len() == n as usize, "Not all approved");
ensure!(Approvals::<T, I>::get().len() == n as usize, "Not all approved");
Ok(())
}

Expand Down Expand Up @@ -130,7 +130,7 @@ mod benchmarks {
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
let (_, value, beneficiary_lookup) = setup_proposal::<T, _>(SEED);
Treasury::<T, _>::spend_local(origin, value, beneficiary_lookup)?;
let proposal_id = Treasury::<T, _>::proposal_count() - 1;
let proposal_id = ProposalCount::<T, _>::get() - 1;
let reject_origin =
T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;

Expand Down
34 changes: 23 additions & 11 deletions substrate/frame/treasury/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ use frame_support::{
ReservableCurrency, WithdrawReasons,
},
weights::Weight,
PalletId,
BoundedVec, PalletId,
};

pub use pallet::*;
Expand Down Expand Up @@ -278,12 +278,10 @@ pub mod pallet {

/// Number of proposals that have been made.
#[pallet::storage]
#[pallet::getter(fn proposal_count)]
pub(crate) type ProposalCount<T, I = ()> = StorageValue<_, ProposalIndex, ValueQuery>;
pub type ProposalCount<T, I = ()> = StorageValue<_, ProposalIndex, ValueQuery>;

/// Proposals that have been made.
#[pallet::storage]
#[pallet::getter(fn proposals)]
pub type Proposals<T: Config<I>, I: 'static = ()> = StorageMap<
_,
Twox64Concat,
Expand All @@ -299,7 +297,6 @@ pub mod pallet {

/// Proposal indices that have been approved but not yet awarded.
#[pallet::storage]
#[pallet::getter(fn approvals)]
pub type Approvals<T: Config<I>, I: 'static = ()> =
StorageValue<_, BoundedVec<ProposalIndex, T::MaxApprovals>, ValueQuery>;

Expand Down Expand Up @@ -335,7 +332,7 @@ pub mod pallet {
impl<T: Config<I>, I: 'static> BuildGenesisConfig for GenesisConfig<T, I> {
fn build(&self) {
// Create Treasury account
let account_id = <Pallet<T, I>>::account_id();
let account_id = Pallet::<T, I>::account_id();
let min = T::Currency::minimum_balance();
if T::Currency::free_balance(&account_id) < min {
let _ = T::Currency::make_free_balance_be(&account_id, min);
Expand Down Expand Up @@ -501,7 +498,7 @@ pub mod pallet {
.unwrap_or(Ok(()))?;

let beneficiary = T::Lookup::lookup(beneficiary)?;
let proposal_index = Self::proposal_count();
let proposal_index = ProposalCount::<T, I>::get();
Approvals::<T, I>::try_append(proposal_index)
.map_err(|_| Error::<T, I>::TooManyApprovals)?;
let proposal = Proposal {
Expand Down Expand Up @@ -794,6 +791,21 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
T::PalletId::get().into_account_truncating()
}

/// Public function to proposal_count storage.
pub fn proposal_count() -> ProposalIndex {
ProposalCount::<T, I>::get()
}

/// Public function to proposals storage.
pub fn proposals(index: ProposalIndex) -> Option<Proposal<T::AccountId, BalanceOf<T, I>>> {
Proposals::<T, I>::get(index)
}

/// Public function to approvals storage.
pub fn approvals() -> BoundedVec<ProposalIndex, T::MaxApprovals> {
Approvals::<T, I>::get()
}

/// Spend some money! returns number of approvals before spend.
pub fn spend_funds() -> Weight {
let mut total_weight = Weight::zero();
Expand All @@ -803,15 +815,15 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let account_id = Self::account_id();

let mut missed_any = false;
let mut imbalance = <PositiveImbalanceOf<T, I>>::zero();
let mut imbalance = PositiveImbalanceOf::<T, I>::zero();
let proposals_len = Approvals::<T, I>::mutate(|v| {
let proposals_approvals_len = v.len() as u32;
v.retain(|&index| {
// Should always be true, but shouldn't panic if false or we're screwed.
if let Some(p) = Self::proposals(index) {
if let Some(p) = Proposals::<T, I>::get(index) {
if p.value <= budget_remaining {
budget_remaining -= p.value;
<Proposals<T, I>>::remove(index);
Proposals::<T, I>::remove(index);

// return their deposit.
let err_amount = T::Currency::unreserve(&p.proposer, p.bond);
Expand Down Expand Up @@ -982,6 +994,6 @@ where
{
type Type = <R as frame_system::Config>::AccountId;
fn get() -> Self::Type {
<crate::Pallet<R>>::account_id()
crate::Pallet::<R>::account_id()
}
}
6 changes: 3 additions & 3 deletions substrate/frame/treasury/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ fn get_payment_id(i: SpendIndex) -> Option<u64> {
fn genesis_config_works() {
ExtBuilder::default().build().execute_with(|| {
assert_eq!(Treasury::pot(), 0);
assert_eq!(Treasury::proposal_count(), 0);
assert_eq!(ProposalCount::<Test>::get(), 0);
});
}

Expand Down Expand Up @@ -437,9 +437,9 @@ fn remove_already_removed_approval_fails() {

assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(14), 100, 3));

assert_eq!(Treasury::approvals(), vec![0]);
assert_eq!(Approvals::<Test>::get(), vec![0]);
assert_ok!(Treasury::remove_approval(RuntimeOrigin::root(), 0));
assert_eq!(Treasury::approvals(), vec![]);
assert_eq!(Approvals::<Test>::get(), vec![]);

assert_noop!(
Treasury::remove_approval(RuntimeOrigin::root(), 0),
Expand Down
Loading