feat: verify unused bits on types derived with Compact (#11131)

crates/storage/codecs/Cargo.toml

@@ -61,3 +61,4 @@ alloy = [
     "dep:serde"
 ]
 optimism = ["alloy", "dep:op-alloy-consensus"]
+test-utils = []

crates/storage/codecs/derive/src/compact/flags.rs

@@ -53,6 +53,7 @@ pub(crate) fn generate_flag_struct(
     let docs =
         format!("Fieldset that facilitates compacting the parent type. Used bytes: {total_bytes} | Unused bits: {unused_bits}");
     let bitflag_encoded_bytes = format!("Used bytes by [`{flags_ident}`]");
+    let bitflag_unused_bits = format!("Unused bits for new fields by [`{flags_ident}`]");
     let impl_bitflag_encoded_bytes = if has_lifetime {
         quote! {
             impl<'a> #ident<'a> {
@@ -60,6 +61,10 @@ pub(crate) fn generate_flag_struct(
                 pub const fn bitflag_encoded_bytes() -> usize {
                     #total_bytes as usize
                 }
+                #[doc = #bitflag_unused_bits]
+                pub const fn bitflag_unused_bits() -> usize {
+                    #unused_bits as usize
+                }
            }
         }
     } else {
@@ -69,6 +74,10 @@ pub(crate) fn generate_flag_struct(
                 pub const fn bitflag_encoded_bytes() -> usize {
                     #total_bytes as usize
                 }
+                #[doc = #bitflag_unused_bits]
+                pub const fn bitflag_unused_bits() -> usize {
+                    #unused_bits as usize
+                }
            }
         }
     };

crates/storage/codecs/derive/src/compact/mod.rs

@@ -245,6 +245,10 @@ mod tests {
                 pub const fn bitflag_encoded_bytes() -> usize {
                     2u8 as usize
                 }
+                #[doc = "Unused bits for new fields by [`TestStructFlags`]"]
+                pub const fn bitflag_unused_bits() -> usize {
+                    1u8 as usize
+                }
             }
 
             pub use TestStruct_flags::TestStructFlags;

crates/storage/codecs/derive/src/lib.rs

@@ -20,12 +20,42 @@ use syn::{
 mod arbitrary;
 mod compact;
 
+/// Derives the `Compact` trait for custom structs, optimizing serialization with a possible
+/// bitflag struct.
+///
+/// ## Implementation:
+/// The derived `Compact` implementation leverages a bitflag struct when needed to manage the
+/// presence of certain field types, primarily for compacting fields efficiently. This bitflag
+/// struct records information about fields that require a small, fixed number of bits for their
+/// encoding, such as `bool`, `Option<T>`, or other small types.
+///
+/// ### Bit Sizes for Fields:
+/// The amount of bits used to store a field size is determined by the field's type. For specific
+/// types, a fixed number of bits is allocated (from `fn get_bit_size`):
+/// - `bool`, `Option<T>`, `TransactionKind`, `Signature`: **1 bit**
+/// - `TxType`: **2 bits**
+/// - `u64`, `BlockNumber`, `TxNumber`, `ChainId`, `NumTransactions`: **4 bits**
+/// - `u128`: **5 bits**
+/// - `U256`: **6 bits**
+///
+/// ### Warning: Extending structs, unused bits and backwards compatibility:
+/// When the bitflag only has one bit left (for example, when adding many `Option<T>` fields),
+/// you should introduce a new struct (e.g., `TExtension`) with additional fields, and use
+/// `Option<TExtension>` in the original struct. This approach allows further field extensions while
+/// maintaining backward compatibility.
+///
+/// ### Limitations:
+/// - Fields not listed above, or types such `Vec`, or large composite types, should manage their
+///   own encoding and do not rely on the bitflag struct.
+/// - `Bytes` fields and any types containing a `Bytes` field should be placed last to ensure
+///   efficient decoding.
 #[proc_macro_derive(Compact, attributes(maybe_zero))]
 pub fn derive(input: TokenStream) -> TokenStream {
     let is_zstd = false;
     compact::derive(input, is_zstd)
 }
 
+/// Adds `zstd` compression to derived [`Compact`].
 #[proc_macro_derive(CompactZstd, attributes(maybe_zero))]
 pub fn derive_zstd(input: TokenStream) -> TokenStream {
     let is_zstd = true;

crates/storage/codecs/src/alloy/authorization_list.rs

@@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize};
 #[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, Compact)]
 #[cfg_attr(test, derive(arbitrary::Arbitrary))]
 #[add_arbitrary_tests(compact)]
-struct Authorization {
+pub(crate) struct Authorization {
     chain_id: U256,
     address: Address,
     nonce: u64,

crates/storage/codecs/src/alloy/genesis_account.rs

@@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize};
 ///
 /// Notice: Make sure this struct is 1:1 with `alloy_genesis::GenesisAccount`
 #[derive(Debug, Clone, PartialEq, Eq, Compact)]
-struct GenesisAccountRef<'a> {
+pub(crate) struct GenesisAccountRef<'a> {
     /// The nonce of the account at genesis.
     nonce: Option<u64>,
     /// The balance of the account at genesis.
@@ -26,7 +26,7 @@ struct GenesisAccountRef<'a> {
 #[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, Compact)]
 #[cfg_attr(test, derive(arbitrary::Arbitrary))]
 #[add_arbitrary_tests(compact)]
-struct GenesisAccount {
+pub(crate) struct GenesisAccount {
     /// The nonce of the account at genesis.
     nonce: Option<u64>,
     /// The balance of the account at genesis.
@@ -42,14 +42,14 @@ struct GenesisAccount {
 #[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, Compact)]
 #[cfg_attr(test, derive(arbitrary::Arbitrary))]
 #[add_arbitrary_tests(compact)]
-struct StorageEntries {
+pub(crate) struct StorageEntries {
     entries: Vec<StorageEntry>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, Compact)]
 #[cfg_attr(test, derive(arbitrary::Arbitrary))]
 #[add_arbitrary_tests(compact)]
-struct StorageEntry {
+pub(crate) struct StorageEntry {
     key: B256,
     value: B256,
 }

crates/storage/codecs/src/alloy/header.rs

@@ -12,7 +12,7 @@ use serde::{Deserialize, Serialize};
 ///
 /// Notice: Make sure this struct is 1:1 with [`alloy_consensus::Header`]
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Default, Serialize, Deserialize, Compact)]
-struct Header {
+pub(crate) struct Header {
     parent_hash: B256,
     ommers_hash: B256,
     beneficiary: Address,

crates/storage/codecs/src/alloy/mod.rs

@@ -9,3 +9,41 @@ mod transaction;
 mod trie;
 mod txkind;
 mod withdrawal;
+
+#[cfg(test)]
+mod tests {
+    use crate::{
+        alloy::{
+            authorization_list::Authorization,
+            genesis_account::{GenesisAccount, GenesisAccountRef, StorageEntries, StorageEntry},
+            header::Header,
+            transaction::{
+                eip1559::TxEip1559, eip2930::TxEip2930, eip4844::TxEip4844, eip7702::TxEip7702,
+                legacy::TxLegacy,
+            },
+            withdrawal::Withdrawal,
+        },
+        test_utils::UnusedBits,
+        validate_bitflag_backwards_compat,
+    };
+
+    #[test]
+    fn validate_bitflag_backwards_compat() {
+        // In case of failure, refer to the documentation of the
+        // [`validate_bitflag_backwards_compat`] macro for detailed instructions on handling
+        // it.
+        validate_bitflag_backwards_compat!(Header, UnusedBits::Zero);
+        validate_bitflag_backwards_compat!(TxEip2930, UnusedBits::Zero);
+        validate_bitflag_backwards_compat!(StorageEntries, UnusedBits::Zero);
+        validate_bitflag_backwards_compat!(StorageEntry, UnusedBits::Zero);
+
+        validate_bitflag_backwards_compat!(Authorization, UnusedBits::NotZero);
+        validate_bitflag_backwards_compat!(GenesisAccountRef<'_>, UnusedBits::NotZero);
+        validate_bitflag_backwards_compat!(GenesisAccount, UnusedBits::NotZero);
+        validate_bitflag_backwards_compat!(TxEip1559, UnusedBits::NotZero);
+        validate_bitflag_backwards_compat!(TxEip4844, UnusedBits::NotZero);
+        validate_bitflag_backwards_compat!(TxEip7702, UnusedBits::NotZero);
+        validate_bitflag_backwards_compat!(TxLegacy, UnusedBits::NotZero);
+        validate_bitflag_backwards_compat!(Withdrawal, UnusedBits::NotZero);
+    }
+}

crates/storage/codecs/src/alloy/transaction/mod.rs

@@ -1,10 +1,10 @@
-mod eip1559;
-mod eip2930;
-mod eip4844;
-mod eip7702;
-mod legacy;
+pub(crate) mod eip1559;
+pub(crate) mod eip2930;
+pub(crate) mod eip4844;
+pub(crate) mod eip7702;
+pub(crate) mod legacy;
 #[cfg(feature = "optimism")]
-mod optimism;
+pub(crate) mod optimism;
 
 #[cfg(test)]
 mod tests {

crates/storage/codecs/src/alloy/withdrawal.rs

@@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize};
 #[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, Compact)]
 #[cfg_attr(test, derive(arbitrary::Arbitrary))]
 #[add_arbitrary_tests(compact)]
-struct Withdrawal {
+pub(crate) struct Withdrawal {
     /// Monotonically increasing identifier issued by consensus layer.
     index: u64,
     /// Index of validator associated with withdrawal.

crates/storage/codecs/src/lib.rs

@@ -28,6 +28,9 @@ use alloc::vec::Vec;
 #[cfg(any(test, feature = "alloy"))]
 mod alloy;
 
+#[cfg(any(test, feature = "test-utils"))]
+pub mod test_utils;
+
 /// Trait that implements the `Compact` codec.
 ///
 /// When deriving the trait for custom structs, be aware of certain limitations/recommendations:

crates/storage/codecs/src/test_utils.rs

@@ -0,0 +1,81 @@
+//! Test utilities for `Compact` derive macro
+
+/// Macro to ensure that derived `Compact` types can be extended with new fields while maintaining
+/// backwards compatibility.
+///
+/// Verifies that the unused bits in the bitflag struct remain as expected: `Zero` or `NotZero`. For
+/// more on bitflag struct: [`reth_codecs_derive::Compact`].
+///
+/// Possible failures:
+/// ### 1. `NotZero` -> `Zero`
+///    This wouldn't allow new fields to be added in the future. Instead, the new field of `T`
+/// should be `Option<TExtension>`  to allow for new fields. The new user field should be included
+/// in `TExtension` type. **Only then, update the test to expect `Zero` for `T` and
+/// add a new test for `TExtension`.**
+///
+/// **Goal:**
+///
+///    ```rust,ignore
+/// {
+///    struct T {
+///        // ... other fields
+///        ext: Option<TExtension>
+///    }
+///    
+///    // Use an extension type for new fields:
+///    struct TExtension {
+///        new_field_b: Option<u8>,
+///    }
+///    
+///    // Change tests
+///    validate_bitflag_backwards_compat!(T, UnusedBits::Zero);
+///    validate_bitflag_backwards_compat!(TExtension, UnusedBits::NotZero);
+/// }   
+/// ```
+/// 
+/// ### 2. `Zero` -> `NotZero`
+/// If it becomes `NotZero`, it would break backwards compatibility, so there is not an action item,
+/// and should be handled with care in a case by case scenario.
+#[macro_export]
+macro_rules! validate_bitflag_backwards_compat {
+    ($type:ty, $expected_unused_bits:expr) => {
+        let actual_unused_bits = <$type>::bitflag_unused_bits();
+
+        match $expected_unused_bits {
+            UnusedBits::NotZero => {
+                assert_ne!(
+                    actual_unused_bits,
+                    0,
+                    "Assertion failed: `bitflag_unused_bits` for type `{}` unexpectedly went from non-zero to zero!",
+                    stringify!($type)
+                );
+            }
+            UnusedBits::Zero => {
+                assert_eq!(
+                    actual_unused_bits,
+                    0,
+                    "Assertion failed: `bitflag_unused_bits` for type `{}` unexpectedly went from zero to non-zero!",
+                    stringify!($type)
+                );
+            }
+        }
+    };
+}
+
+/// Whether there are zero or more unused bits on `Compact` bitflag struct.
+///
+/// To be used with [`validate_bitflag_backwards_compat`].
+#[derive(Debug)]
+pub enum UnusedBits {
+    /// Zero bits available for a new field.
+    Zero,
+    /// Bits available for a new field.
+    NotZero,
+}
+
+impl UnusedBits {
+    /// Returns true if the variant is [`Self::NotZero`].
+    pub const fn not_zero(&self) -> bool {
+        matches!(self, Self::NotZero)
+    }
+}