Forbid transactions without chain ID by default

client/rpc/src/eth/mod.rs

@@ -85,6 +85,8 @@ pub struct Eth<B: BlockT, C, P, CT, BE, CIDP, EC> {
 	/// When using eth_call/eth_estimateGas, the maximum allowed gas limit will be
 	/// block.gas_limit * execute_gas_limit_multiplier
 	execute_gas_limit_multiplier: u64,
+	/// Whether RPC submission accepts legacy transactions without EIP-155 chain id.
+	rpc_allow_unprotected_txs: bool,
 	forced_parent_hashes: Option<BTreeMap<H256, H256>>,
 	/// Something that can create the inherent data providers for pending state.
 	pending_create_inherent_data_providers: CIDP,
@@ -113,6 +115,7 @@ where
 		fee_history_cache: FeeHistoryCache,
 		fee_history_cache_limit: FeeHistoryCacheLimit,
 		execute_gas_limit_multiplier: u64,
+		rpc_allow_unprotected_txs: bool,
 		forced_parent_hashes: Option<BTreeMap<H256, H256>>,
 		pending_create_inherent_data_providers: CIDP,
 		pending_consensus_data_provider: Option<Box<dyn pending::ConsensusDataProvider<B>>>,
@@ -130,6 +133,7 @@ where
 			fee_history_cache,
 			fee_history_cache_limit,
 			execute_gas_limit_multiplier,
+			rpc_allow_unprotected_txs,
 			forced_parent_hashes,
 			pending_create_inherent_data_providers,
 			pending_consensus_data_provider,
@@ -326,6 +330,7 @@ where
 			fee_history_cache,
 			fee_history_cache_limit,
 			execute_gas_limit_multiplier,
+			rpc_allow_unprotected_txs,
 			forced_parent_hashes,
 			pending_create_inherent_data_providers,
 			pending_consensus_data_provider,
@@ -345,6 +350,7 @@ where
 			fee_history_cache,
 			fee_history_cache_limit,
 			execute_gas_limit_multiplier,
+			rpc_allow_unprotected_txs,
 			forced_parent_hashes,
 			pending_create_inherent_data_providers,
 			pending_consensus_data_provider,

client/rpc/src/eth/submit.rs

@@ -37,6 +37,20 @@ use crate::{
 	internal_err, public_key,
 };
 
+fn is_unprotected_legacy_tx(transaction: &ethereum::TransactionV3) -> bool {
+	matches!(
+		transaction,
+		ethereum::TransactionV3::Legacy(tx) if tx.signature.chain_id().is_none()
+	)
+}
+
+fn should_reject_unprotected_legacy_tx(
+	transaction: &ethereum::TransactionV3,
+	allow_unprotected_txs: bool,
+) -> bool {
+	is_unprotected_legacy_tx(transaction) && !allow_unprotected_txs
+}
+
 impl<B, C, P, CT, BE, CIDP, EC> Eth<B, C, P, CT, BE, CIDP, EC>
 where
 	B: BlockT,
@@ -48,6 +62,21 @@ where
 	CT: ConvertTransaction<<B as BlockT>::Extrinsic> + 'static,
 	CIDP: CreateInherentDataProviders<B, ()> + Send + 'static,
 {
+	fn ensure_unprotected_legacy_tx_allowed(
+		&self,
+		transaction: &ethereum::TransactionV3,
+	) -> RpcResult<()> {
+		if should_reject_unprotected_legacy_tx(transaction, self.rpc_allow_unprotected_txs) {
+			// Runtime validation remains authoritative since peers can still gossip transactions
+			// over p2p and bypass RPC submission.
+			return Err(internal_err(
+				"unprotected legacy transactions are not allowed by RPC policy",
+			));
+		}
+
+		Ok(())
+	}
+
 	pub async fn send_transaction(&self, request: TransactionRequest) -> RpcResult<H256> {
 		let from = match request.from {
 			Some(from) => from,
@@ -143,6 +172,7 @@ where
 			Some(transaction) => transaction,
 			None => return Err(internal_err("no signer available")),
 		};
+		self.ensure_unprotected_legacy_tx_allowed(&transaction)?;
 		let transaction_hash = transaction.hash();
 
 		let extrinsic = self.convert_transaction(block_hash, transaction)?;
@@ -165,6 +195,7 @@ where
 				Ok(transaction) => transaction,
 				Err(_) => return Err(internal_err("decode transaction failed")),
 			};
+		self.ensure_unprotected_legacy_tx_allowed(&transaction)?;
 		let transaction_hash = transaction.hash();
 
 		let block_hash = self.client.info().best_hash;
@@ -290,3 +321,78 @@ where
 		}
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::{is_unprotected_legacy_tx, should_reject_unprotected_legacy_tx};
+	use ethereum::{
+		legacy::TransactionSignature as LegacyTransactionSignature, LegacyTransaction,
+		TransactionAction, TransactionV3,
+	};
+	use rlp::RlpStream;
+	use sp_core::{hashing::keccak_256, H160, H256, U256};
+
+	fn legacy_tx_with_v(v: u64) -> TransactionV3 {
+		let nonce = U256::zero();
+		let gas_price = U256::from(1u8);
+		let gas_limit = U256::from(21_000u64);
+		let action = TransactionAction::Call(H160::default());
+		let value = U256::zero();
+		let input = Vec::new();
+
+		let mut stream = RlpStream::new_list(6);
+		stream.append(&nonce);
+		stream.append(&gas_price);
+		stream.append(&gas_limit);
+		stream.append(&action);
+		stream.append(&value);
+		stream.append(&input);
+		let hash = H256::from(keccak_256(&stream.out()));
+		let msg = libsecp256k1::Message::parse(hash.as_fixed_bytes());
+		let secret = libsecp256k1::SecretKey::parse_slice(&[1u8; 32]).expect("valid secret key");
+		let (signature, _) = libsecp256k1::sign(&msg, &secret);
+		let rs = signature.serialize();
+
+		TransactionV3::Legacy(LegacyTransaction {
+			nonce,
+			gas_price,
+			gas_limit,
+			action,
+			value,
+			input,
+			signature: LegacyTransactionSignature::new(
+				v,
+				H256::from_slice(&rs[0..32]),
+				H256::from_slice(&rs[32..64]),
+			)
+			.expect("valid legacy signature"),
+		})
+	}
+
+	#[test]
+	fn detects_unprotected_legacy_transaction() {
+		let tx = legacy_tx_with_v(27);
+
+		assert!(is_unprotected_legacy_tx(&tx));
+	}
+
+	#[test]
+	fn ignores_protected_legacy_transaction() {
+		let tx = legacy_tx_with_v(37);
+
+		assert!(!is_unprotected_legacy_tx(&tx));
+	}
+
+	#[test]
+	fn reject_decision_respects_policy() {
+		let unprotected_legacy = legacy_tx_with_v(27);
+		assert!(should_reject_unprotected_legacy_tx(
+			&unprotected_legacy,
+			false
+		));
+		assert!(!should_reject_unprotected_legacy_tx(
+			&unprotected_legacy,
+			true
+		));
+	}
+}

frame/ethereum/src/lib.rs

@@ -209,6 +209,8 @@ pub mod pallet {
 		type PostLogContent: Get<PostLogContent>;
 		/// The maximum length of the extra data in the Executed event.
 		type ExtraDataLength: Get<u32>;
+		/// Whether transactional ethereum calls accept legacy transactions without EIP-155 chain id.
+		type AllowUnprotectedTxs: Get<bool>;
 	}
 
 	pub mod config_preludes {
@@ -225,13 +227,15 @@ pub mod pallet {
 
 		parameter_types! {
 			pub const PostBlockAndTxnHashes: PostLogContent = PostLogContent::BlockAndTxnHashes;
+			pub const AllowUnprotectedTxs: bool = false;
 		}
 
 		#[register_default_impl(TestDefaultConfig)]
 		impl DefaultConfig for TestDefaultConfig {
 			type StateRoot = IntermediateStateRoot<Self::Version>;
 			type PostLogContent = PostBlockAndTxnHashes;
 			type ExtraDataLength = ConstU32<30>;
+			type AllowUnprotectedTxs = AllowUnprotectedTxs;
 		}
 	}
 
@@ -561,6 +565,7 @@ impl<T: Config> Pallet<T> {
 				base_fee,
 				chain_id: T::ChainId::get(),
 				is_transactional: true,
+				allow_unprotected_txs: T::AllowUnprotectedTxs::get(),
 			},
 			transaction_data.clone().into(),
 			weight_limit,
@@ -1010,6 +1015,7 @@ impl<T: Config> Pallet<T> {
 				base_fee,
 				chain_id: T::ChainId::get(),
 				is_transactional: true,
+				allow_unprotected_txs: T::AllowUnprotectedTxs::get(),
 			},
 			transaction_data.into(),
 			weight_limit,

frame/ethereum/src/mock.rs

@@ -90,6 +90,7 @@ impl FindAuthor<H160> for FindAuthorTruncated {
 parameter_types! {
 	pub const TransactionByteFee: u64 = 1;
 	pub const GasLimitStorageGrowthRatio: u64 = 0;
+	pub static AllowUnprotectedTxs: bool = false;
 	// Alice is allowed to create contracts via CREATE and CALL(CREATE)
 	pub AllowedAddressesCreate: Vec<H160> = vec![H160::from_str("0x1a642f0e3c3af545e7acbd38b07251b3990914f1").expect("alice address")];
 	pub AllowedAddressesCreateInner: Vec<H160> = vec![H160::from_str("0x1a642f0e3c3af545e7acbd38b07251b3990914f1").expect("alice address")];
@@ -111,7 +112,9 @@ impl pallet_evm::Config for Test {
 }
 
 #[derive_impl(crate::config_preludes::TestDefaultConfig)]
-impl Config for Test {}
+impl Config for Test {
+	type AllowUnprotectedTxs = AllowUnprotectedTxs;
+}
 
 impl fp_self_contained::SelfContainedCall for RuntimeCall {
 	type SignedInfo = H160;
@@ -285,6 +288,17 @@ impl LegacyUnsignedTransaction {
 		H256::from(keccak_256(&stream.out()))
 	}
 
+	fn unprotected_signing_hash(&self) -> H256 {
+		let mut stream = RlpStream::new_list(6);
+		stream.append(&self.nonce);
+		stream.append(&self.gas_price);
+		stream.append(&self.gas_limit);
+		stream.append(&self.action);
+		stream.append(&self.value);
+		stream.append(&self.input);
+		H256::from(keccak_256(&stream.out()))
+	}
+
 	pub fn sign(&self, key: &H256) -> Transaction {
 		self.sign_with_chain_id(key, ChainId::get())
 	}
@@ -315,6 +329,33 @@ impl LegacyUnsignedTransaction {
 			signature: sig,
 		})
 	}
+
+	pub fn sign_without_chain_id(&self, key: &H256) -> Transaction {
+		let hash = self.unprotected_signing_hash();
+		let msg = libsecp256k1::Message::parse(hash.as_fixed_bytes());
+		let s = libsecp256k1::sign(
+			&msg,
+			&libsecp256k1::SecretKey::parse_slice(&key[..]).unwrap(),
+		);
+		let sig = s.0.serialize();
+
+		let sig = LegacyTransactionSignature::new(
+			s.1.serialize() as u64 % 2 + 27,
+			H256::from_slice(&sig[0..32]),
+			H256::from_slice(&sig[32..64]),
+		)
+		.unwrap();
+
+		Transaction::Legacy(ethereum::LegacyTransaction {
+			nonce: self.nonce,
+			gas_price: self.gas_price,
+			gas_limit: self.gas_limit,
+			action: self.action,
+			value: self.value,
+			input: self.input.clone(),
+			signature: sig,
+		})
+	}
 }
 
 pub struct EIP2930UnsignedTransaction {

frame/ethereum/src/tests/legacy.rs

@@ -209,6 +209,54 @@ fn transaction_with_invalid_chain_id_should_fail_in_block() {
 	});
 }
 
+#[test]
+fn unprotected_transaction_should_fail_when_not_allowed() {
+	let (pairs, mut ext) = new_test_ext(1);
+	let alice = &pairs[0];
+
+	ext.execute_with(|| {
+		AllowUnprotectedTxs::set(false);
+		let transaction =
+			legacy_erc20_creation_unsigned_transaction().sign_without_chain_id(&alice.private_key);
+
+		let call = crate::Call::<Test>::transact { transaction };
+		let source = call.check_self_contained().unwrap().unwrap();
+		let extrinsic = CheckedExtrinsic::<_, _, SignedExtra, _> {
+			signed: fp_self_contained::CheckedSignature::SelfContained(source),
+			function: RuntimeCall::Ethereum(call),
+		};
+		let dispatch_info = extrinsic.get_dispatch_info();
+		assert_err!(
+			extrinsic.apply::<Test>(&dispatch_info, 0),
+			TransactionValidityError::Invalid(InvalidTransaction::Custom(
+				fp_evm::TransactionValidationError::InvalidChainId as u8,
+			))
+		);
+	});
+}
+
+#[test]
+fn unprotected_transaction_should_succeed_when_allowed() {
+	let (pairs, mut ext) = new_test_ext(1);
+	let alice = &pairs[0];
+
+	ext.execute_with(|| {
+		AllowUnprotectedTxs::set(true);
+		let transaction =
+			legacy_erc20_creation_unsigned_transaction().sign_without_chain_id(&alice.private_key);
+
+		let call = crate::Call::<Test>::transact { transaction };
+		let source = call.check_self_contained().unwrap().unwrap();
+		let extrinsic = CheckedExtrinsic::<_, _, SignedExtra, _> {
+			signed: fp_self_contained::CheckedSignature::SelfContained(source),
+			function: RuntimeCall::Ethereum(call),
+		};
+		let dispatch_info = extrinsic.get_dispatch_info();
+		assert_ok!(extrinsic.apply::<Test>(&dispatch_info, 0));
+		AllowUnprotectedTxs::set(false);
+	});
+}
+
 #[test]
 fn contract_constructor_should_get_executed() {
 	let (pairs, mut ext) = new_test_ext(1);

frame/evm/src/runner/stack.rs

@@ -490,6 +490,7 @@ where
 				base_fee,
 				chain_id: T::ChainId::get(),
 				is_transactional,
+				allow_unprotected_txs: true,
 			},
 			fp_evm::CheckEvmTransactionInput {
 				chain_id: Some(T::ChainId::get()),