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
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
Copy link
Member

Choose a reason for hiding this comment

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

Im not sure if we settled on a preferred syntax, this turbofish vs ::.
Personally i think :: everywhere is better, but without linter it will probably not work out anyway.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that Something::<Test>::get() is preferred

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes and yes.

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
8 changes: 0 additions & 8 deletions substrate/frame/examples/basic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,6 @@ 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();

// Calculate the new value.
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
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
2 changes: 0 additions & 2 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
Loading