Skip to content

Commit

Permalink
Eip 3607 (#2589)
Browse files Browse the repository at this point in the history 
* check eoa for evm calls

* add test

* fix

* tests

* fix
  • Loading branch information
@xlc
xlc authored Aug 16, 2023
1 parent 86c73c3 commit 511dc16
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 21 deletions.
4 changes: 2 additions & 2 deletions modules/asset-registry/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ where
fn calculate_rate(location: MultiLocation) -> Option<Ratio> {
let currency = key_to_currency(location);
match currency {
Some(CurrencyId::Erc20(address)) if !is_system_contract(address) => {
Some(CurrencyId::Erc20(address)) if !is_system_contract(&address) => {
if let Some(asset_metadata) = Pallet::<T>::asset_metadatas(AssetIds::Erc20(address)) {
let minimum_balance = asset_metadata.minimal_balance.into();
let rate =
Expand Down Expand Up @@ -898,7 +898,7 @@ impl<T: Config> Erc20InfoMapping for EvmErc20InfoMapping<T> {
// If is CurrencyId::DexShare and contain DexShare::Erc20,
// will use the u32 to get the DexShare::Erc20 from the mapping.
fn decode_evm_address(addr: EvmAddress) -> Option<CurrencyId> {
if !is_system_contract(addr) {
if !is_system_contract(&addr) {
return Some(CurrencyId::Erc20(addr));
}

Expand Down
28 changes: 26 additions & 2 deletions modules/evm/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ pub use module_support::{
pub use orml_traits::{currency::TransferAll, MultiCurrency};
pub use primitives::{
evm::{
convert_decimals_from_evm, convert_decimals_to_evm, decode_gas_limit, CallInfo, CreateInfo, EvmAddress,
ExecutionInfo, Vicinity, MIRRORED_NFT_ADDRESS_START, MIRRORED_TOKENS_ADDRESS_START,
convert_decimals_from_evm, convert_decimals_to_evm, decode_gas_limit, is_system_contract, CallInfo, CreateInfo,
EvmAddress, ExecutionInfo, Vicinity, MIRRORED_NFT_ADDRESS_START, MIRRORED_TOKENS_ADDRESS_START,
},
task::TaskResult,
Balance, CurrencyId, Nonce, ReserveIdentifier,
Expand Down Expand Up @@ -526,6 +526,8 @@ pub mod module {
InvalidDecimals,
/// Strict call failed
StrictCallFailed,
/// Caller is not externally owned account
NotEOA,
}

#[pallet::pallet]
Expand Down Expand Up @@ -623,6 +625,8 @@ pub mod module {
let who = ensure_signed(origin)?;
let source = T::AddressMapping::get_or_create_evm_address(&who);

Self::ensure_eoa(&source)?;

let outcome = T::Runner::call(
source,
source,
Expand Down Expand Up @@ -810,6 +814,8 @@ pub mod module {
let who = ensure_signed(origin)?;
let source = T::AddressMapping::get_or_create_evm_address(&who);

Self::ensure_eoa(&source)?;

let outcome = T::Runner::create(
source,
input,
Expand Down Expand Up @@ -888,6 +894,8 @@ pub mod module {
let who = ensure_signed(origin)?;
let source = T::AddressMapping::get_or_create_evm_address(&who);

Self::ensure_eoa(&source)?;

let outcome = T::Runner::create2(
source,
input,
Expand Down Expand Up @@ -1263,6 +1271,8 @@ pub mod module {
let who = ensure_signed(origin)?;
let source = T::AddressMapping::get_or_create_evm_address(&who);

Self::ensure_eoa(&source)?;

match T::Runner::call(
source,
source,
Expand Down Expand Up @@ -1315,6 +1325,20 @@ pub mod module {
}

impl<T: Config> Pallet<T> {
/// EIP-3607: https://eips.ethereum.org/EIPS/eip-3607
/// Do not allow transactions for which `tx.sender` has any code deployed.
//
/// We extend the principle of this EIP to also prevent `tx.sender` to be the address
/// of a precompile. While mainnet Ethereum currently only has stateless precompiles,
/// Acala EVM+ can have stateful precompiles that can manage funds or
/// which calls other contracts that expects this precompile address to be trustworthy.
fn ensure_eoa(caller: &EvmAddress) -> DispatchResult {
if is_system_contract(caller) || Self::is_contract(caller) {
return Err(Error::<T>::NotEOA.into());
}
Ok(())
}

/// Get StorageDepositPerByte of actual decimals
pub fn get_storage_deposit_per_byte() -> BalanceOf<T> {
// StorageDepositPerByte decimals is 18, KAR/ACA decimals is 12, convert to 12 here.
Expand Down
9 changes: 8 additions & 1 deletion modules/evm/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use frame_system::EnsureSignedBy;
use module_support::mocks::MockAddressMapping;
use orml_traits::parameter_type_with_key;
use primitives::{define_combined_task, Amount, BlockNumber, CurrencyId, ReserveIdentifier, TokenSymbol};
use sp_core::{H160, H256};
use sp_core::{bytes::from_hex, H160, H256};
use sp_runtime::{
traits::{BlakeTwo256, BlockNumberProvider, IdentityLookup},
AccountId32, BuildStorage,
Expand Down Expand Up @@ -266,10 +266,17 @@ pub fn new_test_ext() -> sp_io::TestExternalities {

let mut accounts = BTreeMap::new();

// pragma solidity >=0.8.2 <0.9.0;
// contract Test {}
let contract = from_hex(
"0x6080604052348015600f57600080fd5b50603f80601d6000396000f3fe6080604052600080fdfea2646970667358221220199b6fd928fecd2e7ce866eb76c49927191c7a839fd75192acc84b773e5dbf1e64736f6c63430008120033"
).unwrap();

accounts.insert(
contract_a(),
GenesisAccount {
nonce: 1,
code: contract.clone(),
..Default::default()
},
);
Expand Down
152 changes: 151 additions & 1 deletion modules/evm/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ fn inc_nonce_if_needed() {
#[test]
fn fail_call_return_ok_and_inc_nonce() {
new_test_ext().execute_with(|| {
let mut data = [0u8; 32];
let mut data = [5u8; 32];
data[0..4].copy_from_slice(b"evm:");
let signer: AccountId32 = AccountId32::from(data);
let alice = MockAddressMapping::get_or_create_evm_address(&signer);
Expand Down Expand Up @@ -2829,3 +2829,153 @@ fn aggregated_storage_logs_works() {
}));
})
}

#[allow(deprecated)]
#[test]
fn should_not_allow_contracts_send_tx() {
new_test_ext().execute_with(|| {
let origin = RuntimeOrigin::signed(MockAddressMapping::get_account_id(&contract_a()));
assert_noop!(
EVM::eth_call(
origin.clone(),
TransactionAction::Call(contract_a()),
vec![],
0,
1_000_000,
100,
vec![],
0
),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::eth_call(
origin.clone(),
TransactionAction::Create,
vec![],
0,
1_000_000,
100,
vec![],
0
),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::eth_call_v2(
origin.clone(),
TransactionAction::Call(contract_a()),
vec![],
0,
1_000_000,
100,
vec![]
),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::eth_call_v2(
origin.clone(),
TransactionAction::Create,
vec![],
0,
1_000_000,
100,
vec![]
),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::call(origin.clone(), contract_a(), vec![], 0, 1_000_000, 100, vec![]),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::create(origin.clone(), vec![], 0, 1_000_000, 100, vec![]),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::create2(origin.clone(), vec![], Default::default(), 0, 1_000_000, 100, vec![]),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::strict_call(origin, contract_a(), vec![], 0, 1_000_000, 100, vec![]),
Error::<Runtime>::NotEOA
);
});
}

#[allow(deprecated)]
#[test]
fn should_not_allow_system_contracts_send_tx() {
new_test_ext().execute_with(|| {
let origin = RuntimeOrigin::signed(MockAddressMapping::get_account_id(
&H160::from_str("000000000000000000ffffffffffffffffffffff").unwrap(),
));
assert_noop!(
EVM::eth_call(
origin.clone(),
TransactionAction::Call(contract_a()),
vec![],
0,
1_000_000,
100,
vec![],
0
),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::eth_call(
origin.clone(),
TransactionAction::Create,
vec![],
0,
1_000_000,
100,
vec![],
0
),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::eth_call_v2(
origin.clone(),
TransactionAction::Call(contract_a()),
vec![],
0,
1_000_000,
100,
vec![]
),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::eth_call_v2(
origin.clone(),
TransactionAction::Create,
vec![],
0,
1_000_000,
100,
vec![]
),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::call(origin.clone(), contract_a(), vec![], 0, 1_000_000, 100, vec![]),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::create(origin.clone(), vec![], 0, 1_000_000, 100, vec![]),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::create2(origin.clone(), vec![], Default::default(), 0, 1_000_000, 100, vec![]),
Error::<Runtime>::NotEOA
);
assert_noop!(
EVM::strict_call(origin, contract_a(), vec![], 0, 1_000_000, 100, vec![]),
Error::<Runtime>::NotEOA
);
});
}
2 changes: 1 addition & 1 deletion primitives/src/evm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ pub const SYSTEM_CONTRACT_ADDRESS_PREFIX: [u8; 9] = [0u8; 9];
/// Check if the given `address` is a system contract.
///
/// It's system contract if the address starts with SYSTEM_CONTRACT_ADDRESS_PREFIX.
pub fn is_system_contract(address: EvmAddress) -> bool {
pub fn is_system_contract(address: &EvmAddress) -> bool {
address.as_bytes().starts_with(&SYSTEM_CONTRACT_ADDRESS_PREFIX)
}

Expand Down
8 changes: 4 additions & 4 deletions primitives/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,18 +169,18 @@ fn generate_function_selector_works() {

#[test]
fn is_system_contract_works() {
assert!(is_system_contract(H160::from_low_u64_be(0)));
assert!(is_system_contract(H160::from_low_u64_be(u64::max_value())));
assert!(is_system_contract(&H160::from_low_u64_be(0)));
assert!(is_system_contract(&H160::from_low_u64_be(u64::max_value())));

let mut bytes = [0u8; 20];
bytes[SYSTEM_CONTRACT_ADDRESS_PREFIX.len() - 1] = 1u8;

assert!(!is_system_contract(bytes.into()));
assert!(!is_system_contract(&bytes.into()));

bytes = [0u8; 20];
bytes[0] = 1u8;

assert!(!is_system_contract(bytes.into()));
assert!(!is_system_contract(&bytes.into()));
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions runtime/acala/src/xcm_config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ impl Convert<CurrencyId, Option<MultiLocation>> for CurrencyIdConvert {
Token(ACA) | Token(AUSD) | Token(LDOT) | Token(TAP) => {
native_currency_location(ParachainInfo::get().into(), id.encode())
}
Erc20(address) if !is_system_contract(address) => {
Erc20(address) if !is_system_contract(&address) => {
native_currency_location(ParachainInfo::get().into(), id.encode())
}
LiquidCrowdloan(_lease) => native_currency_location(ParachainInfo::get().into(), id.encode()),
Expand Down Expand Up @@ -328,7 +328,7 @@ impl Convert<MultiLocation, Option<CurrencyId>> for CurrencyIdConvert {
// check `currency_id` is cross-chain asset
match currency_id {
Token(ACA) | Token(AUSD) | Token(LDOT) | Token(TAP) => Some(currency_id),
Erc20(address) if !is_system_contract(address) => Some(currency_id),
Erc20(address) if !is_system_contract(&address) => Some(currency_id),
LiquidCrowdloan(_lease) => Some(currency_id),
StableAssetPoolToken(_pool_id) => Some(currency_id),
_ => None,
Expand All @@ -350,7 +350,7 @@ impl Convert<MultiLocation, Option<CurrencyId>> for CurrencyIdConvert {
let currency_id = CurrencyId::decode(&mut &*key).ok()?;
match currency_id {
Token(ACA) | Token(AUSD) | Token(LDOT) | Token(TAP) => Some(currency_id),
Erc20(address) if !is_system_contract(address) => Some(currency_id),
Erc20(address) if !is_system_contract(&address) => Some(currency_id),
LiquidCrowdloan(_lease) => Some(currency_id),
StableAssetPoolToken(_pool_id) => Some(currency_id),
_ => None,
Expand Down
2 changes: 1 addition & 1 deletion runtime/common/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ parameter_types! {
pub struct SystemContractsFilter;
impl PrecompileCallerFilter for SystemContractsFilter {
fn is_allowed(caller: H160) -> bool {
is_system_contract(caller)
is_system_contract(&caller)
}
}

Expand Down
6 changes: 3 additions & 3 deletions runtime/karura/src/xcm_config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ impl Convert<CurrencyId, Option<MultiLocation>> for CurrencyIdConvert {
Token(KAR) | Token(KUSD) | Token(LKSM) | Token(TAI) => {
native_currency_location(ParachainInfo::get().into(), id.encode())
}
Erc20(address) if !is_system_contract(address) => {
Erc20(address) if !is_system_contract(&address) => {
native_currency_location(ParachainInfo::get().into(), id.encode())
}
StableAssetPoolToken(_pool_id) => native_currency_location(ParachainInfo::get().into(), id.encode()),
Expand Down Expand Up @@ -437,7 +437,7 @@ impl Convert<MultiLocation, Option<CurrencyId>> for CurrencyIdConvert {
// check `currency_id` is cross-chain asset
match currency_id {
Token(KAR) | Token(KUSD) | Token(LKSM) | Token(TAI) => Some(currency_id),
Erc20(address) if !is_system_contract(address) => Some(currency_id),
Erc20(address) if !is_system_contract(&address) => Some(currency_id),
StableAssetPoolToken(_pool_id) => Some(currency_id),
_ => None,
}
Expand All @@ -462,7 +462,7 @@ impl Convert<MultiLocation, Option<CurrencyId>> for CurrencyIdConvert {
let currency_id = CurrencyId::decode(&mut &*key).ok()?;
match currency_id {
Token(KAR) | Token(KUSD) | Token(LKSM) | Token(TAI) => Some(currency_id),
Erc20(address) if !is_system_contract(address) => Some(currency_id),
Erc20(address) if !is_system_contract(&address) => Some(currency_id),
StableAssetPoolToken(_pool_id) => Some(currency_id),
_ => None,
}
Expand Down
Loading

0 comments on commit 511dc16

Please sign in to comment.