Inherents and empty block production overhaul

paritytech / try-runtime-cli

Substrate's programatic testing framework.

https://paritytech.github.io/try-runtime-cli/try_runtime

GNU General Public License v3.0

14 stars 13 forks source link

Inherents and empty block production overhaul #86

Closed liamaharon closed 5 months ago

liamaharon commented 6 months ago

A pre-requisite to producing empty blocks for MBMs is fixing inherents for relay and parachains, and moving block production logic from being tied to fast_forward to common logic.

It will also be very beneficial to allow producing blocks based on a snapshot, without any live node (--block-ws-uri).

Based on the changes in this PR, I'll add MBM support to the on-runtime-upgrade subcommand.

Fixes inherent providers for relay and parachains
Introduces a SMART inherent provider designed to work across different chains, removing the requirement for a cli arg to specify which inherents to use
Moves inherent and block production to a common utils dir
Removes requirement for a remote node to produce empty blocks
Closes #32 (no longer compiles kitchensink runtime)

kianenigma commented 5 months ago

Based on the changes in this PR, I'll add MBM support to the on-runtime-upgrade subcommand.

How do you imagine the interface for this to look like?

also, I wonder after this, how different on-runtime-upgrade is vs. fast-forward -n 1? or in the case of MBMs fast-forward -n until-all-ext-inclusion (which produces empty blocks until ExtrinsicInclusionMode::All is reached)

liamaharon commented 5 months ago

Based on the changes in this PR, I'll add MBM support to the on-runtime-upgrade subcommand.

How do you imagine the interface for this to look like?

also, I wonder after this, how different on-runtime-upgrade is vs. fast-forward -n 1? or in the case of MBMs fast-forward -n until-all-ext-inclusion (which produces empty blocks until ExtrinsicInclusionMode::All is reached)

I'm imagining that there's a new --mbms flag for on-runtime-upgrade, which by default is enabled. When enabled, after the migrations are executed, empty blocks will be mined continuously until MBMs are completed.

ExtrinsicInclusionMode::All is not enough to signify successful MBMs, because it's possible to configure your chain to go back into that mode when MBMs fail. Instead, we can

Try to decode frame-system events (probably won't work because we don't know how to decode them)
Create a try-runtime variant of execute block that returns MBM progress.

kianenigma commented 5 months ago

I'm imagining that there's a new --mbms flag for on-runtime-upgrade, which by default is enabled. When enabled, after the migrations are executed, empty blocks will be mined continuously until MBMs are completed.

I don't want to be too picky and ack that we need something quick here, but for the sake of a pedantic argument, what argument is making you lean towards this design, as opposed to making everything be a variant of fast-forward?

it's possible to configure your chain to go back into that mode when MBMs fail

Can you elaborate on the scenario here? I am not familiar, and anticipated that at least for successful MBMs, this return type would be enough.

liamaharon commented 5 months ago

I don't want to be too picky and ack that we need something quick here, but for the sake of a pedantic argument, what argument is making you lean towards this design, as opposed to making everything be a variant of fast-forward?

fast-forward

won't tell you about whether your MBMs succeeded, it just fast forwards blocks
does not have any of the migration safety checks that we have implemented in the on-runtime-upgrade subcommand, such as idempotency or spec version checks

Can you elaborate on the scenario here? I am not familiar, and anticipated that at least for successful MBMs, this return type would be enough.

There is no 'return type' for MBMs. If the runtime specifies ForceUnstuck as the action to take on failure, then ExtrinsicInclusionMode::All will be set regardless of whether the MBMs completed successfully or encountered a critical error.