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

try-runtime-cli: support testing new multi-block migrations #14291

Closed
liamaharon opened this issue Jun 2, 2023 · 3 comments
Closed

try-runtime-cli: support testing new multi-block migrations #14291

liamaharon opened this issue Jun 2, 2023 · 3 comments
Labels
J0-enhancement An additional feature request. S4-blocked Issue is blocked, see comments for further information. T1-runtime This PR/Issue is related to the topic “runtime”. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.

Comments

@liamaharon
Copy link
Contributor

liamaharon commented Jun 2, 2023

          @kianenigma am I right to assume we'll want to make it easy to test these with try-runtime-cli? Ideally should be as easy as `on-runtime-upgrade`, maybe make it auto-mine blocks until all multi-block migrations are complete & provide a hook that runs afterwards?

Originally posted by @liamaharon in #14275 (comment)

Kian response:

Good to think about it early, and yes I think it is possible with little effort. I believe follow-chain will already do everything needed to test an MBM?


I don't think follow-chain is necessarily a good solution.

  1. We need to call each upgrade's post_upgrade hook once its migration has finished executing.
  2. We can't use snapshots with follow-chain, making development iterations much slower and requiring a live node running. Some of the plumbing in fast-forward is might be a better solution here?

Ideally, I think we'd be able to run on-runtime-upgrade like usual and it'll fast-forward until all migrations have completed, and then run a post_migration hook. Unfortunately fast-forward is not production ready due to the inherents issue, it seems this feature makes fixing the inherents issue much higher priority now?

@liamaharon liamaharon added J0-enhancement An additional feature request. U2-some_time_soon Issue is worth doing soon. Z3-substantial Can be fixed by an experienced coder with a working knowledge of the codebase. labels Jun 2, 2023
@ggwpez
Copy link
Member

ggwpez commented Jun 2, 2023

I think fast-forward is good, since follow-chain applies extrinsics; those get rejected while MBMs are active anyway. (and is slow)
Just calling on_runtime_upgrade and then on_initialize repeatedly until event UpgradeCompleted | UpgradeFailed is received is enough AFAIK.

@liamaharon
Copy link
Contributor Author

liamaharon commented Jun 2, 2023

Yeah, calling on_runtime_upgrade and then on_initialize repeatedly is a better idea and will be easier to implement, thanks @ggwpez 👌

What do you think about adding a try-runtime feature gated method to the migrations pallet called something like run_to_completion that does that? That way, the cli tool needs not be concerned about the mechanism that migrations progress.

@liamaharon liamaharon added Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. and removed U2-some_time_soon Issue is worth doing soon. Z3-substantial Can be fixed by an experienced coder with a working knowledge of the codebase. labels Jun 4, 2023
@juangirini juangirini added the T1-runtime This PR/Issue is related to the topic “runtime”. label Jun 7, 2023
@liamaharon liamaharon added the S4-blocked Issue is blocked, see comments for further information. label Jul 27, 2023
@liamaharon
Copy link
Contributor Author

Moved to paritytech/try-runtime-cli#17

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request. S4-blocked Issue is blocked, see comments for further information. T1-runtime This PR/Issue is related to the topic “runtime”. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.
Projects
Status: Done
Development

No branches or pull requests

3 participants