fix(derive): Small Fixes and Span Batch Validation Fix (#139)

* fix(derive): small fixes across the codebase * fix(batch): span batch validation * Update batch.rs Co-authored-by: clabby <[email protected]> --------- Co-authored-by: clabby <[email protected]>
op-rs · Apr 24, 2024 · c0020f8 · c0020f8
1 parent 7e8f8e0
commit c0020f8
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 26 deletions.
diff --git a/crates/derive/src/sources/blobs.rs b/crates/derive/src/sources/blobs.rs
@@ -13,7 +13,6 @@ use tracing::warn;
 
 /// A data iterator that reads from a blob.
 #[derive(Debug, Clone)]
-#[allow(dead_code)]
 pub struct BlobSource<F, B>
 where
     F: ChainProvider + Send,

diff --git a/crates/derive/src/sources/calldata.rs b/crates/derive/src/sources/calldata.rs
@@ -11,7 +11,6 @@ use async_trait::async_trait;
 
 /// A data iterator that reads from calldata.
 #[derive(Debug, Clone)]
-#[allow(dead_code)]
 pub struct CalldataSource<CP>
 where
     CP: ChainProvider + Send,

diff --git a/crates/derive/src/traits/test_utils/data_sources.rs b/crates/derive/src/traits/test_utils/data_sources.rs
@@ -16,6 +16,8 @@ use hashbrown::HashMap;
 pub struct MockBlockFetcher {
     /// Blocks
     pub blocks: Vec<L2BlockInfo>,
+    /// Short circuit the block return to be the first block.
+    pub short_circuit: bool,
     /// Payloads
     pub payloads: Vec<L2ExecutionPayloadEnvelope>,
     /// System configs
@@ -29,13 +31,16 @@ impl MockBlockFetcher {
         payloads: Vec<L2ExecutionPayloadEnvelope>,
         system_configs: HashMap<u64, SystemConfig>,
     ) -> Self {
-        Self { blocks, payloads, system_configs }
+        Self { blocks, short_circuit: false, payloads, system_configs }
     }
 }
 
 #[async_trait]
 impl L2ChainProvider for MockBlockFetcher {
     async fn l2_block_info_by_number(&mut self, number: u64) -> Result<L2BlockInfo> {
+        if self.short_circuit {
+            return self.blocks.first().copied().ok_or_else(|| anyhow::anyhow!("Block not found"));
+        }
         self.blocks
             .iter()
             .find(|b| b.block_info.number == number)

diff --git a/crates/derive/src/types/batch/mod.rs b/crates/derive/src/types/batch/mod.rs
@@ -62,7 +62,6 @@ impl BatchWithInclusionBlock {
 
 /// A Batch.
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[allow(clippy::large_enum_variant)]
 pub enum Batch {
     /// A single batch
     Single(SingleBatch),

diff --git a/crates/derive/src/types/batch/span_batch/batch.rs b/crates/derive/src/types/batch/span_batch/batch.rs
@@ -1,7 +1,5 @@
 //! The Span Batch Type
 
-#![allow(unused)]
-
 use super::{SpanBatchError, SpanBatchTransactions};
 use crate::{
     traits::L2ChainProvider,
@@ -10,7 +8,7 @@ use crate::{
         SpanBatchBits, SpanBatchElement, SpanBatchPayload, SpanBatchPrefix,
     },
 };
-use alloc::{vec, vec::Vec};
+use alloc::vec::Vec;
 use alloy_primitives::FixedBytes;
 use op_alloy_consensus::OpTxType;
 use tracing::{info, warn};
@@ -115,7 +113,7 @@ impl SpanBatch {
         // If the span batch does not overlap the current safe chain, parent block should be the L2
         // safe head.
         let mut parent_num = l2_safe_head.block_info.number;
-        let parent_block = l2_safe_head;
+        let mut parent_block = l2_safe_head;
         if self.timestamp() < next_timestamp {
             if self.timestamp() > l2_safe_head.block_info.timestamp {
                 // Batch timestamp cannot be between safe head and next timestamp.
@@ -129,7 +127,7 @@ impl SpanBatch {
             parent_num = l2_safe_head.block_info.number -
                 (l2_safe_head.block_info.timestamp - self.timestamp()) / cfg.block_time -
                 1;
-            let parent_block = match fetcher.l2_block_info_by_number(parent_num).await {
+            parent_block = match fetcher.l2_block_info_by_number(parent_num).await {
                 Ok(block) => block,
                 Err(e) => {
                     warn!("failed to fetch L2 block number {parent_num}: {e}");
@@ -321,9 +319,9 @@ impl SpanBatch {
     /// Converts the span batch to a raw span batch.
     pub fn to_raw_span_batch(
         &self,
-        origin_changed_bit: u8,
+        _origin_changed_bit: u8,
         genesis_timestamp: u64,
-        chain_id: u64,
+        _chain_id: u64,
     ) -> Result<RawSpanBatch, SpanBatchError> {
         if self.batches.is_empty() {
             return Err(SpanBatchError::EmptySpanBatch);
@@ -365,13 +363,13 @@ impl SpanBatch {
             let origin_epoch_hash = l1_origins[origin_index..l1_origins.len()]
                 .iter()
                 .enumerate()
-                .find(|(i, origin)| origin.timestamp == batch.timestamp)
+                .find(|(_, origin)| origin.timestamp == batch.timestamp)
                 .map(|(i, origin)| {
                     origin_index = i;
                     origin.hash
                 })
                 .ok_or(SpanBatchError::MissingL1Origin)?;
-            let mut single_batch = SingleBatch {
+            let single_batch = SingleBatch {
                 epoch_num: batch.epoch_num,
                 epoch_hash: origin_epoch_hash,
                 timestamp: batch.timestamp,
@@ -437,6 +435,7 @@ mod tests {
         traits::test_utils::MockBlockFetcher,
         types::{BlockID, Genesis, L2ExecutionPayload, L2ExecutionPayloadEnvelope, RawTransaction},
     };
+    use alloc::vec;
     use alloy_primitives::{b256, Bytes, B256};
     use op_alloy_consensus::OpTxType;
     use tracing::Level;
@@ -446,7 +445,7 @@ mod tests {
     fn test_timestamp() {
         let timestamp = 10;
         let first_element = SpanBatchElement { timestamp, ..Default::default() };
-        let mut batch =
+        let batch =
             SpanBatch { batches: vec![first_element, Default::default()], ..Default::default() };
         assert_eq!(batch.timestamp(), timestamp);
     }
@@ -455,7 +454,7 @@ mod tests {
     fn test_starting_epoch_num() {
         let epoch_num = 10;
         let first_element = SpanBatchElement { epoch_num, ..Default::default() };
-        let mut batch =
+        let batch =
             SpanBatch { batches: vec![first_element, Default::default()], ..Default::default() };
         assert_eq!(batch.starting_epoch_num(), epoch_num);
     }
@@ -730,10 +729,12 @@ mod tests {
         };
         let inclusion_block = BlockInfo::default();
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 41, timestamp: 10, ..Default::default() },
+            l1_origin: BlockID { number: 9, ..Default::default() },
             ..Default::default()
         };
         let mut fetcher = MockBlockFetcher { blocks: vec![l2_block], ..Default::default() };
+        fetcher.short_circuit = true;
         let first = SpanBatchElement { epoch_num: 10, timestamp: 10, ..Default::default() };
         let second = SpanBatchElement { epoch_num: 11, timestamp: 20, ..Default::default() };
         let batch = SpanBatch {
@@ -769,7 +770,7 @@ mod tests {
         };
         let inclusion_block = BlockInfo { number: 50, ..Default::default() };
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 40, parent_hash, timestamp: 10, ..Default::default() },
             ..Default::default()
         };
         let mut fetcher = MockBlockFetcher { blocks: vec![l2_block], ..Default::default() };
@@ -812,7 +813,8 @@ mod tests {
         };
         let inclusion_block = BlockInfo { number: 50, ..Default::default() };
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 40, parent_hash, timestamp: 10, ..Default::default() },
+            l1_origin: BlockID { number: 8, ..Default::default() },
             ..Default::default()
         };
         let mut fetcher = MockBlockFetcher { blocks: vec![l2_block], ..Default::default() };
@@ -862,7 +864,8 @@ mod tests {
         };
         let inclusion_block = BlockInfo { number: 50, ..Default::default() };
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 40, timestamp: 10, parent_hash, ..Default::default() },
+            l1_origin: BlockID { number: 9, ..Default::default() },
             ..Default::default()
         };
         let mut fetcher = MockBlockFetcher { blocks: vec![l2_block], ..Default::default() };
@@ -911,7 +914,8 @@ mod tests {
         };
         let inclusion_block = BlockInfo { number: 50, ..Default::default() };
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 40, timestamp: 10, parent_hash, ..Default::default() },
+            l1_origin: BlockID { number: 9, ..Default::default() },
             ..Default::default()
         };
         let mut fetcher = MockBlockFetcher { blocks: vec![l2_block], ..Default::default() };
@@ -957,7 +961,8 @@ mod tests {
         };
         let inclusion_block = BlockInfo { number: 50, ..Default::default() };
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 40, timestamp: 10, parent_hash, ..Default::default() },
+            l1_origin: BlockID { number: 14, ..Default::default() },
             ..Default::default()
         };
         let mut fetcher = MockBlockFetcher { blocks: vec![l2_block], ..Default::default() };
@@ -977,7 +982,7 @@ mod tests {
         assert_eq!(logs.len(), 1);
         let str = alloc::format!(
             "dropped batch, epoch is too old, minimum: {}",
-            l2_safe_head.block_info.id(),
+            l2_block.block_info.id(),
         );
         assert!(logs[0].contains(&str));
     }
@@ -1297,7 +1302,8 @@ mod tests {
         };
         let inclusion_block = BlockInfo { number: 50, ..Default::default() };
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 40, timestamp: 10, parent_hash, ..Default::default() },
+            l1_origin: BlockID { number: 9, ..Default::default() },
             ..Default::default()
         };
         let mut fetcher = MockBlockFetcher { blocks: vec![l2_block], ..Default::default() };
@@ -1347,7 +1353,8 @@ mod tests {
         };
         let inclusion_block = BlockInfo { number: 50, ..Default::default() };
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 40, timestamp: 10, parent_hash, ..Default::default() },
+            l1_origin: BlockID { number: 9, ..Default::default() },
             ..Default::default()
         };
         let payload = L2ExecutionPayloadEnvelope {
@@ -1411,7 +1418,8 @@ mod tests {
         };
         let inclusion_block = BlockInfo { number: 50, ..Default::default() };
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 40, parent_hash, timestamp: 10, ..Default::default() },
+            l1_origin: BlockID { number: 9, ..Default::default() },
             ..Default::default()
         };
         let payload = L2ExecutionPayloadEnvelope {
@@ -1476,7 +1484,8 @@ mod tests {
         };
         let inclusion_block = BlockInfo { number: 50, ..Default::default() };
         let l2_block = L2BlockInfo {
-            block_info: BlockInfo { number: 40, ..Default::default() },
+            block_info: BlockInfo { number: 40, parent_hash, timestamp: 10, ..Default::default() },
+            l1_origin: BlockID { number: 9, ..Default::default() },
             ..Default::default()
         };
         let payload = L2ExecutionPayloadEnvelope {