Production / Non-Production Feature Flags for FRAME Pallets and Runtime

[shawntabrizi]

In FRAME we should try to satisfy these two, sometime conflicting requirements as best as possible:

1. Make it dead simple easy to build new pallets with FRAME
2. Allow developers to make completely safe, production ready pallets

The original SRML macros was really good at satisfying (1), where you could just start writing code and try it out right away.

However, over time, and as we transitioned to FRAME, we had to add more and more requirements to pallets in order to make it all production ready for Polkadot and Parachains.

Some example of "production features" that make the tinkerer experience worse:

• Weights
• MaxEncodedLen (PoV Size)
• Explicit Call / Pallet / Storage indexing (Use Mandatory Indices for FRAME Storage API (and others) #8964)

On the flip side, we also have introduced things within FRAME which should only be used in non-production environments:

• Transactional Extrinsics (is it production ready?)
• () or test impl for many traits
• Many pallets which are not fully audited or use real weights and stff

Using Rust's feature system, we should easily be able to enable / disable production and non-production features using feature flags.

For example, when building a pallet or working with substrate for fun, you can design a pallet without needing to define any weights, or maxencodedlen stuff.

However, when you are going to production, you can compile with --features=production, and these compiler errors will appear saying that you need to implement this stuff.

Then, at in the runtime definition, you can ensure that your runtime is always compiled with the production flag by using something like:

// instead of `construct_runtime!(...)

construct_production_runtime!(...)

This will only compile with the --features=production flag, ensuring that all pallets included in your runtime are ready to be used in production.

[stale]

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

[gui1117]

I understand the interest but actually I don't see that much possible improvments in practice:

• Weights: people should simply writes 0 for their toy pallet.
• MaxEncodedLen: I was thinking keeping the constraint optional as it is currently.
• transactional extrinsic: I think it is a logical change, people can use them or not, in both toy pallet and production pallet.
• () implementation of traits: This one makes sense to me, maybe we want to prevent dumb implementation in production network. Maybe we can start the habit with explicit name for dumb implementation, like we did for filter IIRC, so instead of () implementation we would have some explicit name which state whether it makes sense in production.

For the mock configuration when testing pallet, I know people complains about setting it, but it is always the same configuration of frame-system, providing a good example to copy and paste could be enough to be able to set easily.

[shawntabrizi]

For implementing this on the pallet, I suggest a syntax like:

#[frame_support::pallet]
#[frame_support::pallet::dev_mode] // <----
pub mod pallet {
    ...
}

Which will automatically remove the need for #[weight()] attribute (probably set weight to 0 for all extrinsics), and also automatically add #[pallet::without_storage_info] flag (#10662).

[shawntabrizi]

As a test, this minimal frame pallet should compile:

#![cfg_attr(not(feature = "std"), no_std)]

pub use pallet::*;

#[frame_support::pallet]
#[frame_support::pallet::dev_mode]
pub mod pallet {
	use frame_support::pallet_prelude::*;
	use frame_system::pallet_prelude::*;

	// The struct on which we build all of our Pallet logic.
	#[pallet::pallet]
	pub struct Pallet<T>(_);

	// Your Pallet's configuration trait, representing custom external types and interfaces.
	#[pallet::config]
	pub trait Config: frame_system::Config {}


	#[pallet::storage]
	type MyStorage<T: Config> = StorageValue<_, Vec<u8>>;

	// Your Pallet's callable functions.
	#[pallet::call]
	impl<T: Config> Pallet<T> {
		pub fn my_call(origin: OriginFor<T>) -> DispatchResult {
			Ok(())
		}
	}

	// Your Pallet's internal functions.
	impl<T: Config> Pallet<T> {}
}

[sam0x17]

Would it instead be desirable to make the existing construct_runtime! macro be the production one and do a construct_dev_runtime! that can be opted into? Otherwise I feel like this is going to be a significant breaking change across the ecosystem possibly?

[sam0x17]

Note that after a while I realized that the suggested syntax:

#[frame_support::pallet]
#[frame_support::pallet::dev_mode]
pub mod pallet {

will not work because the pallet:: macros are only valid inside of a pallet module and the way the pallet macro machinery works, an additional outer macro would conflict with pallet's role as the outer macro that parses everything within the pallet module. In particular this would lead to problems depending on whether #[pallet] or #[pallet::dev_mode] is specified first, even if we could get this double outer macro thing working.

Instead, I recommend this much easier to parse syntax:

#[frame_support::pallet(dev_mode)]
pub mod pallet {

In other words add dev_mode as an optional argument for the pallet attribute macro. Since pallet is actually expanded, it can then make use of this information during parsing and expansion of the pallet module.