chore(engine): Remove ConsistentDbView #19188

mediocregopher · 2025-10-21T15:04:38Z

This PR replaces usage of ConsistenDbView with the OverlayStateProviderFactory. The OverlayStateProviderFactory will handle reverting both hashed and trie state to the correct target block whenever a new tx is made, as well as applying any necessary in-memory overlay on top of that.

mediocregopher · 2025-10-21T15:09:46Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

 /// Input parameters for spawning a dedicated storage multiproof calculation.
 #[derive(Debug)]
 struct StorageMultiproofInput {
-    config: MultiProofConfig,


MultiProofConfig is how we previously conveyed reverts+in-memory overlay to the proof tasks. OverlayStateProvider now does this job, so it's not necessary to convey the config anymore.

The prefix_sets field of the config is no longer applicable either, as we are always generating proofs off a DB state where hashed state and trie tables are at the same block.

mediocregopher · 2025-10-21T15:18:18Z

crates/engine/tree/src/tree/payload_validator.rs

                // Use state root task only if prefix sets are empty, otherwise proof generation is
                // too expensive because it requires walking all paths in every proof.
                let spawn_start = Instant::now();
-                let (handle, strategy) = if trie_input.prefix_sets.is_empty() {


I'm not sure that prefix_sets ever wasn't empty, but it definitely is always empty now

mediocregopher · 2025-10-21T15:19:37Z

crates/engine/tree/src/tree/payload_validator.rs

            .ok_or_else(|| ProviderError::BlockHashNotFound(historical.as_hash().unwrap()))?;

-        // Retrieve revert state for historical block.
-        let (revert_state, revert_trie) = if block_number == best_block_number {


Reverts are now handled by the OverlayStateProviderFactory, it's only necessary to collect the highest persisted ancestor block number here (so that the OverlayStateProviderFactory knows which block to revert to) and any in-memory overlay data

mediocregopher · 2025-10-21T15:26:55Z

crates/storage/storage-api/src/database_provider.rs

 pub type FactoryTx<F> = <<F as DatabaseProviderFactory>::DB as Database>::TX;

+/// A trait which can be used to describe any factory-like type which returns a read-only provider.
+pub trait DatabaseProviderROFactory {


This trait serves multiple purposes:

Allow passing OverlayStateProviderFactory as a generic throughout the code, which will help keep things flexible in the future.

By not constraining Provider to DBProvider here we don't force OverlayStateProvider to expose its inner TX, which will make it difficult to misuse.

Allows us to constrain the Provider to be TrieCursorFactory + HashedCursorFactory. The alternative would be defining two new TrieCursorFactoryFactory + HashedCursorFactoryFactory traits, which seemed worse.

mediocregopher · 2025-10-21T15:28:42Z

crates/trie/parallel/benches/root.rs

        }

-        let view = ConsistentDbView::new(provider_factory.clone(), None);
+        let view = OverlayStateProviderFactory::new(provider_factory.clone());


If we could add a blanket implementation for TrieCursorFactory and HashedCursorFactory on DBProvider, then (combined with the DatabaseProviderROFactory blanket implementation in this PR) I don't think wrapping this factory would be necessary. But doing that created some import cycles so I left it alone for now.

gakonst · 2025-10-21T15:59:00Z

is this a chore improvement or does it unlock other things?

When determining the available range of trie changeset data we previously only took into account the StageCheckpoint. This was wrong for two reasons: * The StageCheckpoint can have its inner block range cleared, specifically on unwind or reorgs. So we can't rely on the starting bound being present. * The StageCheckpoint's lower bound doesn't get updated by pruning, only the prune checkpoint does. So in general we'll want to rely on the prune checkpoint.

…iocregopher/18906-rm-consistentdbview

jenpaff · 2025-10-22T10:17:52Z

is this a chore improvement or does it unlock other things?

chore but major milestone for us towards simplifying core abstractions which was enabled by #18460

…6-rm-consistentdbview

shekhirin · 2025-10-27T16:24:56Z

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

+
+        // We rely on the cursor factory to provide whatever DB overlay is necessary to see a
+        // consistent view of the database, including the trie tables. Because of this there is no
+        // need for an overarching prefix set to invalidate any section of the trie tables, and so
+        // we use an empty prefix set.
+        let prefix_sets = Arc::new(TriePrefixSetsMut::default());


can prefix_sets be removed from ProofTaskCtx too then?

I think so yes, I had thought it would be good to keep it around in case we wanted to/needed to use it for some future use-case, but at the moment removing it would be possible.

can do as a follow-up, just nicer to not pass defaults everywhere if it's unused anyway

mediocregopher · 2025-10-27T16:30:06Z

crates/storage/provider/src/providers/state/overlay.rs

-            })
-        };
+        // Use the higher of the two lower bounds. If neither is available assume unbounded.
+        let lower_bound = stage_lower_bound.max(prune_lower_bound).unwrap_or(0);


Without this change the e2e tests wouldn't complete, because they don't run a pipeline to set the PruneCheckpoint. I think it's fine generally, as we shouldn't ever have a real condition where the PruneCheckpoint wouldn't be set.

I think this is fine

crates/trie/parallel/benches/root.rs

shekhirin · 2025-10-27T16:45:36Z

crates/trie/parallel/src/proof.rs

 /// that has proof targets.
 #[derive(Debug)]
 pub struct ParallelProof {
-    /// The sorted collection of cached in-memory intermediate trie nodes that


oh, this was already unused 👍

Co-authored-by: Alexey Shekhirin <[email protected]>

shekhirin · 2025-10-27T17:03:38Z

crates/engine/tree/src/tree/payload_validator.rs


        // Plan the strategy used for state root computation.
-        let state_root_plan = self.plan_state_root_computation(&input, &ctx);
-        let persisting_kind = state_root_plan.persisting_kind;


PersistingKind can be removed completely in a follow-up PR

shekhirin · 2025-10-27T17:09:03Z

crates/engine/tree/src/tree/payload_validator.rs

-                                StateRootStrategy::Parallel,
-                            )
-                        }
+                let (handle, strategy) = match self.payload_processor.spawn(


unrelated to this PR: I don't think self.payload_processor.spawn is actually fallible, should be possible to simplify this even more without a fallback to self.payload_processor.spawn_cache_exclusive, can be done in a follow-up PR

can we track this @shekhirin

shekhirin · 2025-10-27T17:09:39Z

crates/engine/tree/src/tree/payload_validator.rs

-                StateRootStrategy::Parallel
-            }
-        } else {
+    fn plan_state_root_computation(&self) -> StateRootPlan {


can be done in a follow-up PR: can just return an enum now, the function is much simpler

shekhirin · 2025-10-27T17:11:49Z

crates/engine/tree/src/tree/payload_validator.rs

-            (revert_state, revert_trie)
-        };
-
-        input.append_cached(revert_trie.into(), revert_state);


checks out, this was needed only for reverting the trie state

shekhirin

LGTM, only nits and follow-ups for cleanup

…rm-consistentdbview

Rjected

this makes sense to me, also just nits that can be done as a follow up

Rjected · 2025-10-27T17:32:27Z

crates/engine/tree/src/tree/payload_validator.rs

+        // The `hashed_state` argument is already taken into account as part of the overlay, but we
+        // need to use the prefix sets which were generated from it to indicate to the
+        // ParallelStateRoot which parts of the trie need to be recomputed.
+        let prefix_sets = Arc::into_inner(multiproof_config.prefix_sets)
+            .expect("MultiProofConfig was never cloned")
+            .freeze();


the use of into_inner here makes me think we should have a function that constructs a MultiProofConfig from trie input, that gives us back the original prefix sets.

Rjected · 2025-10-27T17:43:41Z

crates/trie/parallel/src/proof_task.rs

+    // Create provider from factory
+    let provider = task_ctx
+        .factory
+        .database_provider_ro()
+        .expect("Account worker failed to initialize: unable to create provider");


nit for followup - we should make this fn return a Result<()> so we can just propagate this error

mattsse

awesome

this now unlocks a ton of additional simplifications

mattsse · 2025-10-27T18:50:25Z

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

-    /// A cleared trie input, kept around to be reused so allocations can be minimized.
-    trie_input: Option<TrieInput>,


mattsse · 2025-10-27T18:51:00Z

crates/engine/tree/src/tree/payload_validator.rs


        // Plan the strategy used for state root computation.
-        let state_root_plan = self.plan_state_root_computation(&input, &ctx);
-        let persisting_kind = state_root_plan.persisting_kind;


mattsse · 2025-10-27T18:55:57Z

crates/engine/tree/src/tree/payload_validator.rs

+        let provider = self.provider.database_provider_ro()?;
+
+        let (mut input, block_number) =
+            self.compute_trie_input(provider, parent_hash, state, None)?;


I think all of this is tracked in #19250

mattsse · 2025-10-27T18:56:43Z

crates/engine/tree/src/tree/payload_validator.rs

-                                StateRootStrategy::Parallel,
-                            )
-                        }
+                let (handle, strategy) = match self.payload_processor.spawn(


can we track this @shekhirin

mattsse · 2025-10-27T18:57:50Z

crates/storage/provider/src/providers/state/overlay.rs

-            })
-        };
+        // Use the higher of the two lower bounds. If neither is available assume unbounded.
+        let lower_bound = stage_lower_bound.max(prune_lower_bound).unwrap_or(0);


I think this is fine

github-project-automation bot added this to Reth Tracker Oct 21, 2025

github-project-automation bot moved this to Backlog in Reth Tracker Oct 21, 2025

mediocregopher force-pushed the mediocregopher/18906-rm-consistentdbview branch 2 times, most recently from 1e58915 to 50c3d20 Compare October 21, 2025 15:48

mediocregopher commented Oct 21, 2025

View reviewed changes

chore(engine): Remove ConsistentDbView

50c3d20

mediocregopher added 4 commits October 22, 2025 10:31

Merge branch 'mediocregopher/merkle-changesets-validate-fix' into med…

05e519e

…iocregopher/18906-rm-consistentdbview

Fix compilation errors after merge

15221ec

Fix OverlayStateProviderFactory reverting to wrong block number

9404f81

mediocregopher added 2 commits October 22, 2025 14:40

Merge remote-tracking branch 'upstream/main' into mediocregopher/1890…

902ae06

…6-rm-consistentdbview

Simplify compute_trie_updates further, no more persisting action check

fe51aef

mediocregopher force-pushed the mediocregopher/18906-rm-consistentdbview branch from 16bdc39 to fe51aef Compare October 27, 2025 11:10

mediocregopher changed the base branch from main to mediocregopher/in-mem-trie-overlay-fix October 27, 2025 11:13

mediocregopher added 2 commits October 27, 2025 13:59

Merge remote-tracking branch 'upstream/main' into mediocregopher/1890…

6e5b299

…6-rm-consistentdbview

Fix parallel root test

8504f4a

Base automatically changed from mediocregopher/in-mem-trie-overlay-fix to main October 27, 2025 15:34

mediocregopher marked this pull request as ready for review October 27, 2025 16:11

mediocregopher requested review from Rjected, fgimenez, gakonst, joshieDo, mattsse, rakita, shekhirin and yongkangc as code owners October 27, 2025 16:11

Fix checkpoint validation and docs

c2892cb

shekhirin reviewed Oct 27, 2025

View reviewed changes

mediocregopher commented Oct 27, 2025

View reviewed changes

shekhirin reviewed Oct 27, 2025

View reviewed changes

crates/trie/parallel/benches/root.rs Outdated Show resolved Hide resolved

shekhirin reviewed Oct 27, 2025

View reviewed changes

mediocregopher and others added 2 commits October 27, 2025 17:48

Update crates/trie/parallel/benches/root.rs

403930e

Co-authored-by: Alexey Shekhirin <[email protected]>

fix bench

c1010b9

shekhirin reviewed Oct 27, 2025

View reviewed changes

shekhirin approved these changes Oct 27, 2025

View reviewed changes

github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Oct 27, 2025

Merge remote-tracking branch 'origin/main' into mediocregopher/18906-…

a360554

…rm-consistentdbview

Rjected approved these changes Oct 27, 2025

View reviewed changes

mattsse approved these changes Oct 27, 2025

View reviewed changes

shekhirin added this pull request to the merge queue Oct 27, 2025

Merged via the queue into main with commit ffeaa47 Oct 27, 2025
45 checks passed

shekhirin deleted the mediocregopher/18906-rm-consistentdbview branch October 27, 2025 19:24

github-project-automation bot moved this from In Progress to Done in Reth Tracker Oct 27, 2025

shekhirin mentioned this pull request Oct 28, 2025

Payload validator cleanup after ConsistentDbView removal #19341

Open

		/// A cleared trie input, kept around to be reused so allocations can be minimized.
		trie_input: Option<TrieInput>,

Uh oh!

chore(engine): Remove ConsistentDbView #19188

chore(engine): Remove ConsistentDbView #19188

Uh oh!

Conversation

mediocregopher commented Oct 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mediocregopher Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gakonst commented Oct 21, 2025

Uh oh!

jenpaff commented Oct 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shekhirin left a comment

Choose a reason for hiding this comment

Uh oh!

Rjected left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattsse left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mediocregopher Oct 21, 2025 •

edited

Loading