Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add migration idempotency checks #48

Merged
merged 16 commits into from
Nov 10, 2023
Merged

Add migration idempotency checks #48

merged 16 commits into from
Nov 10, 2023

Conversation

liamaharon
Copy link
Contributor

Closes #42

Checks that on_runtime_upgrade succeeds when called twice, and if it does succeed after being called twice, the storage state root did not change.

@liamaharon liamaharon added the enhancement New feature or request label Oct 31, 2023
pmikolajczyk41
pmikolajczyk41 approved these changes Nov 7, 2023
View reviewed changes
Copy link
Collaborator

@pmikolajczyk41 pmikolajczyk41 left a comment

Choose a reason for hiding this comment

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

cool!

core/tests/execute_block.rs Show resolved Hide resolved
core/tests/on_runtime_upgrade.rs Outdated Show resolved Hide resolved
core/src/lib.rs Show resolved Hide resolved
core/tests/on_runtime_upgrade.rs Outdated Show resolved Hide resolved
core/src/commands/on_runtime_upgrade.rs Show resolved Hide resolved
ggwpez
ggwpez approved these changes Nov 7, 2023
View reviewed changes
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Nice! I will try it.

core/src/commands/on_runtime_upgrade.rs Show resolved Hide resolved
core/src/commands/on_runtime_upgrade.rs Outdated Show resolved Hide resolved
core/tests/on_runtime_upgrade.rs Outdated Show resolved Hide resolved
@ggwpez
Copy link
Member

ggwpez commented Nov 9, 2023 •
edited
Loading

As a follow up: we can mock the host function ext_logging_log_version_1 to silence the logs on the second run, or selectively set the target/level to make it less noisy.

@liamaharon
Copy link
Contributor Author

liamaharon commented Nov 9, 2023 •
edited
Loading

As a follow up: we can mock the host function ext_logging_log_version_1 to silence the logs on the second run, or selectively set the target/level to make it less noisy.

Oh this will be very useful also for silencing logs when measuring PoV. Thanks!

@liamaharon liamaharon mentioned this pull request Nov 10, 2023
Open
@liamaharon liamaharon merged commit f621966 into main Nov 10, 2023
4 checks passed
@liamaharon liamaharon deleted the liam-check-migrations-are-idempotent branch November 10, 2023 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmikolajczyk41 pmikolajczyk41 pmikolajczyk41 approved these changes

@ggwpez ggwpez ggwpez approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

on-runtime-upgrade should enforce migrations are idempotent
3 participants
@liamaharon @ggwpez @pmikolajczyk41