Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove AccountMapping trait #1067

Merged
merged 3 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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).into_address(),
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).into_address(),
tx.into(),
CheckedEthereumTxKind::Xcm,
true,
Expand Down
38 changes: 25 additions & 13 deletions pallets/ethereum-checked/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,21 +127,33 @@ impl AddressMapping<AccountId32> for MockAddressMapping {
}
}

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

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

// this method is not used in tests
fn to_default_account_id(_: &H160) -> AccountId32 {
unimplemented!("not used in tests")
}

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

Expand Down Expand Up @@ -193,7 +205,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
11 changes: 5 additions & 6 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 All @@ -87,7 +86,7 @@ mod benchmarks {
#[benchmark]
fn to_account_id() {
let caller: T::AccountId = whitelisted_caller();
let evm_address = T::DefaultNativeToEvm::into_h160(caller.clone());
let evm_address = T::DefaultMappings::to_default_h160(&caller.clone());
assert_ok!(T::Currency::mint_into(
&caller,
T::AccountMappingStorageFee::get()
Expand All @@ -106,7 +105,7 @@ mod benchmarks {
#[benchmark]
fn to_account_id_or_default() {
let caller: T::AccountId = whitelisted_caller();
let evm_address = T::DefaultNativeToEvm::into_h160(caller.clone());
let evm_address = T::DefaultMappings::to_default_h160(&caller.clone());
assert_ok!(T::Currency::mint_into(
&caller,
T::AccountMappingStorageFee::get()
Expand Down
42 changes: 7 additions & 35 deletions pallets/unified-accounts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,13 @@
//! * [`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, UnifiedAddress, UnifiedAddressMapper},
evm::{EvmAddress, UnifiedAddressMapper},
Balance,
};
use frame_support::{
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,39 +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) -> UnifiedAddress<T::AccountId> {
if let Some(native) = Self::to_account_id(&evm_address) {
UnifiedAddress::Mapped(native)
} else {
UnifiedAddress::Default(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) -> UnifiedAddress<EvmAddress> {
if let Some(h160) = Self::to_h160(&account_id) {
UnifiedAddress::Mapped(h160)
} else {
UnifiedAddress::Default(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).into_address()
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).into_address();
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
Loading