Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add fast-runtime Cargo Feature for Quick Test Runs #4332

Merged
merged 31 commits into from Jan 13, 2022

Conversation

apopiak
Copy link
Contributor

@apopiak apopiak commented Nov 19, 2021

This is meant as a first stab at making certain runtime configuration values configurable via feature flag.
This PR:

  • introduces the fast-runtime feature flag to toggle at build time whether the production or fast/testing values should be used for configuration values like the session time.
  • introduces the prod_or_fast macro to easily define config values taking advantage of the fast-runtime flag.
  • adjusts a lot of durations down to 2 minutes = 20 blocks, 1 minute = 10 blocks or 1 block (if the feature is selected).
  • also introduces environment variables that can be set at build time to specify custom periods (if the feature is selected).

Usage

So to take advantage of the features introduced here you might call cargo like so:

KSM_EPOCH_DURATION=40 KSM_LAUNCH_PERIOD=3 cargo build --release --features=fast-runtime

or with cargo remote:

cargo remote -c release/polkadot -b 'KSM_LAUNCH_PERIOD=2; DOT_LAUNCH_PERIOD=2' -- build --release --features=fast-runtime

Kusama values are prefixed with KSM, Polkadot values with DOT.

Motivation

This allows for easier local testing of the interaction between the production chains with parachains (e.g. Kusama and Statemine).

aims to make progress towards #4333

@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Nov 19, 2021
@apopiak apopiak added B1-releasenotes C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels Nov 19, 2021
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@apopiak
Copy link
Contributor Author

apopiak commented Nov 22, 2021

I tried parsing a const out of an env variable with

core::option_env!("ENV")
  .map(|s| s.parse().ok())
  .flatten()
  .unwrap_or(default)

but failed because parse is not const. Open to ideas on how we could add something equivalent.

@kianenigma
Copy link
Contributor

I tried parsing a const out of an env variable with

core::option_env!("ENV")
  .map(|s| s.parse().ok())
  .flatten()
  .unwrap_or(default)

but failed because parse is not const. Open to ideas on how we could add something equivalent.

If you need const for parameter types, it does support non-const as well.

@apopiak
Copy link
Contributor Author

apopiak commented Nov 23, 2021

If you need const for parameter types, it does support non-const as well.

Mmh, but would we be comfortable with making it non-const in production for the test usecase?

@kianenigma
Copy link
Contributor

kianenigma commented Nov 23, 2021

If you need const for parameter types, it does support non-const as well.

Mmh, but would we be comfortable with making it non-const in production for the test usecase?

I am pretty comfortable with both, as long as there's no unreasonable shenanigans going on. Somethings are hard to make 'const', event though they are constant in reality. For example, see Type BlockWeights in system.

runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
apopiak and others added 3 commits December 4, 2021 12:19
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@apopiak
Copy link
Contributor Author

apopiak commented Dec 10, 2021

I think another alternative implementation here is to just have the macro apply some constant reduction to all the constants than defining both.

So having fast-runtime divides any macro wrapped value by 1_000 or an environment variable amount.

Not at flexible, but probably just as useful with less code.

This would definitely only work for very few values. E.g. launch period can be reduced to 1 block without issues, but the election workers need enough time to submit their solutions (5 blocks (2min session time / 4) seems to work, didn't figure out the lower bound).
I much prefer the approach in this PR.

Copy link
Contributor

@gilescope gilescope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@crystalin
Copy link

How about disabling the UnexpectedEpochChange also with it ? It is quite common for test network to get stuck by it

@gilescope
Copy link
Contributor

I take your point but talking with a few people it sounds like there should be a separate feature that turns on settings that make things more robust in terms of max finality lag and max backoffs. This is purely timings, but features are designed to be additive so I think they would play well together.

@gilescope
Copy link
Contributor

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 8261642 into master Jan 13, 2022
@paritytech-processbot paritytech-processbot bot deleted the apopiak/fast-runtime branch January 13, 2022 20:08
Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this pull request Feb 3, 2022
* add fast-runtime feature for reduced session times

* make democracy periods fast on fast-runtime

* propagate fast-runtime feature through cargo.toml files

* add fast motion and term durations to Kusama

* Update runtime/westend/Cargo.toml

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* set session time to 2 minutes to avoid block production issues

* formatting

* update Substrate

* set democracy fast periods back to 1min

* set launch period and enactment period to 1 block in fast-runtime

* remove unnecessary westend period configs

* add prod_or_test macro to allow specifying prod, test and env values for parameter types

* move prod_or_test macro into common module and use it consistently

* rename macro to prod_or_fast

* cargo +nightly fmt

* bump impl_versions

* newline

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* add note that env variable is evaluated at compile time

* newline

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* newline

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* cargo fmt

* impl_version: 0

* impl_version: 0

* use prod_or_fast macro for LeasePeriod and LeaseOffset

* use prod_or_fast macro in WND and ROC constants

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Giles Cope <gilescope@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants