Skip to content

Commit

Permalink
[pallet-revive] remove custom migration framework
Browse files Browse the repository at this point in the history 
Fixes #5621
  • Loading branch information
@pgherveou
pgherveou committed Sep 6, 2024
1 parent e72a4af commit cf44f41
Show file tree
Hide file tree
Showing 9 changed files with 3 additions and 1,329 deletions.
4 changes: 0 additions & 4 deletions substrate/bin/node/runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1418,10 +1418,6 @@ impl pallet_revive::Config for Runtime {
type UploadOrigin = EnsureSigned<Self::AccountId>;
type InstantiateOrigin = EnsureSigned<Self::AccountId>;
type RuntimeHoldReason = RuntimeHoldReason;
#[cfg(not(feature = "runtime-benchmarks"))]
type Migrations = ();
#[cfg(feature = "runtime-benchmarks")]
type Migrations = pallet_revive::migration::codegen::BenchMigrations;
type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent;
type Debug = ();
type Xcm = ();
Expand Down
1 change: 0 additions & 1 deletion substrate/frame/revive/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ name = "pallet-revive"
version = "0.1.0"
authors.workspace = true
edition.workspace = true
build = "build.rs"
license = "Apache-2.0"
homepage.workspace = true
repository.workspace = true
Expand Down
78 changes: 0 additions & 78 deletions substrate/frame/revive/build.rs
View file

This file was deleted.

69 changes: 0 additions & 69 deletions substrate/frame/revive/src/benchmarking/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use self::{call_builder::CallSetup, code::WasmModule};
use crate::{
exec::{Key, MomentOf},
limits,
migration::codegen::LATEST_MIGRATION_VERSION,
storage::WriteOutcome,
Pallet as Contracts, *,
};
Expand All @@ -34,7 +33,6 @@ use codec::{Encode, MaxEncodedLen};
use frame_benchmarking::v2::*;
use frame_support::{
self, assert_ok,
pallet_prelude::StorageVersion,
storage::child,
traits::{fungible::InspectHold, Currency},
weights::{Weight, WeightMeter},
Expand Down Expand Up @@ -251,73 +249,6 @@ mod benchmarks {
Ok(())
}

// This benchmarks the weight of executing Migration::migrate to execute a noop migration.
#[benchmark(pov_mode = Measured)]
fn migration_noop() {
let version = LATEST_MIGRATION_VERSION;
StorageVersion::new(version).put::<Pallet<T>>();
#[block]
{
Migration::<T>::migrate(&mut WeightMeter::new());
}
assert_eq!(StorageVersion::get::<Pallet<T>>(), version);
}

// This benchmarks the weight of dispatching migrate to execute 1 `NoopMigration`
#[benchmark(pov_mode = Measured)]
fn migrate() {
let latest_version = LATEST_MIGRATION_VERSION;
StorageVersion::new(latest_version - 2).put::<Pallet<T>>();
<Migration<T, false> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();

#[extrinsic_call]
_(RawOrigin::Signed(whitelisted_caller()), Weight::MAX);

assert_eq!(StorageVersion::get::<Pallet<T>>(), latest_version - 1);
}

// This benchmarks the weight of running on_runtime_upgrade when there are no migration in
// progress.
#[benchmark(pov_mode = Measured)]
fn on_runtime_upgrade_noop() {
let latest_version = LATEST_MIGRATION_VERSION;
StorageVersion::new(latest_version).put::<Pallet<T>>();
#[block]
{
<Migration<T, false> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();
}
assert!(MigrationInProgress::<T>::get().is_none());
}

// This benchmarks the weight of running on_runtime_upgrade when there is a migration in
// progress.
#[benchmark(pov_mode = Measured)]
fn on_runtime_upgrade_in_progress() {
let latest_version = LATEST_MIGRATION_VERSION;
StorageVersion::new(latest_version - 2).put::<Pallet<T>>();
let v = vec![42u8].try_into().ok();
MigrationInProgress::<T>::set(v.clone());
#[block]
{
<Migration<T, false> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();
}
assert!(MigrationInProgress::<T>::get().is_some());
assert_eq!(MigrationInProgress::<T>::get(), v);
}

// This benchmarks the weight of running on_runtime_upgrade when there is a migration to
// process.
#[benchmark(pov_mode = Measured)]
fn on_runtime_upgrade() {
let latest_version = LATEST_MIGRATION_VERSION;
StorageVersion::new(latest_version - 2).put::<Pallet<T>>();
#[block]
{
<Migration<T, false> as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();
}
assert!(MigrationInProgress::<T>::get().is_some());
}

// This benchmarks the overhead of loading a code of size `c` byte from storage and into
// the execution engine. This does **not** include the actual execution for which the gas meter
// is responsible. This is achieved by generating all code to the `deploy` function
Expand Down
93 changes: 1 addition & 92 deletions substrate/frame/revive/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ mod wasm;

pub mod chain_extension;
pub mod debug;
pub mod migration;
pub mod test_utils;
pub mod weights;

Expand All @@ -57,7 +56,7 @@ use environmental::*;
use frame_support::{
dispatch::{
DispatchErrorWithPostInfo, DispatchResultWithPostInfo, GetDispatchInfo, Pays,
PostDispatchInfo, RawOrigin, WithPostDispatchInfo,
PostDispatchInfo, RawOrigin,
},
ensure,
traits::{
Expand All @@ -82,7 +81,6 @@ use sp_runtime::{
pub use crate::{
address::{AddressMapper, DefaultAddressMapper},
debug::Tracing,
migration::{MigrateSequence, Migration, NoopMigration},
pallet::*,
};
pub use weights::WeightInfo;
Expand Down Expand Up @@ -275,25 +273,6 @@ pub mod pallet {
#[pallet::no_default_bounds]
type InstantiateOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Self::AccountId>;

/// The sequence of migration steps that will be applied during a migration.
///
/// # Examples
/// ```ignore
/// use pallet_revive::migration::{v10, v11};
/// # struct Runtime {};
/// # struct Currency {};
/// type Migrations = (v10::Migration<Runtime, Currency>, v11::Migration<Runtime>);
/// ```
///
/// If you have a single migration step, you can use a tuple with a single element:
/// ```ignore
/// use pallet_revive::migration::v10;
/// # struct Runtime {};
/// # struct Currency {};
/// type Migrations = (v10::Migration<Runtime, Currency>,);
/// ```
type Migrations: MigrateSequence;

/// For most production chains, it's recommended to use the `()` implementation of this
/// trait. This implementation offers additional logging when the log target
/// "runtime::revive" is set to trace.
Expand Down Expand Up @@ -386,7 +365,6 @@ pub mod pallet {
type DepositPerByte = DepositPerByte;
type DepositPerItem = DepositPerItem;
type MaxCodeLen = ConstU32<{ 123 * 1024 }>;
type Migrations = ();
type Time = Self;
type UnsafeUnstableInterface = ConstBool<true>;
type UploadOrigin = EnsureSigned<AccountId>;
Expand Down Expand Up @@ -553,10 +531,6 @@ pub mod pallet {
/// A more detailed error can be found on the node console if debug messages are enabled
/// by supplying `-lruntime::revive=debug`.
CodeRejected,
/// A pending migration needs to complete before the extrinsic can be called.
MigrationInProgress,
/// Migrate dispatch call was attempted but no migration was performed.
NoMigrationPerformed,
/// The contract has reached its maximum number of delegate dependencies.
MaxDelegateDependenciesReached,
/// The dependency was not found in the contract's delegate dependencies.
Expand Down Expand Up @@ -611,12 +585,6 @@ 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>;

#[pallet::extra_constants]
impl<T: Config> Pallet<T> {
#[pallet::constant_name(ApiVersion)]
Expand All @@ -631,29 +599,12 @@ pub mod pallet {
T::Hash: IsType<H256>,
{
fn on_idle(_block: BlockNumberFor<T>, limit: Weight) -> Weight {
use migration::MigrateResult::*;
let mut meter = WeightMeter::with_limit(limit);

loop {
match Migration::<T>::migrate(&mut meter) {
// There is not enough weight to perform a migration.
// We can't do anything more, so we return the used weight.
NoMigrationPerformed | InProgress { steps_done: 0 } => return meter.consumed(),
// 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(&mut meter);
meter.consumed()
}

fn integrity_test() {
Migration::<T>::integrity_test();

// Total runtime memory limit
let max_runtime_mem: u32 = T::RuntimeMemory::get();
// Memory limits for a single contract:
Expand Down Expand Up @@ -969,7 +920,6 @@ pub mod pallet {
origin: OriginFor<T>,
code_hash: sp_core::H256,
) -> DispatchResultWithPostInfo {
Migration::<T>::ensure_migrated()?;
let origin = ensure_signed(origin)?;
<WasmBlob<T>>::remove(&origin, code_hash)?;
// we waive the fee because removing unused code is beneficial
Expand All @@ -993,7 +943,6 @@ pub mod pallet {
dest: H160,
code_hash: sp_core::H256,
) -> DispatchResult {
Migration::<T>::ensure_migrated()?;
ensure_root(origin)?;
<ContractInfoOf<T>>::try_mutate(&dest, |contract| {
let contract = if let Some(contract) = contract {
Expand All @@ -1012,40 +961,6 @@ pub mod pallet {
Ok(())
})
}

/// When a migration is in progress, this dispatchable can be used to run migration steps.
/// Calls that contribute to advancing the migration have their fees waived, as it's helpful
/// for the chain. Note that while the migration is in progress, the pallet will also
/// leverage the `on_idle` hooks to run migration steps.
#[pallet::call_index(6)]
#[pallet::weight(T::WeightInfo::migrate().saturating_add(*weight_limit))]
pub fn migrate(origin: OriginFor<T>, weight_limit: Weight) -> DispatchResultWithPostInfo {
use migration::MigrateResult::*;
ensure_signed(origin)?;

let weight_limit = weight_limit.saturating_add(T::WeightInfo::migrate());
let mut meter = WeightMeter::with_limit(weight_limit);
let result = Migration::<T>::migrate(&mut meter);

match result {
Completed => Ok(PostDispatchInfo {
actual_weight: Some(meter.consumed()),
pays_fee: Pays::No,
}),
InProgress { steps_done, .. } if steps_done > 0 => Ok(PostDispatchInfo {
actual_weight: Some(meter.consumed()),
pays_fee: Pays::No,
}),
InProgress { .. } => Ok(PostDispatchInfo {
actual_weight: Some(meter.consumed()),
pays_fee: Pays::Yes,
}),
NoMigrationInProgress | NoMigrationPerformed => {
let err: DispatchError = <Error<T>>::NoMigrationPerformed.into();
Err(err.with_weight(meter.consumed()))
},
}
}
}
}

Expand Down Expand Up @@ -1095,7 +1010,6 @@ where
None
};
let try_call = || {
Migration::<T>::ensure_migrated()?;
let origin = Origin::from_runtime_origin(origin)?;
let mut storage_meter = StorageMeter::new(&origin, storage_deposit_limit, value)?;
let result = ExecStack::<T, WasmBlob<T>>::run_call(
Expand Down Expand Up @@ -1148,7 +1062,6 @@ where
let mut debug_message =
if debug == DebugInfo::UnsafeDebug { Some(DebugBuffer::default()) } else { None };
let try_instantiate = || {
Migration::<T>::ensure_migrated()?;
let instantiate_account = T::InstantiateOrigin::ensure_origin(origin.clone())?;
let (executable, upload_deposit) = match code {
Code::Upload(code) => {
Expand Down Expand Up @@ -1209,17 +1122,13 @@ where
code: Vec<u8>,
storage_deposit_limit: BalanceOf<T>,
) -> CodeUploadResult<BalanceOf<T>> {
Migration::<T>::ensure_migrated()?;
let origin = T::UploadOrigin::ensure_origin(origin)?;
let (module, deposit) = Self::try_upload_code(origin, code, storage_deposit_limit, None)?;
Ok(CodeUploadReturnValue { code_hash: *module.code_hash(), deposit })
}

/// Query storage of a specified contract under a specified key.
pub fn get_storage(address: H160, key: [u8; 32]) -> GetStorageResult {
if Migration::<T>::in_progress() {
return Err(ContractAccessError::MigrationInProgress)
}
let contract_info =
ContractInfoOf::<T>::get(&address).ok_or(ContractAccessError::DoesntExist)?;

Expand Down
Loading

0 comments on commit cf44f41

Please sign in to comment.