Skip to content

Commit

Permalink
Update contract multi-block migration (paritytech#14313)
Browse files Browse the repository at this point in the history 
* move migrate sequence to config

* remove commented out code

* Update frame/contracts/src/lib.rs

Co-authored-by: PG Herveou <pgherveou@gmail.com>

* remove Migrations generic

* make runtime use noop migrations

* restrict is_upgrade_supported

* Update contract multi-block migration

Ensure that we do as many steps as possible given the weight limit passed to on_idle

* undo is_upgrade_supported change

* Update bin/node/runtime/src/lib.rs

Co-authored-by: PG Herveou <pgherveou@gmail.com>

* wip

* fix comment (paritytech#14316)

* fix test

* fix

* Update frame/contracts/src/migration.rs

Co-authored-by: Juan <juangirini@gmail.com>

* fix test doc

* Apply suggestions from code review

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

* Fix compilation with feature runtime-benchmarks

* fix example

* fix  cargo doc --document-private-items

* private links

* Remove dup comment

* add doc for MigrationInProgress

* PR review remove duplicate asserts

* simplify upper bound

* fix link

* typo

* typo

* no unwrap()

* correct log message

* missing

* fix typo

* PR comment

* Add example with single element tuple

* Improve migration message

* Update frame/contracts/src/benchmarking/mod.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

* Update frame/contracts/src/migration.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

* Update frame/contracts/src/migration.rs

Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

* use saturating_accrue instead of +=

* add more doc

* Contracts: Better migration types (paritytech#14418)

* Add explicit error, if try-runtime runs a noop migration

* use mut remaining_weight

---------

Co-authored-by: Juan Girini <juangirini@gmail.com>
Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>
  • Loading branch information
@pgherveou @juangirini
@agryaznov @nathanwhit
3 people authored and nathanwhit committed Jul 19, 2023
1 parent dbc6a8f commit ccb4e91
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 104 deletions.
16 changes: 8 additions & 8 deletions frame/contracts/src/benchmarking/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use self::{
};
use crate::{
exec::{AccountIdOf, Key},
migration::{v10, v11, v9, Migrate},
migration::{v10, v11, v9, MigrationStep},
wasm::CallFlags,
Pallet as Contracts, *,
};
Expand Down Expand Up @@ -237,7 +237,7 @@ benchmarks! {
// This benchmarks the v9 migration step. (update codeStorage)
#[pov_mode = Measured]
v9_migration_step {
let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get());
let c in 0 .. T::MaxCodeLen::get();
v9::store_old_dummy_code::<T>(c as usize);
let mut m = v9::Migration::<T>::default();
}: {
Expand All @@ -251,7 +251,7 @@ benchmarks! {
whitelisted_caller(), WasmModule::dummy(), vec![],
)?;

v10::store_old_contrat_info::<T>(contract.account_id.clone(), contract.info()?);
v10::store_old_contract_info::<T>(contract.account_id.clone(), contract.info()?);
let mut m = v10::Migration::<T>::default();
}: {
m.step();
Expand All @@ -277,15 +277,15 @@ benchmarks! {
assert_eq!(StorageVersion::get::<Pallet<T>>(), 2);
}

// This benchmarks the weight of executing Migration::migrate when there are no migration in progress.
// This benchmarks the weight of dispatching migrate to execute 1 `NoopMigraton`
#[pov_mode = Measured]
migrate {
StorageVersion::new(0).put::<Pallet<T>>();
<Migration::<T, false> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();
let origin: RawOrigin<<T as frame_system::Config>::AccountId> = RawOrigin::Signed(whitelisted_caller());
}: {
<Contracts<T>>::migrate(origin.into(), Weight::MAX).unwrap()
} verify {
let caller: T::AccountId = whitelisted_caller();
let origin = RawOrigin::Signed(caller.clone());
}: _(origin, Weight::MAX)
verify {
assert_eq!(StorageVersion::get::<Pallet<T>>(), 1);
}

Expand Down
31 changes: 25 additions & 6 deletions frame/contracts/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
//! an [`eDSL`](https://wiki.haskell.org/Embedded_domain_specific_language) that enables writing
//! WebAssembly based smart contracts in the Rust programming language.
#![allow(rustdoc::private_intra_doc_links)]
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(feature = "runtime-benchmarks", recursion_limit = "1024")]

Expand Down Expand Up @@ -328,19 +329,35 @@ pub mod pallet {
/// # struct Runtime {};
/// type Migrations = (v9::Migration<Runtime>, v10::Migration<Runtime>, v11::Migration<Runtime>);
/// ```
///
/// If you have a single migration step, you can use a tuple with a single element:
/// ```
/// use pallet_contracts::migration::v9;
/// # struct Runtime {};
/// type Migrations = (v9::Migration<Runtime>,);
/// ```
type Migrations: MigrateSequence;
}

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_idle(_block: T::BlockNumber, remaining_weight: Weight) -> Weight {
fn on_idle(_block: T::BlockNumber, mut remaining_weight: Weight) -> Weight {
use migration::MigrateResult::*;

let (result, weight) = Migration::<T>::migrate(remaining_weight);
let remaining_weight = remaining_weight.saturating_sub(weight);

if !matches!(result, Completed | NoMigrationInProgress) {
return weight
loop {
let (result, weight) = Migration::<T>::migrate(remaining_weight);
remaining_weight.saturating_reduce(weight);

match result {
// There is not enough weight to perform a migration, or make any progress, we
// just return the remaining weight.
NoMigrationPerformed | InProgress { steps_done: 0 } => return remaining_weight,
// Migration is still in progress, we can start the next step.
InProgress { .. } => continue,
// Either no migration is in progress, or we are done with all migrations, we
// can do some more other work with the remaining weight.
Completed | NoMigrationInProgress => break,
}
}

ContractInfo::<T>::process_deletion_queue_batch(remaining_weight)
Expand Down Expand Up @@ -987,6 +1004,8 @@ pub mod pallet {
pub(crate) type DeletionQueueCounter<T: Config> =
StorageValue<_, DeletionQueueManager<T>, ValueQuery>;

/// A migration can span across multiple blocks. This storage defines a cursor to track the
/// progress of the migration, enabling us to resume from the last completed position.
#[pallet::storage]
pub(crate) type MigrationInProgress<T: Config> =
StorageValue<_, migration::Cursor, OptionQuery>;
Expand Down
121 changes: 67 additions & 54 deletions frame/contracts/src/migration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,46 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! Migration framework for pallets.
//! Multi-block Migration framework for pallet-contracts.
//!
//! This module allows us to define a migration as a sequence of [`MigrationStep`]s that can be
//! executed across multiple blocks.
//!
//! # Usage
//!
//! A migration step is defined under `src/migration/vX.rs`, where `X` is the version number.
//! For example, `vX.rs` defines a migration from version `X - 1` to version `X`.
//!
//! ## Example:
//!
//! To configure a migration to `v11` for a runtime using `v8` of pallet-contracts on the chain, you
//! would set the `Migrations` type as follows:
//!
//! ```
//! use pallet_contracts::migration::{v9, v10, v11};
//! # pub enum Runtime {};
//! type Migrations = (v9::Migration<Runtime>, v10::Migration<Runtime>, v11::Migration<Runtime>);
//! ```
//!
//! ## Notes:
//!
//! - Migrations should always be tested with `try-runtime` before being deployed.
//! - By testing with `try-runtime` against a live network, you ensure that all migration steps work
//! and that you have included the required steps.
//!
//! ## Low Level / Implementation Details
//!
//! When a migration starts and [`OnRuntimeUpgrade::on_runtime_upgrade`] is called, instead of
//! performing the actual migration, we set a custom storage item [`MigrationInProgress`].
//! This storage item defines a [`Cursor`] for the current migration.
//!
//! If the [`MigrationInProgress`] storage item exists, it means a migration is in progress, and its
//! value holds a cursor for the current migration step. These migration steps are executed during
//! [`Hooks<BlockNumber>::on_idle`] or when the [`Pallet::migrate`] dispatchable is
//! called.
//!
//! While the migration is in progress, all dispatchables except `migrate`, are blocked, and returns
//! a `MigrationInProgress` error.
pub mod v10;
pub mod v11;
Expand All @@ -28,6 +67,7 @@ use frame_support::{
pallet_prelude::*,
traits::{ConstU32, OnRuntimeUpgrade},
};
use sp_runtime::Saturating;
use sp_std::marker::PhantomData;

#[cfg(feature = "try-runtime")]
Expand All @@ -44,7 +84,8 @@ fn invalid_version(version: StorageVersion) -> ! {
panic!("Required migration {version:?} not supported by this runtime. This is a bug.");
}

/// The cursor used to store the state of the current migration step.
/// The cursor used to encode the position (usually the last iterated key) of the current migration
/// step.
pub type Cursor = BoundedVec<u8, ConstU32<1024>>;

/// IsFinished describes whether a migration is finished or not.
Expand All @@ -57,7 +98,7 @@ pub enum IsFinished {
///
/// The migration is done in steps. The migration is finished when
/// `step()` returns `IsFinished::Yes`.
pub trait Migrate: Codec + MaxEncodedLen + Default {
pub trait MigrationStep: Codec + MaxEncodedLen + Default {
/// Returns the version of the migration.
const VERSION: u16;

Expand Down Expand Up @@ -110,7 +151,7 @@ pub trait Migrate: Codec + MaxEncodedLen + Default {
#[derive(frame_support::DefaultNoBound, Encode, Decode, MaxEncodedLen)]
pub struct NoopMigration<const N: u16>;

impl<const N: u16> Migrate for NoopMigration<N> {
impl<const N: u16> MigrationStep for NoopMigration<N> {
const VERSION: u16 = N;
fn max_step_weight() -> Weight {
Weight::zero()
Expand All @@ -122,10 +163,10 @@ impl<const N: u16> Migrate for NoopMigration<N> {
}

mod private {
use crate::migration::Migrate;
use crate::migration::MigrationStep;
pub trait Sealed {}
#[impl_trait_for_tuples::impl_for_tuples(10)]
#[tuple_types_custom_trait_bound(Migrate)]
#[tuple_types_custom_trait_bound(MigrationStep)]
impl Sealed for Tuple {}
}

Expand All @@ -134,11 +175,11 @@ mod private {
/// The sequence must be defined by a tuple of migrations, each of which must implement the
/// `Migrate` trait. Migrations must be ordered by their versions with no gaps.
pub trait MigrateSequence: private::Sealed {
/// Returns the range of versions that this migration can handle.
/// Returns the range of versions that this migrations sequence can handle.
/// Migrations must be ordered by their versions with no gaps.
/// The following code will fail to compile:
///
/// The following code will fail to compile:
///
/// ```compile_fail
/// # use pallet_contracts::{NoopMigration, MigrateSequence};
/// let _ = <(NoopMigration<1>, NoopMigration<3>)>::VERSION_RANGE;
Expand Down Expand Up @@ -172,21 +213,10 @@ pub trait MigrateSequence: private::Sealed {

/// Returns whether migrating from `in_storage` to `target` is supported.
///
/// A migration is supported if (in_storage + 1, target) is contained by `VERSION_RANGE`.
/// A migration is supported if `VERSION_RANGE` is (in_storage + 1, target).
fn is_upgrade_supported(in_storage: StorageVersion, target: StorageVersion) -> bool {
if in_storage == target {
return true
}
if in_storage > target {
return false
}

let (low, high) = Self::VERSION_RANGE;
let Some(first_supported) = low.checked_sub(1) else {
return false
};

in_storage >= first_supported && target == high
target == high && in_storage + 1 == low
}
}

Expand Down Expand Up @@ -276,17 +306,20 @@ impl<T: Config, const TEST_ALL_STEPS: bool> OnRuntimeUpgrade for Migration<T, TE
let storage_version = <Pallet<T>>::on_chain_storage_version();
let target_version = <Pallet<T>>::current_storage_version();

ensure!(
storage_version != target_version,
"No upgrade: Please remove this migration from your runtime upgrade configuration."
);

log::debug!(
target: LOG_TARGET,
"{}: Range supported {:?}, range requested {:?}",
<Pallet<T>>::name(),
T::Migrations::VERSION_RANGE,
(storage_version, target_version)
"Requested migration of {} from {:?}(on-chain storage version) to {:?}(current storage version)",
<Pallet<T>>::name(), storage_version, target_version
);

ensure!(
T::Migrations::is_upgrade_supported(storage_version, target_version),
"Unsupported upgrade"
"Unsupported upgrade: VERSION_RANGE should be (on-chain storage version + 1, current storage version)"
);
Ok(Default::default())
}
Expand All @@ -313,7 +346,8 @@ pub enum StepResult {
}

impl<T: Config, const TEST_ALL_STEPS: bool> Migration<T, TEST_ALL_STEPS> {
/// Verify that each migration's step of the [`T::Migrations`] sequence fits into `Cursor`.
/// Verify that each migration's step of the [`Config::Migrations`] sequence fits into
/// `Cursor`.
pub(crate) fn integrity_test() {
let max_weight = <T as frame_system::Config>::BlockWeights::get().max_block;
T::Migrations::integrity_test(max_weight)
Expand Down Expand Up @@ -394,7 +428,7 @@ impl<T: Config, const TEST_ALL_STEPS: bool> Migration<T, TEST_ALL_STEPS> {
}

#[impl_trait_for_tuples::impl_for_tuples(10)]
#[tuple_types_custom_trait_bound(Migrate)]
#[tuple_types_custom_trait_bound(MigrationStep)]
impl MigrateSequence for Tuple {
const VERSION_RANGE: (u16, u16) = {
let mut versions: (u16, u16) = (0, 0);
Expand Down Expand Up @@ -461,7 +495,7 @@ impl MigrateSequence for Tuple {
let mut steps_done = 0;
while weight_left.all_gt(max_weight) {
let (finished, weight) = migration.step();
steps_done += 1;
steps_done.saturating_accrue(1);
weight_left.saturating_reduce(weight);
if matches!(finished, IsFinished::Yes) {
return StepResult::Completed{ steps_done }
Expand Down Expand Up @@ -494,7 +528,7 @@ mod test {
count: u16,
}

impl<const N: u16> Migrate for MockMigration<N> {
impl<const N: u16> MigrationStep for MockMigration<N> {
const VERSION: u16 = N;
fn max_step_weight() -> Weight {
Weight::from_all(1)
Expand All @@ -519,30 +553,9 @@ mod test {
#[test]
fn is_upgrade_supported_works() {
type Migrations = (MockMigration<9>, MockMigration<10>, MockMigration<11>);

[(1, 1), (8, 11), (9, 11)].into_iter().for_each(|(from, to)| {
assert!(
Migrations::is_upgrade_supported(
StorageVersion::new(from),
StorageVersion::new(to)
),
"{} -> {} is supported",
from,
to
)
});

[(1, 0), (0, 3), (7, 11), (8, 10)].into_iter().for_each(|(from, to)| {
assert!(
!Migrations::is_upgrade_supported(
StorageVersion::new(from),
StorageVersion::new(to)
),
"{} -> {} is not supported",
from,
to
)
});
assert!(Migrations::is_upgrade_supported(StorageVersion::new(8), StorageVersion::new(11)));
assert!(!Migrations::is_upgrade_supported(StorageVersion::new(9), StorageVersion::new(11)));
assert!(!Migrations::is_upgrade_supported(StorageVersion::new(8), StorageVersion::new(12)));
}

#[test]
Expand Down
Loading

0 comments on commit ccb4e91

Please sign in to comment.