From 65172ec004b95399dbd5d49ffd3e7054be1abdf1 Mon Sep 17 00:00:00 2001 From: Ashutosh Varma Date: Wed, 4 Oct 2023 20:05:38 +0530 Subject: [PATCH] fix: storage item typo in unified accounts (#1038) * fix: storage item typo * feat: add onupgrade hook impl * fix: weights --- pallets/unified-accounts/src/lib.rs | 38 +++++++++++++-------------- pallets/unified-accounts/src/tests.rs | 12 ++++----- runtime/shibuya/src/lib.rs | 16 +++++++++++ 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/pallets/unified-accounts/src/lib.rs b/pallets/unified-accounts/src/lib.rs index c2c53643e8..b0ac498832 100644 --- a/pallets/unified-accounts/src/lib.rs +++ b/pallets/unified-accounts/src/lib.rs @@ -142,15 +142,15 @@ pub mod pallet { } /// Native accounts for evm address - /// NativeToEvm: EvmAddress => Option + /// EvmToNative: EvmAddress => Option #[pallet::storage] - pub type NativeToEvm = + pub type EvmToNative = StorageMap<_, Blake2_128Concat, EvmAddress, T::AccountId, OptionQuery>; /// Evm addresses for native accounts - /// EvmToNative: AccountId => Option + /// NativeToEvm: AccountId => Option #[pallet::storage] - pub type EvmToNative = + pub type NativeToEvm = StorageMap<_, Blake2_128Concat, T::AccountId, EvmAddress, OptionQuery>; #[pallet::call] @@ -176,11 +176,11 @@ pub mod pallet { let who = ensure_signed(origin)?; // make sure no prior mapping exists ensure!( - !EvmToNative::::contains_key(&who), + !NativeToEvm::::contains_key(&who), Error::::AlreadyMapped ); ensure!( - !NativeToEvm::::contains_key(evm_address), + !EvmToNative::::contains_key(evm_address), Error::::AlreadyMapped ); @@ -206,8 +206,8 @@ pub mod pallet { } // create double mappings for the pair - NativeToEvm::::insert(&evm_address, &who); - EvmToNative::::insert(&who, &evm_address); + EvmToNative::::insert(&evm_address, &who); + NativeToEvm::::insert(&who, &evm_address); Self::deposit_event(Event::AccountClaimed { account_id: who, @@ -235,7 +235,7 @@ impl Pallet { /// Claim the default evm address fn do_claim_default_evm_address(account_id: T::AccountId) -> Result { ensure!( - !EvmToNative::::contains_key(&account_id), + !NativeToEvm::::contains_key(&account_id), Error::::AlreadyMapped ); // get the default evm address @@ -243,12 +243,12 @@ impl Pallet { // make sure default address is not already mapped, this should not // happen but for sanity check. ensure!( - !NativeToEvm::::contains_key(&evm_address), + !EvmToNative::::contains_key(&evm_address), Error::::AlreadyMapped ); // create double mappings for the pair with default evm address - NativeToEvm::::insert(&evm_address, &account_id); - EvmToNative::::insert(&account_id, &evm_address); + EvmToNative::::insert(&evm_address, &account_id); + NativeToEvm::::insert(&account_id, &evm_address); Self::deposit_event(Event::AccountClaimed { account_id, @@ -326,11 +326,11 @@ impl Pallet { /// and default address scheme from pallet's config impl UnifiedAddressMapper for Pallet { fn to_account_id(evm_address: &EvmAddress) -> Option { - NativeToEvm::::get(evm_address) + EvmToNative::::get(evm_address) } fn to_account_id_or_default(evm_address: &EvmAddress) -> T::AccountId { - NativeToEvm::::get(evm_address).unwrap_or_else(|| { + EvmToNative::::get(evm_address).unwrap_or_else(|| { // fallback to default account_id T::DefaultEvmToNative::into_account_id(evm_address.clone()) }) @@ -341,11 +341,11 @@ impl UnifiedAddressMapper for Pallet { } fn to_h160(account_id: &T::AccountId) -> Option { - EvmToNative::::get(account_id) + NativeToEvm::::get(account_id) } fn to_h160_or_default(account_id: &T::AccountId) -> EvmAddress { - EvmToNative::::get(account_id).unwrap_or_else(|| { + NativeToEvm::::get(account_id).unwrap_or_else(|| { // fallback to default account_id T::DefaultNativeToEvm::into_h160(account_id.clone()) }) @@ -376,9 +376,9 @@ pub struct KillAccountMapping(PhantomData); impl OnKilledAccount for KillAccountMapping { fn on_killed_account(who: &T::AccountId) { // remove mapping created by `claim_account` or `get_or_create_evm_address` - if let Some(evm_addr) = EvmToNative::::take(who) { - NativeToEvm::::remove(evm_addr); - EvmToNative::::remove(who); + if let Some(evm_addr) = NativeToEvm::::take(who) { + EvmToNative::::remove(evm_addr); + NativeToEvm::::remove(who); } } } diff --git a/pallets/unified-accounts/src/tests.rs b/pallets/unified-accounts/src/tests.rs index 93dd97937a..be8f9cbe34 100644 --- a/pallets/unified-accounts/src/tests.rs +++ b/pallets/unified-accounts/src/tests.rs @@ -134,8 +134,8 @@ fn on_killed_account_hook() { ))); // make sure mapping is removed - assert_eq!(EvmToNative::::get(ALICE), None); - assert_eq!(NativeToEvm::::get(alice_eth), None); + assert_eq!(NativeToEvm::::get(ALICE), None); + assert_eq!(EvmToNative::::get(alice_eth), None); }); } @@ -175,10 +175,10 @@ fn account_claim_should_work() { // make sure mappings are in place assert_eq!( - NativeToEvm::::get(alice_eth).unwrap(), ALICE + EvmToNative::::get(alice_eth).unwrap(), ALICE ); assert_eq!( - EvmToNative::::get(ALICE).unwrap(), alice_eth + NativeToEvm::::get(ALICE).unwrap(), alice_eth ) }); } @@ -229,8 +229,8 @@ fn account_default_claim_should_not_work_if_collision() { // create mapping of alice native with bob's default address // in real world possibilty of this happening is minuscule - NativeToEvm::::insert(&bob_default_h160, &ALICE); - EvmToNative::::insert(&ALICE, &bob_default_h160); + EvmToNative::::insert(&bob_default_h160, &ALICE); + NativeToEvm::::insert(&ALICE, &bob_default_h160); // bob try claiming default h160 address, it should fail since alice already // has mapping in place with it. diff --git a/runtime/shibuya/src/lib.rs b/runtime/shibuya/src/lib.rs index a3617a9ac5..460111bf4b 100644 --- a/runtime/shibuya/src/lib.rs +++ b/runtime/shibuya/src/lib.rs @@ -1333,12 +1333,28 @@ impl OnRuntimeUpgrade for DynamicEvmBaseFeeMigration { } } +/// Simple `OnRuntimeUpgrade` logic to remove all corrupted mappings +/// after the fixing the typo in mapping names in pallet. +pub struct ClearCorruptedUnifiedMappings; +impl OnRuntimeUpgrade for ClearCorruptedUnifiedMappings { + fn on_runtime_upgrade() -> Weight { + let maybe_limit = pallet_unified_accounts::EvmToNative::::iter().count(); + let total_rw = maybe_limit as u64 * 2; + // remove all items + let _ = pallet_unified_accounts::EvmToNative::::clear(maybe_limit as u32, None); + let _ = pallet_unified_accounts::NativeToEvm::::clear(maybe_limit as u32, None); + + ::DbWeight::get().reads_writes(total_rw, total_rw) + } +} + /// All migrations that will run on the next runtime upgrade. /// /// Once done, migrations should be removed from the tuple. pub type Migrations = ( frame_support::migrations::RemovePallet, DynamicEvmBaseFeeMigration, + ClearCorruptedUnifiedMappings, ); type EventRecord = frame_system::EventRecord<