From c1b4fd84c5e7a2ae39037760c953af9ba4eeb90a Mon Sep 17 00:00:00 2001 From: ftupas <35031356+ftupas@users.noreply.github.com> Date: Sun, 10 Nov 2024 13:25:55 +0100 Subject: [PATCH] refactor: remove trait bound on `TxType` (#12379) --- Cargo.lock | 37 +++++ Cargo.toml | 1 + crates/primitives-traits/src/tx_type.rs | 1 - crates/primitives/Cargo.toml | 1 + crates/primitives/src/transaction/tx_type.rs | 162 +++++++------------ 5 files changed, 93 insertions(+), 109 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f81a026bd..0434d0384 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6271,6 +6271,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "relative-path" +version = "1.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2" + [[package]] name = "reqwest" version = "0.12.9" @@ -8462,6 +8468,7 @@ dependencies = [ "reth-testing-utils", "reth-trie-common", "revm-primitives", + "rstest", "secp256k1", "serde", "serde_json", @@ -9530,6 +9537,36 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "afab94fb28594581f62d981211a9a4d53cc8130bbcbbb89a0440d9b8e81a7746" +[[package]] +name = "rstest" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a2c585be59b6b5dd66a9d2084aa1d8bd52fbdb806eafdeffb52791147862035" +dependencies = [ + "futures", + "futures-timer", + "rstest_macros", + "rustc_version 0.4.1", +] + +[[package]] +name = "rstest_macros" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "825ea780781b15345a146be27eaefb05085e337e869bff01b4306a4fd4a9ad5a" +dependencies = [ + "cfg-if", + "glob", + "proc-macro-crate", + "proc-macro2", + "quote", + "regex", + "relative-path", + "rustc_version 0.4.1", + "syn 2.0.87", + "unicode-ident", +] + [[package]] name = "ruint" version = "1.12.3" diff --git a/Cargo.toml b/Cargo.toml index 31d2ebb8d..7608756b1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -596,6 +596,7 @@ serial_test = { default-features = false, version = "3" } similar-asserts = { version = "1.5.0", features = ["serde"] } tempfile = "3.8" test-fuzz = "6" +rstest = "0.23.0" tikv-jemalloc-ctl = "0.6" tikv-jemallocator = "0.6" diff --git a/crates/primitives-traits/src/tx_type.rs b/crates/primitives-traits/src/tx_type.rs index aebf7584f..058f02a7e 100644 --- a/crates/primitives-traits/src/tx_type.rs +++ b/crates/primitives-traits/src/tx_type.rs @@ -13,7 +13,6 @@ pub trait TxType: + TryFrom + TryFrom + TryFrom - + From + Debug + Display + Clone diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index 04d96aa36..5bef33e15 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -79,6 +79,7 @@ proptest.workspace = true rand.workspace = true serde_json.workspace = true test-fuzz.workspace = true +rstest.workspace = true criterion.workspace = true pprof = { workspace = true, features = [ diff --git a/crates/primitives/src/transaction/tx_type.rs b/crates/primitives/src/transaction/tx_type.rs index eff1c17a7..0cfb2ff9d 100644 --- a/crates/primitives/src/transaction/tx_type.rs +++ b/crates/primitives/src/transaction/tx_type.rs @@ -217,128 +217,74 @@ impl Decodable for TxType { } } -impl From for TxType { - fn from(value: alloy_consensus::TxType) -> Self { - match value { - alloy_consensus::TxType::Legacy => Self::Legacy, - alloy_consensus::TxType::Eip2930 => Self::Eip2930, - alloy_consensus::TxType::Eip1559 => Self::Eip1559, - alloy_consensus::TxType::Eip4844 => Self::Eip4844, - alloy_consensus::TxType::Eip7702 => Self::Eip7702, - } - } -} - #[cfg(test)] mod tests { use alloy_primitives::hex; - use rand::Rng; use reth_codecs::Compact; + use rstest::rstest; use super::*; - #[test] - fn test_u64_to_tx_type() { - // Test for Legacy transaction - assert_eq!(TxType::try_from(U64::from(LEGACY_TX_TYPE_ID)).unwrap(), TxType::Legacy); - - // Test for EIP2930 transaction - assert_eq!(TxType::try_from(U64::from(EIP2930_TX_TYPE_ID)).unwrap(), TxType::Eip2930); - - // Test for EIP1559 transaction - assert_eq!(TxType::try_from(U64::from(EIP1559_TX_TYPE_ID)).unwrap(), TxType::Eip1559); - - // Test for EIP4844 transaction - assert_eq!(TxType::try_from(U64::from(EIP4844_TX_TYPE_ID)).unwrap(), TxType::Eip4844); - - // Test for EIP7702 transaction - assert_eq!(TxType::try_from(U64::from(EIP7702_TX_TYPE_ID)).unwrap(), TxType::Eip7702); - - // Test for Deposit transaction - #[cfg(feature = "optimism")] - assert_eq!(TxType::try_from(U64::from(DEPOSIT_TX_TYPE_ID)).unwrap(), TxType::Deposit); - - // For transactions with unsupported values - assert!(TxType::try_from(U64::from(EIP7702_TX_TYPE_ID + 1)).is_err()); + #[rstest] + #[case(U64::from(LEGACY_TX_TYPE_ID), Ok(TxType::Legacy))] + #[case(U64::from(EIP2930_TX_TYPE_ID), Ok(TxType::Eip2930))] + #[case(U64::from(EIP1559_TX_TYPE_ID), Ok(TxType::Eip1559))] + #[case(U64::from(EIP4844_TX_TYPE_ID), Ok(TxType::Eip4844))] + #[case(U64::from(EIP7702_TX_TYPE_ID), Ok(TxType::Eip7702))] + #[cfg_attr(feature = "optimism", case(U64::from(DEPOSIT_TX_TYPE_ID), Ok(TxType::Deposit)))] + #[case(U64::MAX, Err("invalid tx type"))] + fn test_u64_to_tx_type(#[case] input: U64, #[case] expected: Result) { + let tx_type_result = TxType::try_from(input); + assert_eq!(tx_type_result, expected); } - #[test] - fn test_txtype_to_compat() { - let cases = vec![ - (TxType::Legacy, COMPACT_IDENTIFIER_LEGACY, vec![]), - (TxType::Eip2930, COMPACT_IDENTIFIER_EIP2930, vec![]), - (TxType::Eip1559, COMPACT_IDENTIFIER_EIP1559, vec![]), - (TxType::Eip4844, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP4844_TX_TYPE_ID]), - (TxType::Eip7702, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP7702_TX_TYPE_ID]), - #[cfg(feature = "optimism")] - (TxType::Deposit, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![DEPOSIT_TX_TYPE_ID]), - ]; + #[rstest] + #[case(TxType::Legacy, COMPACT_IDENTIFIER_LEGACY, vec![])] + #[case(TxType::Eip2930, COMPACT_IDENTIFIER_EIP2930, vec![])] + #[case(TxType::Eip1559, COMPACT_IDENTIFIER_EIP1559, vec![])] + #[case(TxType::Eip4844, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP4844_TX_TYPE_ID])] + #[case(TxType::Eip7702, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP7702_TX_TYPE_ID])] + #[cfg_attr(feature = "optimism", case(TxType::Deposit, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![DEPOSIT_TX_TYPE_ID]))] + fn test_txtype_to_compact( + #[case] tx_type: TxType, + #[case] expected_identifier: usize, + #[case] expected_buf: Vec, + ) { + let mut buf = vec![]; + let identifier = tx_type.to_compact(&mut buf); - for (tx_type, expected_identifier, expected_buf) in cases { - let mut buf = vec![]; - let identifier = tx_type.to_compact(&mut buf); - assert_eq!( - identifier, expected_identifier, - "Unexpected identifier for TxType {tx_type:?}", - ); - assert_eq!(buf, expected_buf, "Unexpected buffer for TxType {tx_type:?}"); - } + assert_eq!(identifier, expected_identifier, "Unexpected identifier for TxType {tx_type:?}",); + assert_eq!(buf, expected_buf, "Unexpected buffer for TxType {tx_type:?}",); } - #[test] - fn test_txtype_from_compact() { - let cases = vec![ - (TxType::Legacy, COMPACT_IDENTIFIER_LEGACY, vec![]), - (TxType::Eip2930, COMPACT_IDENTIFIER_EIP2930, vec![]), - (TxType::Eip1559, COMPACT_IDENTIFIER_EIP1559, vec![]), - (TxType::Eip4844, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP4844_TX_TYPE_ID]), - (TxType::Eip7702, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP7702_TX_TYPE_ID]), - #[cfg(feature = "optimism")] - (TxType::Deposit, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![DEPOSIT_TX_TYPE_ID]), - ]; + #[rstest] + #[case(TxType::Legacy, COMPACT_IDENTIFIER_LEGACY, vec![])] + #[case(TxType::Eip2930, COMPACT_IDENTIFIER_EIP2930, vec![])] + #[case(TxType::Eip1559, COMPACT_IDENTIFIER_EIP1559, vec![])] + #[case(TxType::Eip4844, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP4844_TX_TYPE_ID])] + #[case(TxType::Eip7702, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![EIP7702_TX_TYPE_ID])] + #[cfg_attr(feature = "optimism", case(TxType::Deposit, COMPACT_EXTENDED_IDENTIFIER_FLAG, vec![DEPOSIT_TX_TYPE_ID]))] + fn test_txtype_from_compact( + #[case] expected_type: TxType, + #[case] identifier: usize, + #[case] buf: Vec, + ) { + let (actual_type, remaining_buf) = TxType::from_compact(&buf, identifier); - for (expected_type, identifier, buf) in cases { - let (actual_type, remaining_buf) = TxType::from_compact(&buf, identifier); - assert_eq!(actual_type, expected_type, "Unexpected TxType for identifier {identifier}",); - assert!( - remaining_buf.is_empty(), - "Buffer not fully consumed for identifier {identifier}", - ); - } + assert_eq!(actual_type, expected_type, "Unexpected TxType for identifier {identifier}"); + assert!(remaining_buf.is_empty(), "Buffer not fully consumed for identifier {identifier}"); } - #[test] - fn decode_tx_type() { - // Test for Legacy transaction - let tx_type = TxType::decode(&mut &hex!("80")[..]).unwrap(); - assert_eq!(tx_type, TxType::Legacy); - - // Test for EIP2930 transaction - let tx_type = TxType::decode(&mut &[EIP2930_TX_TYPE_ID][..]).unwrap(); - assert_eq!(tx_type, TxType::Eip2930); - - // Test for EIP1559 transaction - let tx_type = TxType::decode(&mut &[EIP1559_TX_TYPE_ID][..]).unwrap(); - assert_eq!(tx_type, TxType::Eip1559); - - // Test for EIP4844 transaction - let tx_type = TxType::decode(&mut &[EIP4844_TX_TYPE_ID][..]).unwrap(); - assert_eq!(tx_type, TxType::Eip4844); - - // Test for EIP7702 transaction - let tx_type = TxType::decode(&mut &[EIP7702_TX_TYPE_ID][..]).unwrap(); - assert_eq!(tx_type, TxType::Eip7702); - - // Test random byte not in range - let buf = [rand::thread_rng().gen_range(EIP7702_TX_TYPE_ID + 1..=u8::MAX)]; - assert!(TxType::decode(&mut &buf[..]).is_err()); - - // Test for Deposit transaction - #[cfg(feature = "optimism")] - { - let buf = [DEPOSIT_TX_TYPE_ID]; - let tx_type = TxType::decode(&mut &buf[..]).unwrap(); - assert_eq!(tx_type, TxType::Deposit); - } + #[rstest] + #[case(&hex!("80"), Ok(TxType::Legacy))] + #[case(&[EIP2930_TX_TYPE_ID], Ok(TxType::Eip2930))] + #[case(&[EIP1559_TX_TYPE_ID], Ok(TxType::Eip1559))] + #[case(&[EIP4844_TX_TYPE_ID], Ok(TxType::Eip4844))] + #[case(&[EIP7702_TX_TYPE_ID], Ok(TxType::Eip7702))] + #[case(&[u8::MAX], Err(alloy_rlp::Error::InputTooShort))] + #[cfg_attr(feature = "optimism", case(&[DEPOSIT_TX_TYPE_ID], Ok(TxType::Deposit)))] + fn decode_tx_type(#[case] input: &[u8], #[case] expected: Result) { + let tx_type_result = TxType::decode(&mut &input[..]); + assert_eq!(tx_type_result, expected) } }