From 7b74cda24d87e33c6f5dec60b57b960bbd5e0eef Mon Sep 17 00:00:00 2001 From: Ahmad Kaouk <56095276+ahmadkaouk@users.noreply.github.com> Date: Wed, 24 Sep 2025 09:14:56 +0200 Subject: [PATCH] feat: Add Call Filter (#181) ## Add proxy call filtering for EVM accounts This PR implements a `NormalCallFilter` following moonbeam filters. ### Changes - Added `NormalCallFilter` implementation across all runtime configurations (mainnet, stagenet, testnet) - Configured the filter as `BaseCallFilter` in `frame_system::Config` ### Security Improvements The filter blocks: - **Proxy calls to EVM accounts** - Prevents proxying to smart contracts - **Direct EVM pallet calls** - Prevents reentrancy from precompiles (following Moonbeam's security pattern) This aligns with best practices from Moonbeam and addresses known security considerations around EVM/Substrate interaction patterns. --------- Co-authored-by: Steve Degosserie <723552+stiiifff@users.noreply.github.com> --- operator/runtime/mainnet/src/configs/mod.rs | 28 ++++++++++++++-- operator/runtime/mainnet/src/lib.rs | 9 ++++-- operator/runtime/mainnet/tests/lib.rs | 33 ++++++++++++++++++- operator/runtime/stagenet/src/configs/mod.rs | 28 ++++++++++++++-- operator/runtime/stagenet/src/lib.rs | 9 ++++-- operator/runtime/stagenet/tests/lib.rs | 33 ++++++++++++++++++- operator/runtime/testnet/src/configs/mod.rs | 28 ++++++++++++++-- operator/runtime/testnet/src/lib.rs | 9 ++++-- operator/runtime/testnet/tests/lib.rs | 34 +++++++++++++++++++- 9 files changed, 196 insertions(+), 15 deletions(-) diff --git a/operator/runtime/mainnet/src/configs/mod.rs b/operator/runtime/mainnet/src/configs/mod.rs index ec350c88..c29b20d9 100644 --- a/operator/runtime/mainnet/src/configs/mod.rs +++ b/operator/runtime/mainnet/src/configs/mod.rs @@ -98,8 +98,8 @@ use frame_support::{ traits::{ fungible::{Balanced, Credit, HoldConsideration, Inspect}, tokens::{PayFromAccount, UnityAssetBalanceConversion}, - ConstU128, ConstU32, ConstU64, ConstU8, EitherOfDiverse, EqualPrivilegeOnly, FindAuthor, - KeyOwnerProofSystem, LinearStoragePrice, OnUnbalanced, VariantCountOf, + ConstU128, ConstU32, ConstU64, ConstU8, Contains, EitherOfDiverse, EqualPrivilegeOnly, + FindAuthor, KeyOwnerProofSystem, LinearStoragePrice, OnUnbalanced, VariantCountOf, }, weights::{constants::RocksDbWeight, IdentityFee, RuntimeDbWeight, Weight}, PalletId, @@ -203,6 +203,28 @@ parameter_types! { pub const SS58Prefix: u16 = SS58_FORMAT; } +/// Normal Call Filter +pub struct NormalCallFilter; +impl Contains for NormalCallFilter { + fn contains(c: &RuntimeCall) -> bool { + match c { + RuntimeCall::Proxy(method) => match method { + pallet_proxy::Call::proxy { real, .. } => { + !pallet_evm::AccountCodes::::contains_key(H160::from(*real)) + } + _ => true, + }, + // Filtering the EVM prevents possible re-entrancy from the precompiles which could + // lead to unexpected scenarios. + // See https://github.com/PureStake/sr-moonbeam/issues/30 + // Note: It is also assumed that EVM calls are only allowed through `Origin::Root` so + // this can be seen as an additional security + RuntimeCall::Evm(_) => false, + _ => true, + } + } +} + /// The default types are being injected by [`derive_impl`](`frame_support::derive_impl`) from /// [`SoloChainDefaultConfig`](`struct@frame_system::config_preludes::SolochainDefaultConfig`), /// but overridden as needed. @@ -234,6 +256,8 @@ impl frame_system::Config for Runtime { type SS58Prefix = SS58Prefix; type MaxConsumers = frame_support::traits::ConstU32<16>; type SystemWeightInfo = mainnet_weights::frame_system::WeightInfo; + /// Use the NormalCallFilter to restrict certain runtime calls + type BaseCallFilter = NormalCallFilter; } // 1 in 4 blocks (on average, not counting collisions) will be primary babe blocks. diff --git a/operator/runtime/mainnet/src/lib.rs b/operator/runtime/mainnet/src/lib.rs index f96d80c8..78a56d31 100644 --- a/operator/runtime/mainnet/src/lib.rs +++ b/operator/runtime/mainnet/src/lib.rs @@ -22,7 +22,7 @@ use frame_support::{ genesis_builder_helper::{build_state, get_preset}, pallet_prelude::{TransactionValidity, TransactionValidityError}, parameter_types, - traits::{KeyOwnerProofSystem, OnFinalize}, + traits::{Contains, KeyOwnerProofSystem, OnFinalize}, weights::{ constants::ExtrinsicBaseWeight, constants::WEIGHT_REF_TIME_PER_SECOND, Weight, WeightToFeeCoefficient, WeightToFeeCoefficients, WeightToFeePolynomial, @@ -62,7 +62,7 @@ pub use sp_runtime::BuildStorage; use sp_runtime::{ generic, impl_opaque_keys, traits::{Block as BlockT, DispatchInfoOf, Dispatchable, PostDispatchInfoOf}, - transaction_validity::TransactionSource, + transaction_validity::{InvalidTransaction, TransactionSource}, ApplyExtrinsicResult, Perbill, Permill, }; use sp_std::collections::btree_map::BTreeMap; @@ -609,6 +609,11 @@ impl_runtime_apis! { tx: ::Extrinsic, block_hash: ::Hash, ) -> TransactionValidity { + // Filtered calls should not enter the tx pool as they'll fail if inserted. + // If this call is not allowed, we return early. + if !::BaseCallFilter::contains(&tx.0.function) { + return InvalidTransaction::Call.into(); + } Executive::validate_transaction(source, tx, block_hash) } } diff --git a/operator/runtime/mainnet/tests/lib.rs b/operator/runtime/mainnet/tests/lib.rs index c0208a01..bffa5d64 100644 --- a/operator/runtime/mainnet/tests/lib.rs +++ b/operator/runtime/mainnet/tests/lib.rs @@ -6,7 +6,14 @@ mod native_token_transfer; mod proxy; use common::*; -use datahaven_mainnet_runtime::{currency::HAVE, Balances, System, VERSION}; +use datahaven_mainnet_runtime::{ + currency::HAVE, Balances, Runtime, System, UncheckedExtrinsic, VERSION, +}; +use sp_core::H160; +use sp_runtime::transaction_validity::{ + InvalidTransaction, TransactionSource, TransactionValidityError, +}; +use sp_transaction_pool::runtime_api::runtime_decl_for_tagged_transaction_queue::TaggedTransactionQueueV3; // Runtime Tests #[test] @@ -27,3 +34,27 @@ fn test_balances_functionality() { assert_eq!(Balances::free_balance(&account_id(ALICE)), 2_000_000 * HAVE); }); } +#[test] +fn validate_transaction_fails_on_filtered_call() { + ExtBuilder::default().build().execute_with(|| { + let xt = UncheckedExtrinsic::new_bare( + pallet_evm::Call::::call { + source: H160::default(), + target: H160::default(), + input: Vec::new(), + value: sp_core::U256::zero(), + gas_limit: 21000, + max_fee_per_gas: sp_core::U256::zero(), + max_priority_fee_per_gas: Some(sp_core::U256::zero()), + nonce: None, + access_list: Vec::new(), + } + .into(), + ); + + assert_eq!( + Runtime::validate_transaction(TransactionSource::External, xt, Default::default(),), + Err(TransactionValidityError::Invalid(InvalidTransaction::Call)), + ); + }); +} diff --git a/operator/runtime/stagenet/src/configs/mod.rs b/operator/runtime/stagenet/src/configs/mod.rs index 8e8312d8..9d80facd 100644 --- a/operator/runtime/stagenet/src/configs/mod.rs +++ b/operator/runtime/stagenet/src/configs/mod.rs @@ -98,8 +98,8 @@ use frame_support::{ traits::{ fungible::{Balanced, Credit, HoldConsideration, Inspect}, tokens::{PayFromAccount, UnityAssetBalanceConversion}, - ConstU128, ConstU32, ConstU64, ConstU8, EitherOfDiverse, EqualPrivilegeOnly, FindAuthor, - KeyOwnerProofSystem, LinearStoragePrice, OnUnbalanced, VariantCountOf, + ConstU128, ConstU32, ConstU64, ConstU8, Contains, EitherOfDiverse, EqualPrivilegeOnly, + FindAuthor, KeyOwnerProofSystem, LinearStoragePrice, OnUnbalanced, VariantCountOf, }, weights::{constants::RocksDbWeight, IdentityFee, RuntimeDbWeight, Weight}, PalletId, @@ -203,6 +203,28 @@ parameter_types! { pub const SS58Prefix: u16 = SS58_FORMAT; } +/// Normal Call Filter +pub struct NormalCallFilter; +impl Contains for NormalCallFilter { + fn contains(c: &RuntimeCall) -> bool { + match c { + RuntimeCall::Proxy(method) => match method { + pallet_proxy::Call::proxy { real, .. } => { + !pallet_evm::AccountCodes::::contains_key(H160::from(*real)) + } + _ => true, + }, + // Filtering the EVM prevents possible re-entrancy from the precompiles which could + // lead to unexpected scenarios. + // See https://github.com/PureStake/sr-moonbeam/issues/30 + // Note: It is also assumed that EVM calls are only allowed through `Origin::Root` so + // this can be seen as an additional security + RuntimeCall::Evm(_) => false, + _ => true, + } + } +} + /// The default types are being injected by [`derive_impl`](`frame_support::derive_impl`) from /// [`SoloChainDefaultConfig`](`struct@frame_system::config_preludes::SolochainDefaultConfig`), /// but overridden as needed. @@ -234,6 +256,8 @@ impl frame_system::Config for Runtime { type SS58Prefix = SS58Prefix; type MaxConsumers = frame_support::traits::ConstU32<16>; type SystemWeightInfo = stagenet_weights::frame_system::WeightInfo; + /// Use the NormalCallFilter to restrict certain runtime calls + type BaseCallFilter = NormalCallFilter; } // 1 in 4 blocks (on average, not counting collisions) will be primary babe blocks. diff --git a/operator/runtime/stagenet/src/lib.rs b/operator/runtime/stagenet/src/lib.rs index eb5babef..f918719f 100644 --- a/operator/runtime/stagenet/src/lib.rs +++ b/operator/runtime/stagenet/src/lib.rs @@ -22,7 +22,7 @@ use frame_support::{ genesis_builder_helper::{build_state, get_preset}, pallet_prelude::{TransactionValidity, TransactionValidityError}, parameter_types, - traits::{KeyOwnerProofSystem, OnFinalize}, + traits::{Contains, KeyOwnerProofSystem, OnFinalize}, weights::{constants::WEIGHT_REF_TIME_PER_SECOND, Weight}, }; pub use frame_system::Call as SystemCall; @@ -59,7 +59,7 @@ pub use sp_runtime::BuildStorage; use sp_runtime::{ generic, impl_opaque_keys, traits::{Block as BlockT, DispatchInfoOf, Dispatchable, PostDispatchInfoOf}, - transaction_validity::TransactionSource, + transaction_validity::{InvalidTransaction, TransactionSource}, ApplyExtrinsicResult, Perbill, Permill, }; use sp_std::collections::btree_map::BTreeMap; @@ -610,6 +610,11 @@ impl_runtime_apis! { tx: ::Extrinsic, block_hash: ::Hash, ) -> TransactionValidity { + // Filtered calls should not enter the tx pool as they'll fail if inserted. + // If this call is not allowed, we return early. + if !::BaseCallFilter::contains(&tx.0.function) { + return InvalidTransaction::Call.into(); + } Executive::validate_transaction(source, tx, block_hash) } } diff --git a/operator/runtime/stagenet/tests/lib.rs b/operator/runtime/stagenet/tests/lib.rs index 3ae920c4..4213f411 100644 --- a/operator/runtime/stagenet/tests/lib.rs +++ b/operator/runtime/stagenet/tests/lib.rs @@ -6,7 +6,14 @@ mod native_token_transfer; mod proxy; use common::*; -use datahaven_stagenet_runtime::{currency::HAVE, Balances, System, VERSION}; +use datahaven_stagenet_runtime::{ + currency::HAVE, Balances, Runtime, System, UncheckedExtrinsic, VERSION, +}; +use sp_core::H160; +use sp_runtime::transaction_validity::{ + InvalidTransaction, TransactionSource, TransactionValidityError, +}; +use sp_transaction_pool::runtime_api::runtime_decl_for_tagged_transaction_queue::TaggedTransactionQueueV3; // Runtime Tests #[test] @@ -27,3 +34,27 @@ fn test_balances_functionality() { assert_eq!(Balances::free_balance(&account_id(ALICE)), 2_000_000 * HAVE); }); } +#[test] +fn validate_transaction_fails_on_filtered_call() { + ExtBuilder::default().build().execute_with(|| { + let xt = UncheckedExtrinsic::new_bare( + pallet_evm::Call::::call { + source: H160::default(), + target: H160::default(), + input: Vec::new(), + value: sp_core::U256::zero(), + gas_limit: 21000, + max_fee_per_gas: sp_core::U256::zero(), + max_priority_fee_per_gas: Some(sp_core::U256::zero()), + nonce: None, + access_list: Vec::new(), + } + .into(), + ); + + assert_eq!( + Runtime::validate_transaction(TransactionSource::External, xt, Default::default(),), + Err(TransactionValidityError::Invalid(InvalidTransaction::Call)), + ); + }); +} diff --git a/operator/runtime/testnet/src/configs/mod.rs b/operator/runtime/testnet/src/configs/mod.rs index 2eb9eff1..ed0de229 100644 --- a/operator/runtime/testnet/src/configs/mod.rs +++ b/operator/runtime/testnet/src/configs/mod.rs @@ -98,8 +98,8 @@ use frame_support::{ traits::{ fungible::{Balanced, Credit, HoldConsideration, Inspect}, tokens::{PayFromAccount, UnityAssetBalanceConversion}, - ConstU128, ConstU32, ConstU64, ConstU8, EitherOfDiverse, EqualPrivilegeOnly, FindAuthor, - KeyOwnerProofSystem, LinearStoragePrice, OnUnbalanced, VariantCountOf, + ConstU128, ConstU32, ConstU64, ConstU8, Contains, EitherOfDiverse, EqualPrivilegeOnly, + FindAuthor, KeyOwnerProofSystem, LinearStoragePrice, OnUnbalanced, VariantCountOf, }, weights::{constants::RocksDbWeight, IdentityFee, RuntimeDbWeight, Weight}, PalletId, @@ -203,6 +203,28 @@ parameter_types! { pub const SS58Prefix: u16 = SS58_FORMAT; } +/// Normal Call Filter +pub struct NormalCallFilter; +impl Contains for NormalCallFilter { + fn contains(c: &RuntimeCall) -> bool { + match c { + RuntimeCall::Proxy(method) => match method { + pallet_proxy::Call::proxy { real, .. } => { + !pallet_evm::AccountCodes::::contains_key(H160::from(*real)) + } + _ => true, + }, + // Filtering the EVM prevents possible re-entrancy from the precompiles which could + // lead to unexpected scenarios. + // See https://github.com/PureStake/sr-moonbeam/issues/30 + // Note: It is also assumed that EVM calls are only allowed through `Origin::Root` so + // this can be seen as an additional security + RuntimeCall::Evm(_) => false, + _ => true, + } + } +} + /// The default types are being injected by [`derive_impl`](`frame_support::derive_impl`) from /// [`SoloChainDefaultConfig`](`struct@frame_system::config_preludes::SolochainDefaultConfig`), /// but overridden as needed. @@ -234,6 +256,8 @@ impl frame_system::Config for Runtime { type SS58Prefix = SS58Prefix; type MaxConsumers = frame_support::traits::ConstU32<16>; type SystemWeightInfo = testnet_weights::frame_system::WeightInfo; + /// Use the NormalCallFilter to restrict certain runtime calls + type BaseCallFilter = NormalCallFilter; } // 1 in 4 blocks (on average, not counting collisions) will be primary babe blocks. diff --git a/operator/runtime/testnet/src/lib.rs b/operator/runtime/testnet/src/lib.rs index 1a9fa2e1..5ae6bd13 100644 --- a/operator/runtime/testnet/src/lib.rs +++ b/operator/runtime/testnet/src/lib.rs @@ -21,7 +21,7 @@ use frame_support::{ genesis_builder_helper::{build_state, get_preset}, pallet_prelude::{TransactionValidity, TransactionValidityError}, parameter_types, - traits::{KeyOwnerProofSystem, OnFinalize}, + traits::{Contains, KeyOwnerProofSystem, OnFinalize}, weights::{ constants::ExtrinsicBaseWeight, constants::WEIGHT_REF_TIME_PER_SECOND, Weight, WeightToFeeCoefficient, WeightToFeeCoefficients, WeightToFeePolynomial, @@ -61,7 +61,7 @@ pub use sp_runtime::BuildStorage; use sp_runtime::{ generic, impl_opaque_keys, traits::{Block as BlockT, DispatchInfoOf, Dispatchable, PostDispatchInfoOf}, - transaction_validity::TransactionSource, + transaction_validity::{InvalidTransaction, TransactionSource}, ApplyExtrinsicResult, Perbill, Permill, }; use sp_std::collections::btree_map::BTreeMap; @@ -608,6 +608,11 @@ impl_runtime_apis! { tx: ::Extrinsic, block_hash: ::Hash, ) -> TransactionValidity { + // Filtered calls should not enter the tx pool as they'll fail if inserted. + // If this call is not allowed, we return early. + if !::BaseCallFilter::contains(&tx.0.function) { + return InvalidTransaction::Call.into(); + } Executive::validate_transaction(source, tx, block_hash) } } diff --git a/operator/runtime/testnet/tests/lib.rs b/operator/runtime/testnet/tests/lib.rs index 4c5ee8d4..dd8d1ed3 100644 --- a/operator/runtime/testnet/tests/lib.rs +++ b/operator/runtime/testnet/tests/lib.rs @@ -6,7 +6,14 @@ mod native_token_transfer; mod proxy; use common::*; -use datahaven_testnet_runtime::{currency::HAVE, Balances, System, VERSION}; +use datahaven_testnet_runtime::{ + currency::HAVE, Balances, Runtime, System, UncheckedExtrinsic, VERSION, +}; +use sp_core::H160; +use sp_runtime::transaction_validity::{ + InvalidTransaction, TransactionSource, TransactionValidityError, +}; +use sp_transaction_pool::runtime_api::runtime_decl_for_tagged_transaction_queue::TaggedTransactionQueueV3; // Runtime Tests #[test] @@ -27,3 +34,28 @@ fn test_balances_functionality() { assert_eq!(Balances::free_balance(&account_id(ALICE)), 2_000_000 * HAVE); }); } + +#[test] +fn validate_transaction_fails_on_filtered_call() { + ExtBuilder::default().build().execute_with(|| { + let xt = UncheckedExtrinsic::new_bare( + pallet_evm::Call::::call { + source: H160::default(), + target: H160::default(), + input: Vec::new(), + value: sp_core::U256::zero(), + gas_limit: 21000, + max_fee_per_gas: sp_core::U256::zero(), + max_priority_fee_per_gas: Some(sp_core::U256::zero()), + nonce: None, + access_list: Vec::new(), + } + .into(), + ); + + assert_eq!( + Runtime::validate_transaction(TransactionSource::External, xt, Default::default(),), + Err(TransactionValidityError::Invalid(InvalidTransaction::Call)), + ); + }); +}