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.

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 105 commits into from
Oct 3, 2021
Merged
Show file tree
Hide file tree
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 Jun 21, 2021
e4f15ce
Sketch out Migration impls
notlesh Jun 25, 2021
ca3620e
Make it build
notlesh Jun 25, 2021
cdc042a
Squelch warnings
notlesh Jun 25, 2021
c4204b4
Sketch out process_runtime_upgrades
notlesh Jun 25, 2021
0cc885d
Leave note for reviewers
notlesh Jun 25, 2021
95308a1
Add &self to Migrations trait fns
notlesh Jun 28, 2021
93b96ce
Make it compile
notlesh Jun 28, 2021
275531c
Refactor migrations design to use stepping instead of one-shot
notlesh Jun 28, 2021
d5abf1f
Fix typo/bug
notlesh Jun 28, 2021
d25ffc1
Track overall migration doneness
notlesh Jun 28, 2021
0c9ce74
Optimize when progress remains unchanged
notlesh Jun 28, 2021
23837f8
Resolve compiler warnings
notlesh Jun 29, 2021
85ecaa1
Incremental progress on mock
notlesh Jun 29, 2021
f22ce8d
Mock is getting close
notlesh Jun 29, 2021
d8d2c3c
Make mock build
notlesh Jun 29, 2021
bfd24f6
Plumb genesis building in mock
notlesh Jun 30, 2021
9404fbe
Baby's first tests
notlesh Jun 30, 2021
edee830
Fix events
notlesh Jun 30, 2021
d13a471
Use Vec<u8> instead of String
notlesh Jun 30, 2021
986433e
Make MigrationsList part of pallet config; plumb through Moonbase run…
notlesh Jul 1, 2021
92047ee
Appease the compiler
notlesh Jul 1, 2021
b885fc5
Fix up CommonMigrations list
notlesh Jul 1, 2021
aefb302
Remove comment
notlesh Jul 1, 2021
f9f8a2a
Cargo fmt
notlesh Jul 1, 2021
bc2e538
Per-test MigrationsList
notlesh Jul 1, 2021
19a9912
Attempt at a glue
notlesh Jul 3, 2021
1f70d01
Fix FIXME
notlesh Jul 3, 2021
6e581f3
Getting close
notlesh Jul 3, 2021
2ea1e3b
Sort out lifetimes
notlesh Jul 3, 2021
4afc532
Simplify FnMut arguments/storage
notlesh Jul 6, 2021
85ed484
Clean up, fix FIXMEs
notlesh Jul 6, 2021
e377caf
It works
notlesh Jul 7, 2021
95d6b95
Implement Migrations::on_initialize
notlesh Jul 7, 2021
d5da28a
Resolve compilation warnings, add comments about how mock glue works
notlesh Jul 7, 2021
2c0bfe5
Move migration event impl
notlesh Jul 7, 2021
67bede2
Let tests manage ExtBuilder ... execute_with()
notlesh Jul 8, 2021
f924112
Test that migrations are only run once
notlesh Jul 8, 2021
5022dcc
Remove TODO/comment; events are not cheap and should be used conserva…
notlesh Jul 8, 2021
8a925ac
Merge branch 'master' into notlesh-migration-sketch
notlesh Jul 19, 2021
a2177dd
Post merge-master fixes
notlesh Jul 19, 2021
01727f8
Remove cruft
notlesh Jul 19, 2021
8ed861f
Track some db reads and writes and charge accordingly
notlesh Jul 19, 2021
0785e48
cargo fmt
notlesh Jul 20, 2021
8c444e7
Add failing test about one-migration-at-a-time
notlesh Jul 21, 2021
2e12fd2
Don't start next migration until current is done
notlesh Jul 21, 2021
9a9bfd4
Add notes from meeting
notlesh Jul 22, 2021
2744571
Allow multi-block migrations to be disabled
notlesh Jul 22, 2021
e1f49c6
Add failing test about overweight migrations
notlesh Jul 22, 2021
0684874
Explicitly embrace allowing overweight migrations
notlesh Jul 22, 2021
c19a4a1
cargo fmt
notlesh Jul 22, 2021
7d57a4e
Clean up / add comments
notlesh Jul 26, 2021
3f87f79
Derive block weight from Config (still needs improvement)
notlesh Jul 26, 2021
595e865
Merge branch 'master' into notlesh-migration-sketch
notlesh Jul 26, 2021
8f6e9e5
cargo fmt
notlesh Jul 26, 2021
a4e2acc
Configure all runtimes to include pallet-migrations
notlesh Jul 26, 2021
41e9d0b
Add pallet-migrations genesis to specs
notlesh Jul 26, 2021
0a3ae7e
Update pallets/migrations/src/lib.rs
notlesh Jul 29, 2021
26ef7db
Update pallets/migrations/src/lib.rs
notlesh Jul 29, 2021
e6543e6
First pass at ripping out multi-block migration support
notlesh Aug 13, 2021
01bc09f
Incremental work @ removing multi-block migration support
notlesh Aug 13, 2021
d2e5759
Make migration tests compile (not passing yet)
notlesh Aug 13, 2021
38ee3a4
Clean up runtime to reflect removal of multi-block migrations
notlesh Aug 17, 2021
30033ee
You know your tests are good when they catch a critical refactor mistake
notlesh Aug 17, 2021
dfb6f42
Fix test logic to reflect no multi-block migrations
notlesh Aug 17, 2021
f691977
cargo fmt
notlesh Aug 17, 2021
dc42819
Remove phantomdata field from pallet_migrations::GenesisConfig (#701)
4meta5 Aug 18, 2021
f31807a
Better log statement
notlesh Aug 18, 2021
607e0ee
Use ValueQuery instead of OptionQuery
notlesh Aug 18, 2021
3e9c26b
Merge branch 'master' into notlesh-migration-sketch
notlesh Aug 18, 2021
91f92f7
Update Cargo.lock
notlesh Aug 18, 2021
c0e6167
Manually add back version = 3
notlesh Aug 18, 2021
d9e561b
Make some deps dev-dependencies
notlesh Aug 18, 2021
3722eaf
Merge branch 'master' into notlesh-migration-sketch
notlesh Sep 8, 2021
a585e3c
Fix branch
notlesh Sep 8, 2021
bdafb76
Use hotfix branch in Migrations
notlesh Sep 8, 2021
9d27292
Clean up from merge
notlesh Sep 8, 2021
e346883
cargo fmt
notlesh Sep 8, 2021
3cb8bb4
Remove prior hack in test
notlesh Sep 8, 2021
1d86ea1
Initial concept of try-runtime support for pallet-migrations
notlesh Sep 9, 2021
d48140c
Impl for post_upgrade()
notlesh Sep 9, 2021
06ee01f
Extend tests to work with pre_ and post_upgrade hooks
notlesh Sep 10, 2021
5bf5b12
Explicitly invoke try-runtime hooks in tests
notlesh Sep 10, 2021
31d52fd
Leave hint about which migrations are performed for post_upgrade
notlesh Sep 10, 2021
64605c8
cargo fmt
notlesh Sep 10, 2021
e58599f
Add pallet-migration's try-runtime feature to all runtimes
notlesh Sep 10, 2021
85b7095
Merge branch 'master' into notlesh-migration-pallet-try-runtime
JoshOrndorff Sep 20, 2021
3e33836
fix warnings
JoshOrndorff Sep 20, 2021
9370840
Move autho mapping migration to dedicated type
JoshOrndorff Sep 20, 2021
b90b726
brain dump
JoshOrndorff Sep 20, 2021
ac5f99f
Make author hasher migration compatible with pallet migrations.
JoshOrndorff Sep 21, 2021
003a52c
Condense code
notlesh Sep 21, 2021
9a41ed3
Only run try-runtime tests with --features=try-runtime
notlesh Sep 21, 2021
9b476e2
Add missing import
notlesh Sep 21, 2021
4a53747
Add try-runtime-only imports
notlesh Sep 21, 2021
6676265
Hook AuthorMapping migration up to common migrations
notlesh Sep 21, 2021
935d0fd
Actually hook it up... also, trait bounds
notlesh Sep 21, 2021
b9e6a61
cargo fmt
notlesh Sep 21, 2021
85d126b
Remove accidental .cargo/config.toml addition
notlesh Sep 21, 2021
4e459c4
better logging
JoshOrndorff Sep 24, 2021
7f546b7
bump spec version for testing purposes
JoshOrndorff Sep 24, 2021
e651798
Merge branch 'master' into notlesh-migration-pallet-try-runtime
JoshOrndorff Oct 1, 2021
a9da31d
cleanups
JoshOrndorff Oct 1, 2021
95d69c5
line length
JoshOrndorff Oct 1, 2021
fe80835
std feature
JoshOrndorff Oct 2, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

119 changes: 3 additions & 116 deletions pallets/author-mapping/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ mod mock;
#[cfg(test)]
mod tests;

pub mod migrations;

#[pallet]
pub mod pallet {
use crate::WeightInfo;
Expand Down Expand Up @@ -237,7 +239,7 @@ pub mod pallet {
#[pallet::getter(fn account_and_deposit_of)]
/// We maintain a mapping from the AuthorIds used in the consensus layer
/// to the AccountIds runtime (including this staking pallet).
type MappingWithDeposit<T: Config> = StorageMap<
pub type MappingWithDeposit<T: Config> = StorageMap<
Copy link
Collaborator

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

Copy link
Contributor

@JoshOrndorff JoshOrndorff Oct 2, 2021

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

_,
Blake2_128Concat,
T::AuthorId,
Expand Down Expand Up @@ -283,119 +285,4 @@ pub mod pallet {
Self::account_and_deposit_of(author_id).map(|info| info.account)
}
}

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_runtime_upgrade() -> Weight {
use frame_support::storage::migration::{remove_storage_prefix, storage_key_iter};
use sp_std::{convert::TryInto, vec::Vec};

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);
}

// 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, storage_key_iter};
use frame_support::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(())
}
}
}
144 changes: 144 additions & 0 deletions pallets/author-mapping/src/migrations.rs
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(())
}
}
3 changes: 3 additions & 0 deletions pallets/migrations/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,6 @@ std = [
"sp-std/std",
"sp-runtime/std",
]
try-runtime = [
"frame-support/try-runtime",
]
Loading