Skip to content

Comments

refactor(tree): remove unused Factory generic from multiproof system#18933

Merged
yongkangc merged 6 commits intoyk/worker_pool_accfrom
refactor/remove-factory-generic
Oct 13, 2025
Merged

refactor(tree): remove unused Factory generic from multiproof system#18933
yongkangc merged 6 commits intoyk/worker_pool_accfrom
refactor/remove-factory-generic

Conversation

@yongkangc
Copy link
Member

@yongkangc yongkangc commented Oct 10, 2025

Removes the unused Factory generic parameter from ParallelProof, MultiProofConfig, MultiProofTask, and related types. The generic only existed to carry ConsistentDbView<Factory> but was never accessed.

Closes #18932

- Removed the Factory type parameter from the ParallelProof struct, streamlining its definition and implementation.
- Updated the constructor and related methods to reflect this change, enhancing code clarity and maintainability.
- Eliminated unused PhantomData field, reducing complexity in the struct's design.
- Eliminated the Factory type definition from the proof tests, simplifying the code structure.
- This change contributes to improved clarity and maintainability of the test implementation.
- Removed the generic Factory type from MultiProofConfig and related structs, streamlining their definitions and improving code clarity.
- Updated methods to reflect the removal of the Factory type, enhancing maintainability.
- Adjusted the implementation of PendingMultiproofTask and its associated methods to eliminate unnecessary type parameters, simplifying the codebase.
@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Oct 10, 2025
@github-actions github-actions bot added A-trie Related to Merkle Patricia Trie implementation C-debt A clean up/refactor of existing code labels Oct 10, 2025
@yongkangc yongkangc marked this pull request as ready for review October 10, 2025 08:05
@yongkangc yongkangc changed the base branch from main to yk/worker_pool_acc October 10, 2025 08:05
@yongkangc yongkangc self-assigned this Oct 10, 2025
@yongkangc yongkangc requested a review from Copilot October 10, 2025 08:12
@yongkangc yongkangc moved this from Backlog to In Progress in Reth Tracker Oct 10, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This refactoring removes the unused Factory generic parameter from the multiproof system components. The generic was originally added to carry ConsistentDbView<Factory> but analysis showed it was never actually accessed in production code.

  • Removes the Factory generic from ParallelProof, MultiProofConfig, MultiProofTask, and related types
  • Simplifies the API by eliminating unnecessary generic type parameters
  • Updates method signatures and removes phantom data fields that were only used to maintain generic constraints

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
crates/trie/parallel/src/proof.rs Removes Factory generic from ParallelProof struct and impl blocks, removes phantom data field
crates/engine/tree/src/tree/payload_processor/multiproof.rs Removes Factory generic from all multiproof-related structs and simplifies configuration creation
crates/engine/tree/src/tree/payload_processor/mod.rs Updates method call to use simplified configuration creation without Factory generic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +81 to 84
/// Creates a new state root config from the trie input.
///
/// This returns a cleared [`TrieInput`] so that we can reuse any allocated space in the
/// [`TrieInput`].
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

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

The documentation comment is outdated. The method now extracts/splits configuration from input rather than creating 'from' input, and it returns both a cleared TrieInput and the config.

Suggested change
/// Creates a new state root config from the trie input.
///
/// This returns a cleared [`TrieInput`] so that we can reuse any allocated space in the
/// [`TrieInput`].
/// Extracts configuration from the given [`TrieInput`], splitting out the config and returning
/// both a cleared [`TrieInput`] (for reuse of allocated space) and the extracted [`MultiProofConfig`].
///
/// This allows for efficient reuse of the input's allocated space and separates the configuration
/// needed for multiproof computation.

Copilot uses AI. Check for mistakes.
@yongkangc yongkangc changed the title refactor: remove unused Factory generic from multiproof system refactor(tree): remove unused Factory generic from multiproof system Oct 10, 2025
@yongkangc yongkangc merged commit 3e957a5 into yk/worker_pool_acc Oct 13, 2025
38 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Oct 13, 2025
@yongkangc yongkangc deleted the refactor/remove-factory-generic branch October 13, 2025 05:13
@jenpaff jenpaff moved this from Done to Completed in Reth Tracker Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-trie Related to Merkle Patricia Trie implementation C-debt A clean up/refactor of existing code

Projects

Status: Completed

Development

Successfully merging this pull request may close these issues.

Remove Factory from ParallelProof

3 participants