-
Notifications
You must be signed in to change notification settings - Fork 746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MBM try-runtime
support
#4251
MBM try-runtime
support
#4251
Conversation
I remember that you asked somewhere which approach to take. I would take the approach of building multiple blocks until the MBMs are finished. I know that this is more complicated, but this directly checks that your runtime can handle this properly and also that you stay always in the POV limits. (Didn't check the pr yet :D) |
Here's the discussion paritytech/try-runtime-cli#17 (comment) I missed that Kian also supports building proper blocks. Although more involved, I will proceed with that approach :) |
…-sdk into liam-mbm-try-runtime
…-sdk into liam-mbm-try-runtime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -21,6 +21,7 @@ frame-benchmarking = { optional = true, workspace = true } | |||
log = { workspace = true } | |||
scale-info = { workspace = true } | |||
sp-io = { workspace = true } | |||
sp-std = { workspace = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be removed, right?
let bytes = T::Migrations::nth_pre_upgrade(cursor.index) | ||
.expect("Invalid cursor.index") | ||
.expect("Pre-upgrade failed"); | ||
PreUpgradeBytes::<T>::insert(&bounded_id, PreUpgradeBytesWrapper(bytes)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok, it's just try-runtime
storage.
Signed-off-by: georgepisaltu <george.pisaltu@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Waiting for #5649 to merge as to not block the merge queue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to move this check to github action. If this is not urgent I can do it in this PR, if it's urgent - I can do it later.
I am trying to get it in soon since i then want to finally merge this #5676 without any conflicts. I enabled auto-merge last week, but apparently nothing happened. I will re-try it one more time and if that does not work then i guess we have to move the check? |
Draft PR opened for whoever will be taking this over. Next TODO is investigate why MBMs don't seem to progress between blocks being mined. Should be used alongside the changes in this PR: paritytech/polkadot-sdk#4251 --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: claravanstaden <claravanstaden64@gmail.com>
MBM try-runtime support
This MR adds support to the try-runtime trait such that the try-runtime-CLI will be able to support MBM testing here. It mainly adds two feature-gated hooks to the
SteppedMigration
hook to facilitate testing. These hooks are namedpre_upgrade
andpost_upgrade
and have the same signature and implications as for single-block migrations.Integration
To make use of this in your Multi-Block-Migration, just implement the two new hooks and test pre- and post-conditions in them:
You may return an error or panic in these functions to indicate failure. This will then show up in the try-runtime-CLI and can be used in CI for testing.
Changes:
try-runtime
gated methodspre_upgrade
andpost_upgrade
onSteppedMigration
try-runtime
gated methodsnth_pre_upgrade
andnth_post_upgrade
onSteppedMigrations
pallet_migrations
implementation to run pre_upgrade and post_upgrade steps at the appropriate times, and panic in the event of migration failure.