diff --git a/pallets/moonbeam-lazy-migrations/src/lib.rs b/pallets/moonbeam-lazy-migrations/src/lib.rs index d580ec2831..603ac215ce 100644 --- a/pallets/moonbeam-lazy-migrations/src/lib.rs +++ b/pallets/moonbeam-lazy-migrations/src/lib.rs @@ -45,7 +45,6 @@ environmental::environmental!(MIGRATING_FOREIGN_ASSETS: bool); pub mod pallet { use super::*; use crate::foreign_asset::ForeignAssetMigrationStatus; - use cumulus_primitives_storage_weight_reclaim::get_proof_size; use sp_core::{H160, U256}; pub const ARRAY_LIMIT: u32 = 1000; @@ -106,8 +105,6 @@ pub mod pallet { ContractMetadataAlreadySet, /// Contract not exist ContractNotExist, - /// The key lengths exceeds the maximum allowed - KeyTooLong, /// The symbol length exceeds the maximum allowed SymbolTooLong, /// The name length exceeds the maximum allowed @@ -128,231 +125,6 @@ pub mod pallet { ApprovalFailed, } - pub(crate) const MAX_ITEM_PROOF_SIZE: u64 = 30 * 1024; // 30 KB - pub(crate) const PROOF_SIZE_BUFFER: u64 = 100 * 1024; // 100 KB - - #[pallet::hooks] - impl Hooks> for Pallet { - fn on_idle(_n: BlockNumberFor, remaining_weight: Weight) -> Weight { - let proof_size_before: u64 = get_proof_size().unwrap_or(0); - let res = Pallet::::handle_migration(remaining_weight); - let proof_size_after: u64 = get_proof_size().unwrap_or(0); - let proof_size_diff = proof_size_after.saturating_sub(proof_size_before); - - Weight::from_parts(0, proof_size_diff) - .saturating_add(T::DbWeight::get().reads_writes(res.reads, res.writes)) - } - } - - #[derive(Default, Clone, PartialEq, Eq, Encode, Decode, Debug)] - pub(crate) struct ReadWriteOps { - pub reads: u64, - pub writes: u64, - } - - impl ReadWriteOps { - pub fn new() -> Self { - Self { - reads: 0, - writes: 0, - } - } - - pub fn add_one_read(&mut self) { - self.reads += 1; - } - - pub fn add_one_write(&mut self) { - self.writes += 1; - } - - pub fn add_reads(&mut self, reads: u64) { - self.reads += reads; - } - - pub fn add_writes(&mut self, writes: u64) { - self.writes += writes; - } - } - - #[derive(Clone)] - struct StateMigrationResult { - last_key: Option, - error: Option<&'static str>, - migrated: u64, - reads: u64, - writes: u64, - } - - enum NextKeyResult { - NextKey(StorageKey), - NoMoreKeys, - Error(&'static str), - } - - impl Pallet { - /// Handle the migration of the storage keys, returns the number of read and write operations - pub(crate) fn handle_migration(remaining_weight: Weight) -> ReadWriteOps { - let mut read_write_ops = ReadWriteOps::new(); - - // maximum number of items that can be migrated in one block - let migration_limit = remaining_weight - .proof_size() - .saturating_sub(PROOF_SIZE_BUFFER) - .saturating_div(MAX_ITEM_PROOF_SIZE); - - if migration_limit == 0 { - return read_write_ops; - } - - let (status, mut migrated_keys) = StateMigrationStatusValue::::get(); - read_write_ops.add_one_read(); - - let next_key = match &status { - StateMigrationStatus::NotStarted => Default::default(), - StateMigrationStatus::Started(storage_key) => { - let (reads, next_key_result) = Pallet::::get_next_key(storage_key); - read_write_ops.add_reads(reads); - match next_key_result { - NextKeyResult::NextKey(next_key) => next_key, - NextKeyResult::NoMoreKeys => { - StateMigrationStatusValue::::put(( - StateMigrationStatus::Complete, - migrated_keys, - )); - read_write_ops.add_one_write(); - return read_write_ops; - } - NextKeyResult::Error(e) => { - StateMigrationStatusValue::::put(( - StateMigrationStatus::Error( - e.as_bytes().to_vec().try_into().unwrap_or_default(), - ), - migrated_keys, - )); - read_write_ops.add_one_write(); - return read_write_ops; - } - } - } - StateMigrationStatus::Complete | StateMigrationStatus::Error(_) => { - return read_write_ops; - } - }; - - let res = Pallet::::migrate_keys(next_key, migration_limit); - migrated_keys += res.migrated; - read_write_ops.add_reads(res.reads); - read_write_ops.add_writes(res.writes); - - match (res.last_key, res.error) { - (None, None) => { - StateMigrationStatusValue::::put(( - StateMigrationStatus::Complete, - migrated_keys, - )); - read_write_ops.add_one_write(); - } - // maybe we should store the previous key in the storage as well - (_, Some(e)) => { - StateMigrationStatusValue::::put(( - StateMigrationStatus::Error( - e.as_bytes().to_vec().try_into().unwrap_or_default(), - ), - migrated_keys, - )); - read_write_ops.add_one_write(); - } - (Some(key), None) => { - StateMigrationStatusValue::::put(( - StateMigrationStatus::Started(key), - migrated_keys, - )); - read_write_ops.add_one_write(); - } - } - - read_write_ops - } - - /// Tries to get the next key in the storage, returns None if there are no more keys to migrate. - /// Returns an error if the key is too long. - fn get_next_key(key: &StorageKey) -> (u64, NextKeyResult) { - if let Some(next) = sp_io::storage::next_key(key) { - let next: Result = next.try_into(); - match next { - Ok(next_key) => { - if next_key.as_slice() == sp_core::storage::well_known_keys::CODE { - let (reads, next_key_res) = Pallet::::get_next_key(&next_key); - return (1 + reads, next_key_res); - } - (1, NextKeyResult::NextKey(next_key)) - } - Err(_) => (1, NextKeyResult::Error("Key too long")), - } - } else { - (1, NextKeyResult::NoMoreKeys) - } - } - - /// Migrate maximum of `limit` keys starting from `start`, returns the next key to migrate - /// Returns None if there are no more keys to migrate. - /// Returns an error if an error occurred during migration. - fn migrate_keys(start: StorageKey, limit: u64) -> StateMigrationResult { - let mut key = start; - let mut migrated = 0; - let mut next_key_reads = 0; - let mut writes = 0; - - while migrated < limit { - let data = sp_io::storage::get(&key); - if let Some(data) = data { - sp_io::storage::set(&key, &data); - writes += 1; - } - - migrated += 1; - - if migrated < limit { - let (reads, next_key_res) = Pallet::::get_next_key(&key); - next_key_reads += reads; - - match next_key_res { - NextKeyResult::NextKey(next_key) => { - key = next_key; - } - NextKeyResult::NoMoreKeys => { - return StateMigrationResult { - last_key: None, - error: None, - migrated, - reads: migrated + next_key_reads, - writes, - }; - } - NextKeyResult::Error(e) => { - return StateMigrationResult { - last_key: Some(key), - error: Some(e), - migrated, - reads: migrated + next_key_reads, - writes, - }; - } - }; - } - } - - StateMigrationResult { - last_key: Some(key), - error: None, - migrated, - reads: migrated + next_key_reads, - writes, - } - } - } - #[pallet::call] impl Pallet where diff --git a/pallets/moonbeam-lazy-migrations/src/mock.rs b/pallets/moonbeam-lazy-migrations/src/mock.rs index 7a0271766e..6aecf8164c 100644 --- a/pallets/moonbeam-lazy-migrations/src/mock.rs +++ b/pallets/moonbeam-lazy-migrations/src/mock.rs @@ -19,11 +19,8 @@ use super::*; use crate as pallet_moonbeam_lazy_migrations; use frame_support::traits::AsEnsureOriginWithArg; -use frame_support::{ - construct_runtime, parameter_types, - traits::Everything, - weights::{RuntimeDbWeight, Weight}, -}; +use frame_support::weights::constants::RocksDbWeight; +use frame_support::{construct_runtime, parameter_types, traits::Everything, weights::Weight}; use frame_system::{EnsureRoot, EnsureSigned}; use pallet_asset_manager::AssetRegistrar; use pallet_evm::{EnsureAddressNever, EnsureAddressRoot, FrameSystemAccountProvider}; @@ -62,16 +59,9 @@ parameter_types! { pub const SS58Prefix: u8 = 42; } -parameter_types! { - pub const MockDbWeight: RuntimeDbWeight = RuntimeDbWeight { - read: 1_000_000, - write: 1, - }; -} - impl frame_system::Config for Test { type BaseCallFilter = Everything; - type DbWeight = MockDbWeight; + type DbWeight = RocksDbWeight; type RuntimeOrigin = RuntimeOrigin; type RuntimeTask = RuntimeTask; type Nonce = u64; diff --git a/pallets/moonbeam-lazy-migrations/src/tests.rs b/pallets/moonbeam-lazy-migrations/src/tests.rs index 2780651266..def5912724 100644 --- a/pallets/moonbeam-lazy-migrations/src/tests.rs +++ b/pallets/moonbeam-lazy-migrations/src/tests.rs @@ -20,7 +20,6 @@ use crate::{ mock::{AssetId, Balances}, }; use frame_support::traits::fungibles::approvals::Inspect; -use pallet_evm::AddressMapping; use sp_runtime::{BoundedVec, DispatchError}; use xcm::latest::Location; use { @@ -29,51 +28,18 @@ use { AccountId, AssetManager, Assets, ExtBuilder, LazyMigrations, MockAssetType, RuntimeOrigin, Test, ALITH, BOB, }, - Error, StateMigrationStatus, StateMigrationStatusValue, MAX_ITEM_PROOF_SIZE, - PROOF_SIZE_BUFFER, + Error, }, - frame_support::{assert_noop, assert_ok, traits::Hooks, weights::Weight}, - rlp::RlpStream, + frame_support::{assert_noop, assert_ok}, sp_core::{H160, H256}, sp_io::hashing::keccak_256, - sp_runtime::traits::Bounded, }; -// Helper function that calculates the contract address -pub fn contract_address(sender: H160, nonce: u64) -> H160 { - let mut rlp = RlpStream::new_list(2); - rlp.append(&sender); - rlp.append(&nonce); - - H160::from_slice(&keccak_256(&rlp.out())[12..]) -} - fn address_build(seed: u8) -> H160 { let address = H160::from(H256::from(keccak_256(&[seed; 32]))); address } -// Helper function that creates a `num_entries` storage entries for a contract -fn mock_contract_with_entries(seed: u8, nonce: u64, num_entries: u32) -> H160 { - let address = address_build(seed); - - let contract_address = contract_address(address, nonce); - let account_id = - ::AddressMapping::into_account_id(contract_address); - let _ = frame_system::Pallet::::inc_sufficients(&account_id); - - // Add num_entries storage entries to the suicided contract - for i in 0..num_entries { - pallet_evm::AccountStorages::::insert( - contract_address, - H256::from_low_u64_be(i as u64), - H256::from_low_u64_be(i as u64), - ); - } - - contract_address -} - fn create_dummy_contract_without_metadata(seed: u8) -> H160 { let address = address_build(seed); let dummy_code = vec![1, 2, 3]; @@ -118,272 +84,6 @@ fn test_create_contract_metadata_success_path() { }); } -fn count_keys_and_data_without_code() -> (u64, u64) { - let mut keys: u64 = 0; - let mut data: u64 = 0; - - let mut current_key: Option> = Some(Default::default()); - while let Some(key) = current_key { - if key.as_slice() == sp_core::storage::well_known_keys::CODE { - current_key = sp_io::storage::next_key(&key); - continue; - } - keys += 1; - if let Some(_) = sp_io::storage::get(&key) { - data += 1; - } - current_key = sp_io::storage::next_key(&key); - } - - (keys, data) -} - -fn weight_for(read: u64, write: u64) -> Weight { - ::DbWeight::get().reads_writes(read, write) -} - -fn rem_weight_for_entries(num_entries: u64) -> Weight { - let proof = PROOF_SIZE_BUFFER + num_entries * MAX_ITEM_PROOF_SIZE; - Weight::from_parts(u64::max_value(), proof) -} - -#[test] -fn test_state_migration_baseline() { - ExtBuilder::default().build().execute_with(|| { - assert_eq!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::NotStarted, 0) - ); - - let (keys, data) = count_keys_and_data_without_code(); - println!("Keys: {}, Data: {}", keys, data); - - let weight = LazyMigrations::on_idle(0, Weight::max_value()); - - // READS: 2 * keys + 2 (skipped and status) - // Next key requests = keys (we have first key as default which is not counted, and extra - // next_key request to check if we are done) - // - // 1 next key request for the skipped key ":code" - // Read requests = keys (we read each key once) - // 1 Read request for the StateMigrationStatusValue - - // WRITES: data + 1 (status) - // Write requests = data (we write each data once) - // 1 Write request for the StateMigrationStatusValue - assert_eq!(weight, weight_for(2 * keys + 2, data + 1)); - - assert_eq!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::Complete, keys) - ); - }) -} - -#[test] -fn test_state_migration_cannot_fit_any_item() { - ExtBuilder::default().build().execute_with(|| { - StateMigrationStatusValue::::put((StateMigrationStatus::NotStarted, 0)); - - let weight = LazyMigrations::on_idle(0, rem_weight_for_entries(0)); - - assert_eq!(weight, weight_for(0, 0)); - }) -} - -#[test] -fn test_state_migration_when_complete() { - ExtBuilder::default().build().execute_with(|| { - StateMigrationStatusValue::::put((StateMigrationStatus::Complete, 0)); - - let weight = LazyMigrations::on_idle(0, Weight::max_value()); - - // just reading the status of the migration - assert_eq!(weight, weight_for(1, 0)); - }) -} - -#[test] -fn test_state_migration_when_errored() { - ExtBuilder::default().build().execute_with(|| { - StateMigrationStatusValue::::put(( - StateMigrationStatus::Error("Error".as_bytes().to_vec().try_into().unwrap_or_default()), - 1, - )); - - let weight = LazyMigrations::on_idle(0, Weight::max_value()); - - // just reading the status of the migration - assert_eq!(weight, weight_for(1, 0)); - }) -} - -#[test] -fn test_state_migration_can_only_fit_one_item() { - ExtBuilder::default().build().execute_with(|| { - assert_eq!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::NotStarted, 0) - ); - - let data = sp_io::storage::get(Default::default()); - let weight = LazyMigrations::on_idle(0, rem_weight_for_entries(1)); - - let reads = 2; // key read + status read - let writes = 1 + data.map(|_| 1).unwrap_or(0); - assert_eq!(weight, weight_for(reads, writes)); - - assert!(matches!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::Started(_), 1) - )); - - let weight = LazyMigrations::on_idle(0, rem_weight_for_entries(3)); - let reads = 3 + 3 + 1; // next key + key read + status - let writes = 1 + 3; // status write + key write - assert_eq!(weight, weight_for(reads, writes)); - }) -} - -#[test] -fn test_state_migration_can_only_fit_three_item() { - ExtBuilder::default().build().execute_with(|| { - assert_eq!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::NotStarted, 0) - ); - - let weight = LazyMigrations::on_idle(0, rem_weight_for_entries(3)); - - // 2 next key requests (default key dons't need a next key request) + 1 status read - // 3 key reads. - // 1 status write + 2 key writes (default key doesn't have any data) - let reads = 6; - let writes = 3; - assert_eq!(weight, weight_for(reads, writes)); - - assert!(matches!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::Started(_), 3) - )); - }) -} - -#[test] -fn test_state_migration_can_fit_exactly_all_item() { - ExtBuilder::default().build().execute_with(|| { - assert_eq!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::NotStarted, 0) - ); - - let (keys, data) = count_keys_and_data_without_code(); - let weight = LazyMigrations::on_idle(0, rem_weight_for_entries(keys)); - - // we deduct the extra next_key request to check if we are done. - // will know if we are done on the next call to on_idle - assert_eq!(weight, weight_for(2 * keys + 1, data + 1)); - - assert!(matches!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::Started(_), n) if n == keys, - )); - - // after calling on_idle status is added to the storage so we need to account for that - let (new_keys, new_data) = count_keys_and_data_without_code(); - let (diff_keys, diff_data) = (new_keys - keys, new_data - data); - - let weight = LazyMigrations::on_idle(0, rem_weight_for_entries(1 + diff_keys)); - // (next_key + read) for each new key + status + next_key to check if we are done - let reads = diff_keys * 2 + 2; - let writes = 1 + diff_data; // status - assert_eq!(weight, weight_for(reads, writes)); - - assert!(matches!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::Complete, n) if n == new_keys, - )); - }) -} - -#[test] -fn test_state_migration_will_migrate_10_000_items() { - ExtBuilder::default().build().execute_with(|| { - assert_eq!( - StateMigrationStatusValue::::get(), - (StateMigrationStatus::NotStarted, 0) - ); - - for i in 0..100 { - mock_contract_with_entries(i as u8, i as u64, 100); - } - - StateMigrationStatusValue::::put((StateMigrationStatus::NotStarted, 0)); - - let (keys, data) = count_keys_and_data_without_code(); - - // assuming we can only fit 100 items at a time - - let mut total_weight: Weight = Weight::zero(); - let num_of_on_idle_calls = 200; - let entries_per_on_idle = 100; - let needed_on_idle_calls = (keys as f64 / entries_per_on_idle as f64).ceil() as u64; - - // Reads: - // Read status => num_of_on_idle_calls - // Read keys => keys - // Next keys => keys - 1 + 1 skip + 1 done check - // - // Writes: - // Write status => needed_on_idle_calls - // Write keys => data - let expected_reads = (keys - 1 + 2) + keys + num_of_on_idle_calls; - let expected_writes = data + needed_on_idle_calls; - - println!("Keys: {}, Data: {}", keys, data); - println!("entries_per_on_idle: {}", entries_per_on_idle); - println!("num_of_on_idle_calls: {}", num_of_on_idle_calls); - println!("needed_on_idle_calls: {}", needed_on_idle_calls); - println!( - "Expected Reads: {}, Expected Writes: {}", - expected_reads, expected_writes - ); - - for i in 1..=num_of_on_idle_calls { - let weight = LazyMigrations::on_idle(i, rem_weight_for_entries(entries_per_on_idle)); - total_weight = total_weight.saturating_add(weight); - - let status = StateMigrationStatusValue::::get(); - if i < needed_on_idle_calls { - let migrated_so_far = i * entries_per_on_idle; - assert!( - matches!(status, (StateMigrationStatus::Started(_), n) if n == migrated_so_far), - "Status: {:?} at call: #{} doesn't match Started", - status, - i, - ); - assert!(weight.all_gte(weight_for(1, 0))); - } else { - assert!( - matches!(status, (StateMigrationStatus::Complete, n) if n == keys), - "Status: {:?} at call: {} doesn't match Complete", - status, - i, - ); - if i == needed_on_idle_calls { - // last call to on_idle - assert!(weight.all_gte(weight_for(1, 0))); - } else { - // extra calls to on_idle, just status update check - assert_eq!(weight, weight_for(1, 0)); - } - } - } - - assert_eq!(total_weight, weight_for(expected_reads, expected_writes)); - }) -} - // Helper function to create a foreign asset with basic metadata fn create_old_foreign_asset(location: Location) -> AssetId { let asset = MockAssetType::Xcm(location.clone().into());