removed pallet::getter from example pallets (#3371)

part of #3326 

@ggwpez @kianenigma @shawntabrizi

---------

Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
This commit is contained in:
Matteo Muraca
2024-02-22 00:02:31 +01:00
committed by GitHub
parent 318fed32f8
commit cd91c6b782
12 changed files with 41 additions and 36 deletions
@@ -32,7 +32,6 @@ pub mod pallet {
// The pallet's runtime storage items. // The pallet's runtime storage items.
// https://docs.substrate.io/v3/runtime/storage // https://docs.substrate.io/v3/runtime/storage
#[pallet::storage] #[pallet::storage]
#[pallet::getter(fn something)]
// Learn more about declaring storage items: // Learn more about declaring storage items:
// https://docs.substrate.io/v3/runtime/storage#declaring-storage-items // https://docs.substrate.io/v3/runtime/storage#declaring-storage-items
pub type Something<T> = StorageValue<_, u32>; pub type Something<T> = StorageValue<_, u32>;
@@ -1,4 +1,4 @@
use crate::{mock::*, Error}; use crate::{mock::*, Error, Something};
use frame_support::{assert_noop, assert_ok}; use frame_support::{assert_noop, assert_ok};
#[test] #[test]
@@ -7,7 +7,7 @@ fn it_works_for_default_value() {
// Dispatch a signed extrinsic. // Dispatch a signed extrinsic.
assert_ok!(TemplateModule::do_something(RuntimeOrigin::signed(1), 42)); assert_ok!(TemplateModule::do_something(RuntimeOrigin::signed(1), 42));
// Read pallet storage and assert an expected result. // Read pallet storage and assert an expected result.
assert_eq!(TemplateModule::something(), Some(42)); assert_eq!(Something::<Test>::get(), Some(42));
}); });
} }
+19
View File
@@ -0,0 +1,19 @@
# 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` 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
@@ -90,9 +90,7 @@ pub mod pallet {
/// ///
/// In this template, we are declaring a storage item called `Something` that stores a single /// 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/> /// `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::storage]
#[pallet::getter(fn something)]
pub type Something<T> = StorageValue<_, u32>; pub type Something<T> = StorageValue<_, u32>;
/// Events that functions in this pallet can emit. /// Events that functions in this pallet can emit.
@@ -187,7 +185,7 @@ pub mod pallet {
let _who = ensure_signed(origin)?; let _who = ensure_signed(origin)?;
// Read a value from storage. // Read a value from storage.
match Pallet::<T>::something() { match Something::<T>::get() {
// Return an error if the value has not been set. // Return an error if the value has not been set.
None => Err(Error::<T>::NoneValue.into()), None => Err(Error::<T>::NoneValue.into()),
Some(old) => { Some(old) => {
@@ -1,4 +1,4 @@
use crate::{mock::*, Error, Event}; use crate::{mock::*, Error, Event, Something};
use frame_support::{assert_noop, assert_ok}; use frame_support::{assert_noop, assert_ok};
#[test] #[test]
@@ -9,7 +9,7 @@ fn it_works_for_default_value() {
// Dispatch a signed extrinsic. // Dispatch a signed extrinsic.
assert_ok!(TemplateModule::do_something(RuntimeOrigin::signed(1), 42)); assert_ok!(TemplateModule::do_something(RuntimeOrigin::signed(1), 42));
// Read pallet storage and assert an expected result. // 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 // Assert that the correct event was deposited
System::assert_last_event(Event::SomethingStored { something: 42, who: 1 }.into()); System::assert_last_event(Event::SomethingStored { something: 42, who: 1 }.into());
}); });
@@ -48,7 +48,7 @@ mod benchmarks {
set_dummy(RawOrigin::Root, value); // The execution phase is just running `set_dummy` extrinsic call 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. // 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 // An example method that returns a Result that can be called within a benchmark
+3 -11
View File
@@ -286,9 +286,7 @@ pub mod pallet {
let _sender = ensure_signed(origin)?; let _sender = ensure_signed(origin)?;
// Read the value of dummy from storage. // Read the value of dummy from storage.
// let dummy = Self::dummy(); // let dummy = Dummy::<T>::get();
// Will also work using the `::get` on the storage item type itself:
// let dummy = <Dummy<T>>::get();
// Calculate the new value. // Calculate the new value.
// let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by); // let new_dummy = dummy.map_or(increase_by, |dummy| dummy + increase_by);
@@ -381,20 +379,14 @@ pub mod pallet {
// - `Foo::put(1); Foo::get()` returns `1`; // - `Foo::put(1); Foo::get()` returns `1`;
// - `Foo::kill(); Foo::get()` returns `0` (u32::default()). // - `Foo::kill(); Foo::get()` returns `0` (u32::default()).
#[pallet::storage] #[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>; pub(super) type Dummy<T: Config> = StorageValue<_, T::Balance>;
// A map that has enumerable entries. // A map that has enumerable entries.
#[pallet::storage] #[pallet::storage]
#[pallet::getter(fn bar)]
pub(super) type Bar<T: Config> = StorageMap<_, Blake2_128Concat, T::AccountId, T::Balance>; 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. // this one uses the query kind: `ValueQuery`, we'll demonstrate the usage of 'mutate' API.
#[pallet::storage] #[pallet::storage]
#[pallet::getter(fn foo)]
pub(super) type Foo<T: Config> = StorageValue<_, T::Balance, ValueQuery>; pub(super) type Foo<T: Config> = StorageValue<_, T::Balance, ValueQuery>;
#[pallet::storage] #[pallet::storage]
@@ -433,10 +425,10 @@ impl<T: Config> Pallet<T> {
fn accumulate_foo(origin: T::RuntimeOrigin, increase_by: T::Balance) -> DispatchResult { fn accumulate_foo(origin: T::RuntimeOrigin, increase_by: T::Balance) -> DispatchResult {
let _sender = ensure_signed(origin)?; 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 // Because Foo has 'default', the type of 'foo' in closure is the raw type instead of an
// Option<> type. // Option<> type.
let result = <Foo<T>>::mutate(|foo| { let result = Foo::<T>::mutate(|foo| {
*foo = foo.saturating_add(increase_by); *foo = foo.saturating_add(increase_by);
*foo *foo
}); });
+6 -6
View File
@@ -119,25 +119,25 @@ fn it_works_for_optional_value() {
// Check that GenesisBuilder works properly. // Check that GenesisBuilder works properly.
let val1 = 42; let val1 = 42;
let val2 = 27; 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. // Check that accumulate works when we have Some value in Dummy already.
assert_ok!(Example::accumulate_dummy(RuntimeOrigin::signed(1), val2)); 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. // Check that accumulate works when we Dummy has None in it.
<Example as OnInitialize<u64>>::on_initialize(2); <Example as OnInitialize<u64>>::on_initialize(2);
assert_ok!(Example::accumulate_dummy(RuntimeOrigin::signed(1), val1)); 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] #[test]
fn it_works_for_default_value() { fn it_works_for_default_value() {
new_test_ext().execute_with(|| { 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_ok!(Example::accumulate_foo(RuntimeOrigin::signed(1), 1));
assert_eq!(Example::foo(), 25); assert_eq!(Foo::<Test>::get(), 25);
}); });
} }
@@ -146,7 +146,7 @@ fn set_dummy_works() {
new_test_ext().execute_with(|| { new_test_ext().execute_with(|| {
let test_val = 133; let test_val = 133;
assert_ok!(Example::set_dummy(RuntimeOrigin::root(), test_val.into())); 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));
}); });
} }
@@ -51,7 +51,7 @@ mod benchmarks {
set_foo(RawOrigin::Root, value, 10u128); // The execution phase is just running `set_foo` extrinsic call 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. // 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: // This line generates test cases for benchmarking, and could be run by:
@@ -125,7 +125,6 @@ pub mod pallet {
#[pallet::storage] #[pallet::storage]
#[pallet::unbounded] // optional #[pallet::unbounded] // optional
#[pallet::storage_prefix = "OtherFoo"] // optional #[pallet::storage_prefix = "OtherFoo"] // optional
#[pallet::getter(fn foo)] // optional
pub type Foo<T> = StorageValue<Value = u32>; pub type Foo<T> = StorageValue<Value = u32>;
#[pallet::type_value] #[pallet::type_value]
@@ -332,7 +332,6 @@ pub mod pallet {
/// ///
/// This is used to calculate average price, should have bounded size. /// This is used to calculate average price, should have bounded size.
#[pallet::storage] #[pallet::storage]
#[pallet::getter(fn prices)]
pub(super) type Prices<T: Config> = StorageValue<_, BoundedVec<u32, T::MaxPrices>, ValueQuery>; pub(super) type Prices<T: Config> = StorageValue<_, BoundedVec<u32, T::MaxPrices>, ValueQuery>;
/// Defines the block when next unsigned transaction will be accepted. /// Defines the block when next unsigned transaction will be accepted.
@@ -341,7 +340,6 @@ pub mod pallet {
/// we only allow one transaction every `T::UnsignedInterval` blocks. /// we only allow one transaction every `T::UnsignedInterval` blocks.
/// This storage entry defines when new transaction is going to be accepted. /// This storage entry defines when new transaction is going to be accepted.
#[pallet::storage] #[pallet::storage]
#[pallet::getter(fn next_unsigned_at)]
pub(super) type NextUnsignedAt<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>; pub(super) type NextUnsignedAt<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;
} }
@@ -479,7 +477,7 @@ impl<T: Config> Pallet<T> {
) -> Result<(), &'static str> { ) -> Result<(), &'static str> {
// Make sure we don't fetch the price if unsigned transaction is going to be rejected // Make sure we don't fetch the price if unsigned transaction is going to be rejected
// anyway. // anyway.
let next_unsigned_at = <NextUnsignedAt<T>>::get(); let next_unsigned_at = NextUnsignedAt::<T>::get();
if next_unsigned_at > block_number { if next_unsigned_at > block_number {
return Err("Too early to send unsigned transaction") return Err("Too early to send unsigned transaction")
} }
@@ -513,7 +511,7 @@ impl<T: Config> Pallet<T> {
) -> Result<(), &'static str> { ) -> Result<(), &'static str> {
// Make sure we don't fetch the price if unsigned transaction is going to be rejected // Make sure we don't fetch the price if unsigned transaction is going to be rejected
// anyway. // anyway.
let next_unsigned_at = <NextUnsignedAt<T>>::get(); let next_unsigned_at = NextUnsignedAt::<T>::get();
if next_unsigned_at > block_number { if next_unsigned_at > block_number {
return Err("Too early to send unsigned transaction") return Err("Too early to send unsigned transaction")
} }
@@ -543,7 +541,7 @@ impl<T: Config> Pallet<T> {
) -> Result<(), &'static str> { ) -> Result<(), &'static str> {
// Make sure we don't fetch the price if unsigned transaction is going to be rejected // Make sure we don't fetch the price if unsigned transaction is going to be rejected
// anyway. // anyway.
let next_unsigned_at = <NextUnsignedAt<T>>::get(); let next_unsigned_at = NextUnsignedAt::<T>::get();
if next_unsigned_at > block_number { if next_unsigned_at > block_number {
return Err("Too early to send unsigned transaction") return Err("Too early to send unsigned transaction")
} }
@@ -664,7 +662,7 @@ impl<T: Config> Pallet<T> {
/// Calculate current average price. /// Calculate current average price.
fn average_price() -> Option<u32> { fn average_price() -> Option<u32> {
let prices = <Prices<T>>::get(); let prices = Prices::<T>::get();
if prices.is_empty() { if prices.is_empty() {
None None
} else { } else {
@@ -677,7 +675,7 @@ impl<T: Config> Pallet<T> {
new_price: &u32, new_price: &u32,
) -> TransactionValidity { ) -> TransactionValidity {
// Now let's check if the transaction has any chance to succeed. // 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 { if &next_unsigned_at > block_number {
return InvalidTransaction::Stale.into() return InvalidTransaction::Stale.into()
} }
+1 -1
View File
@@ -107,7 +107,7 @@ pub mod pallet {
let _who = ensure_signed(origin)?; let _who = ensure_signed(origin)?;
// Read a value from storage. // Read a value from storage.
match <Something<T>>::get() { match Something::<T>::get() {
// Return an error if the value has not been set. // Return an error if the value has not been set.
None => return Err(Error::<T>::NoneValue.into()), None => return Err(Error::<T>::NoneValue.into()),
Some(old) => { Some(old) => {