chore(protocol): Rejigger L1 Block Info Tx Calldata Parsing (#235)

### Description

Re-jiggers the `L1BlockInfoTx` calldata parsing to use concrete error
types and direct slice referencing with safety.

This avoids missing test coverage over errors that cannot be thrown due
to bound checks.

Closes #229

crates/protocol/src/info/bedrock.rs

@@ -1,6 +1,6 @@
 //! Contains bedrock-specific L1 block info types.
 
-use alloc::{format, string::ToString, vec::Vec};
+use alloc::vec::Vec;
 use alloy_primitives::{Address, Bytes, B256, U256};
 
 use crate::DecodeError;
@@ -68,31 +68,34 @@ impl L1BlockInfoBedrock {
     /// Decodes the [L1BlockInfoBedrock] object from ethereum transaction calldata.
     pub fn decode_calldata(r: &[u8]) -> Result<Self, DecodeError> {
         if r.len() != Self::L1_INFO_TX_LEN {
-            return Err(DecodeError::InvalidLength(format!(
-                "Invalid calldata length for Bedrock L1 info transaction, expected {}, got {}",
-                Self::L1_INFO_TX_LEN,
-                r.len()
-            )));
+            return Err(DecodeError::InvalidBedrockLength(Self::L1_INFO_TX_LEN, r.len()));
         }
 
-        let number = u64::from_be_bytes(
-            r[28..36]
-                .try_into()
-                .map_err(|_| DecodeError::ParseError("Conversion error for number".to_string()))?,
-        );
-        let time = u64::from_be_bytes(
-            r[60..68]
-                .try_into()
-                .map_err(|_| DecodeError::ParseError("Conversion error for time".to_string()))?,
-        );
-        let base_fee =
-            u64::from_be_bytes(r[92..100].try_into().map_err(|_| {
-                DecodeError::ParseError("Conversion error for base fee".to_string())
-            })?);
+        // SAFETY: For all below slice operations, the full
+        //         length is validated above to be `260`.
+
+        // SAFETY: 8 bytes are copied directly into the array
+        let mut number = [0u8; 8];
+        number.copy_from_slice(&r[28..36]);
+        let number = u64::from_be_bytes(number);
+
+        // SAFETY: 8 bytes are copied directly into the array
+        let mut time = [0u8; 8];
+        time.copy_from_slice(&r[60..68]);
+        let time = u64::from_be_bytes(time);
+
+        // SAFETY: 8 bytes are copied directly into the array
+        let mut base_fee = [0u8; 8];
+        base_fee.copy_from_slice(&r[92..100]);
+        let base_fee = u64::from_be_bytes(base_fee);
+
         let block_hash = B256::from_slice(r[100..132].as_ref());
-        let sequence_number = u64::from_be_bytes(r[156..164].try_into().map_err(|_| {
-            DecodeError::ParseError("Conversion error for sequence number".to_string())
-        })?);
+
+        // SAFETY: 8 bytes are copied directly into the array
+        let mut sequence_number = [0u8; 8];
+        sequence_number.copy_from_slice(&r[156..164]);
+        let sequence_number = u64::from_be_bytes(sequence_number);
+
         let batcher_address = Address::from_slice(r[176..196].as_ref());
         let l1_fee_overhead = U256::from_be_slice(r[196..228].as_ref());
         let l1_fee_scalar = U256::from_be_slice(r[228..260].as_ref());
@@ -109,3 +112,35 @@ impl L1BlockInfoBedrock {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_decode_calldata_bedrock_invalid_length() {
+        let r = vec![0u8; 1];
+        assert_eq!(
+            L1BlockInfoBedrock::decode_calldata(&r),
+            Err(DecodeError::InvalidBedrockLength(L1BlockInfoBedrock::L1_INFO_TX_LEN, r.len(),))
+        );
+    }
+
+    #[test]
+    fn test_l1_block_info_bedrock_roundtrip_calldata_encoding() {
+        let info = L1BlockInfoBedrock {
+            number: 1,
+            time: 2,
+            base_fee: 3,
+            block_hash: B256::from([4u8; 32]),
+            sequence_number: 5,
+            batcher_address: Address::from([6u8; 20]),
+            l1_fee_overhead: U256::from(7),
+            l1_fee_scalar: U256::from(8),
+        };
+
+        let calldata = info.encode_calldata();
+        let decoded_info = L1BlockInfoBedrock::decode_calldata(&calldata).unwrap();
+        assert_eq!(info, decoded_info);
+    }
+}

crates/protocol/src/info/ecotone.rs

@@ -1,7 +1,7 @@
 //! Contains ecotone-specific L1 block info types.
 
 use crate::DecodeError;
-use alloc::{format, string::ToString, vec::Vec};
+use alloc::vec::Vec;
 use alloy_primitives::{Address, Bytes, B256, U256};
 
 /// Represents the fields within an Ecotone L1 block info transaction.
@@ -84,41 +84,53 @@ impl L1BlockInfoEcotone {
     /// Decodes the [L1BlockInfoEcotone] object from ethereum transaction calldata.
     pub fn decode_calldata(r: &[u8]) -> Result<Self, DecodeError> {
         if r.len() != Self::L1_INFO_TX_LEN {
-            return Err(DecodeError::InvalidLength(format!(
-                "Invalid calldata length for Ecotone L1 info transaction, expected {}, got {}",
-                Self::L1_INFO_TX_LEN,
-                r.len()
-            )));
+            return Err(DecodeError::InvalidEcotoneLength(Self::L1_INFO_TX_LEN, r.len()));
         }
-        let base_fee_scalar = u32::from_be_bytes(r[4..8].try_into().map_err(|_| {
-            DecodeError::ParseError("Conversion error for base fee scalar".to_string())
-        })?);
-        let blob_base_fee_scalar = u32::from_be_bytes(r[8..12].try_into().map_err(|_| {
-            DecodeError::ParseError("Conversion error for blob base fee scalar".to_string())
-        })?);
-        let sequence_number = u64::from_be_bytes(r[12..20].try_into().map_err(|_| {
-            DecodeError::ParseError("Conversion error for sequence number".to_string())
-        })?);
-        let timestamp =
-            u64::from_be_bytes(r[20..28].try_into().map_err(|_| {
-                DecodeError::ParseError("Conversion error for timestamp".to_string())
-            })?);
-        let l1_block_number = u64::from_be_bytes(r[28..36].try_into().map_err(|_| {
-            DecodeError::ParseError("Conversion error for L1 block number".to_string())
-        })?);
-        let base_fee =
-            u64::from_be_bytes(r[60..68].try_into().map_err(|_| {
-                DecodeError::ParseError("Conversion error for base fee".to_string())
-            })?);
-        let blob_base_fee = u128::from_be_bytes(r[84..100].try_into().map_err(|_| {
-            DecodeError::ParseError("Conversion error for blob base fee".to_string())
-        })?);
+
+        // SAFETY: For all below slice operations, the full
+        //         length is validated above to be `164`.
+
+        // SAFETY: 4 bytes are copied directly into the array
+        let mut base_fee_scalar = [0u8; 4];
+        base_fee_scalar.copy_from_slice(&r[4..8]);
+        let base_fee_scalar = u32::from_be_bytes(base_fee_scalar);
+
+        // SAFETY: 4 bytes are copied directly into the array
+        let mut blob_base_fee_scalar = [0u8; 4];
+        blob_base_fee_scalar.copy_from_slice(&r[8..12]);
+        let blob_base_fee_scalar = u32::from_be_bytes(blob_base_fee_scalar);
+
+        // SAFETY: 8 bytes are copied directly into the array
+        let mut sequence_number = [0u8; 8];
+        sequence_number.copy_from_slice(&r[12..20]);
+        let sequence_number = u64::from_be_bytes(sequence_number);
+
+        // SAFETY: 8 bytes are copied directly into the array
+        let mut time = [0u8; 8];
+        time.copy_from_slice(&r[20..28]);
+        let time = u64::from_be_bytes(time);
+
+        // SAFETY: 8 bytes are copied directly into the array
+        let mut number = [0u8; 8];
+        number.copy_from_slice(&r[28..36]);
+        let number = u64::from_be_bytes(number);
+
+        // SAFETY: 8 bytes are copied directly into the array
+        let mut base_fee = [0u8; 8];
+        base_fee.copy_from_slice(&r[60..68]);
+        let base_fee = u64::from_be_bytes(base_fee);
+
+        // SAFETY: 16 bytes are copied directly into the array
+        let mut blob_base_fee = [0u8; 16];
+        blob_base_fee.copy_from_slice(&r[84..100]);
+        let blob_base_fee = u128::from_be_bytes(blob_base_fee);
+
         let block_hash = B256::from_slice(r[100..132].as_ref());
         let batcher_address = Address::from_slice(r[144..164].as_ref());
 
         Ok(Self {
-            number: l1_block_number,
-            time: timestamp,
+            number,
+            time,
             base_fee,
             block_hash,
             sequence_number,
@@ -135,3 +147,38 @@ impl L1BlockInfoEcotone {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_decode_calldata_ecotone_invalid_length() {
+        let r = vec![0u8; 1];
+        assert_eq!(
+            L1BlockInfoEcotone::decode_calldata(&r),
+            Err(DecodeError::InvalidEcotoneLength(L1BlockInfoEcotone::L1_INFO_TX_LEN, r.len(),))
+        );
+    }
+
+    #[test]
+    fn test_l1_block_info_ecotone_roundtrip_calldata_encoding() {
+        let info = L1BlockInfoEcotone {
+            number: 1,
+            time: 2,
+            base_fee: 3,
+            block_hash: B256::from([4u8; 32]),
+            sequence_number: 5,
+            batcher_address: Address::from([6u8; 20]),
+            blob_base_fee: 7,
+            blob_base_fee_scalar: 8,
+            base_fee_scalar: 9,
+            empty_scalars: false,
+            l1_fee_overhead: U256::ZERO,
+        };
+
+        let calldata = info.encode_calldata();
+        let decoded_info = L1BlockInfoEcotone::decode_calldata(&calldata).unwrap();
+        assert_eq!(info, decoded_info);
+    }
+}

crates/protocol/src/info/errors.rs

@@ -1,7 +1,5 @@
 //! Contains error types specific to the L1 block info transaction.
 
-use alloc::string::String;
-
 /// An error type for parsing L1 block info transactions.
 #[derive(Debug, thiserror::Error, PartialEq, Eq, Copy, Clone)]
 pub enum BlockInfoError {
@@ -28,13 +26,26 @@ pub enum BlockInfoError {
 /// An error decoding an L1 block info transaction.
 #[derive(Debug, Eq, PartialEq, Clone, thiserror::Error)]
 pub enum DecodeError {
+    /// Missing selector bytes.
+    #[error("The provided calldata is too short, missing the 4 selector bytes")]
+    MissingSelector,
     /// Invalid selector for the L1 info transaction.
     #[error("Invalid L1 info transaction selector")]
     InvalidSelector,
-    /// Parse error for the L1 info transaction.
-    #[error("Parse error: {0}")]
-    ParseError(String),
-    /// Invalid length for the L1 info transaction.
-    #[error("Invalid data length: {0}")]
-    InvalidLength(String),
+    /// Invalid length for the L1 info bedrock transaction.
+    /// Arguments are the expected length and the actual length.
+    #[error("Invalid bedrock data length. Expected {0}, got {1}")]
+    InvalidBedrockLength(usize, usize),
+    /// Invalid length for the L1 info ecotone transaction.
+    /// Arguments are the expected length and the actual length.
+    #[error("Invalid ecotone data length. Expected {0}, got {1}")]
+    InvalidEcotoneLength(usize, usize),
+    /// Invalid length for the L1 info isthmus transaction.
+    /// Arguments are the expected length and the actual length.
+    #[error("Invalid isthmus data length. Expected {0}, got {1}")]
+    InvalidIsthmusLength(usize, usize),
+    /// Invalid length for the L1 info interop transaction.
+    /// Arguments are the expected length and the actual length.
+    #[error("Invalid interop data length. Expected {0}, got {1}")]
+    InvalidInteropLength(usize, usize),
 }