From c0020f8410aec599ef13ed359cbbbb0d5836d4ad Mon Sep 17 00:00:00 2001 From: refcell Date: Wed, 24 Apr 2024 19:27:35 -0400 Subject: [PATCH] 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 --------- Co-authored-by: clabby --- crates/derive/src/sources/blobs.rs | 1 - crates/derive/src/sources/calldata.rs | 1 - .../src/traits/test_utils/data_sources.rs | 7 ++- crates/derive/src/types/batch/mod.rs | 1 - .../src/types/batch/span_batch/batch.rs | 53 +++++++++++-------- 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/crates/derive/src/sources/blobs.rs b/crates/derive/src/sources/blobs.rs index 869d974ea..245168dff 100644 --- 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 where F: ChainProvider + Send, diff --git a/crates/derive/src/sources/calldata.rs b/crates/derive/src/sources/calldata.rs index c88ffdbe5..f9a4000e5 100644 --- 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 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 index 1fb473f3e..5540f1a40 100644 --- 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, + /// Short circuit the block return to be the first block. + pub short_circuit: bool, /// Payloads pub payloads: Vec, /// System configs @@ -29,13 +31,16 @@ impl MockBlockFetcher { payloads: Vec, system_configs: HashMap, ) -> 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 { + 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 index 877fae810..58541377f 100644 --- 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 index ac50a064f..a4b45860c 100644 --- 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 { 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 {