From 1dab12b33f42a295ac8938f221e6f3ae923d22a2 Mon Sep 17 00:00:00 2001 From: refcell Date: Tue, 24 Sep 2024 19:02:40 -0400 Subject: [PATCH 1/2] fix: hoist attributes builder --- bin/client/src/l1/driver.rs | 3 +- .../builder.rs => attributes/mod.rs} | 150 +++++++++++++++++- crates/derive/src/lib.rs | 1 + crates/derive/src/online/mod.rs | 2 +- crates/derive/src/params.rs | 6 +- crates/derive/src/stages/attributes_queue.rs | 6 - .../src/stages/attributes_queue/deposits.rs | 145 ----------------- crates/derive/src/stages/mod.rs | 2 +- 8 files changed, 149 insertions(+), 166 deletions(-) rename crates/derive/src/{stages/attributes_queue/builder.rs => attributes/mod.rs} (76%) delete mode 100644 crates/derive/src/stages/attributes_queue/deposits.rs diff --git a/bin/client/src/l1/driver.rs b/bin/client/src/l1/driver.rs index d79661b268..70d4db4c7a 100644 --- a/bin/client/src/l1/driver.rs +++ b/bin/client/src/l1/driver.rs @@ -11,12 +11,13 @@ use alloy_primitives::B256; use anyhow::{anyhow, Result}; use core::fmt::Debug; use kona_derive::{ + attributes::StatefulAttributesBuilder, errors::PipelineErrorKind, pipeline::{DerivationPipeline, Pipeline, PipelineBuilder, StepResult}, sources::EthereumDataSource, stages::{ AttributesQueue, BatchQueue, ChannelBank, ChannelReader, FrameQueue, L1Retrieval, - L1Traversal, StatefulAttributesBuilder, + L1Traversal, }, traits::{BlobProvider, ChainProvider, L2ChainProvider, OriginProvider}, }; diff --git a/crates/derive/src/stages/attributes_queue/builder.rs b/crates/derive/src/attributes/mod.rs similarity index 76% rename from crates/derive/src/stages/attributes_queue/builder.rs rename to crates/derive/src/attributes/mod.rs index 38d818393c..c90c683ee2 100644 --- a/crates/derive/src/stages/attributes_queue/builder.rs +++ b/crates/derive/src/attributes/mod.rs @@ -1,22 +1,27 @@ //! The [`AttributesBuilder`] and it's default implementation. -use super::derive_deposits; use crate::{ - errors::{BuilderError, PipelineError, PipelineErrorKind, PipelineResult}, - params::SEQUENCER_FEE_VAULT_ADDRESS, + errors::{ + BuilderError, PipelineEncodingError, PipelineError, PipelineErrorKind, PipelineResult, + }, traits::{AttributesBuilder, ChainProvider, L2ChainProvider}, }; use alloc::{boxed::Box, fmt::Debug, string::ToString, sync::Arc, vec, vec::Vec}; +use alloy_consensus::{Eip658Value, Receipt}; use alloy_eips::{eip2718::Encodable2718, BlockNumHash}; -use alloy_primitives::Bytes; +use alloy_primitives::{address, Address, Bytes, B256}; use alloy_rlp::Encodable; use alloy_rpc_types_engine::PayloadAttributes; use async_trait::async_trait; use op_alloy_consensus::Hardforks; use op_alloy_genesis::RollupConfig; -use op_alloy_protocol::{L1BlockInfoTx, L2BlockInfo}; +use op_alloy_protocol::{decode_deposit, L1BlockInfoTx, L2BlockInfo, DEPOSIT_EVENT_ABI_HASH}; use op_alloy_rpc_types_engine::OptimismPayloadAttributes; +/// The sequencer fee vault address. +pub const SEQUENCER_FEE_VAULT_ADDRESS: Address = + address!("4200000000000000000000000000000000000011"); + /// A stateful implementation of the [AttributesBuilder]. #[derive(Debug, Default)] pub struct StatefulAttributesBuilder @@ -190,6 +195,38 @@ where } } +/// Derive deposits for transaction receipts. +/// +/// Successful deposits must be emitted by the deposit contract and have the correct event +/// signature. So the receipt address must equal the specified deposit contract and the first topic +/// must be the [DEPOSIT_EVENT_ABI_HASH]. +async fn derive_deposits( + block_hash: B256, + receipts: &[Receipt], + deposit_contract: Address, +) -> Result, PipelineEncodingError> { + let mut global_index = 0; + let mut res = Vec::new(); + for r in receipts.iter() { + if Eip658Value::Eip658(false) == r.status { + continue; + } + for l in r.logs.iter() { + let curr_index = global_index; + global_index += 1; + if !l.data.topics().first().map_or(false, |i| *i == DEPOSIT_EVENT_ABI_HASH) { + continue; + } + if l.address != deposit_contract { + continue; + } + let decoded = decode_deposit(block_hash, curr_index, l)?; + res.push(decoded); + } + } + Ok(res) +} + #[cfg(test)] mod tests { use super::*; @@ -197,10 +234,109 @@ mod tests { errors::ResetError, stages::test_utils::MockSystemConfigL2Fetcher, traits::test_utils::TestChainProvider, }; + use alloc::vec; use alloy_consensus::Header; - use alloy_primitives::B256; + use alloy_primitives::{Log, LogData, B256, U256, U64}; use op_alloy_genesis::SystemConfig; - use op_alloy_protocol::BlockInfo; + use op_alloy_protocol::{BlockInfo, DepositError}; + + fn generate_valid_log() -> Log { + let deposit_contract = address!("1111111111111111111111111111111111111111"); + let mut data = vec![0u8; 192]; + let offset: [u8; 8] = U64::from(32).to_be_bytes(); + data[24..32].copy_from_slice(&offset); + let len: [u8; 8] = U64::from(128).to_be_bytes(); + data[56..64].copy_from_slice(&len); + // Copy the u128 mint value + let mint: [u8; 16] = 10_u128.to_be_bytes(); + data[80..96].copy_from_slice(&mint); + // Copy the tx value + let value: [u8; 32] = U256::from(100).to_be_bytes(); + data[96..128].copy_from_slice(&value); + // Copy the gas limit + let gas: [u8; 8] = 1000_u64.to_be_bytes(); + data[128..136].copy_from_slice(&gas); + // Copy the isCreation flag + data[136] = 1; + let from = address!("2222222222222222222222222222222222222222"); + let mut from_bytes = vec![0u8; 32]; + from_bytes[12..32].copy_from_slice(from.as_slice()); + let to = address!("3333333333333333333333333333333333333333"); + let mut to_bytes = vec![0u8; 32]; + to_bytes[12..32].copy_from_slice(to.as_slice()); + Log { + address: deposit_contract, + data: LogData::new_unchecked( + vec![ + DEPOSIT_EVENT_ABI_HASH, + B256::from_slice(&from_bytes), + B256::from_slice(&to_bytes), + B256::default(), + ], + Bytes::from(data), + ), + } + } + + fn generate_valid_receipt() -> Receipt { + let mut bad_dest_log = generate_valid_log(); + bad_dest_log.data.topics_mut()[1] = B256::default(); + let mut invalid_topic_log = generate_valid_log(); + invalid_topic_log.data.topics_mut()[0] = B256::default(); + Receipt { + status: Eip658Value::Eip658(true), + logs: vec![generate_valid_log(), bad_dest_log, invalid_topic_log], + ..Default::default() + } + } + + #[tokio::test] + async fn test_derive_deposits_empty() { + let receipts = vec![]; + let deposit_contract = Address::default(); + let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; + assert!(result.unwrap().is_empty()); + } + + #[tokio::test] + async fn test_derive_deposits_non_deposit_events_filtered_out() { + let deposit_contract = address!("1111111111111111111111111111111111111111"); + let mut invalid = generate_valid_receipt(); + invalid.logs[0].data = LogData::new_unchecked(vec![], Bytes::default()); + let receipts = vec![generate_valid_receipt(), generate_valid_receipt(), invalid]; + let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; + assert_eq!(result.unwrap().len(), 5); + } + + #[tokio::test] + async fn test_derive_deposits_non_deposit_contract_addr() { + let deposit_contract = address!("1111111111111111111111111111111111111111"); + let mut invalid = generate_valid_receipt(); + invalid.logs[0].address = Address::default(); + let receipts = vec![generate_valid_receipt(), generate_valid_receipt(), invalid]; + let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; + assert_eq!(result.unwrap().len(), 5); + } + + #[tokio::test] + async fn test_derive_deposits_decoding_errors() { + let deposit_contract = address!("1111111111111111111111111111111111111111"); + let mut invalid = generate_valid_receipt(); + invalid.logs[0].data = + LogData::new_unchecked(vec![DEPOSIT_EVENT_ABI_HASH], Bytes::default()); + let receipts = vec![generate_valid_receipt(), generate_valid_receipt(), invalid]; + let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; + let downcasted = result.unwrap_err(); + assert_eq!(downcasted, DepositError::UnexpectedTopicsLen(1).into()); + } + + #[tokio::test] + async fn test_derive_deposits_succeeds() { + let deposit_contract = address!("1111111111111111111111111111111111111111"); + let receipts = vec![generate_valid_receipt(), generate_valid_receipt()]; + let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; + assert_eq!(result.unwrap().len(), 4); + } #[tokio::test] async fn test_prepare_payload_block_mismatch_epoch_reset() { diff --git a/crates/derive/src/lib.rs b/crates/derive/src/lib.rs index 8d01260be9..0727fcf915 100644 --- a/crates/derive/src/lib.rs +++ b/crates/derive/src/lib.rs @@ -9,6 +9,7 @@ extern crate alloc; mod macros; +pub mod attributes; pub mod batch; pub mod block; pub mod errors; diff --git a/crates/derive/src/online/mod.rs b/crates/derive/src/online/mod.rs index 182f8ea2c3..58eb9e65a5 100644 --- a/crates/derive/src/online/mod.rs +++ b/crates/derive/src/online/mod.rs @@ -2,9 +2,9 @@ // Re-export types for the online pipeline construction. pub use crate::{ + attributes::StatefulAttributesBuilder, pipeline::{DerivationPipeline, PipelineBuilder}, sources::EthereumDataSource, - stages::StatefulAttributesBuilder, traits::{ChainProvider, L2ChainProvider, OriginProvider, Pipeline, StepResult}, }; diff --git a/crates/derive/src/params.rs b/crates/derive/src/params.rs index 8b12c75b0c..21c044a40c 100644 --- a/crates/derive/src/params.rs +++ b/crates/derive/src/params.rs @@ -1,10 +1,6 @@ //! This module contains the parameters and identifying types for the derivation pipeline. -use alloy_primitives::{address, b256, Address, B256}; - -/// The sequencer fee vault address. -pub const SEQUENCER_FEE_VAULT_ADDRESS: Address = - address!("4200000000000000000000000000000000000011"); +use alloy_primitives::{b256, B256}; /// `keccak256("ConfigUpdate(uint256,uint8,bytes)")` pub const CONFIG_UPDATE_TOPIC: B256 = diff --git a/crates/derive/src/stages/attributes_queue.rs b/crates/derive/src/stages/attributes_queue.rs index 8ceda077e1..3a0a8fdc8c 100644 --- a/crates/derive/src/stages/attributes_queue.rs +++ b/crates/derive/src/stages/attributes_queue.rs @@ -14,12 +14,6 @@ use crate::{ traits::{AttributesBuilder, NextAttributes, OriginAdvancer, OriginProvider, ResettableStage}, }; -mod deposits; -pub(crate) use deposits::derive_deposits; - -mod builder; -pub use builder::StatefulAttributesBuilder; - /// [AttributesProvider] is a trait abstraction that generalizes the [BatchQueue] stage. /// /// [BatchQueue]: crate::stages::BatchQueue diff --git a/crates/derive/src/stages/attributes_queue/deposits.rs b/crates/derive/src/stages/attributes_queue/deposits.rs deleted file mode 100644 index 3a635d6db0..0000000000 --- a/crates/derive/src/stages/attributes_queue/deposits.rs +++ /dev/null @@ -1,145 +0,0 @@ -//! Contains a helper method to derive deposit transactions from L1 Receipts. - -use crate::errors::PipelineEncodingError; -use alloc::vec::Vec; -use alloy_consensus::{Eip658Value, Receipt}; -use alloy_primitives::{Address, Bytes, B256}; -use op_alloy_protocol::{decode_deposit, DEPOSIT_EVENT_ABI_HASH}; - -/// Derive deposits for transaction receipts. -/// -/// Successful deposits must be emitted by the deposit contract and have the correct event -/// signature. So the receipt address must equal the specified deposit contract and the first topic -/// must be the [DEPOSIT_EVENT_ABI_HASH]. -pub(crate) async fn derive_deposits( - block_hash: B256, - receipts: &[Receipt], - deposit_contract: Address, -) -> Result, PipelineEncodingError> { - let mut global_index = 0; - let mut res = Vec::new(); - for r in receipts.iter() { - if Eip658Value::Eip658(false) == r.status { - continue; - } - for l in r.logs.iter() { - let curr_index = global_index; - global_index += 1; - if !l.data.topics().first().map_or(false, |i| *i == DEPOSIT_EVENT_ABI_HASH) { - continue; - } - if l.address != deposit_contract { - continue; - } - let decoded = decode_deposit(block_hash, curr_index, l)?; - res.push(decoded); - } - } - Ok(res) -} - -#[cfg(test)] -mod tests { - use super::*; - use alloc::vec; - use alloy_primitives::{address, Bytes, Log, LogData, U256, U64}; - use op_alloy_protocol::DepositError; - - fn generate_valid_log() -> Log { - let deposit_contract = address!("1111111111111111111111111111111111111111"); - let mut data = vec![0u8; 192]; - let offset: [u8; 8] = U64::from(32).to_be_bytes(); - data[24..32].copy_from_slice(&offset); - let len: [u8; 8] = U64::from(128).to_be_bytes(); - data[56..64].copy_from_slice(&len); - // Copy the u128 mint value - let mint: [u8; 16] = 10_u128.to_be_bytes(); - data[80..96].copy_from_slice(&mint); - // Copy the tx value - let value: [u8; 32] = U256::from(100).to_be_bytes(); - data[96..128].copy_from_slice(&value); - // Copy the gas limit - let gas: [u8; 8] = 1000_u64.to_be_bytes(); - data[128..136].copy_from_slice(&gas); - // Copy the isCreation flag - data[136] = 1; - let from = address!("2222222222222222222222222222222222222222"); - let mut from_bytes = vec![0u8; 32]; - from_bytes[12..32].copy_from_slice(from.as_slice()); - let to = address!("3333333333333333333333333333333333333333"); - let mut to_bytes = vec![0u8; 32]; - to_bytes[12..32].copy_from_slice(to.as_slice()); - Log { - address: deposit_contract, - data: LogData::new_unchecked( - vec![ - DEPOSIT_EVENT_ABI_HASH, - B256::from_slice(&from_bytes), - B256::from_slice(&to_bytes), - B256::default(), - ], - Bytes::from(data), - ), - } - } - - fn generate_valid_receipt() -> Receipt { - let mut bad_dest_log = generate_valid_log(); - bad_dest_log.data.topics_mut()[1] = B256::default(); - let mut invalid_topic_log = generate_valid_log(); - invalid_topic_log.data.topics_mut()[0] = B256::default(); - Receipt { - status: Eip658Value::Eip658(true), - logs: vec![generate_valid_log(), bad_dest_log, invalid_topic_log], - ..Default::default() - } - } - - #[tokio::test] - async fn test_derive_deposits_empty() { - let receipts = vec![]; - let deposit_contract = Address::default(); - let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; - assert!(result.unwrap().is_empty()); - } - - #[tokio::test] - async fn test_derive_deposits_non_deposit_events_filtered_out() { - let deposit_contract = address!("1111111111111111111111111111111111111111"); - let mut invalid = generate_valid_receipt(); - invalid.logs[0].data = LogData::new_unchecked(vec![], Bytes::default()); - let receipts = vec![generate_valid_receipt(), generate_valid_receipt(), invalid]; - let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; - assert_eq!(result.unwrap().len(), 5); - } - - #[tokio::test] - async fn test_derive_deposits_non_deposit_contract_addr() { - let deposit_contract = address!("1111111111111111111111111111111111111111"); - let mut invalid = generate_valid_receipt(); - invalid.logs[0].address = Address::default(); - let receipts = vec![generate_valid_receipt(), generate_valid_receipt(), invalid]; - let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; - assert_eq!(result.unwrap().len(), 5); - } - - #[tokio::test] - async fn test_derive_deposits_decoding_errors() { - let deposit_contract = address!("1111111111111111111111111111111111111111"); - let mut invalid = generate_valid_receipt(); - invalid.logs[0].data = - LogData::new_unchecked(vec![DEPOSIT_EVENT_ABI_HASH], Bytes::default()); - let receipts = vec![generate_valid_receipt(), generate_valid_receipt(), invalid]; - let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; - let downcasted = result.unwrap_err(); - assert_eq!(downcasted, DepositError::UnexpectedTopicsLen(1).into()); - } - - #[tokio::test] - async fn test_derive_deposits_succeeds() { - let deposit_contract = address!("1111111111111111111111111111111111111111"); - let receipts = vec![generate_valid_receipt(), generate_valid_receipt()]; - let result = derive_deposits(B256::default(), &receipts, deposit_contract).await; - assert_eq!(result.unwrap().len(), 4); - } -} diff --git a/crates/derive/src/stages/mod.rs b/crates/derive/src/stages/mod.rs index 1f2797fccd..2b0422be9e 100644 --- a/crates/derive/src/stages/mod.rs +++ b/crates/derive/src/stages/mod.rs @@ -33,7 +33,7 @@ mod batch_queue; pub use batch_queue::{BatchQueue, BatchQueueProvider}; mod attributes_queue; -pub use attributes_queue::{AttributesProvider, AttributesQueue, StatefulAttributesBuilder}; +pub use attributes_queue::{AttributesProvider, AttributesQueue}; mod utils; pub use utils::decompress_brotli; From b2b210e2be99faed239fa34819c47e724703791d Mon Sep 17 00:00:00 2001 From: refcell Date: Wed, 25 Sep 2024 18:26:30 -0400 Subject: [PATCH 2/2] fixes --- crates/derive/src/attributes/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/derive/src/attributes/mod.rs b/crates/derive/src/attributes/mod.rs index c90c683ee2..173f213ded 100644 --- a/crates/derive/src/attributes/mod.rs +++ b/crates/derive/src/attributes/mod.rs @@ -195,7 +195,7 @@ where } } -/// Derive deposits for transaction receipts. +/// Derive deposits as `Vec` for transaction receipts. /// /// Successful deposits must be emitted by the deposit contract and have the correct event /// signature. So the receipt address must equal the specified deposit contract and the first topic