From e359b32f6d742f22d21e0ff1b9220b2f9f056d33 Mon Sep 17 00:00:00 2001 From: Himess Date: Thu, 18 Dec 2025 22:27:19 +0300 Subject: [PATCH 1/7] refactor(trie): return OpProofsStorageError from execute_and_store_block_updates Replace eyre::Result with Result<(), OpProofsStorageError> in the execute_and_store_block_updates method to improve type safety and limit eyre usage to CLI crates. Changes: - Add ExecutionError and ProviderError variants to OpProofsStorageError - Implement From traits for BlockExecutionError and ProviderError - Update method signature to return specific error type - Remove unnecessary .into() conversions Closes #523 --- crates/optimism/trie/src/error.rs | 20 ++++++++++++++++++++ crates/optimism/trie/src/live.rs | 15 ++++++--------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/crates/optimism/trie/src/error.rs b/crates/optimism/trie/src/error.rs index 4a07a594419..1285b1ef727 100644 --- a/crates/optimism/trie/src/error.rs +++ b/crates/optimism/trie/src/error.rs @@ -2,6 +2,8 @@ use alloy_primitives::B256; use reth_db::DatabaseError; +use reth_execution_errors::BlockExecutionError; +use reth_provider::ProviderError; use reth_trie::Nibbles; use std::sync::Arc; use thiserror::Error; @@ -82,6 +84,24 @@ pub enum OpProofsStorageError { /// Error occurred while trying to acquire a lock. #[error("failed lock attempt")] TryLockError, + /// Error occurred during block execution. + #[error(transparent)] + ExecutionError(Box), + /// Error occurred while interacting with the provider. + #[error(transparent)] + ProviderError(Box), +} + +impl From for OpProofsStorageError { + fn from(err: BlockExecutionError) -> Self { + Self::ExecutionError(Box::new(err)) + } +} + +impl From for OpProofsStorageError { + fn from(err: ProviderError) -> Self { + Self::ProviderError(Box::new(err)) + } } impl From for OpProofsStorageError { diff --git a/crates/optimism/trie/src/live.rs b/crates/optimism/trie/src/live.rs index 694c00a53d7..0ad0e2b0779 100644 --- a/crates/optimism/trie/src/live.rs +++ b/crates/optimism/trie/src/live.rs @@ -40,7 +40,7 @@ where pub async fn execute_and_store_block_updates( &self, block: &RecoveredBlock>, - ) -> eyre::Result<()> { + ) -> Result<(), OpProofsStorageError> { let mut operation_durations = OperationDurations::default(); let start = Instant::now(); @@ -49,12 +49,12 @@ where self.storage.get_earliest_block_number().await?, self.storage.get_latest_block_number().await?, ) else { - return Err(OpProofsStorageError::NoBlocksFound.into()); + return Err(OpProofsStorageError::NoBlocksFound); }; let parent_block_number = block.number() - 1; if parent_block_number < earliest { - return Err(OpProofsStorageError::UnknownParent.into()); + return Err(OpProofsStorageError::UnknownParent); } if parent_block_number > latest { @@ -62,8 +62,7 @@ where block_number: block.number(), parent_block_number, latest_block_number: latest, - } - .into()); + }); } let block_ref = @@ -80,8 +79,7 @@ where let db = StateProviderDatabase::new(&state_provider); let block_executor = self.evm_config.batch_executor(db); - let execution_result = - block_executor.execute(&(*block).clone()).map_err(|err| eyre::eyre!(err))?; + let execution_result = block_executor.execute(&(*block).clone())?; operation_durations.execution_duration_seconds = start.elapsed(); @@ -97,8 +95,7 @@ where block_number: block.number(), current_state_hash: state_root, expected_state_hash: block.state_root(), - } - .into()); + }); } let update_result = self From fb22b244dbc574090992d16bb20e37407b1b1135 Mon Sep 17 00:00:00 2001 From: Himess Date: Thu, 18 Dec 2025 22:41:19 +0300 Subject: [PATCH 2/7] fix: store error messages as String to satisfy Clone bound --- crates/optimism/trie/src/error.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/optimism/trie/src/error.rs b/crates/optimism/trie/src/error.rs index 1285b1ef727..65d9e798387 100644 --- a/crates/optimism/trie/src/error.rs +++ b/crates/optimism/trie/src/error.rs @@ -85,22 +85,22 @@ pub enum OpProofsStorageError { #[error("failed lock attempt")] TryLockError, /// Error occurred during block execution. - #[error(transparent)] - ExecutionError(Box), + #[error("block execution error: {0}")] + ExecutionError(String), /// Error occurred while interacting with the provider. - #[error(transparent)] - ProviderError(Box), + #[error("provider error: {0}")] + ProviderError(String), } impl From for OpProofsStorageError { fn from(err: BlockExecutionError) -> Self { - Self::ExecutionError(Box::new(err)) + Self::ExecutionError(err.to_string()) } } impl From for OpProofsStorageError { fn from(err: ProviderError) -> Self { - Self::ProviderError(Box::new(err)) + Self::ProviderError(err.to_string()) } } From 3ff86ed5acb0879422d171646a25362c6bdb3050 Mon Sep 17 00:00:00 2001 From: Himess Date: Thu, 18 Dec 2025 22:53:40 +0300 Subject: [PATCH 3/7] fix: update tests to use OpProofsStorageError directly Since execute_and_store_block_updates now returns Result<(), OpProofsStorageError> instead of eyre::Result<()>, we no longer need downcast_ref to access the error. --- crates/optimism/trie/tests/live.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/crates/optimism/trie/tests/live.rs b/crates/optimism/trie/tests/live.rs index 45a7de174e2..b87c527e24a 100644 --- a/crates/optimism/trie/tests/live.rs +++ b/crates/optimism/trie/tests/live.rs @@ -361,10 +361,7 @@ async fn test_execute_and_store_block_updates_missing_parent_block() { // EXPECT: MissingParentBlock let err = collector.execute_and_store_block_updates(&incorrect_block).await.unwrap_err(); - assert!(matches!( - err.downcast_ref::().unwrap(), - OpProofsStorageError::MissingParentBlock { .. } - )); + assert!(matches!(err, OpProofsStorageError::MissingParentBlock { .. })); } #[tokio::test] @@ -425,10 +422,7 @@ async fn test_execute_and_store_block_updates_state_root_mismatch() { // EXPECT: StateRootMismatch let err = collector.execute_and_store_block_updates(&block).await.unwrap_err(); - assert!(matches!( - err.downcast_ref::(), - Some(OpProofsStorageError::StateRootMismatch { .. }) - )); + assert!(matches!(err, OpProofsStorageError::StateRootMismatch { .. })); } /// Test with multiple blocks before and after backfill From 4d3ab3b4e6993d2e4f218b1320e89e05541351b2 Mon Sep 17 00:00:00 2001 From: Himess Date: Fri, 19 Dec 2025 15:25:10 +0300 Subject: [PATCH 4/7] fix: use Arc to wrap non-Clone error types Use Arc and Arc instead of String to maintain type safety while satisfying the Clone bound on OpProofsStorageError. This addresses the reviewer feedback about not using String for error variants. --- crates/optimism/trie/src/error.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/optimism/trie/src/error.rs b/crates/optimism/trie/src/error.rs index 65d9e798387..44f5ef8f6d2 100644 --- a/crates/optimism/trie/src/error.rs +++ b/crates/optimism/trie/src/error.rs @@ -85,22 +85,22 @@ pub enum OpProofsStorageError { #[error("failed lock attempt")] TryLockError, /// Error occurred during block execution. - #[error("block execution error: {0}")] - ExecutionError(String), + #[error(transparent)] + ExecutionError(Arc), /// Error occurred while interacting with the provider. - #[error("provider error: {0}")] - ProviderError(String), + #[error(transparent)] + ProviderError(Arc), } impl From for OpProofsStorageError { fn from(err: BlockExecutionError) -> Self { - Self::ExecutionError(err.to_string()) + Self::ExecutionError(Arc::new(err)) } } impl From for OpProofsStorageError { fn from(err: ProviderError) -> Self { - Self::ProviderError(err.to_string()) + Self::ProviderError(Arc::new(err)) } } From fa48b6e9e15bb84cf3eacf4a5d4ed14bfa2bfd19 Mon Sep 17 00:00:00 2001 From: Himess Date: Fri, 19 Dec 2025 15:34:02 +0300 Subject: [PATCH 5/7] refactor: use #[from] directly and remove Clone derive --- crates/optimism/trie/src/error.rs | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/crates/optimism/trie/src/error.rs b/crates/optimism/trie/src/error.rs index 44f5ef8f6d2..b082a76825e 100644 --- a/crates/optimism/trie/src/error.rs +++ b/crates/optimism/trie/src/error.rs @@ -10,7 +10,7 @@ use thiserror::Error; use tokio::sync::TryLockError; /// Error type for storage operations -#[derive(Debug, Clone, Error)] +#[derive(Debug, Error)] pub enum OpProofsStorageError { /// No blocks found #[error("No blocks found")] @@ -86,22 +86,10 @@ pub enum OpProofsStorageError { TryLockError, /// Error occurred during block execution. #[error(transparent)] - ExecutionError(Arc), + ExecutionError(#[from] BlockExecutionError), /// Error occurred while interacting with the provider. #[error(transparent)] - ProviderError(Arc), -} - -impl From for OpProofsStorageError { - fn from(err: BlockExecutionError) -> Self { - Self::ExecutionError(Arc::new(err)) - } -} - -impl From for OpProofsStorageError { - fn from(err: ProviderError) -> Self { - Self::ProviderError(Arc::new(err)) - } + ProviderError(#[from] ProviderError), } impl From for OpProofsStorageError { @@ -121,11 +109,6 @@ impl From for DatabaseError { impl From for OpProofsStorageError { fn from(error: DatabaseError) -> Self { - if let DatabaseError::Custom(ref err) = error && - let Some(err) = err.downcast_ref::() - { - return err.clone() - } Self::DatabaseError(error) } } From 7a6a4ee8d04d4bbdcf09d9f6fe44ab060afca37a Mon Sep 17 00:00:00 2001 From: Himess Date: Fri, 19 Dec 2025 16:04:04 +0300 Subject: [PATCH 6/7] test: fix roundtrip test after Clone removal --- crates/optimism/trie/src/error.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/optimism/trie/src/error.rs b/crates/optimism/trie/src/error.rs index b082a76825e..63091cad3fa 100644 --- a/crates/optimism/trie/src/error.rs +++ b/crates/optimism/trie/src/error.rs @@ -126,7 +126,9 @@ mod test { let db_error: DatabaseError = original_error.into(); let converted_error: OpProofsStorageError = db_error.into(); - assert!(matches!(converted_error, OpProofsStorageError::NoBlocksFound)) + // After converting to DatabaseError and back, the error is wrapped in DatabaseError variant + // because we can't recover the original type without Clone + assert!(matches!(converted_error, OpProofsStorageError::DatabaseError(_))) } #[test] From 741f9f9b76ec996e693fd4b97818fb8d3fbfaa65 Mon Sep 17 00:00:00 2001 From: Himess Date: Tue, 23 Dec 2025 17:58:13 +0300 Subject: [PATCH 7/7] refactor(trie): wrap errors in Arc and restore DatabaseError conversion - Restore Clone derive on OpProofsStorageError - Wrap BlockExecutionError and ProviderError in Arc for Clone compatibility - Restore original From conversion that recovers the original type --- crates/optimism/trie/src/error.rs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/crates/optimism/trie/src/error.rs b/crates/optimism/trie/src/error.rs index 63091cad3fa..d2bb6226a3e 100644 --- a/crates/optimism/trie/src/error.rs +++ b/crates/optimism/trie/src/error.rs @@ -10,7 +10,7 @@ use thiserror::Error; use tokio::sync::TryLockError; /// Error type for storage operations -#[derive(Debug, Error)] +#[derive(Debug, Clone, Error)] pub enum OpProofsStorageError { /// No blocks found #[error("No blocks found")] @@ -86,10 +86,10 @@ pub enum OpProofsStorageError { TryLockError, /// Error occurred during block execution. #[error(transparent)] - ExecutionError(#[from] BlockExecutionError), + ExecutionError(Arc), /// Error occurred while interacting with the provider. #[error(transparent)] - ProviderError(#[from] ProviderError), + ProviderError(Arc), } impl From for OpProofsStorageError { @@ -98,6 +98,18 @@ impl From for OpProofsStorageError { } } +impl From for OpProofsStorageError { + fn from(error: BlockExecutionError) -> Self { + Self::ExecutionError(Arc::new(error)) + } +} + +impl From for OpProofsStorageError { + fn from(error: ProviderError) -> Self { + Self::ProviderError(Arc::new(error)) + } +} + impl From for DatabaseError { fn from(error: OpProofsStorageError) -> Self { match error { @@ -109,6 +121,11 @@ impl From for DatabaseError { impl From for OpProofsStorageError { fn from(error: DatabaseError) -> Self { + if let DatabaseError::Custom(ref err) = error && + let Some(err) = err.downcast_ref::() + { + return err.clone() + } Self::DatabaseError(error) } } @@ -126,9 +143,7 @@ mod test { let db_error: DatabaseError = original_error.into(); let converted_error: OpProofsStorageError = db_error.into(); - // After converting to DatabaseError and back, the error is wrapped in DatabaseError variant - // because we can't recover the original type without Clone - assert!(matches!(converted_error, OpProofsStorageError::DatabaseError(_))) + assert!(matches!(converted_error, OpProofsStorageError::NoBlocksFound)) } #[test]