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

Add contract creation allow lists to EVM domains #3350

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/subspace-runtime/src/signed_extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use sp_runtime::transaction_validity::{
};
use sp_std::prelude::*;

/// Disable specific pallets.
/// Disable balance transfers, if configured in the runtime.
#[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, Default, TypeInfo)]
pub struct DisablePallets;

Expand Down
4 changes: 3 additions & 1 deletion domains/pallets/evm_nonce_tracker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ edition = "2021"
license = "Apache-2.0"
homepage = "https://subspace.network"
repository = "https://github.com/autonomys/subspace"
description = "Subspace node pallet for EVM account nonce tracker."
description = "Subspace node pallet for EVM account nonce tracker and contract creation allow list."
include = [
"/src",
"/Cargo.toml",
Expand All @@ -18,6 +18,7 @@ frame-support = { default-features = false, git = "https://github.com/subspace/p
frame-system = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "94a1a8143a89bbe9f938c1939ff68abc1519a305" }
scale-info = { version = "2.11.2", default-features = false, features = ["derive"] }
sp-core = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "94a1a8143a89bbe9f938c1939ff68abc1519a305" }
sp-domains = { version = "0.1.0", default-features = false, path = "../../../crates/sp-domains" }
sp-runtime = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "94a1a8143a89bbe9f938c1939ff68abc1519a305" }

[features]
Expand All @@ -28,5 +29,6 @@ std = [
"frame-system/std",
"scale-info/std",
"sp-core/std",
"sp-domains/std",
"sp-runtime/std",
]
30 changes: 29 additions & 1 deletion domains/pallets/evm_nonce_tracker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ use sp_core::U256;
#[frame_support::pallet]
mod pallet {
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::BlockNumberFor;
use frame_system::pallet_prelude::*;
use sp_core::U256;
use sp_domains::PermissionedActionAllowedBy;

#[pallet::config]
pub trait Config: frame_system::Config {}
Expand All @@ -41,11 +42,31 @@ mod pallet {
pub(super) type AccountNonce<T: Config> =
StorageMap<_, Identity, T::AccountId, U256, OptionQuery>;

/// Storage to hold EVM contract creation allow list accounts.
#[pallet::storage]
pub(super) type ContractCreationAllowedBy<T: Config> =
StorageValue<_, PermissionedActionAllowedBy<T::AccountId>, OptionQuery>;

teor2345 marked this conversation as resolved.
Show resolved Hide resolved
/// Pallet EVM account nonce tracker.
#[pallet::pallet]
#[pallet::without_storage_info]
pub struct Pallet<T>(_);

#[pallet::call]
impl<T: Config> Pallet<T> {
/// Update ContractCreationAllowedBy storage by Sudo.
#[pallet::call_index(0)]
#[pallet::weight(<T as frame_system::Config>::DbWeight::get().reads_writes(0, 1))]
pub fn set_contract_creation_allowed_by(
origin: OriginFor<T>,
contract_creation_allowed_by: PermissionedActionAllowedBy<T::AccountId>,
) -> DispatchResult {
ensure_root(origin)?;
ContractCreationAllowedBy::<T>::put(contract_creation_allowed_by);
Ok(())
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
}
}

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_finalize(_now: BlockNumberFor<T>) {
Expand All @@ -66,4 +87,11 @@ impl<T: Config> Pallet<T> {
pub fn set_account_nonce(account: T::AccountId, nonce: U256) {
AccountNonce::<T>::set(account, Some(nonce))
}

/// Returns true if the account is allowed to create contracts.
pub fn is_allowed_to_create_contracts(signer: &T::AccountId) -> bool {
ContractCreationAllowedBy::<T>::get()
.map(|allowed_by| allowed_by.is_allowed(signer))
.unwrap_or_default()
}
}
113 changes: 108 additions & 5 deletions domains/runtime/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ use frame_support::weights::{ConstantMultiplier, Weight};
use frame_support::{construct_runtime, parameter_types};
use frame_system::limits::{BlockLength, BlockWeights};
use pallet_block_fees::fees::OnChargeDomainTransaction;
use pallet_ethereum::Call::transact;
use pallet_ethereum::{
Call, PostLogContent, Transaction as EthereumTransaction, TransactionData, TransactionStatus,
PostLogContent, Transaction as EthereumTransaction, TransactionAction, TransactionData,
TransactionStatus,
};
use pallet_evm::{
Account as EVMAccount, EnsureAddressNever, EnsureAddressRoot, FeeCalculator,
Expand All @@ -69,6 +69,7 @@ use sp_runtime::traits::{
};
use sp_runtime::transaction_validity::{
InvalidTransaction, TransactionSource, TransactionValidity, TransactionValidityError,
ValidTransaction,
};
use sp_runtime::{
generic, impl_opaque_keys, ApplyExtrinsicResult, ConsensusEngineId, Digest,
Expand Down Expand Up @@ -121,6 +122,7 @@ pub type SignedExtra = (
frame_system::CheckNonce<Runtime>,
domain_check_weight::CheckWeight<Runtime>,
pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
CheckContractCreation,
);

/// Custom signed extra for check_and_pre_dispatch.
Expand All @@ -134,6 +136,7 @@ type CustomSignedExtra = (
pallet_evm_nonce_tracker::CheckNonce<Runtime>,
domain_check_weight::CheckWeight<Runtime>,
pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
CheckContractCreation,
);

/// Unchecked extrinsic type as expected by this runtime.
Expand All @@ -152,6 +155,95 @@ pub type Executive = domain_pallet_executive::Executive<
AllPalletsWithSystem,
>;

/// Rejects contracts that can't be created under the current allow list.
/// Returns false if the call is a contract call, and the account is *not* allowed to call it.
/// Otherwise, returns true.
pub fn is_create_contract_allowed(call: &RuntimeCall, signer: &AccountId) -> bool {
if is_create_contract(call)
&& !pallet_evm_nonce_tracker::Pallet::<Runtime>::is_allowed_to_create_contracts(signer)
{
return false;
}

// If it's not a contract call, or the account is allowed to create contracts, return true.
true
}

/// Returns true if the call is a contract creation call.
pub fn is_create_contract(call: &RuntimeCall) -> bool {
match call {
RuntimeCall::EVM(pallet_evm::Call::create { .. })
| RuntimeCall::EVM(pallet_evm::Call::create2 { .. }) => true,
RuntimeCall::Ethereum(pallet_ethereum::Call::transact {
transaction: EthereumTransaction::Legacy(transaction),
..
}) => transaction.action == TransactionAction::Create,
RuntimeCall::Ethereum(pallet_ethereum::Call::transact {
transaction: EthereumTransaction::EIP2930(transaction),
..
}) => transaction.action == TransactionAction::Create,
RuntimeCall::Ethereum(pallet_ethereum::Call::transact {
transaction: EthereumTransaction::EIP1559(transaction),
..
}) => transaction.action == TransactionAction::Create,
// TODO: does this need a recursion limit?
RuntimeCall::Utility(utility_call) => match utility_call {
pallet_utility::Call::batch { calls }
| pallet_utility::Call::batch_all { calls }
| pallet_utility::Call::force_batch { calls } => calls.iter().any(is_create_contract),
pallet_utility::Call::as_derivative { call, .. }
| pallet_utility::Call::dispatch_as { call, .. }
| pallet_utility::Call::with_weight { call, .. } => is_create_contract(call),
pallet_utility::Call::__Ignore(..) => false,
},
_ => false,
}
}

/// Reject contract creation, unless the account is in the current evm contract allow list.
#[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, Default, TypeInfo)]
pub struct CheckContractCreation;

impl SignedExtension for CheckContractCreation {
const IDENTIFIER: &'static str = "CheckContractCreation";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this Extension to the pallet itself?
Also ideal if we change the name of pallet to something more alighed with new changes
maybe just pallet-evm-tracker ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this Extension to the pallet itself?

This also makes sense to me, as we can use this extension in the test evm runtime too, and we can write integration test for it.

Also ideal if we change the name of pallet to something more alighed with new changes
maybe just pallet-evm-tracker ?

It is okay to change the crate name but notice we can't change the pallet name in the runtime, because the pallet name is used to construct the storage key changing it make break compatibility with the existing evm domain on Taurus

Copy link
Member Author

@teor2345 teor2345 Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this Extension to the pallet itself?

Not easily. The extension matches on the Runtime::Call enum, which is composed via a macro. Similar code isn't located in pallets, it's located in crates/subspace-runtime/src/signed_extensions.rs and crates/subspace-runtime/src/object_mapping.rs.

We could move the extension to the pallet, but we'd need to keep the code that identifies a contract creation transaction next to the code that constructs the domain runtime. And add a new ContainsContractCreation trait so the extension can call that runtime-specific code.

Also ideal if we change the name of pallet to something more alighed with new changes
maybe just pallet-evm-tracker ?

Sure, I'll do this last.
Edit: done in PR.

type AccountId = <Runtime as frame_system::Config>::AccountId;
type Call = <Runtime as frame_system::Config>::RuntimeCall;
type AdditionalSigned = ();
type Pre = ();

fn additional_signed(&self) -> Result<Self::AdditionalSigned, TransactionValidityError> {
Ok(())
}

fn validate(
&self,
who: &Self::AccountId,
call: &Self::Call,
_info: &DispatchInfoOf<Self::Call>,
_len: usize,
) -> TransactionValidity {
// Reject contract creation unless the account is in the allow list.
if !is_create_contract_allowed(call, who) {
InvalidTransaction::Call.into()
} else {
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
Ok(ValidTransaction::default())
}
}

fn pre_dispatch(
self,
who: &Self::AccountId,
call: &Self::Call,
info: &DispatchInfoOf<Self::Call>,
len: usize,
) -> Result<Self::Pre, TransactionValidityError> {
self.validate(who, call, info, len)?;
Ok(())
}

// TODO: can unsigned calls create contracts?
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
}
teor2345 marked this conversation as resolved.
Show resolved Hide resolved

impl fp_self_contained::SelfContainedCall for RuntimeCall {
type SignedInfo = H160;

Expand All @@ -175,6 +267,11 @@ impl fp_self_contained::SelfContainedCall for RuntimeCall {
dispatch_info: &DispatchInfoOf<RuntimeCall>,
len: usize,
) -> Option<TransactionValidity> {
if !is_create_contract_allowed(self, &(*info).into()) {
// TODO: should this be Custom() instead?
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
return Some(Err(InvalidTransaction::Call.into()));
}

match self {
RuntimeCall::Ethereum(call) => {
// Ensure the caller can pay for the consensus chain storage fee
Expand All @@ -199,6 +296,11 @@ impl fp_self_contained::SelfContainedCall for RuntimeCall {
dispatch_info: &DispatchInfoOf<RuntimeCall>,
len: usize,
) -> Option<Result<(), TransactionValidityError>> {
if !is_create_contract_allowed(self, &(*info).into()) {
// TODO: should this be Custom() instead?
return Some(Err(InvalidTransaction::Call.into()));
}

match self {
RuntimeCall::Ethereum(call) => {
// Withdraw the consensus chain storage fee from the caller and record
Expand Down Expand Up @@ -946,7 +1048,7 @@ fn pre_dispatch_evm_transaction(
{
let _ = transaction_validity?;

let Call::transact { transaction } = call;
let pallet_ethereum::Call::transact { transaction } = call;
domain_check_weight::CheckWeight::<Runtime>::do_pre_dispatch(dispatch_info, len)?;

let transaction_data: TransactionData = (&transaction).into();
Expand Down Expand Up @@ -1009,6 +1111,7 @@ fn check_transaction_and_do_pre_dispatch_inner(
pallet_evm_nonce_tracker::CheckNonce::from(extra.5 .0),
extra.6,
extra.7,
extra.8,
);

custom_extra
Expand Down Expand Up @@ -1526,7 +1629,7 @@ impl_runtime_apis! {
xts: Vec<<Block as BlockT>::Extrinsic>,
) -> Vec<EthereumTransaction> {
xts.into_iter().filter_map(|xt| match xt.0.function {
RuntimeCall::Ethereum(transact { transaction }) => Some(transaction),
RuntimeCall::Ethereum(pallet_ethereum::Call::transact { transaction }) => Some(transaction),
_ => None
}).collect::<Vec<EthereumTransaction>>()
}
Expand Down Expand Up @@ -1560,7 +1663,7 @@ impl_runtime_apis! {
impl fp_rpc::ConvertTransactionRuntimeApi<Block> for Runtime {
fn convert_transaction(transaction: EthereumTransaction) -> <Block as BlockT>::Extrinsic {
UncheckedExtrinsic::new_unsigned(
pallet_ethereum::Call::<Runtime>::transact { transaction }.into(),
pallet_ethereum::Call::transact { transaction }.into(),
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
)
}
}
Expand Down