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

Selectable on-runtime-upgrade checks #13045

Merged
merged 10 commits into from
Jan 4, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Jan 2, 2023

🚨 breaking changes:

  • frame_try_runtime::TryRuntime::on_runtime_upgrade changes its argument type from bool to UpgradeCheckSelect. See the companion for an integration example.

Non breaking:

  • try-runtime-cli: The --checks arg of on-runtime-upgrade now also accepts: all, none, pre-and-post and try-state to select the check to run, instead of running all. The values need to be passed with an =.

This makes the type of checks selectable, which is desirable for faster testing since the try-state can take up to two hours for Polkadot when you only want to test the pre- and post-hooks.

(The CI does not realize it needs a companion, but it does)
Polkadot companion: paritytech/polkadot#6498

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added B5-clientnoteworthy C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jan 2, 2023
@ggwpez ggwpez requested a review from kianenigma January 2, 2023 21:50
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Jan 2, 2023
frame/support/src/traits/try_runtime.rs Outdated Show resolved Hide resolved
ggwpez and others added 2 commits January 3, 2023 12:49
Co-authored-by: Bastian Köcher <git@kchr.de>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
ggwpez and others added 6 commits January 3, 2023 14:41
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
…ech/substrate into oty-try-runtime-cli-select-check
…ech/substrate into oty-try-runtime-cli-select-check
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
#[clap(long,
default_value = "None",
default_missing_value = "All",
num_args = 0..=1,
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this or? default_value should indicate this?

Copy link
Contributor

Choose a reason for hiding this comment

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

could also use some context on what these flags do, but I believe they help us remain backwards compat.

Copy link
Member Author

@ggwpez ggwpez Jan 4, 2023

Choose a reason for hiding this comment

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

It is needed since otherwise it does not work as flag without arg. The difference in the help print:
--checks=<CHECKS> (without num_args)
vs
--checks[=<CHECKS>] (with num_args)

I added a comment.

@@ -2470,12 +2470,17 @@ impl<T: Config> Pallet<T> {

for id in reward_pools {
let account = Self::create_reward_account(id);
assert!(
Copy link
Member

Choose a reason for hiding this comment

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

Why did you removed the assert?

Copy link
Contributor

Choose a reason for hiding this comment

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

I did, it does not hold anymore because we changed ED in Kusama :(

Copy link
Member Author

Choose a reason for hiding this comment

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

@kianenigma
Copy link
Contributor

(The CI does not realize it needs a companion, but it does)

The CI also does not realize that this needs a cumulus companion.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented Jan 4, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit a052224 into master Jan 4, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-try-runtime-cli-select-check branch January 4, 2023 12:44
trusch added a commit to KILTprotocol/kilt-node that referenced this pull request Jan 23, 2023
## fixes KILTProtocol/ticket#2392

## Breaking Changes for us

~~None! 🥳~~

Edit: Forgot to also check with try-runtime feature enabled. There is a
small tweak necessary because of [This PR about
on-runtime-upgrade](paritytech/substrate#13045)

No database migrations, no runtime migrations and no new host functions.

## Polkadot Release Link

https://github.com/paritytech/polkadot/releases/tag/v0.9.37

## Release Analysis Forum Post

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-37/1736

## Cool new stuff that might be useful (or not)

* [frame_support::storage: Add
StorageStreamIter](paritytech/substrate#12721)
* If we have a StorageValue that contains something iterable, we can
directly iterate over it, without copying the memory first by a regular
get() call.
* [Add ensure_* mathematical
methods](paritytech/substrate#12754)
* The checked_* family of calls returns an Option which is in 99% of the
cases mapped to an error
* ensure_* calls directly return an error which can be propagated using
questionmark operator more easily
* [Kusama shows how to express complex local origins in XCM
messages](paritytech/polkadot#6273)
* Perhaps the most interesting one in this release, would be a good idea
for @weichweich and @ntn-x2 to have a look into this
* [pallet_uniques successor NFTv2 is out! 🥳 😄
](paritytech/substrate#12765)
* Finally we can have NFTs with owner controlled metadata on our chain.
* They even literally mention that this way users can write DIDs
directly on their NFT!
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* Make try-runtime checks selectable

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/support/src/traits/try_runtime.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Add Clap wrapper for enum UpgradeCheckSelect

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Revert "Add Clap wrapper for enum UpgradeCheckSelect"

This reverts commit e29538c.

* fix pools sanity check

* Set default for --checks to None

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Make --checks backwards comp

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add clap attr comment

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: kianenigma <kian@parity.io>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Make try-runtime checks selectable

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/support/src/traits/try_runtime.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Add Clap wrapper for enum UpgradeCheckSelect

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Revert "Add Clap wrapper for enum UpgradeCheckSelect"

This reverts commit e29538c.

* fix pools sanity check

* Set default for --checks to None

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Make --checks backwards comp

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add clap attr comment

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: kianenigma <kian@parity.io>
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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants