FRAME v2 (#[pallet] macro)

[shawntabrizi]

Here is a discussion on all things related to FRAME v2!

Ask questions, request features, share your projects, and tell us about bugs!

Happy hacking and holidays to everyone!

---

Documentation

Documentation for the new macro can be found here:

• https://crates.parity.io/frame_support/attr.pallet.html

An example of fully using these macros to create a pallet can be found here:

• https://crates.parity.io/frame_support/attr.pallet.html#example-for-pallet-without-instance

cc @thiolliere

[Swader]

I wouldn't mind finding out about the backstory here - when was this major update decided on, how come, is it a breaking change for Substrate, does it affect the tutorials and / or node-template, etc.?

[shawntabrizi]

It is not a breaking change. Everything from the old FRAME is backwards compatible here.

The short story summary is the following:

• There are different kinds of macros supported by Rust.

• All the kinds of macros basically do the same thing, take some code and write some more code in a programmable way.

• The end result of all the macro expansion is regular Rust code, but usually lots of it is "filler" programming, for example satisfying traits or defining certain known structures. Macros can save thousands of lines of repetitive programming.

• The existing FRAME macros use declarative macros, where basically you define your own custom syntax.

• It was common that the old Substrate macros invented custom syntax which was not standard for Rust, and you would only know how to use it if you knew Substrate and read our documentation.

• Our declarative macros were very large and complex, and thus minor errors could result in impossible to understand errors.

• FRAME v2 uses attribute like macros: https://doc.rust-lang.org/book/ch19-06-macros.html#attribute-like-macros

• Attribute macros are applied onto regular Rust objects and normal Rust syntax. While there may be some custom syntax within the attribute itself, the rest of the code is just normal Rust.

• This means that meta-programming tools like Rust analyzer, visual studio code, RustFmt, etc can all work on Pallets written with the v2 macros, whereas it was impossible for them to do this on the old macros due to the custom syntax that was introduced.

• The FRAME v2 macros have been design to be much more modular and thus errors are reported to the user in a much more friendly way, allowing users to more easily figure out problems.

• Both the old and new macros expand to the same final Rust code, thus they are completely compatible with one another.

In the end, FRAME v1 was trying more to invent a new language for writing pallets, but FRAME v2 admits that Rust is already a great language on its own, and instead just enhances Rust to make it work. This will make it so that to learn Substrate, you really just need to learn Rust.

[gui1117]

We wanted to improve our macro since quite a long time, because it gives difficult error message in some situation, because syntax is difficult to grasp, (and also because of IDE support). This was also inspired by the successful ink! macro.

Next version of substrate will still holds decl_* macros. Either can be used to declare a pallet. But in the future we'll probably deprecate decl_* macros, at least new features will probably only be added to the new pallet macro. Anway transition must be smooth.

But migrating from delcl_* to pallet macro shouldn't break pallets (except some minor changes), the logic inside is just expressed differently, but concepts are same, you can see the migration guide on master.

[Swader]

Fantastic, thanks to both of you, all clear!

[adoerr]

Just out of curiosity: has there been a specific reason to not make the pallet::getter attribute an optional argument for the pallet::storage attribute macro? So, allowing something like #[pallet::storage(getter = fn get_balance)]. Ofc, you could argue that this would again introduce some piece of custom syntax.

[gui1117]

I think we tend to organize so that each item is given an attribute like #[pallet::$something] and then additional optional information using inner attributes, I prefer as it is now, but your proposition seems also fine to me.

[shawntabrizi]

TBH I don't find that the getter shortcut is actually that useful. Either you can write a simple function which returns the value, or better you can directly call MyStorage::get() which for me makes auditing usage of storage operations much easier.

I would prefer we get rid of it all together if possible 😈

[adoerr]

I have a question regarding the pallet macro and pallet source file organisation. Given the following adaptation of the Substrate Recipe Basic Token example. Constructing a runtime with construct_runtime! I would end up with the following

BasicToken: basic_token::pallet::{Module, Call, Config<T>, Storage, Event<T>},

This is causing compile time issues, since construct_runtime! does not seem to support a Path for a ModuleDeclaration.name. After basic_token:: a pallet instance is expected.

What is supposed to be the right/canonical approach here?

[gui1117]

there is 2 approach possible here:

• reexport the object of pallet module into the main scope of the crate: pub use pallet::*;, this would also allow to not break your public API when upgrading from old macros.
• in node-runtime you can alias the path: use basic_token::pallet as basic_token_pallet; and then use basic_token_pallet in construct_runtime.

But handling path in construct_runtime is also on my todo list.

[rphmeier]

It would be nice to have the English in the documentation improved. It needs to be easy to read and understand especially as such a crucial piece to developers.

[rphmeier]

For more complicated runtimes one of the things on my wishlist is to have "batch" pallets. Where internal pieces that work together are split up, because scoping behavior is important for correctness and code is easier to maintain when it's not all in a 10k LoC file. Within a batch of modules, initialization order is important.

How do you think that might look with the #[pallet] attribute? Maybe declaring "sub-pallet"s in different files and referencing them somehow?

[gui1117]

I definitely agree, I don't have that much idea yet.

Do you think the initialization order is the only information that should checked/handled/helped when having "batched" pallet ? or do you see other kind of intrication ?

Maybe this could be solved by general mechanism for on_initialize order helper.

[xlc]

Am I the only finding the readability of storage syntax is not as good as the old one?

This is the old code

pub Locks get(fn locks):
    double_map hasher(blake2_128_concat) T::AccountId, hasher(twox_64_concat) T::CurrencyId
        => Vec<BalanceLock<T::Balance>>;
pub Accounts get(fn accounts):
    double_map hasher(blake2_128_concat) T::AccountId, hasher(twox_64_concat) T::CurrencyId
        => AccountData<T::Balance>;

This is the new code

#[pallet::storage]
#[pallet::getter(fn locks)]
pub type Locks<T: Config> = StorageDoubleMap<
	_,
	Blake2_128Concat,
	T::AccountId,
	Twox64Concat,
	T::CurrencyId,
	Vec<BalanceLock<T::Balance>>,
	ValueQuery,
>;

#[pallet::storage]
#[pallet::getter(fn accounts)]
pub type Accounts<T: Config> = StorageDoubleMap<
	_,
	Blake2_128Concat,
	T::AccountId,
	Twox64Concat,
	T::CurrencyId,
	AccountData<T::Balance>,
	ValueQuery,
>;

It is just harder to me to identify the key type and value type at a glance. >= helps me to quickly local the interesting part. I've also already trained myself to skip snake_case identifiers and just looking CamelCase ones (i.e. the Type), but now the hasher is also a type here so that's not helping in readability.

[gui1117]

it looks fine to me,

And it would better if we could get rid of the leading boilerplate _

I think the boilerplate _ is useful to understand what the macro does, in order not to be confuse when you look at doc for StorageDoubleMap

[coriolinus]

I like @rphmeier 's suggestion; it provides a lot more context for what all the generic parameters mean.

Yes, associated types are only for traits, but the point of doing this as a procedural macro is that we get to invent our own syntax and semantics. The goal isn't to map 100% to existing semantics, because if so why bother? It's to write the shorthand that provides the best clarity. It wouldn't hard to make the context cues optional, allowing something like this:

#[pallet::storage]
#[pallet::getter(fn locks)]
pub type Locks<T: Config> = StorageDoubleMap<
	_,
	PrimaryKey = T::AccountId<Blake2_128Concat>,
	SecondaryKey = T::CurrencyId<Twox64Concat>,
	Value = Vec<BalanceLock<T::Balance>>,
	ValueQuery,
>;

[gui1117]

Yes, associated types are only for traits, but the point of doing this as a procedural macro is that we get to invent our own syntax and semantics.

No we use rust syntax in the procedural macro. to use non rust syntax we must parse ourself or wrap the non-rust syntax into macro block: storage!(...), I'm not sure the syntax Primary = Type is accepted by syn parser.

T::AccountId<Blake2_128Concat>

I woud prefer not to create fake types. the point to stick to rust semantics is to avoid having to understand another DSL in order to use pallets.

[xlc]

if we are going to invent new syntax, we might as well go back to the old decl_storage syntax

[gui1117]

I opened this PR #8751
Which allows to give name to generics like:

#[pallet::storage]
pub(super) type MyStorage<T> = StorageMap<Hasher = Blake2_128Concat, Key = u32, Value = u32>;

[shaunxw]

Regarding the storage syntax, another thing is that if a pallet has several/dozen storage items, it will be verbose to repeat #[pallet::storage] on everyone of them.

It might be more clean and easier to tell which pallet item is a storage, if we could group storage items in a mod, like:

// The macro can add `use storage::*;` to make items available in upper level mod.
#[pallet::storage]
mod storage {
    #[pallet::getter(fn total_issuance)]
    pub type TotalIssuance<T: Config> = StorageMap<_, Twox64Concat, T::CurrencyId, T::Balance, ValueQuery>;

    #[pallet::getter(fn locks)]
    pub type Locks<T: Config> = StorageDoubleMap<
        _,
        Blake2_128Concat,
        T::AccountId,
        Twox64Concat,
        T::CurrencyId,
        Vec<BalanceLock<T::Balance>>,
        ValueQuery,
    >;

    #[pallet::getter(fn accounts)]
    pub type Accounts<T: Config> = StorageDoubleMap<
        _,
        Blake2_128Concat,
        T::AccountId,
        Twox64Concat,
        T::CurrencyId,
        AccountData<T::Balance>,
        ValueQuery,
    >;
}

[olanod]

Perhaps there could be default getters generated with the name of the type to snake_case as it is usually what people do and that way we also omit that attribute

[gui1117]

I'm a fan of having yet another indentation level but I'm ok with it.

Perhaps there could be default getters generated with the name of the type to snake_case as it is usually what people do and that way we also omit that attribute

well not all pallet always use getters, see pallet-contracts. Also when we hide it then it is harder to see where the function is coming from.

[KiChjang]

I may have encountered a situation while attempting to migrate to FRAMEv2:

Suppose I have an instantiable pallet Pallet<T, I>. I leave everything unchanged in my runtime during my migration, i.e. the construct_runtime! still looks like the following:

construct_runtime!(
    pub enum Runtime // where clause
    {
        // other pallet declarations
        InstanceName: pallet_name::<Instance1>::{Pallet, Call, Storage, Origin<T>, Event<T>, Config<T>},
    }
)

In FRAMEv1, the storage prefix for the pallet in question is Instance1PalletName, whereas in FRAMEv2, it seems to have changed -- it's now simply InstanceName. Does that mean a pallet upgrade from FRAMEv1 to FRAMEv2 would require a storage migration if the pallet is instantiable?

[bkchr]

Yes, it also requires a storage migration if the name of the storage changed.

[shaunxw]

This issue #8547 reminds me of a change I forgot to propose about pallet macro.

In dispatchable calls impl, currently it's quite easy to miss something and break "Check First, Write Last" rule. Since in most cases extrinsics are transactional, I think we can make #[transactional] as default, and provide a #[non_transactional] for edges like batch call.

[shawntabrizi]

I think it is not clear that #[transactional] has zero overhead. Some kind of benchmarking of this needs to be done before a switch would occur.

[asmita-mf]

Can anyone suggest that how can I resolve the below error

<Wallet>::get(&account);
| ^^^ function or associated item cannot be called on frame_support::pallet_prelude::StorageMap<_GeneratedPrefixForStorageWallet<T>, frame_support::Blake2_128Concat, sp_runtime::AccountId32, Wallet<CryptoDetail>, ValueQuery> due to unsatisfied trait bounds
|
::: /home/asmita-mfs/vtb-test/vtb_substrate/vtb-node/pallets/vtbdex/src/lib.rs:269:15
|
269 | #[pallet::storage]
| _______-
270 | | #[pallet::getter( fn user_record)]
271 | | pub type Wallet = StorageMap<, Blake2_128Concat, AccountId32, custom_types::Wallet<custom_types::CryptoDetail>, ValueQuery>;
| |- doesn't satisfy _: StorageInstance
|
= note: the following trait bounds were not satisfied:
_GeneratedPrefixForStorageWallet<T>: StorageInstance

[gui1117]

can you format the error message into 3 "`", some element are removed.

My expectation is that you wrote <Wallet<T>::get(&account); but T doesn't implement Config trait.