Skip to content

feat: more paranoid reth db checks #911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ZanCorDX merged 1 commit into from
Apr 8, 2026
Merged

feat: more paranoid reth db checks #911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/eth-sparse-mpt/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl SparseTrieSharedCache {
parent_state_root,
);
let mut cache_v2 = v2::SharedCacheV2::default();
cache_v2.last_block_hash = parent_block_hash;
cache_v2.parent_state_root = parent_state_root;
let mut cache_v_experimental = v_experimental::SharedCacheVExperimental::default();
cache_v_experimental.last_block_hash = parent_block_hash;
Self {
Expand Down
47 changes: 22 additions & 25 deletions crates/eth-sparse-mpt/src/v2/fetch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,30 @@ use rayon::prelude::*;
use alloy_primitives::B256;
use nybbles::Nibbles;
use reth_provider::{
providers::ConsistentDbView, BlockHashReader, BlockNumReader, BlockReader, DBProvider,
DatabaseProviderFactory,
providers::ConsistentDbView, BlockReader, DBProvider, DatabaseProviderFactory,
};
use reth_trie::{
proof::{Proof, StorageProof},
MultiProofTargets,
MultiProofTargets, StateRoot,
};
use reth_trie_db::{DatabaseHashedCursorFactory, DatabaseTrieCursorFactory};
use reth_trie_db::{DatabaseHashedCursorFactory, DatabaseStateRoot, DatabaseTrieCursorFactory};

use super::SharedCacheV2;

pub fn check_state_root_in_db(
provider: &impl DBProvider,
expected_state_root: B256,
) -> Result<(), SparseTrieError> {
let db_state_root = StateRoot::from_tx(provider.tx_ref())
.root()
.map_err(SparseTrieError::other)?;
if db_state_root == expected_state_root {
Ok(())
} else {
Err(SparseTrieError::WrongDatabaseTrieError)
}
}
Comment on lines +24 to +36
Copy link
Copy Markdown
Contributor

@claude claude Bot Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance concern: StateRoot::from_tx(provider.tx_ref()).root() recomputes the state root by walking the trie cursors. This is significantly more expensive than the previous check which was a simple block hash lookup.

This function is called once per storage proof target (in a parallel iterator at line 84) plus once for account proofs (line 119). For blocks with many touched accounts, this could add substantial overhead.

Is this intentional? If the goal is just to verify the DB hasn't been updated underneath, could you read the root node directly from the account trie table instead of recomputing it?

Copy link
Copy Markdown
Contributor Author

@dvush dvush Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional?
Yes it is, is once per database transaction.


#[derive(Debug, Default)]
pub struct MissingNodesFetcher {
storage_proof_targets: HashMap<B256, (B256Set, Vec<Nibbles>)>,
Expand Down Expand Up @@ -59,24 +72,16 @@ impl MissingNodesFetcher {
{
let fetched_nodes: Arc<Mutex<usize>> = Default::default();

let last_block_hash = shared_cache.last_block_hash;
let parent_state_root = shared_cache.parent_state_root;
std::mem::take(&mut self.storage_proof_targets)
.into_par_iter()
.map(
|(hashed_address, (targets, requested_proofs))| -> Result<(), SparseTrieError> {
let provider = consistent_db_view
.provider_ro()
.map_err(SparseTrieError::other)?;
if !last_block_hash.is_zero() {
let block_number = provider
.last_block_number()
.map_err(SparseTrieError::other)?;
let block_hash = provider
.block_hash(block_number)
.map_err(SparseTrieError::other)?;
if block_hash != Some(shared_cache.last_block_hash) {
return Err(SparseTrieError::WrongDatabaseTrieError);
}
if !parent_state_root.is_zero() {
check_state_root_in_db(&provider, parent_state_root)?;
}

let proof = StorageProof::new_hashed(
Expand Down Expand Up @@ -110,16 +115,8 @@ impl MissingNodesFetcher {
let provider = consistent_db_view
.provider_ro()
.map_err(SparseTrieError::other)?;
if !last_block_hash.is_zero() {
let block_number = provider
.last_block_number()
.map_err(SparseTrieError::other)?;
let block_hash = provider
.block_hash(block_number)
.map_err(SparseTrieError::other)?;
if block_hash != Some(shared_cache.last_block_hash) {
return Err(SparseTrieError::WrongDatabaseTrieError);
}
if !parent_state_root.is_zero() {
check_state_root_in_db(&provider, parent_state_root)?
}

let proof = Proof::new(
Expand Down
2 changes: 1 addition & 1 deletion crates/eth-sparse-mpt/src/v2/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const PARALLEL_HASHING_STORAGE_NODES: bool = true;
pub struct SharedCacheV2 {
pub account_trie: ProofStore,
pub storage_tries: Arc<DashMap<B256, ProofStore, FxBuildHasher>>,
pub last_block_hash: B256,
pub parent_state_root: B256,
}

impl SharedCacheV2 {
Expand Down
Loading