Inherents and empty block production overhaul #86

liamaharon · 2024-05-07T17:02:54Z

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 Reduce unnecessary dependencies #32 (no longer compiles kitchensink runtime)

kianenigma · 2024-05-14T11:21:21Z

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)

ggwpez · 2024-05-14T11:50:56Z

core/src/common/state.rs

+                    serde_json::Value::String(format!("0x{}", hex::encode(value))),
+                )
+            })
+            .collect(),


Is this sorted?

Why does it need to be sorted?

core/src/commands/fast_forward.rs

core/src/common/empty_block/inherents/custom_idps/para_parachain.rs

ggwpez

Looks pretty good

core/src/common/empty_block/inherents/pre_apply.rs

core/src/common/empty_block/inherents/providers.rs

ggwpez · 2024-05-14T12:51:45Z

core/src/common/empty_block/inherents/providers.rs

+                })
+                .encode(),
+            ),
+            DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode()),


Could you try with some ecosystem runtimes? I assume that moonbeam would not work, but maybe some others could be used to test it.

Added --blocktime arg to make this work for other runtimes like Aleph Zero that have blocktime other than 6s.

Disapointingly, more ecosystem runtimes I tried than not failed to even compile with the try-runtime feature 😅

Out of those that did compile:

Success

Aleph Zero

Polimec

Zeitgeist

Failed

HydraHX - Digest issue

Unknown

Acala - couldn't find a public node that would serve the on-chain data

Astar - couldn't find a public node that would serve the on-chain data

Findings

HydraDX issue seems specific to them, I think best to work out later when in contact with their devs who hopefully know what's up with their digest.

Parachains are 12s, relay chain 6s, Aleph Zero 1s. If you get the blocktime wrong, you get a potentially confusing error. So I'm going to remove the default for --blocktime so it must be included explicitly.

Yea a lot of parachains also dont compile with the benchmark features 🙈

But at least it shows that it can work. I think that is good enough for now 👍

core/src/common/empty_block/production.rs

liamaharon · 2024-05-14T15:07:03Z

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.

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

kianenigma · 2024-05-15T15:35:15Z

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 · 2024-05-16T06:30:25Z

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.

liamaharon added 2 commits May 7, 2024 20:31

inherent overhaul

d1bce56

bump version

20878ee

liamaharon added the enhancement New feature or request label May 7, 2024

liamaharon requested review from kianenigma and ggwpez May 7, 2024 17:02

liamaharon marked this pull request as draft May 7, 2024 17:49

liamaharon added 2 commits May 8, 2024 17:37

clean up custom idps

d58941c

smart inherent provider

ff30415

liamaharon requested a review from pmikolajczyk41 May 9, 2024 12:14

liamaharon added 7 commits May 9, 2024 17:17

licence and basic docs

5883f46

clippy

b27fd10

remove node-executor node-primitive deps

62d9696

fmt

9c5fcbe

update cargo.lock

ac898c2

move shared params to common

36a8cf1

move misc logging to common

4424f61

liamaharon force-pushed the liam-inherent-overhaul branch from 1e3fae5 to 4424f61 Compare May 10, 2024 02:22

liamaharon added 4 commits May 10, 2024 07:24

move parse to common

a0a5485

move state to common

16c5b39

move inherents to empty block production dir

9632a14

move block creation out of fast forward

4f7e04e

liamaharon force-pushed the liam-inherent-overhaul branch from 5d0d069 to 4f7e04e Compare May 10, 2024 04:01

liamaharon changed the title ~~Inherents overhaul~~ Inherents and empty block production overhaul May 10, 2024

liamaharon added 5 commits May 10, 2024 09:17

improve log

b728086

clippy

039cdaf

cargo fmt

bcdd083

update test runtimes and snapshots

4a406c6

use stable for clippy

fd0092e

liamaharon marked this pull request as ready for review May 14, 2024 11:05

fix typos

4b4648b

ggwpez reviewed May 14, 2024

View reviewed changes

core/src/commands/fast_forward.rs Outdated Show resolved Hide resolved

ggwpez reviewed May 14, 2024

View reviewed changes

core/src/common/empty_block/inherents/custom_idps/para_parachain.rs Outdated Show resolved Hide resolved

ggwpez reviewed May 14, 2024

View reviewed changes

core/src/common/empty_block/inherents/custom_idps/para_parachain.rs Show resolved Hide resolved

ggwpez reviewed May 14, 2024

View reviewed changes

Update core/src/common/empty_block/inherents/pre_apply.rs

527c857

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

liamaharon added 6 commits May 16, 2024 10:36

address comments

c7dbbfe

doc comment

8b465e2

doc

828a357

use valueenum

5980993

rename execute_next_block to mine_block

aee8086

bump relay_offset +1

2254202

liamaharon mentioned this pull request May 20, 2024

Check the version of the Core Runtime Interface to prevent weird future errors when we change something about it #87

Open

blocktime arg

00e2962

liamaharon force-pushed the liam-inherent-overhaul branch from a4b0c84 to 00e2962 Compare May 20, 2024 08:39

liamaharon added 2 commits May 20, 2024 14:12

make blocktime a required value

e80f524

hardcode relaychain blocktime

47a76ca

ggwpez approved these changes May 24, 2024

View reviewed changes

liamaharon added 2 commits May 26, 2024 11:52

clippy

c6a143b

free disk space on test runner

5fdd6d5

liamaharon merged commit 251438f into main May 26, 2024
5 checks passed

ggwpez deleted the liam-inherent-overhaul branch May 28, 2024 19:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inherents and empty block production overhaul #86

Inherents and empty block production overhaul #86

liamaharon commented May 7, 2024 •

edited

kianenigma commented May 14, 2024

ggwpez May 14, 2024

liamaharon May 15, 2024

ggwpez left a comment

ggwpez May 14, 2024

liamaharon May 20, 2024

liamaharon May 20, 2024 •

edited

ggwpez May 24, 2024

liamaharon commented May 14, 2024 •

edited

kianenigma commented May 15, 2024

liamaharon commented May 16, 2024 •

edited

Inherents and empty block production overhaul #86

Inherents and empty block production overhaul #86

Conversation

liamaharon commented May 7, 2024 • edited

kianenigma commented May 14, 2024

ggwpez May 14, 2024

Choose a reason for hiding this comment

liamaharon May 15, 2024

Choose a reason for hiding this comment

ggwpez left a comment

Choose a reason for hiding this comment

ggwpez May 14, 2024

Choose a reason for hiding this comment

liamaharon May 20, 2024

Choose a reason for hiding this comment

liamaharon May 20, 2024 • edited

Choose a reason for hiding this comment

Findings

ggwpez May 24, 2024

Choose a reason for hiding this comment

liamaharon commented May 14, 2024 • edited

kianenigma commented May 15, 2024

liamaharon commented May 16, 2024 • edited

liamaharon commented May 7, 2024 •

edited

liamaharon May 20, 2024 •

edited

liamaharon commented May 14, 2024 •

edited

liamaharon commented May 16, 2024 •

edited