-
Notifications
You must be signed in to change notification settings - Fork 336
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
pallet-migrations: try-runtime support, and manage author mapping blake2 migration #796
Merged
Merged
Changes from all commits
Commits
Show all changes
105 commits
Select commit
Hold shift + click to select a range
9ecdc74
Add initial migrations pallet sketch
notlesh e4f15ce
Sketch out Migration impls
notlesh ca3620e
Make it build
notlesh cdc042a
Squelch warnings
notlesh c4204b4
Sketch out process_runtime_upgrades
notlesh 0cc885d
Leave note for reviewers
notlesh 95308a1
Add &self to Migrations trait fns
notlesh 93b96ce
Make it compile
notlesh 275531c
Refactor migrations design to use stepping instead of one-shot
notlesh d5abf1f
Fix typo/bug
notlesh d25ffc1
Track overall migration doneness
notlesh 0c9ce74
Optimize when progress remains unchanged
notlesh 23837f8
Resolve compiler warnings
notlesh 85ecaa1
Incremental progress on mock
notlesh f22ce8d
Mock is getting close
notlesh d8d2c3c
Make mock build
notlesh bfd24f6
Plumb genesis building in mock
notlesh 9404fbe
Baby's first tests
notlesh edee830
Fix events
notlesh d13a471
Use Vec<u8> instead of String
notlesh 986433e
Make MigrationsList part of pallet config; plumb through Moonbase run…
notlesh 92047ee
Appease the compiler
notlesh b885fc5
Fix up CommonMigrations list
notlesh aefb302
Remove comment
notlesh f9f8a2a
Cargo fmt
notlesh bc2e538
Per-test MigrationsList
notlesh 19a9912
Attempt at a glue
notlesh 1f70d01
Fix FIXME
notlesh 6e581f3
Getting close
notlesh 2ea1e3b
Sort out lifetimes
notlesh 4afc532
Simplify FnMut arguments/storage
notlesh 85ed484
Clean up, fix FIXMEs
notlesh e377caf
It works
notlesh 95d6b95
Implement Migrations::on_initialize
notlesh d5da28a
Resolve compilation warnings, add comments about how mock glue works
notlesh 2c0bfe5
Move migration event impl
notlesh 67bede2
Let tests manage ExtBuilder ... execute_with()
notlesh f924112
Test that migrations are only run once
notlesh 5022dcc
Remove TODO/comment; events are not cheap and should be used conserva…
notlesh 8a925ac
Merge branch 'master' into notlesh-migration-sketch
notlesh a2177dd
Post merge-master fixes
notlesh 01727f8
Remove cruft
notlesh 8ed861f
Track some db reads and writes and charge accordingly
notlesh 0785e48
cargo fmt
notlesh 8c444e7
Add failing test about one-migration-at-a-time
notlesh 2e12fd2
Don't start next migration until current is done
notlesh 9a9bfd4
Add notes from meeting
notlesh 2744571
Allow multi-block migrations to be disabled
notlesh e1f49c6
Add failing test about overweight migrations
notlesh 0684874
Explicitly embrace allowing overweight migrations
notlesh c19a4a1
cargo fmt
notlesh 7d57a4e
Clean up / add comments
notlesh 3f87f79
Derive block weight from Config (still needs improvement)
notlesh 595e865
Merge branch 'master' into notlesh-migration-sketch
notlesh 8f6e9e5
cargo fmt
notlesh a4e2acc
Configure all runtimes to include pallet-migrations
notlesh 41e9d0b
Add pallet-migrations genesis to specs
notlesh 0a3ae7e
Update pallets/migrations/src/lib.rs
notlesh 26ef7db
Update pallets/migrations/src/lib.rs
notlesh e6543e6
First pass at ripping out multi-block migration support
notlesh 01bc09f
Incremental work @ removing multi-block migration support
notlesh d2e5759
Make migration tests compile (not passing yet)
notlesh 38ee3a4
Clean up runtime to reflect removal of multi-block migrations
notlesh 30033ee
You know your tests are good when they catch a critical refactor mistake
notlesh dfb6f42
Fix test logic to reflect no multi-block migrations
notlesh f691977
cargo fmt
notlesh dc42819
Remove phantomdata field from pallet_migrations::GenesisConfig (#701)
4meta5 f31807a
Better log statement
notlesh 607e0ee
Use ValueQuery instead of OptionQuery
notlesh 3e9c26b
Merge branch 'master' into notlesh-migration-sketch
notlesh 91f92f7
Update Cargo.lock
notlesh c0e6167
Manually add back version = 3
notlesh d9e561b
Make some deps dev-dependencies
notlesh 3722eaf
Merge branch 'master' into notlesh-migration-sketch
notlesh a585e3c
Fix branch
notlesh bdafb76
Use hotfix branch in Migrations
notlesh 9d27292
Clean up from merge
notlesh e346883
cargo fmt
notlesh 3cb8bb4
Remove prior hack in test
notlesh 1d86ea1
Initial concept of try-runtime support for pallet-migrations
notlesh d48140c
Impl for post_upgrade()
notlesh 06ee01f
Extend tests to work with pre_ and post_upgrade hooks
notlesh 5bf5b12
Explicitly invoke try-runtime hooks in tests
notlesh 31d52fd
Leave hint about which migrations are performed for post_upgrade
notlesh 64605c8
cargo fmt
notlesh e58599f
Add pallet-migration's try-runtime feature to all runtimes
notlesh 85b7095
Merge branch 'master' into notlesh-migration-pallet-try-runtime
JoshOrndorff 3e33836
fix warnings
JoshOrndorff 9370840
Move autho mapping migration to dedicated type
JoshOrndorff b90b726
brain dump
JoshOrndorff ac5f99f
Make author hasher migration compatible with pallet migrations.
JoshOrndorff 003a52c
Condense code
notlesh 9a41ed3
Only run try-runtime tests with --features=try-runtime
notlesh 9b476e2
Add missing import
notlesh 4a53747
Add try-runtime-only imports
notlesh 6676265
Hook AuthorMapping migration up to common migrations
notlesh 935d0fd
Actually hook it up... also, trait bounds
notlesh b9e6a61
cargo fmt
notlesh 85d126b
Remove accidental .cargo/config.toml addition
notlesh 4e459c4
better logging
JoshOrndorff 7f546b7
bump spec version for testing purposes
JoshOrndorff e651798
Merge branch 'master' into notlesh-migration-pallet-try-runtime
JoshOrndorff a9da31d
cleanups
JoshOrndorff 95d69c5
line length
JoshOrndorff fe80835
std feature
JoshOrndorff 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,144 @@ | ||
// Copyright 2019-2021 PureStake Inc. | ||
// This file is part of Moonbeam. | ||
|
||
// Moonbeam is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
|
||
// Moonbeam is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
|
||
// You should have received a copy of the GNU General Public License | ||
// along with Moonbeam. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
use crate::{BalanceOf, Config, MappingWithDeposit, RegistrationInfo}; | ||
use frame_support::{ | ||
pallet_prelude::PhantomData, | ||
storage::migration::{remove_storage_prefix, storage_key_iter}, | ||
traits::{Get, OnRuntimeUpgrade}, | ||
weights::Weight, | ||
Twox64Concat, | ||
}; | ||
|
||
use sp_std::convert::TryInto; | ||
//TODO sometimes this is unused, sometimes its necessary | ||
use sp_std::vec::Vec; | ||
|
||
/// Migrates the AuthorMapping's storage map fro mthe insecure Twox64 hasher to the secure | ||
/// BlakeTwo hasher. | ||
pub struct TwoXToBlake<T>(PhantomData<T>); | ||
impl<T: Config> OnRuntimeUpgrade for TwoXToBlake<T> { | ||
fn on_runtime_upgrade() -> Weight { | ||
log::info!(target: "TwoXToBlake", "actually running it"); | ||
let pallet_prefix: &[u8] = b"AuthorMapping"; | ||
let storage_item_prefix: &[u8] = b"MappingWithDeposit"; | ||
|
||
// Read all the data into memory. | ||
// https://crates.parity.io/frame_support/storage/migration/fn.storage_key_iter.html | ||
let stored_data: Vec<_> = storage_key_iter::< | ||
T::AuthorId, | ||
RegistrationInfo<T::AccountId, BalanceOf<T>>, | ||
Twox64Concat, | ||
>(pallet_prefix, storage_item_prefix) | ||
.collect(); | ||
|
||
let migrated_count: Weight = stored_data | ||
.len() | ||
.try_into() | ||
.expect("There are between 0 and 2**64 mappings stored."); | ||
|
||
// Now remove the old storage | ||
// https://crates.parity.io/frame_support/storage/migration/fn.remove_storage_prefix.html | ||
remove_storage_prefix(pallet_prefix, storage_item_prefix, &[]); | ||
|
||
// Assert that old storage is empty | ||
assert!(storage_key_iter::< | ||
T::AuthorId, | ||
RegistrationInfo<T::AccountId, BalanceOf<T>>, | ||
Twox64Concat, | ||
>(pallet_prefix, storage_item_prefix) | ||
.next() | ||
.is_none()); | ||
|
||
// Write the mappings back to storage with the new secure hasher | ||
for (author_id, account_id) in stored_data { | ||
MappingWithDeposit::<T>::insert(author_id, account_id); | ||
} | ||
|
||
log::info!(target: "TwoXToBlake", "almost done"); | ||
|
||
// Return the weight used. For each migrated mapping there is a red to get it into | ||
// memory, a write to clear the old stored value, and a write to re-store it. | ||
let db_weights = T::DbWeight::get(); | ||
migrated_count.saturating_mul(2 * db_weights.write + db_weights.read) | ||
} | ||
|
||
#[cfg(feature = "try-runtime")] | ||
fn pre_upgrade() -> Result<(), &'static str> { | ||
use frame_support::{storage::migration::storage_iter, traits::OnRuntimeUpgradeHelpersExt}; | ||
|
||
let pallet_prefix: &[u8] = b"AuthorMapping"; | ||
let storage_item_prefix: &[u8] = b"MappingWithDeposit"; | ||
|
||
// We want to test that: | ||
// There are no entries in the new storage beforehand | ||
// The same number of mappings exist before and after | ||
// As long as there are some mappings stored, one representative key maps to the | ||
// same value after the migration. | ||
// There are no entries in the old storage afterward | ||
|
||
// Assert new storage is empty | ||
// Because the pallet and item prefixes are the same, the old storage is still at this | ||
// key. However, the values can't be decoded so the assertion passes. | ||
assert!(MappingWithDeposit::<T>::iter().next().is_none()); | ||
|
||
// Check number of entries, and set it aside in temp storage | ||
let mapping_count = storage_iter::<RegistrationInfo<T::AccountId, BalanceOf<T>>>( | ||
pallet_prefix, | ||
storage_item_prefix, | ||
) | ||
.count() as u64; | ||
Self::set_temp_storage(mapping_count, "mapping_count"); | ||
|
||
// Read an example pair from old storage and set it aside in temp storage | ||
if mapping_count > 0 { | ||
let example_pair = storage_key_iter::< | ||
T::AuthorId, | ||
RegistrationInfo<T::AccountId, BalanceOf<T>>, | ||
Twox64Concat, | ||
>(pallet_prefix, storage_item_prefix) | ||
.next() | ||
.expect("We already confirmed that there was at least one item stored"); | ||
|
||
Self::set_temp_storage(example_pair, "example_pair"); | ||
} | ||
|
||
Ok(()) | ||
} | ||
|
||
#[cfg(feature = "try-runtime")] | ||
fn post_upgrade() -> Result<(), &'static str> { | ||
use frame_support::traits::OnRuntimeUpgradeHelpersExt; | ||
|
||
// Check number of entries matches what was set aside in pre_upgrade | ||
let old_mapping_count: u64 = Self::get_temp_storage("mapping_count") | ||
.expect("We stored a mapping count; it should be there; qed"); | ||
let new_mapping_count = MappingWithDeposit::<T>::iter().count() as u64; | ||
assert_eq!(old_mapping_count, new_mapping_count); | ||
|
||
// Check that our example pair is still well-mapped after the migration | ||
if new_mapping_count > 0 { | ||
let (account, original_info): ( | ||
T::AuthorId, | ||
RegistrationInfo<T::AccountId, BalanceOf<T>>, | ||
) = Self::get_temp_storage("example_pair").expect("qed"); | ||
let migrated_info = MappingWithDeposit::<T>::get(account).expect("qed"); | ||
assert_eq!(original_info, migrated_info); | ||
} | ||
|
||
Ok(()) | ||
} | ||
} |
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 |
---|---|---|
|
@@ -26,3 +26,6 @@ std = [ | |
"sp-std/std", | ||
"sp-runtime/std", | ||
] | ||
try-runtime = [ | ||
"frame-support/try-runtime", | ||
] |
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.
We are only using this in the crate context, we can make it pub crate only
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.
That's a valid point, but our other pallets also have them just
pub
(eg https://github.com/PureStake/moonbeam/blob/master/pallets/asset-manager/src/lib.rs#L119). And I worry that making stuff too restricted will make it hard to write precompiles in the short term.Here's a followup issue to treat this throughout the repo. https://purestake.atlassian.net/browse/MOON-933