Skip to content

Conversation

silvekkk
Copy link
Contributor

Improves performance of log query operations by optimizing how data is passed between internal functions. Refactoring reduces redundant operations by passing additional context that's already available at call sites, resulting in measurable performance improvements particularly for range queries.

Internal optimization only.

&& let Some(block) = storage.blocks.get(&block_hash).cloned()
{
drop(storage);
all_logs.extend(self.mined_logs_for_block(filter.clone(), block, block_hash));
Copy link
Contributor

@onbjerg onbjerg Oct 1, 2025

Choose a reason for hiding this comment

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

idt this change makes sense, using get_block is fine, what is the reasoning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change avoids redundant hash_slow() calls. get_block(number) internally does hashes.get(&number) + blocks.get(&hash) but discards the hash, forcing mined_logs_for_block to recompute it via expensive RLP encoding + Keccak256.


/// Returns all `Log`s mined by the node that were emitted in the `block` and match the `Filter`
fn mined_logs_for_block(&self, filter: Filter, block: Block) -> Vec<Log> {
fn mined_logs_for_block(&self, filter: Filter, block: Block, block_hash: B256) -> Vec<Log> {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@onbjerg onbjerg self-assigned this Oct 1, 2025
@silvekkk silvekkk requested a review from onbjerg October 1, 2025 13:04
@onbjerg onbjerg merged commit 632c586 into foundry-rs:master Oct 2, 2025
16 checks passed
@github-project-automation github-project-automation bot moved this to Done in Foundry Oct 2, 2025
silvekkk added a commit to silvekkk/foundry that referenced this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants