feat(bin, engine): make state root task default (#14371)

book/cli/reth/node.md

@@ -707,8 +707,8 @@ Engine:
 
           [default: 2]
 
-      --engine.state-root-task
+      --engine.legacy-state-root
-          Enable state root task
+          Enable legacy state root
 
       --engine.caching-and-prewarming
           Enable cross-block caching and parallel prewarming

crates/e2e-test-utils/src/node.rs

@@ -277,7 +277,7 @@ where
             tokio::time::sleep(std::time::Duration::from_millis(100)).await;
             self.engine_api.update_forkchoice(block, block).await?;
 
-            assert!(start.elapsed() <= std::time::Duration::from_secs(10), "timed out");
+            assert!(start.elapsed() <= std::time::Duration::from_secs(40), "timed out");
         }
 
         // Hack to make sure that all components have time to process canonical state update.

crates/engine/tree/src/tree/config.rs

@@ -43,8 +43,9 @@ pub struct TreeConfig {
     /// This is used as a cutoff to prevent long-running sequential block execution when we receive
     /// a batch of downloaded blocks.
     max_execute_block_batch_size: usize,
-    /// Whether to use the new state root task calculation method instead of parallel calculation
+    /// Whether to use the legacy state root calculation method instead of the
-    use_state_root_task: bool,
+    /// new state root task
+    legacy_state_root: bool,
     /// Whether to always compare trie updates from the state root task to the trie updates from
     /// the regular state root calculation.
     always_compare_trie_updates: bool,
@@ -62,7 +63,7 @@ impl Default for TreeConfig {
             block_buffer_limit: DEFAULT_BLOCK_BUFFER_LIMIT,
             max_invalid_header_cache_length: DEFAULT_MAX_INVALID_HEADER_CACHE_LENGTH,
             max_execute_block_batch_size: DEFAULT_MAX_EXECUTE_BLOCK_BATCH_SIZE,
-            use_state_root_task: false,
+            legacy_state_root: false,
             always_compare_trie_updates: false,
             use_caching_and_prewarming: false,
             cross_block_cache_size: DEFAULT_CROSS_BLOCK_CACHE_SIZE,
@@ -79,7 +80,7 @@ impl TreeConfig {
         block_buffer_limit: u32,
         max_invalid_header_cache_length: u32,
         max_execute_block_batch_size: usize,
-        use_state_root_task: bool,
+        legacy_state_root: bool,
         always_compare_trie_updates: bool,
         use_caching_and_prewarming: bool,
         cross_block_cache_size: u64,
@@ -90,7 +91,7 @@ impl TreeConfig {
             block_buffer_limit,
             max_invalid_header_cache_length,
             max_execute_block_batch_size,
-            use_state_root_task,
+            legacy_state_root,
             always_compare_trie_updates,
             use_caching_and_prewarming,
             cross_block_cache_size,
@@ -122,9 +123,10 @@ impl TreeConfig {
         self.max_execute_block_batch_size
     }
 
-    /// Returns whether to use the state root task calculation method.
+    /// Returns whether to use the legacy state root calculation method instead
-    pub const fn use_state_root_task(&self) -> bool {
+    /// of the new state root task
-        self.use_state_root_task
+    pub const fn legacy_state_root(&self) -> bool {
+        self.legacy_state_root
     }
 
     /// Returns whether or not cross-block caching and parallel prewarming should be used.
@@ -182,9 +184,9 @@ impl TreeConfig {
         self
     }
 
-    /// Setter for whether to use the new state root task calculation method.
+    /// Setter for whether to use the legacy state root calculation method.
-    pub const fn with_state_root_task(mut self, use_state_root_task: bool) -> Self {
+    pub const fn with_legacy_state_root(mut self, legacy_state_root: bool) -> Self {
-        self.use_state_root_task = use_state_root_task;
+        self.legacy_state_root = legacy_state_root;
         self
     }
 

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

@@ -2420,7 +2420,7 @@ where
         let cancel_execution = ManualCancel::default();
 
         let (state_root_handle, state_root_task_config, state_root_sender, state_hook) =
-            if is_descendant_of_persisting_blocks && self.config.use_state_root_task() {
+            if is_descendant_of_persisting_blocks && !self.config.legacy_state_root() {
                 let consistent_view = ConsistentDbView::new_with_latest_tip(self.provider.clone())?;
 
                 // Compute trie input
@@ -2548,21 +2548,7 @@ where
         // a different database transaction per thread and it might end up with a
         // different view of the database.
         let (state_root, trie_output, root_elapsed) = if is_descendant_of_persisting_blocks {
-            if self.config.use_state_root_task() {
+            if self.config.legacy_state_root() {
-                let state_root_handle = state_root_handle
-                    .expect("state root handle must exist if use_state_root_task is true");
-                let state_root_config = state_root_task_config.expect("task config is present");
-
-                // Handle state root result from task using handle
-                self.handle_state_root_result(
-                    state_root_handle,
-                    state_root_config,
-                    block.sealed_block(),
-                    &hashed_state,
-                    &state_provider,
-                    root_time,
-                )?
-            } else {
                 match self.compute_state_root_parallel(block.header().parent_hash(), &hashed_state)
                 {
                     Ok(result) => {
@@ -2582,6 +2568,20 @@ where
                     }
                     Err(error) => return Err(InsertBlockErrorKind::Other(Box::new(error))),
                 }
+            } else {
+                let state_root_handle = state_root_handle
+                    .expect("state root handle must exist if use_state_root_task is true");
+                let state_root_config = state_root_task_config.expect("task config is present");
+
+                // Handle state root result from task using handle
+                self.handle_state_root_result(
+                    state_root_handle,
+                    state_root_config,
+                    block.sealed_block(),
+                    &hashed_state,
+                    &state_provider,
+                    root_time,
+                )?
             }
         } else {
             debug!(target: "engine::tree", block=?block_num_hash, ?is_descendant_of_persisting_blocks, "Failed to compute state root in parallel");
@@ -3341,7 +3341,8 @@ mod tests {
                 persistence_handle,
                 PersistenceState::default(),
                 payload_builder,
-                TreeConfig::default(),
+                // TODO: fix tests for state root task https://github.com/paradigmxyz/reth/issues/14376
+                TreeConfig::default().with_legacy_state_root(true),
                 EngineApiKind::Ethereum,
                 evm_config,
             );

crates/node/builder/src/builder/mod.rs

@@ -564,7 +564,7 @@ where
         let engine_tree_config = TreeConfig::default()
             .with_persistence_threshold(builder.config.engine.persistence_threshold)
             .with_memory_block_buffer_target(builder.config.engine.memory_block_buffer_target)
-            .with_state_root_task(builder.config.engine.state_root_task_enabled)
+            .with_legacy_state_root(builder.config.engine.legacy_state_root_task_enabled)
             .with_caching_and_prewarming(builder.config.engine.caching_and_prewarming_enabled)
             .with_always_compare_trie_updates(builder.config.engine.state_root_task_compare_updates)
             .with_cross_block_cache_size(

crates/node/core/src/args/engine.rs

@@ -19,9 +19,9 @@ pub struct EngineArgs {
     #[arg(long = "engine.memory-block-buffer-target", default_value_t = DEFAULT_MEMORY_BLOCK_BUFFER_TARGET)]
     pub memory_block_buffer_target: u64,
 
-    /// Enable state root task
+    /// Enable legacy state root
-    #[arg(long = "engine.state-root-task")]
+    #[arg(long = "engine.legacy-state-root", default_value = "false")]
-    pub state_root_task_enabled: bool,
+    pub legacy_state_root_task_enabled: bool,
 
     /// Enable cross-block caching and parallel prewarming
     #[arg(long = "engine.caching-and-prewarming")]
@@ -42,7 +42,7 @@ impl Default for EngineArgs {
         Self {
             persistence_threshold: DEFAULT_PERSISTENCE_THRESHOLD,
             memory_block_buffer_target: DEFAULT_MEMORY_BLOCK_BUFFER_TARGET,
-            state_root_task_enabled: false,
+            legacy_state_root_task_enabled: false,
             state_root_task_compare_updates: false,
             caching_and_prewarming_enabled: false,
             cross_block_cache_size: DEFAULT_CROSS_BLOCK_CACHE_SIZE_MB,