From 780161a6472952704c5e1cc6d8f56fbda172d660 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Sat, 1 Nov 2025 16:13:51 +0100 Subject: [PATCH] chore: OverlayStateProviderFactory: don't query for reverts unless necessary (#19412) --- .../provider/src/providers/state/overlay.rs | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/crates/storage/provider/src/providers/state/overlay.rs b/crates/storage/provider/src/providers/state/overlay.rs index 5c086c273b..f912411f1e 100644 --- a/crates/storage/provider/src/providers/state/overlay.rs +++ b/crates/storage/provider/src/providers/state/overlay.rs @@ -73,16 +73,30 @@ where F: DatabaseProviderFactory, F::Provider: TrieReader + StageCheckpointReader + PruneCheckpointReader + BlockNumReader, { - /// Validates that there are sufficient changesets to revert to the requested block number. + /// Returns the block number for [`Self`]'s `block_hash` field, if any. + fn get_block_number(&self, provider: &F::Provider) -> ProviderResult> { + if let Some(block_hash) = self.block_hash { + Ok(Some( + provider + .convert_hash_or_number(block_hash.into())? + .ok_or_else(|| ProviderError::BlockHashNotFound(block_hash))?, + )) + } else { + Ok(None) + } + } + + /// Returns whether or not it is required to collect reverts, and validates that there are + /// sufficient changesets to revert to the requested block number if so. /// /// Returns an error if the `MerkleChangeSets` checkpoint doesn't cover the requested block. /// Takes into account both the stage checkpoint and the prune checkpoint to determine the /// available data range. - fn validate_changesets_availability( + fn reverts_required( &self, provider: &F::Provider, requested_block: BlockNumber, - ) -> ProviderResult<()> { + ) -> ProviderResult { // Get the MerkleChangeSets stage and prune checkpoints. let stage_checkpoint = provider.get_stage_checkpoint(StageId::MerkleChangeSets)?; let prune_checkpoint = provider.get_prune_checkpoint(PruneSegment::MerkleChangeSets)?; @@ -99,7 +113,7 @@ where // If the requested block is the DB tip (determined by the MerkleChangeSets stage // checkpoint) then there won't be any reverts necessary, and we can simply return Ok. if upper_bound == requested_block { - return Ok(()) + return Ok(false) } // Extract the lower bound from prune checkpoint if available @@ -123,7 +137,7 @@ where }); } - Ok(()) + Ok(true) } } @@ -140,15 +154,10 @@ where let provider = self.factory.database_provider_ro()?; // If block_hash is provided, collect reverts - let (trie_updates, hashed_state) = if let Some(block_hash) = self.block_hash { - // Convert block hash to block number - let from_block = provider - .convert_hash_or_number(block_hash.into())? - .ok_or_else(|| ProviderError::BlockHashNotFound(block_hash))?; - - // Validate that we have sufficient changesets for the requested block - self.validate_changesets_availability(&provider, from_block)?; - + let (trie_updates, hashed_state) = if let Some(from_block) = + self.get_block_number(&provider)? && + self.reverts_required(&provider, from_block)? + { // Collect trie reverts let mut trie_reverts = provider.trie_reverts(from_block + 1)?; @@ -186,7 +195,7 @@ where debug!( target: "providers::state::overlay", - ?block_hash, + block_hash = ?self.block_hash, ?from_block, num_trie_updates = ?trie_updates.total_len(), num_state_updates = ?hashed_state_updates.total_len(),