From a212e1b36f199429ff541a7e7d3d6a98b6022164 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 13 Dec 2024 01:27:12 +0400 Subject: [PATCH] chore: remove the workaround for pre-bedrock OP transactions (#13365) --- .../primitives/src/transaction/signed.rs | 15 +-- crates/primitives/src/transaction/mod.rs | 100 +++++------------- crates/primitives/src/transaction/pooled.rs | 10 +- .../primitives/src/transaction/signature.rs | 26 ----- 4 files changed, 34 insertions(+), 117 deletions(-) diff --git a/crates/optimism/primitives/src/transaction/signed.rs b/crates/optimism/primitives/src/transaction/signed.rs index 0d4c20447c..c9b13bd0d7 100644 --- a/crates/optimism/primitives/src/transaction/signed.rs +++ b/crates/optimism/primitives/src/transaction/signed.rs @@ -4,7 +4,7 @@ use crate::OpTxType; use alloc::vec::Vec; use alloy_consensus::{ transaction::RlpEcdsaTx, SignableTransaction, Transaction, TxEip1559, TxEip2930, TxEip7702, - Typed2718, + TxLegacy, Typed2718, }; use alloy_eips::{ eip2718::{Decodable2718, Eip2718Error, Eip2718Result, Encodable2718}, @@ -25,10 +25,7 @@ use once_cell::sync::OnceCell as OnceLock; use op_alloy_consensus::{OpTypedTransaction, TxDeposit}; #[cfg(any(test, feature = "reth-codec"))] use proptest as _; -use reth_primitives::{ - transaction::{recover_signer, recover_signer_unchecked}, - TransactionSigned, -}; +use reth_primitives::transaction::{recover_signer, recover_signer_unchecked}; use reth_primitives_traits::{FillTxEnv, InMemorySize, SignedTransaction}; use revm_primitives::{AuthorizationList, OptimismFields, TxEnv}; #[cfg(feature = "std")] @@ -63,7 +60,7 @@ impl OpTransactionSigned { /// Creates a new signed transaction from the given transaction and signature without the hash. /// - /// Note: this only calculates the hash on the first [`TransactionSigned::hash`] call. + /// Note: this only calculates the hash on the first [`OpTransactionSigned::hash`] call. pub fn new_unhashed(transaction: OpTypedTransaction, signature: Signature) -> Self { Self { hash: Default::default(), signature, transaction } } @@ -222,7 +219,6 @@ impl InMemorySize for OpTransactionSigned { } impl alloy_rlp::Encodable for OpTransactionSigned { - /// See [`alloy_rlp::Encodable`] impl for [`TransactionSigned`]. fn encode(&self, out: &mut dyn alloy_rlp::bytes::BufMut) { self.network_encode(out); } @@ -238,7 +234,6 @@ impl alloy_rlp::Encodable for OpTransactionSigned { } impl alloy_rlp::Decodable for OpTransactionSigned { - /// See [`alloy_rlp::Decodable`] impl for [`TransactionSigned`]. fn decode(buf: &mut &[u8]) -> alloy_rlp::Result { Self::network_decode(buf).map_err(Into::into) } @@ -321,10 +316,8 @@ impl Decodable2718 for OpTransactionSigned { } fn fallback_decode(buf: &mut &[u8]) -> Eip2718Result { - let (transaction, hash, signature) = - TransactionSigned::decode_rlp_legacy_transaction_tuple(buf)?; + let (transaction, signature) = TxLegacy::rlp_decode_with_signature(buf)?; let signed_tx = Self::new_unhashed(OpTypedTransaction::Legacy(transaction), signature); - signed_tx.hash.get_or_init(|| hash); Ok(signed_tx) } diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index 4431d1f9cd..a23ecbc716 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -27,7 +27,6 @@ use rayon::prelude::{IntoParallelIterator, ParallelIterator}; use reth_primitives_traits::{InMemorySize, SignedTransaction}; use revm_primitives::{AuthorizationList, TxEnv}; use serde::{Deserialize, Serialize}; -use signature::decode_with_eip155_chain_id; #[cfg(feature = "std")] use std::sync::{LazyLock, OnceLock}; @@ -966,68 +965,6 @@ impl TransactionSigned { let hash = self.hash(); (self.transaction, self.signature, hash) } - - /// Decodes legacy transaction from the data buffer into a tuple. - /// - /// This expects `rlp(legacy_tx)` - /// - /// Refer to the docs for [`Self::decode_rlp_legacy_transaction`] for details on the exact - /// format expected. - pub fn decode_rlp_legacy_transaction_tuple( - data: &mut &[u8], - ) -> alloy_rlp::Result<(TxLegacy, TxHash, Signature)> { - // keep this around, so we can use it to calculate the hash - let original_encoding = *data; - - let header = Header::decode(data)?; - let remaining_len = data.len(); - - let transaction_payload_len = header.payload_length; - - if transaction_payload_len > remaining_len { - return Err(RlpError::InputTooShort) - } - - let mut transaction = TxLegacy { - nonce: Decodable::decode(data)?, - gas_price: Decodable::decode(data)?, - gas_limit: Decodable::decode(data)?, - to: Decodable::decode(data)?, - value: Decodable::decode(data)?, - input: Decodable::decode(data)?, - chain_id: None, - }; - let (signature, extracted_id) = decode_with_eip155_chain_id(data)?; - transaction.chain_id = extracted_id; - - // check the new length, compared to the original length and the header length - let decoded = remaining_len - data.len(); - if decoded != transaction_payload_len { - return Err(RlpError::UnexpectedLength) - } - - let tx_length = header.payload_length + header.length(); - let hash = keccak256(&original_encoding[..tx_length]); - Ok((transaction, hash, signature)) - } - - /// Decodes legacy transaction from the data buffer. - /// - /// This should be used _only_ be used in general transaction decoding methods, which have - /// already ensured that the input is a legacy transaction with the following format: - /// `rlp(legacy_tx)` - /// - /// Legacy transactions are encoded as lists, so the input should start with a RLP list header. - /// - /// This expects `rlp(legacy_tx)` - // TODO: make buf advancement semantics consistent with `decode_enveloped_typed_transaction`, - // so decoding methods do not need to manually advance the buffer - pub fn decode_rlp_legacy_transaction(data: &mut &[u8]) -> alloy_rlp::Result { - let (transaction, hash, signature) = Self::decode_rlp_legacy_transaction_tuple(data)?; - let signed = - Self { transaction: Transaction::Legacy(transaction), hash: hash.into(), signature }; - Ok(signed) - } } impl SignedTransaction for TransactionSigned { @@ -1322,20 +1259,36 @@ impl Decodable2718 for TransactionSigned { match ty.try_into().map_err(|_| Eip2718Error::UnexpectedType(ty))? { TxType::Legacy => Err(Eip2718Error::UnexpectedType(0)), TxType::Eip2930 => { - let (tx, signature, hash) = TxEip2930::rlp_decode_signed(buf)?.into_parts(); - Ok(Self { transaction: Transaction::Eip2930(tx), signature, hash: hash.into() }) + let (tx, signature) = TxEip2930::rlp_decode_with_signature(buf)?; + Ok(Self { + transaction: Transaction::Eip2930(tx), + signature, + hash: Default::default(), + }) } TxType::Eip1559 => { - let (tx, signature, hash) = TxEip1559::rlp_decode_signed(buf)?.into_parts(); - Ok(Self { transaction: Transaction::Eip1559(tx), signature, hash: hash.into() }) + let (tx, signature) = TxEip1559::rlp_decode_with_signature(buf)?; + Ok(Self { + transaction: Transaction::Eip1559(tx), + signature, + hash: Default::default(), + }) } TxType::Eip7702 => { - let (tx, signature, hash) = TxEip7702::rlp_decode_signed(buf)?.into_parts(); - Ok(Self { transaction: Transaction::Eip7702(tx), signature, hash: hash.into() }) + let (tx, signature) = TxEip7702::rlp_decode_with_signature(buf)?; + Ok(Self { + transaction: Transaction::Eip7702(tx), + signature, + hash: Default::default(), + }) } TxType::Eip4844 => { - let (tx, signature, hash) = TxEip4844::rlp_decode_signed(buf)?.into_parts(); - Ok(Self { transaction: Transaction::Eip4844(tx), signature, hash: hash.into() }) + let (tx, signature) = TxEip4844::rlp_decode_with_signature(buf)?; + Ok(Self { + transaction: Transaction::Eip4844(tx), + signature, + hash: Default::default(), + }) } #[cfg(feature = "optimism")] TxType::Deposit => Ok(Self::new_unhashed( @@ -1346,7 +1299,8 @@ impl Decodable2718 for TransactionSigned { } fn fallback_decode(buf: &mut &[u8]) -> Eip2718Result { - Ok(Self::decode_rlp_legacy_transaction(buf)?) + let (tx, signature) = TxLegacy::rlp_decode_with_signature(buf)?; + Ok(Self { transaction: Transaction::Legacy(tx), signature, hash: Default::default() }) } } @@ -2177,7 +2131,7 @@ mod tests { #[test] fn recover_legacy_singer() { let data = hex!("f9015482078b8505d21dba0083022ef1947a250d5630b4cf539739df2c5dacb4c659f2488d880c46549a521b13d8b8e47ff36ab50000000000000000000000000000000000000000000066ab5a608bd00a23f2fe000000000000000000000000000000000000000000000000000000000000008000000000000000000000000048c04ed5691981c42154c6167398f95e8f38a7ff00000000000000000000000000000000000000000000000000000000632ceac70000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc20000000000000000000000006c6ee5e31d828de241282b9606c8e98ea48526e225a0c9077369501641a92ef7399ff81c21639ed4fd8fc69cb793cfa1dbfab342e10aa0615facb2f1bcf3274a354cfe384a38d0cc008a11c2dd23a69111bc6930ba27a8"); - let tx = TransactionSigned::decode_rlp_legacy_transaction(&mut data.as_slice()).unwrap(); + let tx = TransactionSigned::fallback_decode(&mut data.as_slice()).unwrap(); assert!(tx.is_legacy()); let sender = tx.recover_signer().unwrap(); assert_eq!(sender, address!("a12e1462d0ceD572f396F58B6E2D03894cD7C8a4")); diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index ffcffef4f6..7031f76f4b 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -8,7 +8,7 @@ use crate::{BlobTransaction, RecoveredTx, Transaction, TransactionSigned}; use alloc::vec::Vec; use alloy_consensus::{ constants::EIP4844_TX_TYPE_ID, - transaction::{TxEip1559, TxEip2930, TxEip4844, TxLegacy}, + transaction::{RlpEcdsaTx, TxEip1559, TxEip2930, TxEip4844, TxLegacy}, SignableTransaction, Signed, TxEip4844WithSidecar, Typed2718, }; use alloy_eips::{ @@ -374,11 +374,7 @@ impl Decodable2718 for PooledTransactionsElement { } fn fallback_decode(buf: &mut &[u8]) -> Eip2718Result { - // decode as legacy transaction - let (transaction, hash, signature) = - TransactionSigned::decode_rlp_legacy_transaction_tuple(buf)?; - - Ok(Self::Legacy(Signed::new_unchecked(transaction, signature, hash))) + Ok(Self::Legacy(TxLegacy::rlp_decode_signed(buf)?)) } } @@ -793,7 +789,7 @@ mod tests { // this is a legacy tx so we can attempt the same test with // decode_rlp_legacy_transaction_tuple let input_rlp = &mut &data[..]; - let res = TransactionSigned::decode_rlp_legacy_transaction_tuple(input_rlp); + let res = TxLegacy::rlp_decode_signed(input_rlp); assert_matches!(res, Ok(_tx)); assert!(input_rlp.is_empty()); diff --git a/crates/primitives/src/transaction/signature.rs b/crates/primitives/src/transaction/signature.rs index 33eb44950d..03b6327df2 100644 --- a/crates/primitives/src/transaction/signature.rs +++ b/crates/primitives/src/transaction/signature.rs @@ -1,31 +1,5 @@ -use alloy_consensus::transaction::from_eip155_value; -use alloy_primitives::{PrimitiveSignature as Signature, U256}; -use alloy_rlp::Decodable; - pub use reth_primitives_traits::crypto::secp256k1::{recover_signer, recover_signer_unchecked}; -pub(crate) fn decode_with_eip155_chain_id( - buf: &mut &[u8], -) -> alloy_rlp::Result<(Signature, Option)> { - let v = Decodable::decode(buf)?; - let r: U256 = Decodable::decode(buf)?; - let s: U256 = Decodable::decode(buf)?; - - let Some((parity, chain_id)) = from_eip155_value(v) else { - // pre bedrock system transactions were sent from the zero address as legacy - // transactions with an empty signature - // - // NOTE: this is very hacky and only relevant for op-mainnet pre bedrock - #[cfg(feature = "optimism")] - if v == 0 && r.is_zero() && s.is_zero() { - return Ok((Signature::new(r, s, false), None)) - } - return Err(alloy_rlp::Error::Custom("invalid parity for legacy transaction")) - }; - - Ok((Signature::new(r, s, parity), chain_id)) -} - #[cfg(test)] mod tests { use crate::transaction::signature::{recover_signer, recover_signer_unchecked};