Skip to content

Commit

Permalink
chore: remove the workaround for pre-bedrock OP transactions (#13365)
Browse files Browse the repository at this point in the history
  • Loading branch information
@klkvr
klkvr authored Dec 12, 2024
1 parent d29bca8 commit a212e1b
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 117 deletions.
15 changes: 4 additions & 11 deletions crates/optimism/primitives/src/transaction/signed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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")]
Expand Down Expand Up @@ -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 }
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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> {
Self::network_decode(buf).map_err(Into::into)
}
Expand Down Expand Up @@ -321,10 +316,8 @@ impl Decodable2718 for OpTransactionSigned {
}

fn fallback_decode(buf: &mut &[u8]) -> Eip2718Result<Self> {
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)
}
Expand Down
100 changes: 27 additions & 73 deletions crates/primitives/src/transaction/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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<Self> {
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 {
Expand Down Expand Up @@ -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(
Expand All @@ -1346,7 +1299,8 @@ impl Decodable2718 for TransactionSigned {
}

fn fallback_decode(buf: &mut &[u8]) -> Eip2718Result<Self> {
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() })
}
}

Expand Down Expand Up @@ -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"));
Expand Down
10 changes: 3 additions & 7 deletions crates/primitives/src/transaction/pooled.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -374,11 +374,7 @@ impl Decodable2718 for PooledTransactionsElement {
}

fn fallback_decode(buf: &mut &[u8]) -> Eip2718Result<Self> {
// 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)?))
}
}

Expand Down Expand Up @@ -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());

Expand Down
26 changes: 0 additions & 26 deletions crates/primitives/src/transaction/signature.rs
View file
Original file line number Diff line number Diff line change
@@ -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<u64>)> {
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};
Expand Down

0 comments on commit a212e1b

Please sign in to comment.