Repository navigation
Make construct_runtime macro more flexible #232
Description
Activity
I am sure I've pasted this in another issue as well (found it), but I think the new version of this macro should look like this:
#[frame::contruct_rutnime] mod runtime { // The rest, like `AllPalletsReserved` and such have to be // unfortunately aut-generated, or better, deprecated. #[frame::pallets] type AllPallets = ( frame_system::pallet, pallet_balances::pallet, pallet_staking::pallet, pallet_sessin::pallet, ); // don't think there is much to this. #[frame::runtime] pub struct Runtime; // Auto-generate call from `AllPallets` #[frame::call] pub enum RuntimeCall = ..AllPallets; // Alternative to the spread syntax above: Explicitly build an enum with the given // pallet paths. // // This is very dangerous, only for experts. Probably no real use case for it either.. #[frame::event] pub enum RuntimeEvent = (frame_system, pallet_staking); #[frame::origin] pub enum RuntimeOrigin = ..AllPallets; #[frame::genesis_config] pun enum RuntimeGenesis = ..AllPallet; // Have not thought it through, but see https://github.com/paritytech/substrate/issues/13463#issuecomment-1464013060 #[frame::ext_builder] pub struct ExtBuilder {...} } pub use runtime::{Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin};
Where if you write
pub enum Origin = ..AllPalletsyou get the default implementation, but you can opt out of it any at any point as well.Reacted by Gabriel Facco de Arruda and liamaharon- I like this a lot, it not only solves the flexibility issues I mentioned, but also helps with visibility for the enums generated, which are right now created inside the macro and thus hidden in the source code of the runtime. ```…On Sun, Jan 15, 2023 at 9:36 AM Kian Paimani ***@***.***> wrote: I am sure I've pasted this in another issue as well, but I think the new version of this macro should look like this: #[frame::contruct_rutnime] mod runtime { // The rest, like `AllPalletsReserved` and such have to be // unfortunately aut-generated, or better, deprecated. #[frame::pallets] type AllPallets = ( frame_system::pallet, pallet_balances::pallet, pallet_staking::pallet, pallet_sessin::pallet, ); // don't think there is much to this. #[frame::runtime] pub struct Runtime; // Auto-generate call from `AllPallets` #[frame::call] pub enum RuntimeCall = ..AllPallets; // Alternative to the speard syntax above: Explicitely build an enum with the given // pallet paths. // // This is very dangerous, only for experts. Probably no real use case for it either.. #[frame::event] pub enum RuntimeEvent = (frame_system, pallet_staking); #[frame::origin] pub enum Origin = ..AllPallets } pub use runtime::{Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin}; Where if you write pub enum Origin = ..AllPallets you get the default implementation, but you can opt out of it any at any point as well. — Reply to this email directly, view it on GitHub <#232>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHGXNIUDDRNXPJG2AW45S5TWSPVMLANCNFSM6AAAAAAT2PWTBI> . You are receiving this because you authored the thread.Message ID: ***@***.***>-- *Gabriel Facco de Arruda* +55 (51) 99826-0101 ***@***.*** ***@***.***>*
In my case, for example, I want to override the implementation of
From<Origin> for Result<RawOrigin<AccountId>, Origin>so I can allow my custom origins to pass through the defaultensure_signedassertions in most pallets' calls.This doesn't really makes sense, why passing your custom origin when the function only needs a signed origin? And how should the
Fromimplementation look like? You don't have any context in there and would convert your origin just "blindly" into theRawOrigin. If you need help with what you are doing, please provide some proper write up (what you want to achieve etc) somewhere (maybe Polkadot forum). Then we can look into it and give some tips.The general idea of @kianenigma looks good and something we should speak about!
In my case, for example, I want to override the implementation of
From<Origin> for Result<RawOrigin<AccountId>, Origin>so I can allow my custom origins to pass through the defaultensure_signedassertions in most pallets' calls.This doesn't really makes sense, why passing your custom origin when the function only needs a signed origin? And how should the
Fromimplementation look like? You don't have any context in there and would convert your origin just "blindly" into theRawOrigin. If you need help with what you are doing, please provide some proper write up (what you want to achieve etc) somewhere (maybe Polkadot forum). Then we can look into it and give some tips.The general idea of @kianenigma looks good and something we should speak about!
I don't actually need help with that, I already have it working how I want it but I had to clone and modify the frame-support-procedural crate so I could implement it myself, which is why I came to the conclusion that making the construct_runtime macro more flexible would be of help not only to me but possibly to others that want to have unconventional implementation like mine.
And to answer your question of why I want my custom origin to act as signed, it's because the origin is for a multisig pallet that has calls within it to change parameters and handle operations that modify the multisig itself, those need specific data like the multisig id and the original caller who initiated the multisig, I have that data in the custom origin, but I also want the multisig to act as a normal account for everything else. I know it can be easily solved by adding those to the function arguments and then verifying within the function, but I'm building with UX in mind, if something can be achieved by the runtime without concerning the users and developers, then I'll handle that.
I see what you want to achieve. Currently this is already achievable without forking
frame-supportby creating a newtype wrapper for theRuntimeOrigin. On this newtype wrapper you could implement theInto<Result<RawOrigin, Self>>as you need it. Then you would set thisWrappedRuntimeOriginas your runtime origin to all the pallets.We could also add some attribute
#[do_not_implement_into_raw_origin](shitty naming :P) to the currentconstruct_runtimemacro to skip the implementation. This is then a short term fix for your issue. I think even in the future dreamed by @kianenigma you would need such an attribute as per default this should always be implemented.This issue has been mentioned on Polkadot Forum. There might be relevant details there:
https://forum.polkadot.network/t/less-magic-in-frame-good-or-bad/2287/8
#[frame::contruct_rutnime] mod runtime { // The rest, like `AllPalletsReserved` and such have to be // unfortunately aut-generated, or better, deprecated. #[frame::pallets] type AllPallets = ( frame_system::pallet, pallet_balances::pallet, pallet_staking::pallet, pallet_sessin::pallet, ); }
How does it handle instances, indices and instance naming?
Reacted by Kian Paimani and gupnikNot a whole lot has changed, but here is a more up-to-date and well-documented version of the above idea, which also answers the question raised by @gavofyork:
#[frame::construct_runtime] mod runtime { /// Mandatory, just to declare `pub struct Runtime`. #[frame::runtime] pub struct Runtime; /// Mandatory; and the benefit now is that we can elide a LOT of the types for this. The macro /// will elide all generics for you as: /// 1. `System` is already know. /// 2. `Block` is already known from system (assuming /// https://github.com/paritytech/substrate/pull/14193) /// 3. `Context` is already know from System. /// 4. `UnsignedValidator` is known from `AllPallets` /// 5. `AllPallets` is known /// 6. `OnRuntimeUpgrade` will be left blank to its default to `()`. /// /// All of the above can be replaced with `_`. So the typical syntax is: /// /// `pub struct Executive<_, _, _, _, _, Migration>` /// /// Which can be further reduced to: /// /// If you provide `pub struct Executive<Migrations>` /// /// NOTE: In the future, the execution of `OnRuntimeUpgrade` should be fully delegated to a /// single "migration dispatcher" pallet (probably the same one as /// https://github.com/paritytech/substrate/pull/14275), so this last argument can be removed. #[frame::executive] pub struct Executive<_, _, _, _ , _>; /// Almost exactly the same syntax as the existing one. /// /// This will not be real rust-like, but it will demonstrate the existence of `type AllPallets`. #[frame::pallets] pub type AllPallets = ( System = frame_system::pallet = 0, BalancesFoo = pallet_balances::pallet = 1, BalancesBar = pallet_balances::pallet = 2, Staking = pallet_staking::pallet = 42, Session = pallet_session::pallet = 43, ); /// This can either be a tuple of pallet names, eg /// /// #[frame::call] pub enum RuntimeCall = (System, BalancesFoo, Staking) /// /// which omits a number of calls, or be the below syntax, which means "derive it from /// AllPallets, in the exact same manner". #[frame::call] pub enum RuntimeCall = ..AllPallets; /// Same semantics as `RuntimeCall`. #[frame::event] pub enum RuntimeEvent = ..AllPallets; /// Same semantics as `RuntimeCall`. #[frame::origin] pub enum RuntimeOrigin = ..AllPallets; /// Same semantics as `RuntimeCall`. #[frame::genesis_config] pub enum RuntimeGenesis = ..AllPallet; /// Same semantics as `RuntimeCall`. /// This is slightly different from how we do this now. Now, we implement /// `ValidateUnsigned` for `Runtime`, now we should implement it for a new /// type and pass that one to executive. #[frame::validate_unsigned] pub type UnsignedValidator = ..AllPallets; /// This will auto-populate this module with a known list of types and re-exports that will be /// useful either in the rest of this block (`Block`, `Header` etc), or the ones that are /// typically used by the node to build genesis (`AccountId`, `OpaqueBlock`) etc. #[frame::runtime_prelude] pub mod prelude {} /// Further Idea paritytech/substrate#1: This can automatically generate nested builders that create a struct with /// fields corresponding to the `GenesisConfig` of each pallet. /// /// See: https://github.com/paritytech/substrate/issues/13463#issuecomment-1464013060 #[frame::ext_builder] pub struct ExtBuilder {} /// Further idea paritytech/substrate#2: The parent annotation could be /// #[frame::construct_runtime(testing|parachain|solo-chain)] which could enforce more rules, /// elide more types. /// /// See: https://github.com/paritytech/substrate/issues/13321 } /// Finally, other than `#[frame::runtime] mod runtime`, this is your single exposed pub use runtime::prelude::*;
Reacted by gupnik and Gabriel Facco de Arruda/// NOTE: In the future, the execution of `OnRuntimeUpgrade` should be fully delegated to a /// single "migration dispatcher" pallet (probably the same one as /// https://github.com/paritytech/substrate/pull/14275), so this last argument can be removed.
Migrations are running before
on_initializeandon_initializeis controlled byExecutive. We could add some extraHooks, however I'm not sure if this is better.13 remaining items
As discussed during the retreat, we could use the following syntax to provide a reasonable default:
#[frame::construct_runtime] mod runtime { #[frame::runtime] pub struct Runtime; #[frame::pallets] #[derive(RuntimeGenesisConfig, RuntimeCall, RuntimeOrigin)] // Aggregate all pallets by default pub type AllPallets = ( System = frame_system::pallet = 0, BalancesFoo = pallet_balances::pallet = 1, BalancesBar = pallet_balances::pallet = 2, Staking = pallet_staking::pallet = 42, Session = pallet_session::pallet = 43, ); }
There could be an additional syntax to exclude some pallet parts if needed.
I wanted exactly this when trying to learn about
type RuntimeHoldReason = RuntimeHoldReason;and I could not findRuntimeHoldReasonin the codebase.Deeper digging, I realized it was being generated by
construct_runtimeand this was not obvious.Reacted by Kian Paimani, liamaharon, gupnik, Juan Girini and Guillaume Thiolliere- added a commit that references this issue
on Mar 13, 2024 - added a commit that references this issue
on Mar 24, 2024
Metadata
Metadata
Assignees
Type
Projects
- Status
- Status
Usually the assumptions made by the
construct_runtimemacro are correct and help keep the runtime boilerplate free, but sometimes it's desirable to override the implementations generated by the macro.In my case, for example, I want to override the implementation of
From<Origin> for Result<RawOrigin<AccountId>, Origin>so I can allow my custom origins to pass through the defaultensure_signedassertions in most pallets' calls.I would like to propose and get feedback/suggestions on how to make the
construct_runtimemacro more flexible, while still maintaining the minimum amount of boilerplate. The solution I currently have in mind is probably not the best possible, thus I'd like to brainstorm solutions.I'll preface this with a note that I'm not super knowledgeable in Rust macros, so I'm not sure if this is possible to implement like this, but here's what I had in mind:
Perhaps we could even break down the implementations from
construct_runtimeinto multiple macros (likeimpl_call,impl_origin, etc...) and these could be used by themselves or if the runtime does not require any special implementations, we could have the overarchingconstruct_runtimecalling these within itself if they are declared in the macro.