avoid cloning block in EngineApiTreeHandler::insert_block

crates/engine/tree/src/tree/mod.rs

@@ -2164,28 +2164,31 @@ where
         &mut self,
         block: SealedBlockWithSenders<N::Block>,
     ) -> Result<InsertPayloadOk2, InsertBlockErrorTwo<N::Block>> {
-        self.insert_block_inner(block.clone())
-            .map_err(|kind| InsertBlockErrorTwo::new(block.block, kind))
-    }
+        macro_rules! unwrap_or_return_error {
+            ($e:expr, $block:expr) => {
+                match $e {
+                    Ok(t) => t,
+                    Err(e) => return Err(InsertBlockErrorTwo::new($block, e.into())),
+                }
+            };
+        }
 
-    fn insert_block_inner(
-        &mut self,
-        block: SealedBlockWithSenders<N::Block>,
-    ) -> Result<InsertPayloadOk2, InsertBlockErrorKindTwo> {
         debug!(target: "engine::tree", block=?block.num_hash(), parent = ?block.parent_hash(), state_root = ?block.state_root(), "Inserting new block into tree");
 
-        if self.block_by_hash(block.hash())?.is_some() {
+        if unwrap_or_return_error!(self.block_by_hash(block.hash()), block.block).is_some() {
             return Ok(InsertPayloadOk2::AlreadySeen(BlockStatus2::Valid))
         }
 
         let start = Instant::now();
 
         trace!(target: "engine::tree", block=?block.num_hash(), "Validating block consensus");
         // validate block consensus rules
-        self.validate_block(&block)?;
+        unwrap_or_return_error!(self.validate_block(&block), block.block);
 
         trace!(target: "engine::tree", block=?block.num_hash(), parent=?block.parent_hash(), "Fetching block state provider");
-        let Some(state_provider) = self.state_provider(block.parent_hash())? else {
+        let Some(state_provider) =
+            unwrap_or_return_error!(self.state_provider(block.parent_hash()), block.block)
+        else {
             // we don't have the state required to execute this block, buffering it and find the
             // missing parent block
             let missing_ancestor = self
@@ -2204,22 +2207,26 @@ where
         };
 
         // now validate against the parent
-        let parent_block = self.sealed_header_by_hash(block.parent_hash())?.ok_or_else(|| {
-            InsertBlockErrorKindTwo::Provider(ProviderError::HeaderNotFound(
-                block.parent_hash().into(),
-            ))
-        })?;
+        let parent_block = unwrap_or_return_error!(
+            unwrap_or_return_error!(self.sealed_header_by_hash(block.parent_hash()), block.block)
+                .ok_or_else(|| {
+                    InsertBlockErrorKindTwo::Provider(ProviderError::HeaderNotFound(
+                        block.parent_hash().into(),
+                    ))
+                }),
+            block.block
+        );
         if let Err(e) = self.consensus.validate_header_against_parent(&block, &parent_block) {
             warn!(target: "engine::tree", ?block, "Failed to validate header {} against parent: {e}", block.hash());
-            return Err(e.into())
+            return Err(InsertBlockErrorTwo::new(block.block, e.into()))
         }
 
         trace!(target: "engine::tree", block=?block.num_hash(), "Executing block");
         let executor = self.executor_provider.executor(StateProviderDatabase::new(&state_provider));
 
         let block_number = block.number();
         let block_hash = block.hash();
-        let sealed_block = Arc::new(block.block.clone());
+        let sealed_block = block.block.clone();
         let block = block.unseal();
 
         let exec_time = Instant::now();
@@ -2261,7 +2268,10 @@ where
         // };
         let state_hook = Box::new(|_state: &EvmState| {});
 
-        let output = self.metrics.executor.execute_metered(executor, &block, state_hook)?;
+        let output = unwrap_or_return_error!(
+            self.metrics.executor.execute_metered(executor, &block, state_hook),
+            sealed_block
+        );
 
         trace!(target: "engine::tree", elapsed=?exec_time.elapsed(), ?block_number, "Executed block");
 
@@ -2276,7 +2286,7 @@ where
                 &output,
                 None,
             );
-            return Err(err.into())
+            return Err(InsertBlockErrorTwo::new(sealed_block, err.into()))
         }
 
         let hashed_state = self.provider.hashed_post_state(&output.state);
@@ -2314,7 +2324,12 @@ where
                     debug!(target: "engine", %error, "Parallel state root computation failed consistency check, falling back");
                     None
                 }
-                Err(error) => return Err(InsertBlockErrorKindTwo::Other(Box::new(error))),
+                Err(error) => {
+                    return Err(InsertBlockErrorTwo::new(
+                        sealed_block,
+                        InsertBlockErrorKindTwo::Other(Box::new(error)),
+                    ))
+                }
             }
         } else {
             None
@@ -2324,7 +2339,10 @@ where
             result
         } else {
             debug!(target: "engine::tree", block=?sealed_block.num_hash(), ?persistence_not_in_progress, "Failed to compute state root in parallel");
-            state_provider.state_root_with_updates(hashed_state.clone())?
+            unwrap_or_return_error!(
+                state_provider.state_root_with_updates(hashed_state.clone()),
+                sealed_block
+            )
         };
 
         if state_root != block.header().state_root() {
@@ -2335,16 +2353,19 @@ where
                 &output,
                 Some((&trie_output, state_root)),
             );
-            return Err(ConsensusError::BodyStateRootDiff(
-                GotExpected { got: state_root, expected: block.header().state_root() }.into(),
-            )
-            .into())
+            return Err(InsertBlockErrorTwo::consensus_error(
+                ConsensusError::BodyStateRootDiff(
+                    GotExpected { got: state_root, expected: block.header().state_root() }.into(),
+                ),
+                sealed_block,
+            ))
         }
 
         let root_elapsed = root_time.elapsed();
         self.metrics.block_validation.record_state_root(&trie_output, root_elapsed.as_secs_f64());
         debug!(target: "engine::tree", ?root_elapsed, block=?sealed_block.num_hash(), "Calculated state root");
 
+        let sealed_block = Arc::new(sealed_block);
         let executed: ExecutedBlock<N> = ExecutedBlock {
             block: sealed_block.clone(),
             senders: Arc::new(block.senders),
@@ -2364,11 +2385,12 @@ where
 
         // emit insert event
         let elapsed = start.elapsed();
-        let engine_event = if self.is_fork(block_hash)? {
-            BeaconConsensusEngineEvent::ForkBlockAdded(sealed_block, elapsed)
-        } else {
-            BeaconConsensusEngineEvent::CanonicalBlockAdded(sealed_block, elapsed)
-        };
+        let engine_event =
+            if unwrap_or_return_error!(self.is_fork(block_hash), sealed_block.as_ref().clone()) {
+                BeaconConsensusEngineEvent::ForkBlockAdded(sealed_block, elapsed)
+            } else {
+                BeaconConsensusEngineEvent::CanonicalBlockAdded(sealed_block, elapsed)
+            };
         self.emit_event(EngineApiEvent::BeaconConsensus(engine_event));
 
         debug!(target: "engine::tree", block=?BlockNumHash::new(block_number, block_hash), "Finished inserting block");