Skip to content

Commit

Permalink
Remove pallet::getter usage from treasury (#4962)
Browse files Browse the repository at this point in the history
ISSUE
Link to the issue:
#3326

Deliverables

[Deprecation] remove pallet::getter usage from pallet-treasury

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Dónal Murray <donal.murray@parity.io>
Co-authored-by: Dónal Murray <donalm@seadanda.dev>
Co-authored-by: Bastian Köcher <git@kchr.de>
  • Loading branch information
5 people authored Aug 28, 2024
1 parent 9423718 commit 4096ad7
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 16 deletions.
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.
Instead use the syntax `StorageItem::<T, I>::get()`.

crates:
- name: pallet-treasury
bump: minor
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

0 comments on commit 4096ad7

Please sign in to comment.