[FRAME/Meta/PM] "experimental" `FRAME` features

[ggwpez]

Analogous to the deprecation process it would be nice to have an on-ramp for new features in FRAME. This could be used to introduce new feature in a non-semver way, so that we can start integrating them before stabilizing the design.
I think it could increase our development velocity since we can iterate quicker on features without having to worry too much of breaking downstream. Currently I often try to keep breaking changes to a minimum - even when a feature is not used outside of Parity yet…

Possible implementations

There were some internal discussions about how to achieve this. Some ideas:

• A) Some attribute macro like frame_unstable(id) or similar and a matching allow_unstable(id) where ID is the identifier of the issue, similar to rust nightly features. This would be the more fine-grained and elegant solution - although not sure how achievable without something like https://github.com/rust-lang/rust/issues/62398
• B) Rust compiler features where we have one feature that guards all unstable features. This would need to be propagated to all dependencies… we already have enough problems just from try-runtime and runtime-benchmarks though.
• C) Simplest would be to have a frame_support::unstable module, that export all unstable types. Or in the future just frame::unstable. The unstable features should not be accessible through another path. We can do this in frame_support since the modules can be made pub(super), such that the unstable module can export them.

Inside of pallets we have more control and could opt for a better DevEx. For example with the upcoming Task API we could mark that as unstable and emit a warning that can be silenced through pallet(unstable) or something.

Process

Without making this too cooperate-style bureaucratic I think we should still have some kind of process. For example:

1. Put new non-trivial traits and types, where future changes are expected, into the unstable module. For example we could use that for the VersionedRuntimeUpgrade or the Multi Block Migrations.
2. Start integrating the feature to discover sharp edges and improvements.
3. Set a deadline for further changes and move it out to the normal FRAME crates afterwards.
4. Maybe announce the feature somehow.

cc @kianenigma @sam0x17 @juangirini

[kianenigma]

• big no to features, which as you said are already a PITA.
• something like an unstable module seems like the most reasonable path forward for now.
• for now, we can add frame_support::unstable::pallet_prelude, but once paritytech/substrate#14137 is done, it can be neatly packaged in that.
• I guess @sam0x17 can come up with some wacky hack that would allow us to (with some false negative) detect if anything from frame::unstable is being used, and if so, raise a warning.
• This warning can be configurable based on the context of the pallet/runtime, ie https://github.com/paritytech/polkadot-sdk/issues/218
• paritytech/polkadot-sdk#171 is a good example to go through this process.

[xlc]

unstable doesn’t means insecure right? if that’s the case, why do we need warnings? you need to implicitly import from unstable module to use it and I think that’s enough

[sam0x17]

I think it's more like "could change at any time"

Maybe the word "experimental" would be better

[bkchr]

• something like an unstable module seems like the most reasonable path forward for now.

How should this work for things that require changes to any Config trait?

IMO, as annoying as features can be, they are probably the best way forward.

[sam0x17]

IMO, as annoying as features can be, they are probably the best way forward.

Yeah I agree that unless this is only occurring within pallets, in which case we have outer macro pattern powers and can make up a syntax, something like a feature is unfortunately our best bet.

If we're doing features, IMO an easy albeit backwards way to do this is to mark all the experimental things as deprecated, with a deprecation message saying they are experimental, but if the experimental feature is present, we don't emit the deprecation attribute.

So to opt in to using something experimental, you simply enable the experimental feature and now you won't get deprecation warnings on any of those things.

A simpler approach ofc is to just only emit those things if the experimental feature is present, but doing it this way might aid easier discovery of experimental features while they are still being worked on, which is something that I think we want

[xlc]

Please don't abuse deprecation message. Only mark deprecated features deprecated.

[juangirini]

unstable doesn’t means insecure right? if that’s the case, why do we need warnings? you need to implicitly import from unstable module to use it and I think that’s enough

Independently of whether we go for the features or the modules solution, this is a very good point and it would simplify things. I guess it was meant read explicitly import, in this or in the features case the developer is already aware of the experimental feature.

Maybe the word "experimental" would be better

+1 on this one, it sounds less scary

[kianenigma]

Having thought about this a bit, I am leaning toward backing using a feature flag for this.

[juangirini]

Having thought about this a bit, I am leaning toward backing using a feature flag for this.

Yes, me too, I think it's the best way to go.

[sam0x17]

And consensus seems to be we call it "experimental" instead of "unstable"

[ggwpez]

So its settled then; we use an experimental feature that is never enabled per default.
We can even apply this to pallets and non-FRAME crates :tada:

---

We could use https://github.com/paritytech/substrate/pull/14311 as an example to try it out.

[liamaharon]

I believe we need to update the Substrate CI to run experimental tests? It seems that the test CI is configured outside of this repo, what's the way to go about tweaking that?

[bkchr]

No, the CI is configured in here: https://github.com/paritytech/substrate/blob/master/scripts/ci/gitlab/pipeline/test.yml

There you will need to change/add a new job.

[kianenigma]

@juangirini @ggwpez @paritytech/frame-coders any reason to not close this?

[juangirini]

@ggwpez https://github.com/paritytech/substrate/pull/14311 should have close this issue, right? or is there anything else that you want to do?

[ggwpez]

Yea it should be good. There were some follow up discussions on how to separate feature-gates and modules, but we can reopen it then.