chore(net): Add proptest roundtrip to rlp types (#829)

crates/primitives/src/bits.rs

@@ -0,0 +1,74 @@
+//! Fixed hash types
+use bytes::Buf;
+use derive_more::{AsRef, Deref};
+use fixed_hash::construct_fixed_hash;
+use impl_serde::impl_fixed_hash_serde;
+use reth_codecs::{impl_hash_compact, Compact};
+use reth_rlp::{RlpDecodableWrapper, RlpEncodableWrapper, RlpMaxEncodedLen};
+
+/// Implements a fixed hash type (eg. H512) with `serde`, `Arbitrary`, `proptest::Arbitrary` and
+/// `Compact` support.
+#[macro_export]
+macro_rules! impl_fixed_hash_type {
+    ($(($name:tt, $size:expr)),+) => {
+
+        #[cfg(any(test, feature = "arbitrary"))]
+        use proptest::{
+            arbitrary::{any_with, ParamsFor},
+            strategy::{BoxedStrategy, Strategy},
+        };
+
+        #[cfg(any(test, feature = "arbitrary"))]
+        use arbitrary::Arbitrary;
+
+        $(
+            construct_fixed_hash! {
+                #[cfg_attr(any(test, feature = "arbitrary"), derive(Arbitrary))]
+                #[derive(AsRef, Deref, RlpEncodableWrapper, RlpDecodableWrapper, RlpMaxEncodedLen)]
+                #[doc = concat!(stringify!($name), " fixed hash type.")]
+                pub struct $name($size);
+            }
+
+            impl_hash_compact!($name);
+
+            impl_fixed_hash_serde!($name, $size);
+
+            #[cfg(any(test, feature = "arbitrary"))]
+            impl proptest::arbitrary::Arbitrary for $name {
+                type Parameters = ParamsFor<u8>;
+                type Strategy = BoxedStrategy<$name>;
+
+                fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
+                    proptest::collection::vec(any_with::<u8>(args), $size)
+                        .prop_map(move |vec| $name::from_slice(&vec))
+                        .boxed()
+                }
+            }
+        )+
+
+        #[cfg(test)]
+        mod hash_tests {
+            use super::*;
+
+            #[test]
+            fn arbitrary() {
+                $(
+                    proptest::proptest!(|(field: $name)| {
+                        let mut buf = vec![];
+                        field.to_compact(&mut buf);
+
+                        // Add noise. We want to make sure that only $size bytes get consumed.
+                        buf.push(1);
+
+                        let (decoded, remaining_buf) = $name::from_compact(&buf, buf.len());
+
+                        assert!(field == decoded);
+                        assert!(remaining_buf.len() == 1);
+                    });
+                )+
+            }
+        }
+    };
+}
+
+impl_fixed_hash_type!((H64, 8), (H512, 64));

crates/primitives/src/block.rs

@@ -1,4 +1,5 @@
 use crate::{Header, SealedHeader, TransactionSigned, H256};
+use reth_codecs::derive_arbitrary;
 use reth_rlp::{Decodable, DecodeError, Encodable, RlpDecodable, RlpEncodable};
 use serde::{Deserialize, Serialize};
 use std::ops::Deref;
@@ -61,6 +62,7 @@ impl Deref for SealedBlock {
 }
 
 /// Either a block hash _or_ a block number
+#[derive_arbitrary(rlp)]
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum BlockHashOrNumber {
     /// A block hash

crates/primitives/src/bloom.rs

@@ -1,45 +1,16 @@
-//! Bloom related utilities.
-use crate::{keccak256, Log};
+//! Bloom type.
+use crate::{impl_fixed_hash_type, keccak256, Log};
 use bytes::Buf;
 use derive_more::{AsRef, Deref};
 use fixed_hash::construct_fixed_hash;
 use impl_serde::impl_fixed_hash_serde;
 use reth_codecs::{impl_hash_compact, Compact};
-use reth_rlp::{RlpDecodableWrapper, RlpEncodableWrapper};
-
-#[cfg(any(test, feature = "arbitrary"))]
-use proptest::{
-    arbitrary::{any_with, Arbitrary as PropTestArbitrary, ParamsFor},
-    strategy::{BoxedStrategy, Strategy},
-};
-
-#[cfg(any(test, feature = "arbitrary"))]
-use arbitrary::Arbitrary;
+use reth_rlp::{RlpDecodableWrapper, RlpEncodableWrapper, RlpMaxEncodedLen};
 
 /// Length of bloom filter used for Ethereum.
 pub const BLOOM_BYTE_LENGTH: usize = 256;
 
-construct_fixed_hash! {
-    /// 2048 bits type.
-    #[cfg_attr(any(test, feature = "arbitrary"), derive(Arbitrary))]
-    #[derive(AsRef, Deref, RlpEncodableWrapper, RlpDecodableWrapper)]
-    pub struct Bloom(BLOOM_BYTE_LENGTH);
-}
-
-impl_hash_compact!(Bloom);
-impl_fixed_hash_serde!(Bloom, BLOOM_BYTE_LENGTH);
-
-#[cfg(any(test, feature = "arbitrary"))]
-impl PropTestArbitrary for Bloom {
-    type Parameters = ParamsFor<u8>;
-    type Strategy = BoxedStrategy<Bloom>;
-
-    fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
-        proptest::collection::vec(any_with::<u8>(args), BLOOM_BYTE_LENGTH)
-            .prop_map(move |vec| Bloom::from_slice(&vec))
-            .boxed()
-    }
-}
+impl_fixed_hash_type!((Bloom, BLOOM_BYTE_LENGTH));
 
 // See Section 4.3.1 "Transaction Receipt" of the Yellow Paper
 fn m3_2048(bloom: &mut Bloom, x: &[u8]) {
@@ -105,19 +76,4 @@ mod tests {
             ))
         );
     }
-    #[test]
-    fn arbitrary() {
-        proptest::proptest!(|(bloom: Bloom)| {
-            let mut buf = vec![];
-            bloom.to_compact(&mut buf);
-
-            // Add noise
-            buf.push(1);
-
-            let (decoded, remaining_buf) = Bloom::from_compact(&buf, buf.len());
-
-            assert!(bloom == decoded);
-            assert!(remaining_buf.len() == 1);
-        });
-    }
 }

crates/primitives/src/chain.rs

@@ -1,10 +1,12 @@
 use crate::U256;
 use ethers_core::types::U64;
+use reth_codecs::add_arbitrary_tests;
 use reth_rlp::{Decodable, Encodable};
 use serde::{Deserialize, Serialize};
 use std::{fmt, str::FromStr};
 
 /// Either a named or chain id or the actual id value
+#[add_arbitrary_tests(rlp)]
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum Chain {
     /// Contains a known chain
@@ -168,6 +170,43 @@ impl Default for Chain {
     }
 }
 
+#[cfg(any(test, feature = "arbitrary"))]
+impl<'a> arbitrary::Arbitrary<'a> for Chain {
+    fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
+        if u.ratio(1, 2)? {
+            let chain = u.int_in_range(0..=(ethers_core::types::Chain::COUNT - 1))?;
+
+            return Ok(Chain::Named(ethers_core::types::Chain::iter().nth(chain).expect("in range")))
+        }
+
+        Ok(Self::Id(u64::arbitrary(u)?))
+    }
+}
+
+#[cfg(any(test, feature = "arbitrary"))]
+use strum::{EnumCount, IntoEnumIterator};
+
+#[cfg(any(test, feature = "arbitrary"))]
+use proptest::{
+    arbitrary::ParamsFor,
+    prelude::{any, Strategy},
+    sample::Selector,
+    strategy::BoxedStrategy,
+};
+
+#[cfg(any(test, feature = "arbitrary"))]
+impl proptest::arbitrary::Arbitrary for Chain {
+    type Parameters = ParamsFor<u32>;
+    type Strategy = BoxedStrategy<Chain>;
+
+    fn arbitrary_with(_: Self::Parameters) -> Self::Strategy {
+        let named = any::<Selector>()
+            .prop_map(move |sel| Chain::Named(sel.select(ethers_core::types::Chain::iter())));
+        let id = any::<u64>().prop_map(Chain::from);
+        proptest::strategy::Union::new_weighted(vec![(50, named.boxed()), (50, id.boxed())]).boxed()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/primitives/src/forkid.rs

@@ -5,6 +5,7 @@
 
 use crate::{BlockNumber, H256};
 use crc::crc32;
+use reth_codecs::derive_arbitrary;
 use reth_rlp::*;
 use serde::{Deserialize, Serialize};
 use std::{
@@ -15,6 +16,7 @@ use std::{
 use thiserror::Error;
 
 /// `CRC32` hash of all previous forks starting from genesis block.
+#[derive_arbitrary(rlp)]
 #[derive(
     Clone,
     Copy,
@@ -58,6 +60,7 @@ impl Add<BlockNumber> for ForkHash {
 
 /// A fork identifier as defined by EIP-2124.
 /// Serves as the chain compatibility identifier.
+#[derive_arbitrary(rlp)]
 #[derive(
     Clone,
     Copy,

crates/primitives/src/header.rs

@@ -5,7 +5,7 @@ use crate::{
 };
 use bytes::{BufMut, BytesMut};
 use ethers_core::types::H64;
-use reth_codecs::{main_codec, Compact};
+use reth_codecs::{derive_arbitrary, main_codec, Compact};
 use reth_rlp::{length_of_length, Decodable, Encodable};
 use serde::{Deserialize, Serialize};
 use std::ops::Deref;
@@ -290,6 +290,7 @@ impl SealedHeader {
 /// [`HeadersDirection::Falling`] block numbers for `reverse == 1 == true`
 ///
 /// See also <https://github.com/ethereum/devp2p/blob/master/caps/eth.md#getblockheaders-0x03>
+#[derive_arbitrary(rlp)]
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Default, Serialize, Deserialize)]
 pub enum HeadersDirection {
     /// Falling block number.

crates/primitives/src/hex_bytes.rs

@@ -220,7 +220,7 @@ impl proptest::prelude::Arbitrary for Bytes {
     type Strategy = proptest::prelude::BoxedStrategy<Bytes>;
 
     fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
-        proptest::collection::vec(proptest::arbitrary::any_with::<u8>(args), 0..1000)
+        proptest::collection::vec(proptest::arbitrary::any_with::<u8>(args), 0..80)
             .prop_map(move |vec| bytes::Bytes::from(vec).into())
             .boxed()
     }
@@ -229,7 +229,7 @@ impl proptest::prelude::Arbitrary for Bytes {
 #[cfg(any(test, feature = "arbitrary"))]
 impl<'a> arbitrary::Arbitrary<'a> for Bytes {
     fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
-        let size = u.int_in_range(0..=1000)?;
+        let size = u.int_in_range(0..=80)?;
         Ok(Self(bytes::Bytes::copy_from_slice(u.bytes(size)?)))
     }
 }

crates/primitives/src/lib.rs

@@ -10,6 +10,7 @@
 //! This crate contains Ethereum primitive types and helper functions.
 
 mod account;
+mod bits;
 mod block;
 pub mod bloom;
 mod chain;
@@ -32,6 +33,7 @@ mod transaction;
 pub mod proofs;
 
 pub use account::Account;
+pub use bits::H512;
 pub use block::{Block, BlockHashOrNumber, SealedBlock};
 pub use bloom::Bloom;
 pub use chain::Chain;
@@ -77,9 +79,9 @@ pub type TransitionId = u64;
 
 pub use ethers_core::{
     types as rpc,
-    types::{BigEndianHash, H128, H512, H64, U128, U64},
+    types::{BigEndianHash, H128, H64, U64},
 };
-pub use revm_interpreter::{B160 as H160, B256 as H256, U256};
+pub use revm_interpreter::{ruint::aliases::U128, B160 as H160, B256 as H256, U256};
 
 #[doc(hidden)]
 mod __reexport {

crates/primitives/src/log.rs

@@ -3,7 +3,7 @@ use reth_codecs::{main_codec, Compact};
 use reth_rlp::{RlpDecodable, RlpEncodable};
 
 /// Ethereum Log
-#[main_codec]
+#[main_codec(rlp)]
 #[derive(Clone, Debug, PartialEq, Eq, RlpDecodable, RlpEncodable, Default)]
 pub struct Log {
     /// Contract that emitted this log.

crates/primitives/src/peer.rs

@@ -1,4 +1,4 @@
-use ethers_core::types::H512;
+use crate::H512;
 
 // TODO: should we use `PublicKey` for this? Even when dealing with public keys we should try to
 // prevent misuse

crates/primitives/src/receipt.rs

@@ -112,7 +112,7 @@ impl Encodable for Receipt {
         if matches!(self.tx_type, TxType::EIP1559 | TxType::EIP2930) {
             payload_len += 1;
             // we include a string header for typed receipts, so include the length here
-            payload_len = length_of_length(payload_len);
+            payload_len += length_of_length(payload_len);
         }
 
         payload_len

crates/primitives/src/transaction/access_list.rs

@@ -5,7 +5,7 @@ use reth_rlp::{RlpDecodable, RlpDecodableWrapper, RlpEncodable, RlpEncodableWrap
 
 /// A list of addresses and storage keys that the transaction plans to access.
 /// Accesses outside the list are possible, but become more expensive.
-#[main_codec]
+#[main_codec(rlp)]
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Default, RlpDecodable, RlpEncodable)]
 pub struct AccessListItem {
     /// Account addresses that would be loaded at the start of execution
@@ -15,6 +15,6 @@ pub struct AccessListItem {
 }
 
 /// AccessList as defined in EIP-2930
-#[main_codec]
+#[main_codec(rlp)]
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Default, RlpDecodableWrapper, RlpEncodableWrapper)]
 pub struct AccessList(pub Vec<AccessListItem>);

crates/primitives/src/transaction/mod.rs

@@ -2,7 +2,7 @@ use crate::{keccak256, Address, Bytes, ChainId, TxHash, H256};
 pub use access_list::{AccessList, AccessListItem};
 use bytes::{Buf, BytesMut};
 use derive_more::{AsRef, Deref};
-use reth_codecs::{main_codec, Compact};
+use reth_codecs::{add_arbitrary_tests, main_codec, Compact};
 use reth_rlp::{length_of_length, Decodable, DecodeError, Encodable, Header, EMPTY_STRING_CODE};
 pub use signature::Signature;
 pub use tx_type::TxType;
@@ -187,6 +187,15 @@ impl Transaction {
         keccak256(&buf)
     }
 
+    /// Get chain_id.
+    pub fn chain_id(&self) -> Option<&u64> {
+        match self {
+            Transaction::Legacy(TxLegacy { chain_id, .. }) => chain_id.as_ref(),
+            Transaction::Eip2930(TxEip2930 { chain_id, .. }) => Some(chain_id),
+            Transaction::Eip1559(TxEip1559 { chain_id, .. }) => Some(chain_id),
+        }
+    }
+
     /// Sets the transaction's chain id to the provided value.
     pub fn set_chain_id(&mut self, chain_id: u64) {
         match self {
@@ -525,7 +534,8 @@ impl Decodable for TransactionKind {
 }
 
 /// Signed transaction.
-#[main_codec]
+#[main_codec(no_arbitrary)]
+#[add_arbitrary_tests(rlp, compact)]
 #[derive(Debug, Clone, PartialEq, Eq, Hash, AsRef, Deref, Default)]
 pub struct TransactionSigned {
     /// Transaction hash
@@ -538,6 +548,53 @@ pub struct TransactionSigned {
     pub transaction: Transaction,
 }
 
+#[cfg(any(test, feature = "arbitrary"))]
+use proptest::{
+    prelude::{any, Strategy},
+    strategy::BoxedStrategy,
+};
+
+#[cfg(any(test, feature = "arbitrary"))]
+impl proptest::arbitrary::Arbitrary for TransactionSigned {
+    type Parameters = ();
+    type Strategy = BoxedStrategy<TransactionSigned>;
+
+    fn arbitrary_with(_: Self::Parameters) -> Self::Strategy {
+        any::<(Transaction, Signature)>()
+            .prop_map(move |(mut transaction, sig)| {
+                if let Some(chain_id) = transaction.chain_id().cloned() {
+                    // Otherwise we might overflow when calculating `v` on `recalculate_hash`
+                    transaction.set_chain_id(chain_id % (u64::MAX / 2 - 36));
+                }
+                let mut tx =
+                    TransactionSigned { hash: Default::default(), signature: sig, transaction };
+                tx.hash = tx.recalculate_hash();
+                tx
+            })
+            .boxed()
+    }
+}
+
+#[cfg(any(test, feature = "arbitrary"))]
+impl<'a> arbitrary::Arbitrary<'a> for TransactionSigned {
+    fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
+        let mut transaction = Transaction::arbitrary(u)?;
+        if let Some(chain_id) = transaction.chain_id().cloned() {
+            // Otherwise we might overflow when calculating `v` on `recalculate_hash`
+            transaction.set_chain_id(chain_id % (u64::MAX / 2 - 36));
+        }
+
+        let mut tx = TransactionSigned {
+            hash: Default::default(),
+            signature: Signature::arbitrary(u)?,
+            transaction,
+        };
+        tx.hash = tx.recalculate_hash();
+
+        Ok(tx)
+    }
+}
+
 impl From<TransactionSignedEcRecovered> for TransactionSigned {
     fn from(recovered: TransactionSignedEcRecovered) -> Self {
         recovered.signed_transaction

crates/primitives/src/transaction/tx_type.rs

@@ -1,8 +1,8 @@
-use reth_codecs::{derive_compact_arbitrary, Compact};
+use reth_codecs::{derive_arbitrary, Compact};
 use serde::{Deserialize, Serialize};
 
 /// Transaction Type
-#[derive_compact_arbitrary]
+#[derive_arbitrary(compact)]
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Default, Serialize, Deserialize)]
 pub enum TxType {
     /// Legacy transaction pre EIP-2929