Skip to content

Commit

Permalink
feat: remove AccountMapping trait
Browse files Browse the repository at this point in the history
  • Loading branch information
ashutoshvarma committed Oct 24, 2023
1 parent ebbc59d commit 4871911
Show file tree
Hide file tree
Showing 15 changed files with 106 additions and 112 deletions.
10 changes: 3 additions & 7 deletions chain-extensions/unified-accounts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,14 @@

#![cfg_attr(not(feature = "std"), no_std)]

use astar_primitives::{
ethereum_checked::AccountMapping,
evm::{EvmAddress, UnifiedAddressMapper},
};
use astar_primitives::evm::{EvmAddress, UnifiedAddressMapper};
use core::marker::PhantomData;
use sp_runtime::DispatchError;

use frame_support::{traits::Get, DefaultNoBound};
use pallet_contracts::chain_extension::{
ChainExtension, Environment, Ext, InitState, Result as DispatchResult, RetVal,
};
use pallet_evm::AddressMapping;
use parity_scale_codec::Encode;
pub use unified_accounts_chain_extension_types::{
Command::{self, *},
Expand Down Expand Up @@ -69,7 +65,7 @@ where
let evm_address = if let Some(h160) = UA::to_h160(&account_id) {
UnifiedAddress::Mapped(h160)
} else {
UnifiedAddress::Default(T::DefaultNativeToEvm::into_h160(account_id))
UnifiedAddress::Default(UA::to_default_h160(&account_id))
};
// write to buffer
evm_address.using_encoded(|r| env.write(r, false, None))?;
Expand All @@ -92,7 +88,7 @@ where
let native_address = if let Some(native) = UA::to_account_id(&evm_address) {
UnifiedAddress::Mapped(native)
} else {
UnifiedAddress::Default(T::DefaultEvmToNative::into_account_id(evm_address))
UnifiedAddress::Default(UA::to_default_account_id(&evm_address))
};

// write to buffer
Expand Down
11 changes: 6 additions & 5 deletions pallets/ethereum-checked/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ use sp_runtime::traits::TrailingZeroInput;
use sp_runtime::traits::UniqueSaturatedInto;
use sp_std::{marker::PhantomData, result::Result};

use astar_primitives::ethereum_checked::{
AccountMapping, CheckedEthereumTransact, CheckedEthereumTx,
use astar_primitives::{
ethereum_checked::{CheckedEthereumTransact, CheckedEthereumTx},
evm::UnifiedAddressMapper,
};

pub use pallet::*;
Expand Down Expand Up @@ -141,7 +142,7 @@ pub mod pallet {
type ValidatedTransaction: ValidatedTransaction;

/// Account mapping.
type AccountMapping: AccountMapping<Self::AccountId>;
type AddressMapper: UnifiedAddressMapper<Self::AccountId>;

/// Origin for `transact` call.
type XcmTransactOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Self::AccountId>;
Expand Down Expand Up @@ -170,7 +171,7 @@ pub mod pallet {
pub fn transact(origin: OriginFor<T>, tx: CheckedEthereumTx) -> DispatchResultWithPostInfo {
let source = T::XcmTransactOrigin::ensure_origin(origin)?;
Self::do_transact(
T::AccountMapping::into_h160(source),
T::AddressMapper::to_h160_or_default(&source),
tx.into(),
CheckedEthereumTxKind::Xcm,
false,
Expand Down Expand Up @@ -283,7 +284,7 @@ impl<T: Config> Pallet<T> {
) -> DispatchResultWithPostInfo {
let source = T::XcmTransactOrigin::ensure_origin(origin)?;
Self::do_transact(
T::AccountMapping::into_h160(source),
T::AddressMapper::to_h160_or_default(&source),
tx.into(),
CheckedEthereumTxKind::Xcm,
true,
Expand Down
34 changes: 27 additions & 7 deletions pallets/ethereum-checked/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,22 +127,42 @@ impl AddressMapping<AccountId32> for MockAddressMapping {
}
}

pub struct MockAccountMapping;
impl AccountMapping<AccountId32> for MockAccountMapping {
fn into_h160(account_id: AccountId) -> H160 {
if account_id == ALICE {
pub struct MockAddressMapper;
impl UnifiedAddressMapper<AccountId32> for MockAddressMapper {
fn to_h160_or_default(account_id: &AccountId) -> H160 {
if account_id == &ALICE {
return ALICE_H160;
}
if account_id == BOB {
if account_id == &BOB {
return BOB_H160;
}
if account_id == CHARLIE {
if account_id == &CHARLIE {
return CHARLIE_H160;
}

let data = (b"evm:", account_id);
return H160::from_slice(&data.using_encoded(blake2_256)[0..20]);
}

// this method is not used in tests
fn to_account_id(_: &H160) -> Option<AccountId32> {
None
}

// this method is not used in tests
fn to_h160(_: &AccountId32) -> Option<H160> {
None
}

// this method is not used in tests
fn to_default_account_id(_: &H160) -> AccountId32 {
[0u8; 32].into()
}

// this method is not used in tests
fn to_default_h160(_: &AccountId32) -> H160 {
[0u8; 20].into()
}
}

parameter_types! {
Expand Down Expand Up @@ -193,7 +213,7 @@ impl pallet_ethereum_checked::Config for TestRuntime {
type XvmTxWeightLimit = TxWeightLimit;
type InvalidEvmTransactionError = pallet_ethereum::InvalidTransactionWrapper;
type ValidatedTransaction = pallet_ethereum::ValidatedTransaction<Self>;
type AccountMapping = MockAccountMapping;
type AddressMapper = MockAddressMapper;
type XcmTransactOrigin = EnsureXcmEthereumTx<AccountId32>;
type WeightInfo = ();
}
Expand Down
7 changes: 3 additions & 4 deletions pallets/unified-accounts/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,19 @@ mod benchmarks {
#[benchmark]
fn claim_default_evm_address() {
let caller: T::AccountId = whitelisted_caller();
let caller_clone = caller.clone();
let evm_address = T::DefaultNativeToEvm::into_h160(caller.clone());
let evm_address = T::DefaultMappings::to_default_h160(&caller);

assert_ok!(T::Currency::mint_into(
&caller,
T::AccountMappingStorageFee::get()
));

#[extrinsic_call]
_(RawOrigin::Signed(caller));
_(RawOrigin::Signed(caller.clone()));

assert_last_event::<T>(
Event::<T>::AccountClaimed {
account_id: caller_clone,
account_id: caller,
evm_address,
}
.into(),
Expand Down
38 changes: 6 additions & 32 deletions pallets/unified-accounts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,12 @@
//! * [`StaticLookup`](sp_runtime::traits::StaticLookup): Lookup implementations for accepting H160
//! * [`AddressMapping`](pallet_evm::AddressMapping): Wrapper over `UnifiedAddressMapper` for evm address mapping
//! to account id.
//! * [`AccountMapping`](astar_primitives::ethereum_checked::AccountMapping): Wrapper over `UnifiedAddressMapper`
//! for account id mappings to h160.
//! * `KillAccountMapping`: [`OnKilledAccount`](frame_support::traits::OnKilledAccount) implementation to remove
//! the mappings from storage after account is reaped.
#![cfg_attr(not(feature = "std"), no_std)]

use astar_primitives::{
ethereum_checked::AccountMapping,
evm::{EvmAddress, UnifiedAddressMapper},
Balance,
};
Expand Down Expand Up @@ -115,10 +112,8 @@ pub mod pallet {
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
/// The Currency for managing evm address assets
type Currency: FungibleMutate<Self::AccountId, Balance = Balance>;
/// Default evm address to account id conversion
type DefaultEvmToNative: AddressMapping<Self::AccountId>;
/// Default account id to evm address conversion
type DefaultNativeToEvm: AccountMapping<Self::AccountId>;
/// Default address conversion
type DefaultMappings: UnifiedAddressMapper<Self::AccountId>;
/// EVM chain id
#[pallet::constant]
type ChainId: Get<u64>;
Expand Down Expand Up @@ -205,7 +200,7 @@ pub mod pallet {
Self::charge_storage_fee(&who)?;

// Check if the default account id already exists for this evm address
let default_account_id = T::DefaultEvmToNative::into_account_id(evm_address.clone());
let default_account_id = T::DefaultMappings::to_default_account_id(&evm_address);
if frame_system::Pallet::<T>::account_exists(&default_account_id) {
// Transfer all the free native balance from old account id to the newly
// since this `default_account_id` will no longer be connected to evm address
Expand Down Expand Up @@ -253,7 +248,7 @@ impl<T: Config> Pallet<T> {
Error::<T>::AlreadyMapped
);
// get the default evm address
let evm_address = T::DefaultNativeToEvm::into_h160(account_id.clone());
let evm_address = T::DefaultMappings::to_default_h160(&account_id);
// make sure default address is not already mapped, this should not
// happen but for sanity check.
ensure!(
Expand Down Expand Up @@ -351,37 +346,16 @@ impl<T: Config> UnifiedAddressMapper<T::AccountId> for Pallet<T> {
EvmToNative::<T>::get(evm_address)
}

fn to_account_id_or_default(evm_address: &EvmAddress) -> T::AccountId {
EvmToNative::<T>::get(evm_address).unwrap_or_else(|| {
// fallback to default account_id
T::DefaultEvmToNative::into_account_id(evm_address.clone())
})
}

fn to_default_account_id(evm_address: &EvmAddress) -> T::AccountId {
T::DefaultEvmToNative::into_account_id(evm_address.clone())
T::DefaultMappings::to_default_account_id(evm_address)
}

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

fn to_h160_or_default(account_id: &T::AccountId) -> EvmAddress {
NativeToEvm::<T>::get(account_id).unwrap_or_else(|| {
// fallback to default account_id
T::DefaultNativeToEvm::into_h160(account_id.clone())
})
}

fn to_default_h160(account_id: &T::AccountId) -> EvmAddress {
T::DefaultNativeToEvm::into_h160(account_id.clone())
}
}

/// AccountMapping wrapper implementation
impl<T: Config> AccountMapping<T::AccountId> for Pallet<T> {
fn into_h160(account: T::AccountId) -> H160 {
<Self as UnifiedAddressMapper<T::AccountId>>::to_h160_or_default(&account)
T::DefaultMappings::to_default_h160(account_id)
}
}

Expand Down
7 changes: 3 additions & 4 deletions pallets/unified-accounts/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@

use super::*;
use crate as pallet_unified_accounts;
use astar_primitives::ethereum_checked::HashedAccountMapping;
use astar_primitives::evm::HashedDefaultMappings;
use frame_support::{
construct_runtime, parameter_types,
sp_io::TestExternalities,
traits::{ConstU128, ConstU64, FindAuthor},
weights::Weight,
};
use pallet_ethereum::PostLogContent;
use pallet_evm::{FeeCalculator, HashedAddressMapping};
use pallet_evm::FeeCalculator;
use sp_core::{keccak_256, H160, H256, U256};
use sp_runtime::{
testing::Header,
Expand Down Expand Up @@ -155,8 +155,7 @@ parameter_types! {
impl pallet_unified_accounts::Config for TestRuntime {
type RuntimeEvent = RuntimeEvent;
type Currency = Balances;
type DefaultEvmToNative = HashedAddressMapping<BlakeTwo256>;
type DefaultNativeToEvm = HashedAccountMapping<BlakeTwo256>;
type DefaultMappings = HashedDefaultMappings<BlakeTwo256>;
type ChainId = ChainId;
type AccountMappingStorageFee = AccountMappingStorageFee;
type WeightInfo = ();
Expand Down
5 changes: 2 additions & 3 deletions pallets/unified-accounts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ fn account_claim_should_work() {
let alice_eth = UnifiedAccounts::eth_address(&alice_secret());
// default ss58 account associated with eth address
let alice_eth_old_account =
<TestRuntime as Config>::DefaultEvmToNative::into_account_id(alice_eth.clone());
<TestRuntime as Config>::DefaultMappings::to_default_account_id(&alice_eth);
let signature = get_evm_signature(&ALICE, &alice_secret());

// transfer some funds to alice_eth (H160)
Expand Down Expand Up @@ -194,8 +194,7 @@ fn account_claim_should_work() {
#[test]
fn account_default_claim_works() {
ExtBuilder::default().build().execute_with(|| {
let alice_default_evm =
<TestRuntime as Config>::DefaultNativeToEvm::into_h160(ALICE.into());
let alice_default_evm = <TestRuntime as Config>::DefaultMappings::to_default_h160(&ALICE);

// claim default account
assert_ok!(UnifiedAccounts::claim_default_evm_address(
Expand Down
9 changes: 4 additions & 5 deletions pallets/xvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,8 @@ use sp_core::{H160, U256};
use sp_std::{marker::PhantomData, prelude::*};

use astar_primitives::{
ethereum_checked::{
AccountMapping, CheckedEthereumTransact, CheckedEthereumTx, EthereumTxInput,
},
ethereum_checked::{CheckedEthereumTransact, CheckedEthereumTx, EthereumTxInput},
evm::UnifiedAddressMapper,
xvm::{
CallFailure, CallOutput, CallResult, Context, FailureError::*, FailureRevert::*, VmId,
XvmCall,
Expand Down Expand Up @@ -85,7 +84,7 @@ pub mod pallet {
#[pallet::config]
pub trait Config: frame_system::Config + pallet_contracts::Config {
/// Mapping from `Account` to `H160`.
type AccountMapping: AccountMapping<Self::AccountId>;
type AddressMapper: UnifiedAddressMapper<Self::AccountId>;

/// Mapping from Ethereum gas to Substrate weight.
type GasWeightMapping: GasWeightMapping;
Expand Down Expand Up @@ -213,7 +212,7 @@ where
let weight_limit = context.weight_limit.saturating_sub(overheads);
let gas_limit = U256::from(T::GasWeightMapping::weight_to_gas(weight_limit));

let source = T::AccountMapping::into_h160(source);
let source = T::AddressMapper::to_h160_or_default(&source);
let tx = CheckedEthereumTx {
gas_limit,
target: target_decoded,
Expand Down
11 changes: 2 additions & 9 deletions pallets/xvm/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use super::*;
use crate as pallet_xvm;

use astar_primitives::evm::HashedDefaultMappings;
use fp_evm::{CallInfo as EvmCallInfo, ExitReason, ExitSucceed, UsedGas};
use frame_support::{
construct_runtime,
Expand Down Expand Up @@ -124,14 +125,6 @@ impl pallet_contracts::Config for TestRuntime {
type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>;
}

pub struct HashedAccountMapping;
impl astar_primitives::ethereum_checked::AccountMapping<AccountId> for HashedAccountMapping {
fn into_h160(account_id: AccountId) -> H160 {
let data = (b"evm:", account_id);
return H160::from_slice(&data.using_encoded(sp_io::hashing::blake2_256)[0..20]);
}
}

thread_local! {
static TRANSACTED: RefCell<Option<(H160, CheckedEthereumTx)>> = RefCell::new(None);
}
Expand Down Expand Up @@ -180,7 +173,7 @@ impl GasWeightMapping for MockGasWeightMapping {

impl pallet_xvm::Config for TestRuntime {
type GasWeightMapping = MockGasWeightMapping;
type AccountMapping = HashedAccountMapping;
type AddressMapper = HashedDefaultMappings<BlakeTwo256>;
type EthereumTransact = MockEthereumTransact;
type WeightInfo = weights::SubstrateWeight<TestRuntime>;
}
Expand Down
1 change: 0 additions & 1 deletion pallets/xvm/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use mock::*;
use frame_support::{assert_noop, assert_ok, weights::Weight};
use parity_scale_codec::Encode;
use sp_core::H160;
use sp_runtime::MultiAddress;

#[test]
fn calling_into_same_vm_is_not_allowed() {
Expand Down
17 changes: 0 additions & 17 deletions primitives/src/ethereum_checked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,8 @@ use frame_support::{
traits::ConstU32,
BoundedVec,
};
use sp_core::Hasher;
use sp_std::{prelude::*, result::Result};

use crate::AccountId;

/// Max Ethereum tx input size: 65_536 bytes
pub const MAX_ETHEREUM_TX_INPUT_SIZE: u32 = 2u32.pow(16);

Expand Down Expand Up @@ -99,17 +96,3 @@ pub trait CheckedEthereumTransact {
checked_tx: CheckedEthereumTx,
) -> Result<(PostDispatchInfo, CallInfo), DispatchErrorWithPostInfo>;
}

/// Mapping from `Account` to `H160`.
pub trait AccountMapping<AccountId> {
fn into_h160(account: AccountId) -> H160;
}

/// Hashed derive mapping for converting account id to evm address
pub struct HashedAccountMapping<H>(sp_std::marker::PhantomData<H>);
impl<H: Hasher<Out = H256>> AccountMapping<AccountId> for HashedAccountMapping<H> {
fn into_h160(account: AccountId) -> H160 {
let payload = (b"evm:", account);
H160::from_slice(&payload.using_encoded(H::hash)[0..20])
}
}
Loading

0 comments on commit 4871911

Please sign in to comment.