From 4d7f85053321831252da0a4fc0d35104c5d258a5 Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Mon, 26 Jan 2026 19:39:26 +0200 Subject: [PATCH 1/7] feat: :sparkles: add transaction size validation to RPC endpoints --- Cargo.lock | 3 +- Cargo.toml | 4 + client/rpc-core/src/types/mod.rs | 4 +- .../rpc-core/src/types/transaction_request.rs | 221 ++++++++++++++++++ client/rpc/src/eth/submit.rs | 23 +- 5 files changed, 251 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index af1e2c5bf5..b1582e0e6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2862,8 +2862,7 @@ dependencies = [ [[package]] name = "ethereum" version = "0.18.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3ee371ebb7479ed3258617557ab0b3247e741075cb6b02b820d188f68da44441" +source = "git+https://github.com/rust-ethereum/ethereum.git?rev=5185fe8efa62add7e989660a69c6945c8a3ae64e#5185fe8efa62add7e989660a69c6945c8a3ae64e" dependencies = [ "bytes", "ethereum-types", diff --git a/Cargo.toml b/Cargo.toml index adf36ff37c..1436c87d82 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -228,6 +228,10 @@ precompile-utils = { path = "precompiles", default-features = false } # Frontier Template frontier-template-runtime = { path = "template/runtime", default-features = false } +# TODO: Remove this patch before merging once ethereum 0.19.0 is released +[patch.crates-io] +ethereum = { git = "https://github.com/rust-ethereum/ethereum.git", rev = "5185fe8efa62add7e989660a69c6945c8a3ae64e" } + [profile.release] # Substrate runtime requires unwinding. panic = "unwind" diff --git a/client/rpc-core/src/types/mod.rs b/client/rpc-core/src/types/mod.rs index f824cb1143..74981378f2 100644 --- a/client/rpc-core/src/types/mod.rs +++ b/client/rpc-core/src/types/mod.rs @@ -63,7 +63,9 @@ pub use self::{ Peers, PipProtocolInfo, SyncInfo, SyncStatus, TransactionStats, }, transaction::{LocalTransactionStatus, RichRawTransaction, Transaction}, - transaction_request::{TransactionMessage, TransactionRequest}, + transaction_request::{ + TransactionMessage, TransactionRequest, DEFAULT_MAX_TX_INPUT_BYTES, TX_SLOT_BYTE_SIZE, + }, work::Work, }; diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 4eea1cdb01..c5377143a4 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -25,6 +25,21 @@ use serde::{Deserialize, Deserializer}; use crate::types::Bytes; +/// The default byte size of a transaction slot (32 KiB). +/// +/// Reference: +/// - geth: (`txSlotSize`) +/// - reth: +pub const TX_SLOT_BYTE_SIZE: usize = 32 * 1024; + +/// The default maximum size a single transaction can have (128 KiB). +/// This is the RLP-encoded size of the signed transaction. +/// +/// Reference: +/// - geth: (`txMaxSize`) +/// - reth: +pub const DEFAULT_MAX_TX_INPUT_BYTES: usize = 4 * TX_SLOT_BYTE_SIZE; + /// Transaction request from the RPC. #[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize)] #[serde(rename_all = "camelCase")] @@ -174,6 +189,75 @@ impl TransactionRequest { fn chain_id_u64(&self) -> u64 { self.chain_id.map(|id| id.as_u64()).unwrap_or_default() } + + /// Calculates the RLP-encoded size of the signed transaction for DoS protection. + /// + /// This mirrors geth's `tx.Size()` and reth's `transaction.encoded_length()` which use + /// actual RLP encoding to determine transaction size. We convert the request to its + /// transaction message type and use the `encoded_len()` method from the ethereum crate. + /// + /// Reference: + /// - geth: (`tx.Size()`) + /// - reth: (`PoolTransaction::encoded_length()`) + /// - alloy: (`rlp_encoded_fields_length()`) + pub fn encoded_length(&self) -> usize { + // Convert to transaction message and use the ethereum crate's encoded_len() + let message: Option = self.clone().into(); + + match message { + Some(TransactionMessage::Legacy(msg)) => { + // Legacy: RLP([nonce, gasPrice, gasLimit, to, value, data, v, r, s]) + // v is variable (27/28 or chainId*2+35/36), r and s are 32 bytes each + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + Some(TransactionMessage::EIP2930(msg)) => { + // EIP-2930: 0x01 || RLP([chainId, nonce, gasPrice, gasLimit, to, value, data, accessList, yParity, r, s]) + 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + Some(TransactionMessage::EIP1559(msg)) => { + // EIP-1559: 0x02 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, yParity, r, s]) + 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + Some(TransactionMessage::EIP7702(msg)) => { + // EIP-7702: 0x04 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, authorizationList, yParity, r, s]) + 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + None => { + // Fallback for invalid/incomplete requests - use conservative estimate + // This shouldn't happen in normal operation as validation should catch it + Self::DEFAULT_FALLBACK_SIZE + } + } + } + + /// RLP overhead for signature fields (yParity + r + s) + /// - yParity: 1 byte (0x00 or 0x01 encoded as single byte) + /// - r: typically 33 bytes (0x80 + 32 bytes, or less if leading zeros) + /// - s: typically 33 bytes (0x80 + 32 bytes, or less if leading zeros) + const SIGNATURE_RLP_OVERHEAD: usize = 1 + 33 + 33; + + /// Fallback size for invalid requests that can't be converted to a message + const DEFAULT_FALLBACK_SIZE: usize = 256; + + /// Validates that the estimated signed transaction size is within limits. + /// + /// This prevents DoS attacks via oversized transactions before they enter the pool. + /// The limit matches geth's `txMaxSize` and reth's `DEFAULT_MAX_TX_INPUT_BYTES`. + /// + /// Reference: + /// - geth: (`ValidateTransaction`) + /// - reth: + pub fn validate_size(&self) -> Result<(), String> { + let size = self.encoded_length(); + + if size > DEFAULT_MAX_TX_INPUT_BYTES { + return Err(format!( + "oversized data: transaction size {} exceeds limit {}", + size, DEFAULT_MAX_TX_INPUT_BYTES + )); + } + Ok(()) + } } /// Additional data of the transaction. @@ -446,4 +530,141 @@ mod tests { } ); } + + #[test] + fn test_request_size_validation_large_access_list() { + use ethereum::AccessListItem; + use ethereum_types::{H160, H256}; + + // Create access list that exceeds 128KB (131,072 bytes) + // Each storage key RLP-encodes to ~33 bytes + // 4000 keys * 33 bytes = 132,000 bytes > 128KB + let storage_keys: Vec = (0..4000).map(|_| H256::default()).collect(); + let access_list = vec![AccessListItem { + address: H160::default(), + storage_keys, + }]; + let request = TransactionRequest { + access_list: Some(access_list), + ..Default::default() + }; + assert!(request.validate_size().is_err()); + } + + #[test] + fn test_request_size_validation_valid() { + use ethereum::AccessListItem; + use ethereum_types::{H160, H256}; + + // 100 storage keys is well under 128KB + let request = TransactionRequest { + access_list: Some(vec![AccessListItem { + address: H160::default(), + storage_keys: vec![H256::default(); 100], + }]), + ..Default::default() + }; + assert!(request.validate_size().is_ok()); + } + + #[test] + fn test_encoded_length_includes_signature_overhead() { + // A minimal EIP-1559 transaction should include signature overhead + // Default TransactionRequest converts to EIP-1559 (no gas_price, no access_list) + let request = TransactionRequest::default(); + let size = request.encoded_length(); + + // EIP-1559 message RLP: ~11 bytes for minimal fields (all zeros/empty) + // + 1 byte type prefix + 67 bytes signature overhead = ~79 bytes minimum + // The signature overhead (67 bytes) is the key verification + assert!( + size >= TransactionRequest::SIGNATURE_RLP_OVERHEAD, + "Size {} should be at least signature overhead {}", + size, + TransactionRequest::SIGNATURE_RLP_OVERHEAD + ); + + // Verify it's a reasonable size for a minimal transaction + assert!(size < 200, "Size {} should be reasonable for minimal tx", size); + } + + #[test] + fn test_encoded_length_typed_transaction_overhead() { + use ethereum::AccessListItem; + use ethereum_types::H160; + + // EIP-1559 transaction (has max_fee_per_gas) + let request = TransactionRequest { + max_fee_per_gas: Some(U256::from(1000)), + access_list: Some(vec![AccessListItem { + address: H160::default(), + storage_keys: vec![], + }]), + ..Default::default() + }; + let typed_size = request.encoded_length(); + + // Legacy transaction + let legacy_request = TransactionRequest { + gas_price: Some(U256::from(1000)), + ..Default::default() + }; + let legacy_size = legacy_request.encoded_length(); + + // Typed transaction should be larger due to: + // - Type byte (+1) + // - Chain ID (+9) + // - max_priority_fee_per_gas (+33) + // - Access list overhead + assert!( + typed_size > legacy_size, + "Typed tx {} should be larger than legacy {}", + typed_size, + legacy_size + ); + } + + #[test] + fn test_encoded_length_access_list_scaling() { + use ethereum::AccessListItem; + use ethereum_types::{H160, H256}; + + // Transaction with 10 storage keys + let request_10 = TransactionRequest { + access_list: Some(vec![AccessListItem { + address: H160::default(), + storage_keys: vec![H256::default(); 10], + }]), + ..Default::default() + }; + + // Transaction with 100 storage keys + let request_100 = TransactionRequest { + access_list: Some(vec![AccessListItem { + address: H160::default(), + storage_keys: vec![H256::default(); 100], + }]), + ..Default::default() + }; + + let size_10 = request_10.encoded_length(); + let size_100 = request_100.encoded_length(); + + // Size should scale roughly linearly with storage keys + // 90 additional keys * ~34 bytes each ≈ 3060 bytes difference + let diff = size_100 - size_10; + assert!( + diff > 2500 && diff < 4000, + "Size difference {} should be proportional to storage keys", + diff + ); + } + + #[test] + fn test_constants_match_geth_reth() { + // Verify our constants match geth/reth exactly + assert_eq!(TX_SLOT_BYTE_SIZE, 32 * 1024); // 32 KiB + assert_eq!(DEFAULT_MAX_TX_INPUT_BYTES, 128 * 1024); // 128 KiB + assert_eq!(DEFAULT_MAX_TX_INPUT_BYTES, 4 * TX_SLOT_BYTE_SIZE); + } } diff --git a/client/rpc/src/eth/submit.rs b/client/rpc/src/eth/submit.rs index 8e26a83a1a..7dc57ebb2a 100644 --- a/client/rpc/src/eth/submit.rs +++ b/client/rpc/src/eth/submit.rs @@ -29,7 +29,7 @@ use sp_core::H160; use sp_inherents::CreateInherentDataProviders; use sp_runtime::{traits::Block as BlockT, transaction_validity::TransactionSource}; // Frontier -use fc_rpc_core::types::*; +use fc_rpc_core::types::{TransactionRequest, DEFAULT_MAX_TX_INPUT_BYTES, *}; use fp_rpc::{ConvertTransaction, ConvertTransactionRuntimeApi, EthereumRuntimeRPCApi}; use crate::{ @@ -49,6 +49,10 @@ where CIDP: CreateInherentDataProviders + Send + 'static, { pub async fn send_transaction(&self, request: TransactionRequest) -> RpcResult { + request.validate_size().map_err(|msg| { + crate::err(jsonrpsee::types::error::INVALID_PARAMS_CODE, &msg, None) + })?; + let from = match request.from { Some(from) => from, None => { @@ -160,6 +164,23 @@ where return Err(internal_err("transaction data is empty")); } + // Validate transaction size to prevent DoS attacks. + // This matches geth/reth pool validation which rejects transactions > 128 KB. + // Reference: + // - geth: https://github.com/ethereum/go-ethereum/blob/master/core/txpool/validation.go + // - reth: https://github.com/paradigmxyz/reth/blob/main/crates/transaction-pool/src/validate/eth.rs#L342-L363 + if bytes.len() > DEFAULT_MAX_TX_INPUT_BYTES { + return Err(crate::err( + jsonrpsee::types::error::INVALID_PARAMS_CODE, + &format!( + "oversized data: transaction size {} exceeds limit {}", + bytes.len(), + DEFAULT_MAX_TX_INPUT_BYTES + ), + None, + )); + } + let transaction: ethereum::TransactionV3 = match ethereum::EnvelopedDecodable::decode(&bytes) { Ok(transaction) => transaction, From c242d68d4cdbe30fb34d51f36119d5523be47cac Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Mon, 26 Jan 2026 19:49:45 +0200 Subject: [PATCH 2/7] style: :art: fmt --- client/rpc-core/src/types/transaction_request.rs | 6 +++++- client/rpc/src/eth/submit.rs | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index c5377143a4..7a329ae8a1 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -585,7 +585,11 @@ mod tests { ); // Verify it's a reasonable size for a minimal transaction - assert!(size < 200, "Size {} should be reasonable for minimal tx", size); + assert!( + size < 200, + "Size {} should be reasonable for minimal tx", + size + ); } #[test] diff --git a/client/rpc/src/eth/submit.rs b/client/rpc/src/eth/submit.rs index 7dc57ebb2a..11690a0641 100644 --- a/client/rpc/src/eth/submit.rs +++ b/client/rpc/src/eth/submit.rs @@ -49,9 +49,9 @@ where CIDP: CreateInherentDataProviders + Send + 'static, { pub async fn send_transaction(&self, request: TransactionRequest) -> RpcResult { - request.validate_size().map_err(|msg| { - crate::err(jsonrpsee::types::error::INVALID_PARAMS_CODE, &msg, None) - })?; + request + .validate_size() + .map_err(|msg| crate::err(jsonrpsee::types::error::INVALID_PARAMS_CODE, &msg, None))?; let from = match request.from { Some(from) => from, From 3bc944ff9c2e0e9922a681798ae55827fc782e4b Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Tue, 27 Jan 2026 12:21:09 +0200 Subject: [PATCH 3/7] refactor: :rotating_light: lint --- .../rpc-core/src/types/transaction_request.rs | 17 ++++------------- client/rpc/src/eth/submit.rs | 10 +++------- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 7a329ae8a1..ccc6122919 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -243,17 +243,12 @@ impl TransactionRequest { /// /// This prevents DoS attacks via oversized transactions before they enter the pool. /// The limit matches geth's `txMaxSize` and reth's `DEFAULT_MAX_TX_INPUT_BYTES`. - /// - /// Reference: - /// - geth: (`ValidateTransaction`) - /// - reth: pub fn validate_size(&self) -> Result<(), String> { let size = self.encoded_length(); if size > DEFAULT_MAX_TX_INPUT_BYTES { return Err(format!( - "oversized data: transaction size {} exceeds limit {}", - size, DEFAULT_MAX_TX_INPUT_BYTES + "oversized data: transaction size {size} exceeds limit {DEFAULT_MAX_TX_INPUT_BYTES}" )); } Ok(()) @@ -587,8 +582,7 @@ mod tests { // Verify it's a reasonable size for a minimal transaction assert!( size < 200, - "Size {} should be reasonable for minimal tx", - size + "Size {size} should be reasonable for minimal tx" ); } @@ -622,9 +616,7 @@ mod tests { // - Access list overhead assert!( typed_size > legacy_size, - "Typed tx {} should be larger than legacy {}", - typed_size, - legacy_size + "Typed tx {typed_size} should be larger than legacy {legacy_size}" ); } @@ -659,8 +651,7 @@ mod tests { let diff = size_100 - size_10; assert!( diff > 2500 && diff < 4000, - "Size difference {} should be proportional to storage keys", - diff + "Size difference {diff} should be proportional to storage keys" ); } diff --git a/client/rpc/src/eth/submit.rs b/client/rpc/src/eth/submit.rs index 11690a0641..124c3b4b09 100644 --- a/client/rpc/src/eth/submit.rs +++ b/client/rpc/src/eth/submit.rs @@ -166,16 +166,12 @@ where // Validate transaction size to prevent DoS attacks. // This matches geth/reth pool validation which rejects transactions > 128 KB. - // Reference: - // - geth: https://github.com/ethereum/go-ethereum/blob/master/core/txpool/validation.go - // - reth: https://github.com/paradigmxyz/reth/blob/main/crates/transaction-pool/src/validate/eth.rs#L342-L363 if bytes.len() > DEFAULT_MAX_TX_INPUT_BYTES { return Err(crate::err( jsonrpsee::types::error::INVALID_PARAMS_CODE, - &format!( - "oversized data: transaction size {} exceeds limit {}", - bytes.len(), - DEFAULT_MAX_TX_INPUT_BYTES + format!( + "oversized data: transaction size {} exceeds limit {DEFAULT_MAX_TX_INPUT_BYTES}", + bytes.len() ), None, )); From fc974a5f6ccc370dada2e2d3e30add7d510cdd64 Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Tue, 27 Jan 2026 12:23:10 +0200 Subject: [PATCH 4/7] docs: :memo: remove links to geth/reth in comments --- client/rpc-core/src/types/transaction_request.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index ccc6122919..fdf151be1b 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -26,18 +26,10 @@ use serde::{Deserialize, Deserializer}; use crate::types::Bytes; /// The default byte size of a transaction slot (32 KiB). -/// -/// Reference: -/// - geth: (`txSlotSize`) -/// - reth: pub const TX_SLOT_BYTE_SIZE: usize = 32 * 1024; /// The default maximum size a single transaction can have (128 KiB). /// This is the RLP-encoded size of the signed transaction. -/// -/// Reference: -/// - geth: (`txMaxSize`) -/// - reth: pub const DEFAULT_MAX_TX_INPUT_BYTES: usize = 4 * TX_SLOT_BYTE_SIZE; /// Transaction request from the RPC. @@ -195,11 +187,6 @@ impl TransactionRequest { /// This mirrors geth's `tx.Size()` and reth's `transaction.encoded_length()` which use /// actual RLP encoding to determine transaction size. We convert the request to its /// transaction message type and use the `encoded_len()` method from the ethereum crate. - /// - /// Reference: - /// - geth: (`tx.Size()`) - /// - reth: (`PoolTransaction::encoded_length()`) - /// - alloy: (`rlp_encoded_fields_length()`) pub fn encoded_length(&self) -> usize { // Convert to transaction message and use the ethereum crate's encoded_len() let message: Option = self.clone().into(); From b5eef63f5a25357fad2f495ba6d78776e5dba2a8 Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Mon, 2 Feb 2026 17:19:33 +0200 Subject: [PATCH 5/7] refactor: :recycle: return error if the tx request can't be converted to a message type --- .../rpc-core/src/types/transaction_request.rs | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index fdf151be1b..23f0c158fc 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -187,7 +187,9 @@ impl TransactionRequest { /// This mirrors geth's `tx.Size()` and reth's `transaction.encoded_length()` which use /// actual RLP encoding to determine transaction size. We convert the request to its /// transaction message type and use the `encoded_len()` method from the ethereum crate. - pub fn encoded_length(&self) -> usize { + /// + /// Returns an error if the transaction request cannot be converted to a valid message type. + pub fn encoded_length(&self) -> Result { // Convert to transaction message and use the ethereum crate's encoded_len() let message: Option = self.clone().into(); @@ -195,25 +197,23 @@ impl TransactionRequest { Some(TransactionMessage::Legacy(msg)) => { // Legacy: RLP([nonce, gasPrice, gasLimit, to, value, data, v, r, s]) // v is variable (27/28 or chainId*2+35/36), r and s are 32 bytes each - msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + Ok(msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD) } Some(TransactionMessage::EIP2930(msg)) => { // EIP-2930: 0x01 || RLP([chainId, nonce, gasPrice, gasLimit, to, value, data, accessList, yParity, r, s]) - 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + Ok(1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD) } Some(TransactionMessage::EIP1559(msg)) => { // EIP-1559: 0x02 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, yParity, r, s]) - 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + Ok(1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD) } Some(TransactionMessage::EIP7702(msg)) => { // EIP-7702: 0x04 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, authorizationList, yParity, r, s]) - 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD - } - None => { - // Fallback for invalid/incomplete requests - use conservative estimate - // This shouldn't happen in normal operation as validation should catch it - Self::DEFAULT_FALLBACK_SIZE + Ok(1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD) } + None => Err( + "invalid transaction parameters: unable to determine transaction type".to_string(), + ), } } @@ -223,15 +223,12 @@ impl TransactionRequest { /// - s: typically 33 bytes (0x80 + 32 bytes, or less if leading zeros) const SIGNATURE_RLP_OVERHEAD: usize = 1 + 33 + 33; - /// Fallback size for invalid requests that can't be converted to a message - const DEFAULT_FALLBACK_SIZE: usize = 256; - /// Validates that the estimated signed transaction size is within limits. /// /// This prevents DoS attacks via oversized transactions before they enter the pool. /// The limit matches geth's `txMaxSize` and reth's `DEFAULT_MAX_TX_INPUT_BYTES`. pub fn validate_size(&self) -> Result<(), String> { - let size = self.encoded_length(); + let size = self.encoded_length()?; if size > DEFAULT_MAX_TX_INPUT_BYTES { return Err(format!( @@ -554,7 +551,7 @@ mod tests { // A minimal EIP-1559 transaction should include signature overhead // Default TransactionRequest converts to EIP-1559 (no gas_price, no access_list) let request = TransactionRequest::default(); - let size = request.encoded_length(); + let size = request.encoded_length().unwrap(); // EIP-1559 message RLP: ~11 bytes for minimal fields (all zeros/empty) // + 1 byte type prefix + 67 bytes signature overhead = ~79 bytes minimum @@ -587,14 +584,14 @@ mod tests { }]), ..Default::default() }; - let typed_size = request.encoded_length(); + let typed_size = request.encoded_length().unwrap(); // Legacy transaction let legacy_request = TransactionRequest { gas_price: Some(U256::from(1000)), ..Default::default() }; - let legacy_size = legacy_request.encoded_length(); + let legacy_size = legacy_request.encoded_length().unwrap(); // Typed transaction should be larger due to: // - Type byte (+1) @@ -630,8 +627,8 @@ mod tests { ..Default::default() }; - let size_10 = request_10.encoded_length(); - let size_100 = request_100.encoded_length(); + let size_10 = request_10.encoded_length().unwrap(); + let size_100 = request_100.encoded_length().unwrap(); // Size should scale roughly linearly with storage keys // 90 additional keys * ~34 bytes each ≈ 3060 bytes difference @@ -649,4 +646,17 @@ mod tests { assert_eq!(DEFAULT_MAX_TX_INPUT_BYTES, 128 * 1024); // 128 KiB assert_eq!(DEFAULT_MAX_TX_INPUT_BYTES, 4 * TX_SLOT_BYTE_SIZE); } + + #[test] + fn test_encoded_length_invalid_parameters() { + // Transaction with both max_fee_per_gas and gas_price set (invalid combination) + let request = TransactionRequest { + max_fee_per_gas: Some(U256::from(1000)), + gas_price: Some(U256::from(500)), + ..Default::default() + }; + + assert!(request.encoded_length().is_err()); + assert!(request.validate_size().is_err()); + } } From 1b2f4fca5186added5c4c7021c417daee72a6b6e Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Wed, 18 Mar 2026 17:00:56 +0200 Subject: [PATCH 6/7] fix(rpc): validate transaction size after message fields are populated Address review feedback: validate_size() was called on the raw TransactionRequest before nonce, gas limit, gas price, and chain ID were filled in. Near-limit transactions could pass validation then become oversized after field population. Move encoded_length(), validate_size(), and SIGNATURE_RLP_OVERHEAD from TransactionRequest to TransactionMessage. In send_transaction, validate the fully-populated TransactionMessage (after nonce, gas, fees, chain ID are set) right before signing. This ensures the 128 KiB limit is enforced on the actual final transaction size. TransactionRequest::encoded_length() is kept as a convenience method that delegates to TransactionMessage::encoded_length() via conversion. --- .../rpc-core/src/types/transaction_request.rs | 118 ++++++++++-------- client/rpc/src/eth/submit.rs | 10 +- 2 files changed, 75 insertions(+), 53 deletions(-) diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 23f0c158fc..300371dfa8 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -184,59 +184,24 @@ impl TransactionRequest { /// Calculates the RLP-encoded size of the signed transaction for DoS protection. /// - /// This mirrors geth's `tx.Size()` and reth's `transaction.encoded_length()` which use - /// actual RLP encoding to determine transaction size. We convert the request to its - /// transaction message type and use the `encoded_len()` method from the ethereum crate. + /// This converts the request to a `TransactionMessage` using default values for any + /// missing fields, then delegates to `TransactionMessage::encoded_length()`. + /// + /// **Note:** For `eth_sendTransaction`, prefer calling `TransactionMessage::validate_size()` + /// on the fully-populated message (after nonce, gas, fees, chain ID are filled in) + /// to get an accurate size measurement. /// /// Returns an error if the transaction request cannot be converted to a valid message type. pub fn encoded_length(&self) -> Result { - // Convert to transaction message and use the ethereum crate's encoded_len() let message: Option = self.clone().into(); match message { - Some(TransactionMessage::Legacy(msg)) => { - // Legacy: RLP([nonce, gasPrice, gasLimit, to, value, data, v, r, s]) - // v is variable (27/28 or chainId*2+35/36), r and s are 32 bytes each - Ok(msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD) - } - Some(TransactionMessage::EIP2930(msg)) => { - // EIP-2930: 0x01 || RLP([chainId, nonce, gasPrice, gasLimit, to, value, data, accessList, yParity, r, s]) - Ok(1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD) - } - Some(TransactionMessage::EIP1559(msg)) => { - // EIP-1559: 0x02 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, yParity, r, s]) - Ok(1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD) - } - Some(TransactionMessage::EIP7702(msg)) => { - // EIP-7702: 0x04 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, authorizationList, yParity, r, s]) - Ok(1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD) - } + Some(msg) => Ok(msg.encoded_length()), None => Err( "invalid transaction parameters: unable to determine transaction type".to_string(), ), } } - - /// RLP overhead for signature fields (yParity + r + s) - /// - yParity: 1 byte (0x00 or 0x01 encoded as single byte) - /// - r: typically 33 bytes (0x80 + 32 bytes, or less if leading zeros) - /// - s: typically 33 bytes (0x80 + 32 bytes, or less if leading zeros) - const SIGNATURE_RLP_OVERHEAD: usize = 1 + 33 + 33; - - /// Validates that the estimated signed transaction size is within limits. - /// - /// This prevents DoS attacks via oversized transactions before they enter the pool. - /// The limit matches geth's `txMaxSize` and reth's `DEFAULT_MAX_TX_INPUT_BYTES`. - pub fn validate_size(&self) -> Result<(), String> { - let size = self.encoded_length()?; - - if size > DEFAULT_MAX_TX_INPUT_BYTES { - return Err(format!( - "oversized data: transaction size {size} exceeds limit {DEFAULT_MAX_TX_INPUT_BYTES}" - )); - } - Ok(()) - } } /// Additional data of the transaction. @@ -301,6 +266,57 @@ pub enum TransactionMessage { EIP7702(EIP7702TransactionMessage), } +impl TransactionMessage { + /// RLP overhead for signature fields (yParity + r + s) + /// - yParity: 1 byte (0x00 or 0x01 encoded as single byte) + /// - r: typically 33 bytes (0x80 + 32 bytes, or less if leading zeros) + /// - s: typically 33 bytes (0x80 + 32 bytes, or less if leading zeros) + const SIGNATURE_RLP_OVERHEAD: usize = 1 + 33 + 33; + + /// Calculates the RLP-encoded size of the signed transaction for DoS protection. + /// + /// This mirrors geth's `tx.Size()` and reth's `transaction.encoded_length()` which use + /// actual RLP encoding to determine transaction size. We use the `encoded_len()` method + /// from the ethereum crate on the fully-populated message. + pub fn encoded_length(&self) -> usize { + match self { + // Legacy: RLP([nonce, gasPrice, gasLimit, to, value, data, v, r, s]) + TransactionMessage::Legacy(msg) => msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD, + // EIP-2930: 0x01 || RLP([chainId, nonce, gasPrice, gasLimit, to, value, data, accessList, yParity, r, s]) + TransactionMessage::EIP2930(msg) => { + 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + // EIP-1559: 0x02 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, yParity, r, s]) + TransactionMessage::EIP1559(msg) => { + 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + // EIP-7702: 0x04 || RLP([chainId, nonce, maxPriorityFeePerGas, maxFeePerGas, gasLimit, to, value, data, accessList, authorizationList, yParity, r, s]) + TransactionMessage::EIP7702(msg) => { + 1 + msg.encoded_len() + Self::SIGNATURE_RLP_OVERHEAD + } + } + } + + /// Validates that the estimated signed transaction size is within limits. + /// + /// This prevents DoS attacks via oversized transactions before they enter the pool. + /// The limit matches geth's `txMaxSize` and reth's `DEFAULT_MAX_TX_INPUT_BYTES`. + /// + /// This should be called on the fully-populated message (after nonce, gas limit, + /// gas price, chain ID, etc. have been filled in) to ensure the final transaction + /// size is accurately measured. + pub fn validate_size(&self) -> Result<(), String> { + let size = self.encoded_length(); + + if size > DEFAULT_MAX_TX_INPUT_BYTES { + return Err(format!( + "oversized data: transaction size {size} exceeds limit {DEFAULT_MAX_TX_INPUT_BYTES}" + )); + } + Ok(()) + } +} + impl From for Option { fn from(req: TransactionRequest) -> Self { // Common fields extraction - these are used by all transaction types @@ -511,7 +527,7 @@ mod tests { } #[test] - fn test_request_size_validation_large_access_list() { + fn test_message_size_validation_large_access_list() { use ethereum::AccessListItem; use ethereum_types::{H160, H256}; @@ -527,11 +543,12 @@ mod tests { access_list: Some(access_list), ..Default::default() }; - assert!(request.validate_size().is_err()); + let message: Option = request.into(); + assert!(message.unwrap().validate_size().is_err()); } #[test] - fn test_request_size_validation_valid() { + fn test_message_size_validation_valid() { use ethereum::AccessListItem; use ethereum_types::{H160, H256}; @@ -543,7 +560,8 @@ mod tests { }]), ..Default::default() }; - assert!(request.validate_size().is_ok()); + let message: Option = request.into(); + assert!(message.unwrap().validate_size().is_ok()); } #[test] @@ -557,10 +575,10 @@ mod tests { // + 1 byte type prefix + 67 bytes signature overhead = ~79 bytes minimum // The signature overhead (67 bytes) is the key verification assert!( - size >= TransactionRequest::SIGNATURE_RLP_OVERHEAD, + size >= TransactionMessage::SIGNATURE_RLP_OVERHEAD, "Size {} should be at least signature overhead {}", size, - TransactionRequest::SIGNATURE_RLP_OVERHEAD + TransactionMessage::SIGNATURE_RLP_OVERHEAD ); // Verify it's a reasonable size for a minimal transaction @@ -657,6 +675,8 @@ mod tests { }; assert!(request.encoded_length().is_err()); - assert!(request.validate_size().is_err()); + // Invalid parameters also fail conversion, so validate_size is unreachable + let message: Option = request.into(); + assert!(message.is_none()); } } diff --git a/client/rpc/src/eth/submit.rs b/client/rpc/src/eth/submit.rs index 124c3b4b09..9f35c15024 100644 --- a/client/rpc/src/eth/submit.rs +++ b/client/rpc/src/eth/submit.rs @@ -49,10 +49,6 @@ where CIDP: CreateInherentDataProviders + Send + 'static, { pub async fn send_transaction(&self, request: TransactionRequest) -> RpcResult { - request - .validate_size() - .map_err(|msg| crate::err(jsonrpsee::types::error::INVALID_PARAMS_CODE, &msg, None))?; - let from = match request.from { Some(from) => from, None => { @@ -132,6 +128,12 @@ where _ => return Err(internal_err("invalid transaction parameters")), }; + // Validate size on the fully-populated message (after nonce, gas, fees, chain ID + // have been filled in) to accurately enforce the 128 KiB limit. + message + .validate_size() + .map_err(|msg| crate::err(jsonrpsee::types::error::INVALID_PARAMS_CODE, &msg, None))?; + let mut transaction = None; for signer in &self.signers { if signer.accounts().contains(&from) { From a5c026fc7a30038937d0e2ea7353f5398185e521 Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Wed, 18 Mar 2026 17:10:52 +0200 Subject: [PATCH 7/7] style(rpc): use expect with 'should' convention in tests --- .../rpc-core/src/types/transaction_request.rs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/client/rpc-core/src/types/transaction_request.rs b/client/rpc-core/src/types/transaction_request.rs index 300371dfa8..e20f6748e1 100644 --- a/client/rpc-core/src/types/transaction_request.rs +++ b/client/rpc-core/src/types/transaction_request.rs @@ -569,7 +569,9 @@ mod tests { // A minimal EIP-1559 transaction should include signature overhead // Default TransactionRequest converts to EIP-1559 (no gas_price, no access_list) let request = TransactionRequest::default(); - let size = request.encoded_length().unwrap(); + let size = request + .encoded_length() + .expect("should encode valid EIP-1559 request"); // EIP-1559 message RLP: ~11 bytes for minimal fields (all zeros/empty) // + 1 byte type prefix + 67 bytes signature overhead = ~79 bytes minimum @@ -602,14 +604,18 @@ mod tests { }]), ..Default::default() }; - let typed_size = request.encoded_length().unwrap(); + let typed_size = request + .encoded_length() + .expect("should encode valid EIP-1559 request"); // Legacy transaction let legacy_request = TransactionRequest { gas_price: Some(U256::from(1000)), ..Default::default() }; - let legacy_size = legacy_request.encoded_length().unwrap(); + let legacy_size = legacy_request + .encoded_length() + .expect("should encode valid legacy request"); // Typed transaction should be larger due to: // - Type byte (+1) @@ -645,8 +651,12 @@ mod tests { ..Default::default() }; - let size_10 = request_10.encoded_length().unwrap(); - let size_100 = request_100.encoded_length().unwrap(); + let size_10 = request_10 + .encoded_length() + .expect("should encode valid EIP-1559 request with 10 keys"); + let size_100 = request_100 + .encoded_length() + .expect("should encode valid EIP-1559 request with 100 keys"); // Size should scale roughly linearly with storage keys // 90 additional keys * ~34 bytes each ≈ 3060 bytes difference