This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[FRAME] Simple multi block migrations #14275
Draft
ggwpez
wants to merge
66
commits into
master
Choose a base branch
from
oty-multi-step-migration
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+2,155
−95
Draft
Changes from 10 commits
Commits
Show all changes
66 commits
Select commit
Hold shift + click to select a range
dc35acf
Add stepped migrations and pallet
ggwpez 283d51c
Cleanup
ggwpez 7c53a74
Comment
ggwpez edb005c
Suspension and benches
ggwpez 8331fbe
Add generic to Executive
ggwpez c23259f
Cleanup
ggwpez 9ffd3d7
Merge remote-tracking branch 'origin/master' into oty-multi-step-migr…
ggwpez 3afdf0a
Add Executed
ggwpez 104fd6d
Add events
ggwpez 1e18cae
Fixes
ggwpez 7ab621a
Make cursor generic
ggwpez acd99bb
Fix kitchensink
ggwpez d217589
Add testing
ggwpez 3351a54
Tests
ggwpez 4eeb47e
Use default configs for System
ggwpez 80922c9
Tests
ggwpez 4ae8fbf
Fixes
ggwpez e6beda5
Add status handling
ggwpez fa20468
Refactor
ggwpez ad98b43
Fixes
ggwpez 5c3a90d
Clippy
ggwpez 357fa47
Update frame/support/src/migrations.rs
ggwpez da04876
Update frame/migrations/src/mock.rs
ggwpez 2e86915
Update frame/migrations/src/tests.rs
ggwpez 99691a3
fmt
ggwpez e44c902
Add timeout checking
ggwpez 5f512e6
Fix stuff
ggwpez c72c7b7
Add docs
ggwpez 790914d
Fix docs and kitchensink
ggwpez 642d65a
Format Cargo.toml
ggwpez a2b3194
Docs
ggwpez 60c1322
Now it sounds like English 😂
ggwpez 17be927
Fix test
ggwpez 741c476
fmt
ggwpez 6a1438e
Benches still WIP
ggwpez 0e4f8a6
Move code
ggwpez 97b58e1
Docs review fixes
ggwpez 87ac850
Add tests
ggwpez b8c8559
Rename to OnMigrationUpdate
ggwpez 3b27c4f
Fix bugs
ggwpez 07f6889
More fixes
ggwpez f202a80
Fixes
ggwpez 1249955
Undo changes to frame-executive
ggwpez 717b30e
Fix executive
ggwpez c7e8384
Fix
ggwpez a1589a3
Fix docs
ggwpez 4fd5d93
Add log target to sc-basic-authorship
ggwpez e75d0c1
Merge remote-tracking branch 'origin/master' into oty-multi-step-migr…
ggwpez 37cf9b8
Add mock_helpers
ggwpez 69fddc3
Add progress_mbms to BlockBuilder
ggwpez 7435788
Add log target
ggwpez 6b1d6b1
Factor out apply_inherents
ggwpez 0b7dddc
Factor out apply_extrinsics
ggwpez 648e005
Factor out print_summary
ggwpez 24c8bf6
Pimp print_summary
ggwpez 4b99c6e
Cleanup
ggwpez 93f0d32
Fixes
ggwpez c57bbf5
Backup
ggwpez 30c302e
Merge branch 'oty-refactor-block-builder' into oty-multi-step-migration
ggwpez 4101e82
Fix stuff
ggwpez 8f7bda1
Works
ggwpez e430d57
Fix template
ggwpez 89313b3
Fix test
ggwpez 999b0f0
WIP
ggwpez f87a8e4
Use impl trait for tuples
bkchr f3f76b6
Simplify
bkchr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| [package] | ||
| name = "pallet-migrations" | ||
| version = "1.0.0" | ||
| description = "FRAME pallet to execute multi-block migrations." | ||
| authors = ["Parity Technologies <admin@parity.io>"] | ||
| homepage = "https://substrate.io" | ||
| edition = "2021" | ||
| license = "Apache-2.0" | ||
| repository = "https://github.com/paritytech/substrate" | ||
|
|
||
| [package.metadata.docs.rs] | ||
| targets = ["x86_64-unknown-linux-gnu"] | ||
|
|
||
| [dependencies] | ||
| codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = [ "derive"] } | ||
| scale-info = { version = "2.0.0", default-features = false, features = ["derive"] } | ||
|
|
||
| frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" } | ||
| frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } | ||
| frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } | ||
| impl-trait-for-tuples = "0.2.2" | ||
| log = "0.4.18" | ||
| sp-std = { version = "8.0.0", path = "../../primitives/std", default-features = false } | ||
| sp-runtime = { version = "24.0.0", path = "../../primitives/runtime", default-features = false } | ||
|
|
||
| [dev-dependencies] | ||
| sp-tracing = { version = "10.0.0", path = "../../primitives/tracing", features = [ "std" ] } | ||
| sp-core = { version = "21.0.0", path = "../../primitives/core", features = [ "std" ] } | ||
| sp-io = { version = "23.0.0", path = "../../primitives/io", features = [ "std" ] } | ||
|
|
||
| [features] | ||
| default = ["std"] | ||
|
|
||
| std = [ | ||
| "codec/std", | ||
| "frame-benchmarking?/std", | ||
| "frame-support/std", | ||
| "frame-system/std", | ||
| "scale-info/std", | ||
| "sp-std/std", | ||
| "sp-runtime/std", | ||
| ] | ||
|
|
||
| runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks", "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", "sp-runtime/runtime-benchmarks",] | ||
|
|
||
| try-runtime = ["frame-support/try-runtime", "sp-runtime/try-runtime",] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| // This file is part of Substrate. | ||
|
|
||
| // Copyright (C) Parity Technologies (UK) Ltd. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| #![cfg(feature = "runtime-benchmarks")] | ||
|
|
||
| use super::*; | ||
|
|
||
| use frame_benchmarking::v2::*; | ||
| use frame_support::migrations::STEPPED_MIGRATION_CURSOR_LEN as CURSOR_LEN; | ||
| use frame_system::{Pallet as System, RawOrigin}; | ||
|
|
||
| #[benchmarks] | ||
| mod benches { | ||
| use super::*; | ||
| use frame_support::traits::Hooks; | ||
|
|
||
| #[benchmark] | ||
| fn on_runtime_upgrade() { | ||
| assert!(!Cursor::<T>::exists()); | ||
|
|
||
| #[block] | ||
| { | ||
| Pallet::<T>::on_runtime_upgrade(); | ||
| } | ||
| } | ||
|
|
||
| #[benchmark] | ||
| fn on_init_base() { | ||
| assert!(!Cursor::<T>::exists()); | ||
| System::<T>::set_block_number(1u32.into()); | ||
|
|
||
| #[block] | ||
| { | ||
| Pallet::<T>::on_initialize(1u32.into()); | ||
| } | ||
| } | ||
|
|
||
| #[benchmark] | ||
| fn on_init_loop_base() { | ||
| System::<T>::set_block_number(1u32.into()); | ||
| Pallet::<T>::on_runtime_upgrade(); | ||
|
|
||
| #[block] | ||
| { | ||
| Pallet::<T>::on_initialize(1u32.into()); | ||
| } | ||
| } | ||
|
|
||
| /// Benchmarks the slowest path of `change_value`. | ||
| #[benchmark] | ||
| fn force_set_cursor() { | ||
| Cursor::<T>::set(Some(cursor(0))); | ||
|
|
||
| #[extrinsic_call] | ||
| _(RawOrigin::Root, Some(cursor(1))); | ||
| } | ||
|
|
||
| fn cursor(i: u32) -> MigrationCursor { | ||
| MigrationCursor::Active( | ||
| u32::MAX - i, | ||
| Some(vec![1u8; CURSOR_LEN as usize].try_into().expect("Static length is good")), | ||
| ) | ||
| } | ||
|
|
||
| // Implements a test for each benchmark. Execute with: | ||
| // `cargo test -p pallet-migrations --features runtime-benchmarks`. | ||
| impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Orthogonal to my other comment but I feel this is wrong: For a non mandatory suspended extrinsic, the extrinsic is not applied but it is noted applied with
note_applied_extrinsicand the apply extrinsic is a success.Instead what we want is to not apply suspended extrinsics.
So I think here it should do an early return of
Err(InvalidTransaction::Suspended)then in client basic-authorship in the proposer we should end the proposing when we find a suspended transaction. (because runtime only accept inherents when it is suspending transactions)Or we do an early return of
Err(Exhausted)and the proposer will tryMAX_SKIPPED_TRANSACTIONS=8other transaction and then end the proposing. ( if I read the code correctly )(Or we go with making the block full in on_initialize or on_post_inherent)
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.
Yes. This is what i meant above with:
This could also be returned as bool from
initialize_block. So the block builder will then know to not include any extrinsics.Do you think that could work? I think it is better than the approach to first include the extrinsic and then reject it from inside the 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.
I changed it to panic now. The block builder should not be able to include non-mandatory transactions while a MBM is ongoing. And a block will not be importable if it does not respect that.