From 519a10ae99338f58a4f443dea295057469f807c1 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 2 Dec 2024 14:24:21 +0100 Subject: [PATCH] chore: remove OpTxType new type (#12715) --- Cargo.lock | 2 +- crates/optimism/primitives/Cargo.toml | 8 +- .../primitives/src/transaction/tx_type.rs | 289 +----------------- crates/primitives-traits/Cargo.toml | 27 +- crates/primitives-traits/src/size.rs | 7 + .../src/transaction/tx_type.rs | 29 +- .../codecs/src/alloy/transaction/optimism.rs | 52 +++- 7 files changed, 118 insertions(+), 296 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9a3521dcc..cf011d0f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8461,7 +8461,6 @@ dependencies = [ "alloy-consensus", "alloy-eips", "alloy-primitives", - "alloy-rlp", "arbitrary", "bytes", "derive_more 1.0.0", @@ -8664,6 +8663,7 @@ dependencies = [ "bytes", "derive_more 1.0.0", "modular-bitfield", + "op-alloy-consensus", "proptest", "proptest-arbitrary-interop", "rand 0.8.5", diff --git a/crates/optimism/primitives/Cargo.toml b/crates/optimism/primitives/Cargo.toml index 92e02f1d2..075cd0d13 100644 --- a/crates/optimism/primitives/Cargo.toml +++ b/crates/optimism/primitives/Cargo.toml @@ -14,14 +14,13 @@ workspace = true [dependencies] # reth reth-primitives.workspace = true -reth-primitives-traits.workspace = true +reth-primitives-traits = { workspace = true, features = ["op"] } reth-codecs = { workspace = true, optional = true, features = ["optimism"] } # ethereum alloy-primitives.workspace = true alloy-consensus.workspace = true alloy-eips.workspace = true -alloy-rlp.workspace = true # op op-alloy-consensus.workspace = true @@ -37,7 +36,7 @@ derive_more.workspace = true arbitrary = { workspace = true, features = ["derive"], optional = true } [dev-dependencies] -reth-codecs = { workspace = true, features = ["test-utils"] } +reth-codecs = { workspace = true, features = ["test-utils", "optimism"] } rstest.workspace = true arbitrary.workspace = true @@ -51,12 +50,13 @@ std = [ "alloy-eips/std", "alloy-primitives/std", "serde/std", - "alloy-rlp/std" ] reth-codec = [ "dep:reth-codecs", "reth-primitives/reth-codec", "reth-primitives-traits/reth-codec", + "reth-codecs?/optimism", + "reth-primitives/reth-codec" ] serde = [ "dep:serde", diff --git a/crates/optimism/primitives/src/transaction/tx_type.rs b/crates/optimism/primitives/src/transaction/tx_type.rs index 9976221b4..8be5f3a3d 100644 --- a/crates/optimism/primitives/src/transaction/tx_type.rs +++ b/crates/optimism/primitives/src/transaction/tx_type.rs @@ -1,286 +1,21 @@ -//! newtype pattern on `op_alloy_consensus::OpTxType`. -//! `OpTxType` implements `reth_primitives_traits::TxType`. -//! This type is required because a `Compact` impl is needed on the deposit tx type. +//! Optimism transaction type. -use core::fmt::Debug; - -use alloy_primitives::{U64, U8}; -use alloy_rlp::{Decodable, Encodable, Error}; -use bytes::BufMut; -use derive_more::{ - derive::{From, Into}, - Display, -}; -use op_alloy_consensus::OpTxType as AlloyOpTxType; -use reth_primitives_traits::{InMemorySize, TxType}; - -/// Wrapper type for [`op_alloy_consensus::OpTxType`] to implement -/// [`TxType`] trait. -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Display, Ord, Hash, From, Into)] -#[cfg_attr(any(test, feature = "arbitrary"), derive(arbitrary::Arbitrary))] -#[into(u8)] -pub struct OpTxType(AlloyOpTxType); - -impl TxType for OpTxType { - #[inline] - fn is_legacy(&self) -> bool { - matches!(self.0, AlloyOpTxType::Legacy) - } - - #[inline] - fn is_eip2930(&self) -> bool { - matches!(self.0, AlloyOpTxType::Eip2930) - } - - #[inline] - fn is_eip1559(&self) -> bool { - matches!(self.0, AlloyOpTxType::Eip1559) - } - - #[inline] - fn is_eip4844(&self) -> bool { - false - } - - #[inline] - fn is_eip7702(&self) -> bool { - matches!(self.0, AlloyOpTxType::Eip7702) - } -} - -impl InMemorySize for OpTxType { - /// Calculates a heuristic for the in-memory size of the [`OpTxType`]. - #[inline] - fn size(&self) -> usize { - core::mem::size_of::() - } -} - -impl From for U8 { - fn from(tx_type: OpTxType) -> Self { - Self::from(u8::from(tx_type)) - } -} - -impl TryFrom for OpTxType { - type Error = Error; - - fn try_from(value: u8) -> Result { - AlloyOpTxType::try_from(value) - .map(OpTxType) - .map_err(|_| Error::Custom("Invalid transaction type")) - } -} - -impl Default for OpTxType { - fn default() -> Self { - Self(AlloyOpTxType::Legacy) - } -} - -impl PartialEq for OpTxType { - fn eq(&self, other: &u8) -> bool { - let self_as_u8: u8 = (*self).into(); - &self_as_u8 == other - } -} - -impl TryFrom for OpTxType { - type Error = Error; - - fn try_from(value: u64) -> Result { - if value > u8::MAX as u64 { - return Err(Error::Custom("value out of range")); - } - Self::try_from(value as u8) - } -} - -impl TryFrom for OpTxType { - type Error = Error; - - fn try_from(value: U64) -> Result { - let u64_value: u64 = value.try_into().map_err(|_| Error::Custom("value out of range"))?; - Self::try_from(u64_value) - } -} - -impl Encodable for OpTxType { - fn length(&self) -> usize { - let value: u8 = (*self).into(); - value.length() - } - - fn encode(&self, out: &mut dyn BufMut) { - let value: u8 = (*self).into(); - value.encode(out); - } -} - -impl Decodable for OpTxType { - fn decode(buf: &mut &[u8]) -> Result { - // Decode the u8 value from RLP - let value = if buf.is_empty() { - return Err(alloy_rlp::Error::InputTooShort); - } else if buf[0] == 0x80 { - 0 // Special case: RLP encoding for integer 0 is `b"\x80"` - } else { - u8::decode(buf)? - }; - - Self::try_from(value).map_err(|_| alloy_rlp::Error::Custom("Invalid transaction type")) - } -} - -#[cfg(any(test, feature = "reth-codec"))] -impl reth_codecs::Compact for OpTxType { - fn to_compact(&self, buf: &mut B) -> usize - where - B: bytes::BufMut + AsMut<[u8]>, - { - use reth_codecs::txtype::*; - match self.0 { - AlloyOpTxType::Legacy => COMPACT_IDENTIFIER_LEGACY, - AlloyOpTxType::Eip2930 => COMPACT_IDENTIFIER_EIP2930, - AlloyOpTxType::Eip1559 => COMPACT_IDENTIFIER_EIP1559, - AlloyOpTxType::Eip7702 => { - buf.put_u8(alloy_consensus::constants::EIP7702_TX_TYPE_ID); - COMPACT_EXTENDED_IDENTIFIER_FLAG - } - AlloyOpTxType::Deposit => { - buf.put_u8(op_alloy_consensus::DEPOSIT_TX_TYPE_ID); - COMPACT_EXTENDED_IDENTIFIER_FLAG - } - } - } - - fn from_compact(mut buf: &[u8], identifier: usize) -> (Self, &[u8]) { - use bytes::Buf; - ( - match identifier { - reth_codecs::txtype::COMPACT_IDENTIFIER_LEGACY => Self(AlloyOpTxType::Legacy), - reth_codecs::txtype::COMPACT_IDENTIFIER_EIP2930 => Self(AlloyOpTxType::Eip2930), - reth_codecs::txtype::COMPACT_IDENTIFIER_EIP1559 => Self(AlloyOpTxType::Eip1559), - reth_codecs::txtype::COMPACT_EXTENDED_IDENTIFIER_FLAG => { - let extended_identifier = buf.get_u8(); - match extended_identifier { - alloy_consensus::constants::EIP7702_TX_TYPE_ID => { - Self(AlloyOpTxType::Eip7702) - } - op_alloy_consensus::DEPOSIT_TX_TYPE_ID => Self(AlloyOpTxType::Deposit), - _ => panic!("Unsupported OpTxType identifier: {extended_identifier}"), - } - } - _ => panic!("Unknown identifier for OpTxType: {identifier}"), - }, - buf, - ) - } -} +pub use op_alloy_consensus::OpTxType; #[cfg(test)] mod tests { use super::*; use alloy_consensus::constants::EIP7702_TX_TYPE_ID; - use bytes::BytesMut; use op_alloy_consensus::DEPOSIT_TX_TYPE_ID; use reth_codecs::{txtype::*, Compact}; use rstest::rstest; - #[test] - fn test_from_alloy_op_tx_type() { - let alloy_tx = AlloyOpTxType::Legacy; - let op_tx: OpTxType = OpTxType::from(alloy_tx); - assert_eq!(op_tx, OpTxType(AlloyOpTxType::Legacy)); - } - - #[test] - fn test_from_op_tx_type_to_u8() { - let op_tx = OpTxType(AlloyOpTxType::Legacy); - let tx_type_u8: u8 = op_tx.into(); - assert_eq!(tx_type_u8, AlloyOpTxType::Legacy as u8); - } - - #[test] - fn test_from_op_tx_type_to_u8_u8() { - let op_tx = OpTxType(AlloyOpTxType::Legacy); - let tx_type_u8: U8 = op_tx.into(); - assert_eq!(tx_type_u8, U8::from(AlloyOpTxType::Legacy as u8)); - } - - #[test] - fn test_try_from_u8() { - let op_tx = OpTxType::try_from(AlloyOpTxType::Legacy as u8).unwrap(); - assert_eq!(op_tx, OpTxType(AlloyOpTxType::Legacy)); - } - - #[test] - fn test_try_from_invalid_u8() { - let invalid_value: u8 = 255; - let result = OpTxType::try_from(invalid_value); - assert_eq!(result, Err(Error::Custom("Invalid transaction type"))); - } - - #[test] - fn test_try_from_u64() { - let op_tx = OpTxType::try_from(AlloyOpTxType::Legacy as u64).unwrap(); - assert_eq!(op_tx, OpTxType(AlloyOpTxType::Legacy)); - } - - #[test] - fn test_try_from_u64_out_of_range() { - let result = OpTxType::try_from(u64::MAX); - assert_eq!(result, Err(Error::Custom("value out of range"))); - } - - #[test] - fn test_try_from_u64_within_range() { - let valid_value: U64 = U64::from(AlloyOpTxType::Legacy as u64); - let op_tx = OpTxType::try_from(valid_value).unwrap(); - assert_eq!(op_tx, OpTxType(AlloyOpTxType::Legacy)); - } - - #[test] - fn test_default() { - let default_tx = OpTxType::default(); - assert_eq!(default_tx, OpTxType(AlloyOpTxType::Legacy)); - } - - #[test] - fn test_partial_eq_u8() { - let op_tx = OpTxType(AlloyOpTxType::Legacy); - assert_eq!(op_tx, AlloyOpTxType::Legacy as u8); - } - - #[test] - fn test_encodable() { - let op_tx = OpTxType(AlloyOpTxType::Legacy); - let mut buf = BytesMut::new(); - op_tx.encode(&mut buf); - assert_eq!(buf, BytesMut::from(&[0x80][..])); - } - - #[test] - fn test_decodable_success() { - // Using the RLP-encoded form of 0, which is `b"\x80"` - let mut buf: &[u8] = &[0x80]; - let decoded_tx = OpTxType::decode(&mut buf).unwrap(); - assert_eq!(decoded_tx, OpTxType(AlloyOpTxType::Legacy)); - } - - #[test] - fn test_decodable_invalid() { - let mut buf: &[u8] = &[255]; - let result = OpTxType::decode(&mut buf); - assert!(result.is_err()); - } - #[rstest] - #[case(OpTxType(AlloyOpTxType::Legacy), COMPACT_IDENTIFIER_LEGACY, vec![])] - #[case(OpTxType(AlloyOpTxType::Eip2930), COMPACT_IDENTIFIER_EIP2930, vec![])] - #[case(OpTxType(AlloyOpTxType::Eip1559), COMPACT_IDENTIFIER_EIP1559, vec![])] - #[case(OpTxType(AlloyOpTxType::Eip7702), COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP7702_TX_TYPE_ID])] - #[case(OpTxType(AlloyOpTxType::Deposit), COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![DEPOSIT_TX_TYPE_ID])] + #[case(OpTxType::Legacy, COMPACT_IDENTIFIER_LEGACY, vec![])] + #[case(OpTxType::Eip2930, COMPACT_IDENTIFIER_EIP2930, vec![])] + #[case(OpTxType::Eip1559, COMPACT_IDENTIFIER_EIP1559, vec![])] + #[case(OpTxType::Eip7702, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP7702_TX_TYPE_ID])] + #[case(OpTxType::Deposit, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![DEPOSIT_TX_TYPE_ID])] fn test_txtype_to_compact( #[case] tx_type: OpTxType, #[case] expected_identifier: usize, @@ -297,11 +32,11 @@ mod tests { } #[rstest] - #[case(OpTxType(AlloyOpTxType::Legacy), COMPACT_IDENTIFIER_LEGACY, vec![])] - #[case(OpTxType(AlloyOpTxType::Eip2930), COMPACT_IDENTIFIER_EIP2930, vec![])] - #[case(OpTxType(AlloyOpTxType::Eip1559), COMPACT_IDENTIFIER_EIP1559, vec![])] - #[case(OpTxType(AlloyOpTxType::Eip7702), COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP7702_TX_TYPE_ID])] - #[case(OpTxType(AlloyOpTxType::Deposit), COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![DEPOSIT_TX_TYPE_ID])] + #[case(OpTxType::Legacy, COMPACT_IDENTIFIER_LEGACY, vec![])] + #[case(OpTxType::Eip2930, COMPACT_IDENTIFIER_EIP2930, vec![])] + #[case(OpTxType::Eip1559, COMPACT_IDENTIFIER_EIP1559, vec![])] + #[case(OpTxType::Eip7702, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP7702_TX_TYPE_ID])] + #[case(OpTxType::Deposit, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![DEPOSIT_TX_TYPE_ID])] fn test_txtype_from_compact( #[case] expected_type: OpTxType, #[case] identifier: usize, diff --git a/crates/primitives-traits/Cargo.toml b/crates/primitives-traits/Cargo.toml index d56fd5bc0..ceee1e26c 100644 --- a/crates/primitives-traits/Cargo.toml +++ b/crates/primitives-traits/Cargo.toml @@ -23,6 +23,9 @@ alloy-primitives.workspace = true alloy-rlp.workspace = true revm-primitives.workspace = true +# op +op-alloy-consensus = { workspace = true, optional = true } + # misc byteorder = { workspace = true, optional = true } bytes.workspace = true @@ -78,28 +81,34 @@ arbitrary = [ "dep:proptest-arbitrary-interop", "alloy-eips/arbitrary", "revm-primitives/arbitrary", - "reth-codecs?/arbitrary" + "reth-codecs?/arbitrary", + "op-alloy-consensus?/arbitrary" ] serde-bincode-compat = [ "serde", "serde_with", "alloy-consensus/serde-bincode-compat", - "alloy-eips/serde-bincode-compat" + "alloy-eips/serde-bincode-compat", + "op-alloy-consensus?/serde-bincode-compat" ] serde = [ "dep:serde", "alloy-consensus/serde", - "alloy-eips/serde", - "alloy-primitives/serde", - "bytes/serde", - "rand/serde", - "reth-codecs?/serde", - "revm-primitives/serde", - "roaring/serde", + "alloy-eips/serde", + "alloy-primitives/serde", + "bytes/serde", + "rand/serde", + "reth-codecs?/serde", "revm-primitives/serde", + "roaring/serde", + "revm-primitives/serde", + "op-alloy-consensus?/serde" ] reth-codec = [ "dep:reth-codecs", "dep:modular-bitfield", "dep:byteorder", ] +op = [ + "dep:op-alloy-consensus", +] diff --git a/crates/primitives-traits/src/size.rs b/crates/primitives-traits/src/size.rs index 4d721dd00..f9065cda2 100644 --- a/crates/primitives-traits/src/size.rs +++ b/crates/primitives-traits/src/size.rs @@ -46,6 +46,13 @@ macro_rules! impl_in_mem_size { impl_in_mem_size!(Header, TxLegacy, TxEip2930, TxEip1559, TxEip7702, TxEip4844); +#[cfg(feature = "op")] +impl InMemorySize for op_alloy_consensus::OpTxType { + fn size(&self) -> usize { + 1 + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/primitives-traits/src/transaction/tx_type.rs b/crates/primitives-traits/src/transaction/tx_type.rs index d2caebe4c..c2f2e0489 100644 --- a/crates/primitives-traits/src/transaction/tx_type.rs +++ b/crates/primitives-traits/src/transaction/tx_type.rs @@ -1,10 +1,8 @@ //! Abstraction of transaction envelope type ID. -use core::fmt; - -use alloy_primitives::{U64, U8}; - use crate::{InMemorySize, MaybeArbitrary, MaybeCompact}; +use alloy_primitives::{U64, U8}; +use core::fmt; /// Helper trait that unifies all behaviour required by transaction type ID to support full node /// operations. @@ -60,3 +58,26 @@ pub trait TxType: !self.is_eip4844() } } + +#[cfg(feature = "op")] +impl TxType for op_alloy_consensus::OpTxType { + fn is_legacy(&self) -> bool { + matches!(self, Self::Legacy) + } + + fn is_eip2930(&self) -> bool { + matches!(self, Self::Eip2930) + } + + fn is_eip1559(&self) -> bool { + matches!(self, Self::Eip1559) + } + + fn is_eip4844(&self) -> bool { + false + } + + fn is_eip7702(&self) -> bool { + matches!(self, Self::Eip7702) + } +} diff --git a/crates/storage/codecs/src/alloy/transaction/optimism.rs b/crates/storage/codecs/src/alloy/transaction/optimism.rs index bb970b581..631f5c406 100644 --- a/crates/storage/codecs/src/alloy/transaction/optimism.rs +++ b/crates/storage/codecs/src/alloy/transaction/optimism.rs @@ -1,9 +1,11 @@ //! Compact implementation for [`AlloyTxDeposit`] +use alloy_consensus::constants::EIP7702_TX_TYPE_ID; use crate::Compact; use alloy_primitives::{Address, Bytes, TxKind, B256, U256}; -use op_alloy_consensus::TxDeposit as AlloyTxDeposit; +use op_alloy_consensus::{OpTxType, TxDeposit as AlloyTxDeposit}; use reth_codecs_derive::add_arbitrary_tests; +use crate::txtype::{COMPACT_EXTENDED_IDENTIFIER_FLAG, COMPACT_IDENTIFIER_EIP1559, COMPACT_IDENTIFIER_EIP2930, COMPACT_IDENTIFIER_LEGACY}; /// Deposit transactions, also known as deposits are initiated on L1, and executed on L2. /// @@ -65,3 +67,51 @@ impl Compact for AlloyTxDeposit { (alloy_tx, buf) } } + + +impl crate::Compact for OpTxType { + fn to_compact(&self, buf: &mut B) -> usize + where + B: bytes::BufMut + AsMut<[u8]>, + { + use crate::txtype::*; + + match self { + Self::Legacy => COMPACT_IDENTIFIER_LEGACY, + Self::Eip2930 => COMPACT_IDENTIFIER_EIP2930, + Self::Eip1559 => COMPACT_IDENTIFIER_EIP1559, + Self::Eip7702 => { + buf.put_u8(EIP7702_TX_TYPE_ID); + COMPACT_EXTENDED_IDENTIFIER_FLAG + } + Self::Deposit => { + buf.put_u8(op_alloy_consensus::DEPOSIT_TX_TYPE_ID); + COMPACT_EXTENDED_IDENTIFIER_FLAG + } + } + } + + // For backwards compatibility purposes only 2 bits of the type are encoded in the identifier + // parameter. In the case of a [`COMPACT_EXTENDED_IDENTIFIER_FLAG`], the full transaction type + // is read from the buffer as a single byte. + fn from_compact(mut buf: &[u8], identifier: usize) -> (Self, &[u8]) { + use bytes::Buf; + ( + match identifier { + COMPACT_IDENTIFIER_LEGACY => Self::Legacy, + COMPACT_IDENTIFIER_EIP2930 => Self::Eip2930, + COMPACT_IDENTIFIER_EIP1559 => Self::Eip1559, + COMPACT_EXTENDED_IDENTIFIER_FLAG => { + let extended_identifier = buf.get_u8(); + match extended_identifier { + EIP7702_TX_TYPE_ID => Self::Eip7702, + op_alloy_consensus::DEPOSIT_TX_TYPE_ID => Self::Deposit, + _ => panic!("Unsupported TxType identifier: {extended_identifier}"), + } + } + _ => panic!("Unknown identifier for TxType: {identifier}"), + }, + buf, + ) + } +} \ No newline at end of file