-
Notifications
You must be signed in to change notification settings - Fork 704
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
Create some compile-time notion of "runtime mode" #218
Comments
Features in rust are not mutually exclusive and this leads to problems. Yeah, I know that you can do And how would the env work for when you are having some mock runtime? How do you tell the pallets that you just compile the mock runtime in some random crate? We already have some
I don't get this, what should the pallet do with this information? |
An additional thing that might be useful at compile-time would be the "role" like validator, etc if that isn't already available. Also those are all fair points @bkchr. Off the top of my head I don't know if there would be a use case for the "What" one but I think @kianenigma had something in mind with that one |
And yeah, I actually implemented rails-like environments in a personal rust-based web app like this: use strum::IntoStaticStr;
#[derive(Clone, Debug, IntoStaticStr)]
pub enum Environment {
Development,
Test,
Staging,
Production,
}
#[cfg(not(any(test, staging, production)))]
pub const CURRENT: Environment = Environment::Development;
#[cfg(test)]
pub const CURRENT: Environment = Environment::Test;
#[cfg(staging)]
pub const CURRENT: Environment = Environment::Staging;
#[cfg(production)]
pub const CURRENT: Environment = Environment::Production; This coupled with making |
Rails env is used for control the application behavior, Rust Web framework e.g. Rocket has the same design https://rocket.rs/v0.5-rc/guide/upgrading/#profiles I think Rust feature gate are not design for the same sceanario, it's just a tool for conditional compiling, adding too much feature gate may lead some compositions not orthogonal then introducing unexpect bug, rencently I already reported feature-gate relates bug that break parachains compiling with On the other hands, I like the idea of your example, but I think it should be limited in constants level, in test or staging, we usually hope short governance period, charge some money to testing accounts, etc, use the style of code you showed can organize them better. One thing is hard to solve is, in non-production env we hope to have Sudo pallet, in this case, I don't find good way for this, so I'm trying separate staging and production to different runtime, and make pallets configurations and other primitives reusable, then the runtime crate only has |
Yeah I agree that we should probably avoid using cfg for this. That said, would lean towards a solution that makes this info available at compile-time, as that is where a good bit of the utility lays I also haven't gone very deep into |
I haven't try for renaming, I don't have idea, for the randomness pallet, the renaming is good, it's breaking change but not hard to adapt. |
And this raises another common issue. I agree with you, but rust still hasn't stabilized the ability to raise arbitrary warnings from macro world. We are limited to the warnings the compiler normally generates. @ggwpez has a slick workaround that involves making an unused item to intentionally have the compiler issue a warning or something like that (I forget), but it isn't perfect. The env var idea would work well for dev mode, but I guess my point is we're re-creating what could be a global abstraction in numerous places when we should just figure out a global abstraction. |
I think we need this sooner rather than later, and I would also not use runt features for this. Option 1: Check in
|
Any TLDR on exactly the problem this is trying to solve? There are a few things to keep in mind: Features / Flags will introduce complexity to the code and make it harder to test. You can only test in production for code that is guarded with production env. We want to make sure the wasm code is as small as possible and therefore need to make sure the compiler is able to exclude dev code in prod build. |
Should really pallet have different logic depending on argument like Also if we decide to introduce envs like |
This came out of a conversation with @kianenigma, @ggwpez and other members of the FRAME team so I'll do my best to reproduce the highlights of that conversation here. The short version is that right now in a substrate-based project, we don't really have a well-defined, standardized notion of what the build "intent" or "mode" is at runtime, nor do we have this information in an easily-consumable form at compile-time, and this is something we could improve upon. I'll explain what I mean:
Some background:
Before I worked in Rust I was a long-time rubyist. Ruby on Rails (web framework used at a lot of startups) is known for having a very clear notion of what in the Rails world is called an "environment". When you run a Rails app, there are three default "environments" that come built-in: "test", "development" and "production". An app's environment is determined when it launches and cannot change while it is running, and this information can be accessed via globally accessible methods such as
Rails.env.test?
,Rails.env.production?
etc., and is set by the environment variableRAILS_ENV
.When you run unit and integration tests, the app will automatically be in test mode (
RAILS_ENV=test
), and all sorts of configuration and things can then load and happen on an environment-specific basis (just likecfg(test)
in Rust). It is quite common, for example, to have blocks of code that will do a switch statement that does different things for each of the defined environments.The "development" environment is the default and is typically used for local development/tinkering. The "production" environment is used on deployed servers, and people also often add an additional "staging" environment that closely matches "production" but with subtle differences like different db credentials, etc. Secret management is also typically set up at the environment level.
Running development mode uses a lazy asset pipeline suitable for local development, while running the app in production mode will result in performance-optimized compiled assets, tighter security settings (CORS, etc), production-specific API keys and secrets, etc..
By standardizing these "environments" at the framework level, all tooling and plugins that integrate with Rails can make certain assumptions based on what environment we are in, and this is healthy for the ecosystem. Crucially, this allows gem (crate) authors to specify that certain features can or cannot be used in certain environments, and since all Rails apps are structured roughly the same way, this will just work out of the box for arbitrary Rails apps.
It would be great if we could rely on something conceptually similar to this when building with substrate!
My Suggestion
So given my background you can understand why when I started working on substrate, I was surprised we don't have some notion similar to this. For example, at build time the only thing we really know is whether we are in test mode or not, which doesn't tell us much. We don't (at least not in a way that is standardized and easily accessible at compile-time anywhere and everywhere in substrate, for example in individual pallets) know if we are polkadot itself, or a parachain, and we don't know whether we are in production or if we are just some random dev playing around with things locally.
As far as I can tell there are at least two notions of "environment" that would be useful for us in substrate:
RuntimeType
. This already exists in a limited fashion in some of the structs and traits generated byconstruct_runtime!
, but right now this information is not easily consumed at compile-time in other contexts like in a pallet.BuildIntent
orBuildMode
orBuildEnvironment
It would make a lot of sense to formulate these as mutually exclusive cfg flags, though cfg flags have their own pitfalls and we could just as easaily implement something like this entirely without cfg flags.
If we did go the cfg flag route, it would be quite easy to gate things for example something like:
For our purposes, it also might make sense to replace the word "production" with "secure", so we would make the "test", "development", and "secure" features mutually exclusive and require exactly one of them to be set at all times.
We technically already have the
test
environment because rust has a built-intest
mode and we usecfg(test)
in many places in the code.It doesn't have to be cfg flags, again, but regardless, if we were to formalize automatically generated enums at compile-time that provide information like this, or produce cfg flags that accomplish the same thing, it would be much, much easier to do things like gate certain features from being used in "production" or in certain contexts, for example:
randomness-collective-flip
pallet toinsecure-randomness-collective-flip
to discourage its usage in production contexts because it is inherently insecureWhile renaming knowingly insecure pallets to be prefixed with "insecure" is an effecitve counter-measure to some of these scenarios, situations like this could be made even safer if we had the granularity at compile-time to actually know whether this is a context where security is required and issue a compile error accordingly if the programmer is trying to use something inherently insecure in a secure context.
Another advantage of the cfg route, by the way, is Rust already has a strong notion of the
test
cfg, so we'd just be adding more and making them mutually exclusive (which is something I've done before).Thoughts?
The text was updated successfully, but these errors were encountered: