Skip to content

Commit cbb349e

Browse files
fmolettaxqft
authored andcommitted
fix(l1): rlp decode impls for low size transactions (#5139)
**Motivation** Transactions are encoded in the following format: * Legacy: rlp(Transaction) * Non Legacy: rlp(Payload) where Payload is a bytes object containing (TxType || rlp(Transaction)) When decoding in order to differentiate between a legacy and a non legacy transaction we check if the encoded data is a bytes object (`is_encoded_as_bytes`) by checking if the prefix is between 0xb8 and 0xbf. The problem with this is that when encoding the payload the 0xb8..0xbf prefix is only applied if the length of the payload is higher than 56 bytes. This is usually the case for most real-scenario transactions, but if the transaction payload were to be lower than 56 bytes then we would not detect it as a bytes object and would attempt and fail to decode the transaction as a legacy one. This PR fixes this problem by: * Changing the criteria for legacy vs non-legacy transactions to instead check if the incoming encoded data is encoded as a list (if so it will be legacy). Aka `is_encoded_as_bytes` -> `!is_encoded_as_list` * Considering the case where the encoded payload doesn't have the bytes prefix. Aka `get_rlp_bytes_item_payload` -> `decode_rlp_item`. The latter which considers the prefix RLP_NULL+size that is used when encoding less than 56 bytes <!-- Why does this pull request exist? What are its goals? --> **Description** * Fix decoding for `Transaction` & `P2PTransaction` for cases where tx payload encoding is less than 56 bytes <!-- A clear and concise general description of the changes this PR introduces --> <!-- Link to issues: Resolves #111, Resolves #222 --> Closes #issue_number
1 parent f157a78 commit cbb349e

File tree

2 files changed

+37
-29
lines changed

2 files changed

+37
-29
lines changed

crates/common/types/receipt.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,8 @@ impl RLPDecode for ReceiptWithBloom {
227227
/// A) Legacy receipts: rlp(receipt)
228228
/// B) Non legacy receipts: rlp(Bytes(tx_type | rlp(receipt))).
229229
fn decode_unfinished(rlp: &[u8]) -> Result<(Self, &[u8]), RLPDecodeError> {
230+
// The minimum size for a ReceiptWithBloom is > 256 bytes (due to the Bloom type field) meaning that it is safe
231+
// to check for bytes prefix to diferenticate between legacy receipts and non-legacy receipt payloads
230232
let (tx_type, rlp) = if is_encoded_as_bytes(rlp)? {
231233
let payload = get_rlp_bytes_item_payload(rlp)?;
232234
let tx_type = match payload.first().ok_or(RLPDecodeError::InvalidLength)? {

crates/common/types/transaction.rs

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use sha3::{Digest, Keccak256};
1212

1313
use ethrex_rlp::{
1414
constants::RLP_NULL,
15-
decode::{RLPDecode, get_rlp_bytes_item_payload, is_encoded_as_bytes},
15+
decode::{RLPDecode, decode_rlp_item},
1616
encode::{PayloadRLPEncode, RLPEncode},
1717
error::RLPDecodeError,
1818
structs::{Decoder, Encoder},
@@ -77,28 +77,27 @@ impl RLPEncode for P2PTransaction {
7777

7878
impl RLPDecode for P2PTransaction {
7979
fn decode_unfinished(rlp: &[u8]) -> Result<(Self, &[u8]), RLPDecodeError> {
80-
if is_encoded_as_bytes(rlp)? {
81-
// Adjust the encoding to get the payload
82-
let payload = get_rlp_bytes_item_payload(rlp)?;
80+
let (is_list, payload, remainder) = decode_rlp_item(rlp)?;
81+
if !is_list {
8382
let tx_type = payload.first().ok_or(RLPDecodeError::InvalidLength)?;
8483
let tx_encoding = &payload.get(1..).ok_or(RLPDecodeError::InvalidLength)?;
8584
// Look at the first byte to check if it corresponds to a TransactionType
8685
match *tx_type {
8786
// Legacy
88-
0x0 => LegacyTransaction::decode_unfinished(tx_encoding)
89-
.map(|(tx, rem)| (P2PTransaction::LegacyTransaction(tx), rem)), // TODO: check if this is a real case scenario
87+
0x0 => LegacyTransaction::decode(tx_encoding)
88+
.map(|tx| (P2PTransaction::LegacyTransaction(tx), remainder)), // TODO: check if this is a real case scenario
9089
// EIP2930
91-
0x1 => EIP2930Transaction::decode_unfinished(tx_encoding)
92-
.map(|(tx, rem)| (P2PTransaction::EIP2930Transaction(tx), rem)),
90+
0x1 => EIP2930Transaction::decode(tx_encoding)
91+
.map(|tx| (P2PTransaction::EIP2930Transaction(tx), remainder)),
9392
// EIP1559
94-
0x2 => EIP1559Transaction::decode_unfinished(tx_encoding)
95-
.map(|(tx, rem)| (P2PTransaction::EIP1559Transaction(tx), rem)),
93+
0x2 => EIP1559Transaction::decode(tx_encoding)
94+
.map(|tx| (P2PTransaction::EIP1559Transaction(tx), remainder)),
9695
// EIP4844
97-
0x3 => WrappedEIP4844Transaction::decode_unfinished(tx_encoding)
98-
.map(|(tx, rem)| (P2PTransaction::EIP4844TransactionWithBlobs(tx), rem)),
96+
0x3 => WrappedEIP4844Transaction::decode(tx_encoding)
97+
.map(|tx| (P2PTransaction::EIP4844TransactionWithBlobs(tx), remainder)),
9998
// EIP7702
100-
0x4 => EIP7702Transaction::decode_unfinished(tx_encoding)
101-
.map(|(tx, rem)| (P2PTransaction::EIP7702Transaction(tx), rem)),
99+
0x4 => EIP7702Transaction::decode(tx_encoding)
100+
.map(|tx| (P2PTransaction::EIP7702Transaction(tx), remainder)),
102101
ty => Err(RLPDecodeError::Custom(format!(
103102
"Invalid transaction type: {ty}"
104103
))),
@@ -406,31 +405,30 @@ impl RLPDecode for Transaction {
406405
/// B) Non legacy transactions: rlp(Bytes) where Bytes represents the canonical encoding for the transaction as a bytes object.
407406
/// Checkout [Transaction::decode_canonical] for more information
408407
fn decode_unfinished(rlp: &[u8]) -> Result<(Self, &[u8]), RLPDecodeError> {
409-
if is_encoded_as_bytes(rlp)? {
410-
// Adjust the encoding to get the payload
411-
let payload = get_rlp_bytes_item_payload(rlp)?;
408+
let (is_list, payload, remainder) = decode_rlp_item(rlp)?;
409+
if !is_list {
412410
let tx_type = payload.first().ok_or(RLPDecodeError::InvalidLength)?;
413411
let tx_encoding = &payload.get(1..).ok_or(RLPDecodeError::InvalidLength)?;
414412
// Look at the first byte to check if it corresponds to a TransactionType
415413
match *tx_type {
416414
// Legacy
417-
0x0 => LegacyTransaction::decode_unfinished(tx_encoding)
418-
.map(|(tx, rem)| (Transaction::LegacyTransaction(tx), rem)), // TODO: check if this is a real case scenario
415+
0x0 => LegacyTransaction::decode(tx_encoding)
416+
.map(|tx| (Transaction::LegacyTransaction(tx), remainder)), // TODO: check if this is a real case scenario
419417
// EIP2930
420-
0x1 => EIP2930Transaction::decode_unfinished(tx_encoding)
421-
.map(|(tx, rem)| (Transaction::EIP2930Transaction(tx), rem)),
418+
0x1 => EIP2930Transaction::decode(tx_encoding)
419+
.map(|tx| (Transaction::EIP2930Transaction(tx), remainder)),
422420
// EIP1559
423-
0x2 => EIP1559Transaction::decode_unfinished(tx_encoding)
424-
.map(|(tx, rem)| (Transaction::EIP1559Transaction(tx), rem)),
421+
0x2 => EIP1559Transaction::decode(tx_encoding)
422+
.map(|tx| (Transaction::EIP1559Transaction(tx), remainder)),
425423
// EIP4844
426-
0x3 => EIP4844Transaction::decode_unfinished(tx_encoding)
427-
.map(|(tx, rem)| (Transaction::EIP4844Transaction(tx), rem)),
424+
0x3 => EIP4844Transaction::decode(tx_encoding)
425+
.map(|tx| (Transaction::EIP4844Transaction(tx), remainder)),
428426
// EIP7702
429-
0x4 => EIP7702Transaction::decode_unfinished(tx_encoding)
430-
.map(|(tx, rem)| (Transaction::EIP7702Transaction(tx), rem)),
427+
0x4 => EIP7702Transaction::decode(tx_encoding)
428+
.map(|tx| (Transaction::EIP7702Transaction(tx), remainder)),
431429
// PrivilegedL2
432-
0x7e => PrivilegedL2Transaction::decode_unfinished(tx_encoding)
433-
.map(|(tx, rem)| (Transaction::PrivilegedL2Transaction(tx), rem)),
430+
0x7e => PrivilegedL2Transaction::decode(tx_encoding)
431+
.map(|tx| (Transaction::PrivilegedL2Transaction(tx), remainder)),
434432
ty => Err(RLPDecodeError::Custom(format!(
435433
"Invalid transaction type: {ty}"
436434
))),
@@ -3131,4 +3129,12 @@ mod tests {
31313129
assert_eq!(generic_tx.access_list[0].address, access_list[0].0);
31323130
assert_eq!(generic_tx.access_list[0].storage_keys, access_list[0].1);
31333131
}
3132+
3133+
#[test]
3134+
fn encode_decode_low_size_tx() {
3135+
let tx = Transaction::EIP2930Transaction(EIP2930Transaction::default());
3136+
let encoded = tx.encode_to_vec();
3137+
let decoded_tx = Transaction::decode(&encoded).unwrap();
3138+
assert_eq!(tx, decoded_tx);
3139+
}
31343140
}

0 commit comments

Comments
 (0)