Skip to content

Commit

Permalink
fix: storage item typo in unified accounts (#1038)
Browse files Browse the repository at this point in the history
* fix: storage item typo

* feat: add onupgrade hook impl

* fix: weights
  • Loading branch information
ashutoshvarma authored Oct 4, 2023
1 parent 212fa81 commit 65172ec
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 25 deletions.
38 changes: 19 additions & 19 deletions pallets/unified-accounts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,15 @@ pub mod pallet {
}

/// Native accounts for evm address
/// NativeToEvm: EvmAddress => Option<AccountId>
/// EvmToNative: EvmAddress => Option<AccountId>
#[pallet::storage]
pub type NativeToEvm<T: Config> =
pub type EvmToNative<T: Config> =
StorageMap<_, Blake2_128Concat, EvmAddress, T::AccountId, OptionQuery>;

/// Evm addresses for native accounts
/// EvmToNative: AccountId => Option<EvmAddress>
/// NativeToEvm: AccountId => Option<EvmAddress>
#[pallet::storage]
pub type EvmToNative<T: Config> =
pub type NativeToEvm<T: Config> =
StorageMap<_, Blake2_128Concat, T::AccountId, EvmAddress, OptionQuery>;

#[pallet::call]
Expand All @@ -176,11 +176,11 @@ pub mod pallet {
let who = ensure_signed(origin)?;
// make sure no prior mapping exists
ensure!(
!EvmToNative::<T>::contains_key(&who),
!NativeToEvm::<T>::contains_key(&who),
Error::<T>::AlreadyMapped
);
ensure!(
!NativeToEvm::<T>::contains_key(evm_address),
!EvmToNative::<T>::contains_key(evm_address),
Error::<T>::AlreadyMapped
);

Expand All @@ -206,8 +206,8 @@ pub mod pallet {
}

// create double mappings for the pair
NativeToEvm::<T>::insert(&evm_address, &who);
EvmToNative::<T>::insert(&who, &evm_address);
EvmToNative::<T>::insert(&evm_address, &who);
NativeToEvm::<T>::insert(&who, &evm_address);

Self::deposit_event(Event::AccountClaimed {
account_id: who,
Expand Down Expand Up @@ -235,20 +235,20 @@ impl<T: Config> Pallet<T> {
/// Claim the default evm address
fn do_claim_default_evm_address(account_id: T::AccountId) -> Result<EvmAddress, DispatchError> {
ensure!(
!EvmToNative::<T>::contains_key(&account_id),
!NativeToEvm::<T>::contains_key(&account_id),
Error::<T>::AlreadyMapped
);
// get the default evm address
let evm_address = T::DefaultNativeToEvm::into_h160(account_id.clone());
// make sure default address is not already mapped, this should not
// happen but for sanity check.
ensure!(
!NativeToEvm::<T>::contains_key(&evm_address),
!EvmToNative::<T>::contains_key(&evm_address),
Error::<T>::AlreadyMapped
);
// create double mappings for the pair with default evm address
NativeToEvm::<T>::insert(&evm_address, &account_id);
EvmToNative::<T>::insert(&account_id, &evm_address);
EvmToNative::<T>::insert(&evm_address, &account_id);
NativeToEvm::<T>::insert(&account_id, &evm_address);

Self::deposit_event(Event::AccountClaimed {
account_id,
Expand Down Expand Up @@ -326,11 +326,11 @@ impl<T: Config> Pallet<T> {
/// and default address scheme from pallet's config
impl<T: Config> UnifiedAddressMapper<T::AccountId> for Pallet<T> {
fn to_account_id(evm_address: &EvmAddress) -> Option<T::AccountId> {
NativeToEvm::<T>::get(evm_address)
EvmToNative::<T>::get(evm_address)
}

fn to_account_id_or_default(evm_address: &EvmAddress) -> T::AccountId {
NativeToEvm::<T>::get(evm_address).unwrap_or_else(|| {
EvmToNative::<T>::get(evm_address).unwrap_or_else(|| {
// fallback to default account_id
T::DefaultEvmToNative::into_account_id(evm_address.clone())
})
Expand All @@ -341,11 +341,11 @@ impl<T: Config> UnifiedAddressMapper<T::AccountId> for Pallet<T> {
}

fn to_h160(account_id: &T::AccountId) -> Option<EvmAddress> {
EvmToNative::<T>::get(account_id)
NativeToEvm::<T>::get(account_id)
}

fn to_h160_or_default(account_id: &T::AccountId) -> EvmAddress {
EvmToNative::<T>::get(account_id).unwrap_or_else(|| {
NativeToEvm::<T>::get(account_id).unwrap_or_else(|| {
// fallback to default account_id
T::DefaultNativeToEvm::into_h160(account_id.clone())
})
Expand Down Expand Up @@ -376,9 +376,9 @@ pub struct KillAccountMapping<T>(PhantomData<T>);
impl<T: Config> OnKilledAccount<T::AccountId> for KillAccountMapping<T> {
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::<T>::take(who) {
NativeToEvm::<T>::remove(evm_addr);
EvmToNative::<T>::remove(who);
if let Some(evm_addr) = NativeToEvm::<T>::take(who) {
EvmToNative::<T>::remove(evm_addr);
NativeToEvm::<T>::remove(who);
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions pallets/unified-accounts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ fn on_killed_account_hook() {
)));

// make sure mapping is removed
assert_eq!(EvmToNative::<TestRuntime>::get(ALICE), None);
assert_eq!(NativeToEvm::<TestRuntime>::get(alice_eth), None);
assert_eq!(NativeToEvm::<TestRuntime>::get(ALICE), None);
assert_eq!(EvmToNative::<TestRuntime>::get(alice_eth), None);
});
}

Expand Down Expand Up @@ -175,10 +175,10 @@ fn account_claim_should_work() {

// make sure mappings are in place
assert_eq!(
NativeToEvm::<TestRuntime>::get(alice_eth).unwrap(), ALICE
EvmToNative::<TestRuntime>::get(alice_eth).unwrap(), ALICE
);
assert_eq!(
EvmToNative::<TestRuntime>::get(ALICE).unwrap(), alice_eth
NativeToEvm::<TestRuntime>::get(ALICE).unwrap(), alice_eth
)
});
}
Expand Down Expand Up @@ -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::<TestRuntime>::insert(&bob_default_h160, &ALICE);
EvmToNative::<TestRuntime>::insert(&ALICE, &bob_default_h160);
EvmToNative::<TestRuntime>::insert(&bob_default_h160, &ALICE);
NativeToEvm::<TestRuntime>::insert(&ALICE, &bob_default_h160);

// bob try claiming default h160 address, it should fail since alice already
// has mapping in place with it.
Expand Down
16 changes: 16 additions & 0 deletions runtime/shibuya/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Runtime>::iter().count();
let total_rw = maybe_limit as u64 * 2;
// remove all items
let _ = pallet_unified_accounts::EvmToNative::<Runtime>::clear(maybe_limit as u32, None);
let _ = pallet_unified_accounts::NativeToEvm::<Runtime>::clear(maybe_limit as u32, None);

<Runtime as frame_system::Config>::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<BaseFeeStr, RocksDbWeight>,
DynamicEvmBaseFeeMigration,
ClearCorruptedUnifiedMappings,
);

type EventRecord = frame_system::EventRecord<
Expand Down

0 comments on commit 65172ec

Please sign in to comment.