Skip to content

fix(engine): remove redundant parent_to_child removal during eviction#18648

Merged
yongkangc merged 2 commits intoparadigmxyz:mainfrom
Forostovec:fix/block-buffer-eviction-remove-duplicate-parent-removal
Sep 25, 2025
Merged

fix(engine): remove redundant parent_to_child removal during eviction#18648
yongkangc merged 2 commits intoparadigmxyz:mainfrom
Forostovec:fix/block-buffer-eviction-remove-duplicate-parent-removal

Conversation

@Forostovec
Copy link
Contributor

  • Drop duplicate remove_from_parent call in insert_block eviction path; remove_block already handles it.
  • No functional changes; micro-optimization and slight simplification.
  • Tests and metrics unaffected; eviction remains FIFO via block_queue.

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

but would like a sanity check from @yongkangc or @shekhirin perhaps

if let Some(evicted_block) = self.remove_block(&evicted_hash) {
self.remove_from_parent(evicted_block.parent_hash(), &evicted_hash);
}
self.remove_block(&evicted_hash);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is indeed redundant here

fn remove_block(&mut self, hash: &BlockHash) -> Option<RecoveredBlock<B>> {
let block = self.blocks.remove(hash)?;
self.remove_from_earliest_blocks(block.number(), hash);
self.remove_from_parent(block.parent_hash(), hash);

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Sep 23, 2025
Copy link
Member

@yongkangc yongkangc left a comment

Choose a reason for hiding this comment

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

generally looks ok to me

would be nice to maybe add test for parent-child cleanup is properly maintained during eviction during eviction?

@Forostovec
Copy link
Contributor Author

generally looks ok to me

would be nice to maybe add test for parent-child cleanup is properly maintained during eviction during eviction?

added test

@yongkangc yongkangc added this pull request to the merge queue Sep 25, 2025
@yongkangc
Copy link
Member

sending

Merged via the queue into paradigmxyz:main with commit 4d60984 Sep 25, 2025
41 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Sep 25, 2025
@Forostovec Forostovec deleted the fix/block-buffer-eviction-remove-duplicate-parent-removal branch November 3, 2025 08:51
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