From cd91c6b782b60906597ed28c4e4a4fdddd8428c9 Mon Sep 17 00:00:00 2001 From: Matteo Muraca <56828990+muraca@users.noreply.github.com> Date: Thu, 22 Feb 2024 00:02:31 +0100 Subject: [PATCH] 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> --- .../pallets/template/src/lib.rs | 1 - .../pallets/template/src/tests.rs | 4 ++-- prdoc/pr_3371.prdoc | 19 +++++++++++++++++++ .../node-template/pallets/template/src/lib.rs | 4 +--- .../pallets/template/src/tests.rs | 4 ++-- .../frame/examples/basic/src/benchmarking.rs | 2 +- substrate/frame/examples/basic/src/lib.rs | 14 +++----------- substrate/frame/examples/basic/src/tests.rs | 12 ++++++------ .../examples/kitchensink/src/benchmarking.rs | 2 +- .../frame/examples/kitchensink/src/lib.rs | 1 - .../frame/examples/offchain-worker/src/lib.rs | 12 +++++------- substrate/frame/examples/split/src/lib.rs | 2 +- 12 files changed, 41 insertions(+), 36 deletions(-) create mode 100644 prdoc/pr_3371.prdoc diff --git a/cumulus/parachain-template/pallets/template/src/lib.rs b/cumulus/parachain-template/pallets/template/src/lib.rs index 5f3252bfc3a..24226d6cf40 100644 --- a/cumulus/parachain-template/pallets/template/src/lib.rs +++ b/cumulus/parachain-template/pallets/template/src/lib.rs @@ -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>; diff --git a/cumulus/parachain-template/pallets/template/src/tests.rs b/cumulus/parachain-template/pallets/template/src/tests.rs index 527aec8ed00..9ad3076be2c 100644 --- a/cumulus/parachain-template/pallets/template/src/tests.rs +++ b/cumulus/parachain-template/pallets/template/src/tests.rs @@ -1,4 +1,4 @@ -use crate::{mock::*, Error}; +use crate::{mock::*, Error, Something}; use frame_support::{assert_noop, assert_ok}; #[test] @@ -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)); }); } diff --git a/prdoc/pr_3371.prdoc b/prdoc/pr_3371.prdoc new file mode 100644 index 00000000000..605d540772f --- /dev/null +++ b/prdoc/pr_3371.prdoc @@ -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 + diff --git a/substrate/bin/node-template/pallets/template/src/lib.rs b/substrate/bin/node-template/pallets/template/src/lib.rs index 4a2e53baa77..90dfe370145 100644 --- a/substrate/bin/node-template/pallets/template/src/lib.rs +++ b/substrate/bin/node-template/pallets/template/src/lib.rs @@ -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. @@ -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) => { diff --git a/substrate/bin/node-template/pallets/template/src/tests.rs b/substrate/bin/node-template/pallets/template/src/tests.rs index 7c2b853ee4d..83e4bea7377 100644 --- a/substrate/bin/node-template/pallets/template/src/tests.rs +++ b/substrate/bin/node-template/pallets/template/src/tests.rs @@ -1,4 +1,4 @@ -use crate::{mock::*, Error, Event}; +use crate::{mock::*, Error, Event, Something}; use frame_support::{assert_noop, assert_ok}; #[test] @@ -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()); }); diff --git a/substrate/frame/examples/basic/src/benchmarking.rs b/substrate/frame/examples/basic/src/benchmarking.rs index 4b2ebb41fbd..65ca3089aba 100644 --- a/substrate/frame/examples/basic/src/benchmarking.rs +++ b/substrate/frame/examples/basic/src/benchmarking.rs @@ -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 diff --git a/substrate/frame/examples/basic/src/lib.rs b/substrate/frame/examples/basic/src/lib.rs index dad4d01978f..12cadc969fd 100644 --- a/substrate/frame/examples/basic/src/lib.rs +++ b/substrate/frame/examples/basic/src/lib.rs @@ -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); @@ -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] @@ -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 }); diff --git a/substrate/frame/examples/basic/src/tests.rs b/substrate/frame/examples/basic/src/tests.rs index 9434ace35ff..207e46e428d 100644 --- a/substrate/frame/examples/basic/src/tests.rs +++ b/substrate/frame/examples/basic/src/tests.rs @@ -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); }); } @@ -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)); }); } diff --git a/substrate/frame/examples/kitchensink/src/benchmarking.rs b/substrate/frame/examples/kitchensink/src/benchmarking.rs index 24da581fc96..5f1d378e06f 100644 --- a/substrate/frame/examples/kitchensink/src/benchmarking.rs +++ b/substrate/frame/examples/kitchensink/src/benchmarking.rs @@ -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: diff --git a/substrate/frame/examples/kitchensink/src/lib.rs b/substrate/frame/examples/kitchensink/src/lib.rs index 18429bc967d..b7425b0c084 100644 --- a/substrate/frame/examples/kitchensink/src/lib.rs +++ b/substrate/frame/examples/kitchensink/src/lib.rs @@ -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] diff --git a/substrate/frame/examples/offchain-worker/src/lib.rs b/substrate/frame/examples/offchain-worker/src/lib.rs index 6c1fa6ea8ec..d21c8b4cfd2 100644 --- a/substrate/frame/examples/offchain-worker/src/lib.rs +++ b/substrate/frame/examples/offchain-worker/src/lib.rs @@ -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. @@ -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>; } @@ -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") } @@ -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") } @@ -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") } @@ -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 { @@ -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() } diff --git a/substrate/frame/examples/split/src/lib.rs b/substrate/frame/examples/split/src/lib.rs index 74d2e0cc24b..5245d90e390 100644 --- a/substrate/frame/examples/split/src/lib.rs +++ b/substrate/frame/examples/split/src/lib.rs @@ -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) => { -- GitLab