Skip to content

Conversation

RomarQ
Copy link
Contributor

@RomarQ RomarQ commented Aug 7, 2025

Recently, when moving the single block migrations from frame_executive::Executive to SingleBlockMigrations in frame_system::Config, I noticed that try_runtime_upgrade was ignoring the SingleBlockMigrations defined in frame_system. More context at polkadot-fellows/runtimes#844

Based on PR #1781 and PRDoc, the new way for providing the single block migrations should be through SingleBlockMigrations in frame_system::Config. Providing them from frame_executive::Executive is still supported, but from what I understood is or will be deprecated.

SingleBlockMigrations this is the new way of configuring migrations that run in a single block. Previously they were defined as last generic argument of Executive. This shift is brings all central configuration about migrations closer into view of the developer (migrations that are configured in Executive will still work for now but is deprecated).

Follow-up Changes

Will try to open a pull request tomorrow for deprecating the use of OnRuntimeUpgrade in frame_executive::Executive.

@RomarQ RomarQ requested a review from a team as a code owner August 7, 2025 20:52
@RomarQ RomarQ changed the title Call SingleBlockMigrations frame_system::Config on try_on_runtime_upgrade Call SingleBlockMigrations from frame_system::Config on try_on_runtime_upgrade Aug 7, 2025
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Besides a missing test it looks good 👍

@ggwpez ggwpez added A4-backport-stable2506 Pull request must be backported to the stable2506 release branch A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch labels Aug 8, 2025
@ggwpez
Copy link
Member

ggwpez commented Aug 8, 2025

I hope we can still backport stuff into unstable2507 since the runtimes will directly go to that.

@RomarQ
Copy link
Contributor Author

RomarQ commented Aug 8, 2025

@ggwpez @bkchr

Just added a test, let me know if something else needs to be done.

@RomarQ
Copy link
Contributor Author

RomarQ commented Aug 8, 2025

I think all CI checks related to this PR are now passing

@RomarQ
Copy link
Contributor Author

RomarQ commented Aug 12, 2025

@bkchr @ggwpez can this be merged?

@RomarQ
Copy link
Contributor Author

RomarQ commented Aug 25, 2025

@bkchr @ggwpez Can we merge this?

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. (holidays)

I'm not happy with the current test, please fix it and then we can merge this.

@RomarQ
Copy link
Contributor Author

RomarQ commented Sep 1, 2025

Sorry for the delay. (holidays)

I'm not happy with the current test, please fix it and then we can merge this.

The fixed method is inside TryRuntime, the test needs to be behind the try-runtime feature, like the others in the file. I may have not understood your remark correctly, let me know if that was the case.

@RomarQ
Copy link
Contributor Author

RomarQ commented Sep 2, 2025

Also important to note that this is a bug.

@bkchr bkchr enabled auto-merge September 2, 2025 10:14
@bkchr bkchr added this pull request to the merge queue Sep 2, 2025
Merged via the queue into paritytech:master with commit 7753112 Sep 2, 2025
246 checks passed
paritytech-release-backport-bot bot pushed a commit that referenced this pull request Sep 2, 2025
…e_upgrade (#9451)

Recently, when moving the single block migrations from
`frame_executive::Executive` to `SingleBlockMigrations` in
`frame_system::Config`, I noticed that `try_runtime_upgrade` was
ignoring the `SingleBlockMigrations` defined in frame_system. More
context at polkadot-fellows/runtimes#844

Based on PR #1781 and
[PRDoc](https://github.com/paritytech/polkadot-sdk/blob/beb9030b249cc078b3955232074a8495e7e0302a/prdoc/1.9.0/pr_1781.prdoc#L29),
the new way for providing the single block migrations should be through
`SingleBlockMigrations` in `frame_system::Config`. Providing them from
`frame_executive::Executive` is still supported, but from what I
understood is or will be deprecated.

> `SingleBlockMigrations` this is the new way of configuring migrations
that run in a single block. Previously they were defined as last generic
argument of Executive. This shift is brings all central configuration
about migrations closer into view of the developer (migrations that are
configured in Executive will still work for now but is deprecated).

## Follow-up Changes
Will try to open a pull request tomorrow for deprecating the use of
`OnRuntimeUpgrade` in `frame_executive::Executive`.

(cherry picked from commit 7753112)
@paritytech-release-backport-bot

Successfully created backport PR for stable2506:

@paritytech-release-backport-bot

Successfully created backport PR for unstable2507:

paritytech-release-backport-bot bot pushed a commit that referenced this pull request Sep 2, 2025
…e_upgrade (#9451)

Recently, when moving the single block migrations from
`frame_executive::Executive` to `SingleBlockMigrations` in
`frame_system::Config`, I noticed that `try_runtime_upgrade` was
ignoring the `SingleBlockMigrations` defined in frame_system. More
context at polkadot-fellows/runtimes#844

Based on PR #1781 and
[PRDoc](https://github.com/paritytech/polkadot-sdk/blob/beb9030b249cc078b3955232074a8495e7e0302a/prdoc/1.9.0/pr_1781.prdoc#L29),
the new way for providing the single block migrations should be through
`SingleBlockMigrations` in `frame_system::Config`. Providing them from
`frame_executive::Executive` is still supported, but from what I
understood is or will be deprecated.

> `SingleBlockMigrations` this is the new way of configuring migrations
that run in a single block. Previously they were defined as last generic
argument of Executive. This shift is brings all central configuration
about migrations closer into view of the developer (migrations that are
configured in Executive will still work for now but is deprecated).

## Follow-up Changes
Will try to open a pull request tomorrow for deprecating the use of
`OnRuntimeUpgrade` in `frame_executive::Executive`.

(cherry picked from commit 7753112)
@RomarQ RomarQ deleted the rq/fix-try_runtime_upgrade branch September 2, 2025 12:26
EgorPopelyaev pushed a commit that referenced this pull request Sep 2, 2025
Backport #9451 into `stable2506` from RomarQ.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Rodrigo Quelhas <[email protected]>
bkchr pushed a commit that referenced this pull request Sep 4, 2025
Backport #9451 into `unstable2507` from RomarQ.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Rodrigo Quelhas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A4-backport-stable2506 Pull request must be backported to the stable2506 release branch A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants