Skip to content

refactor(trie): restructure proof task worker#19060

Closed
yongkangc wants to merge 5 commits intomainfrom
yk/cleanup_proof
Closed

refactor(trie): restructure proof task worker#19060
yongkangc wants to merge 5 commits intomainfrom
yk/cleanup_proof

Conversation

@yongkangc
Copy link
Copy Markdown
Contributor

@yongkangc yongkangc commented Oct 16, 2025

Closes: #19007

Changes:

  • Turn function based loops that are stateful into structs
    • e.g spawn_account_loop -> AccountProofWorker::run()

Copy link
Copy Markdown
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

Refactor to restructure trie proof task workers from function-based loops into dedicated worker structs for improved clarity and reuse.

  • Introduces StorageProofWorker and AccountProofWorker structs with run methods.
  • Extracts shared transaction/factory logic into ProofTaskTx and centralizes worker pool management in ProofWorkerHandle.

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

Comment thread crates/trie/parallel/src/proof_task.rs Outdated
Comment thread crates/trie/parallel/src/proof_task.rs Outdated
@yongkangc yongkangc requested a review from Copilot October 16, 2025 05:57
Copy link
Copy Markdown
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

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


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

Comment thread crates/trie/parallel/src/proof_task.rs Outdated
@yongkangc yongkangc requested a review from Copilot October 16, 2025 06:03
Copy link
Copy Markdown
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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


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

Comment thread crates/trie/parallel/src/proof_task.rs
Comment thread crates/trie/parallel/src/proof_task.rs
Copy link
Copy Markdown
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

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

Comments suppressed due to low confidence (2)

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

Comment thread crates/trie/parallel/src/proof_task.rs
/// Provided by the user to give the necessary context to retain extra proofs.
multi_added_removed_keys: Option<Arc<MultiAddedRemovedKeys>>,
/// Worker responsible for account trie operations.
struct AccountProofWorker<Factory> {
Copy link
Copy Markdown
Contributor Author

@yongkangc yongkangc Oct 16, 2025

Choose a reason for hiding this comment

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

this is the only change - worker struct

self.task_ctx.state_sorted.as_ref(),
);
/// Worker responsible for storage trie operations.
struct StorageProofWorker<Factory> {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the only change with AccountProofWorker

@yongkangc
Copy link
Copy Markdown
Contributor Author

Will be letting this run in Reth4 for abit

@yongkangc yongkangc marked this pull request as ready for review October 16, 2025 13:35
@yongkangc yongkangc requested a review from mattsse October 16, 2025 13:37
rv64m

This comment was marked as abuse.

@github-project-automation github-project-automation Bot moved this from Backlog to In Progress in Reth Tracker Oct 22, 2025
yongkangc added a commit that referenced this pull request Oct 28, 2025
Following the pattern from PR #19060, extract common blinded node processing
logic into helper methods on ProofTaskTx:

- Add process_blinded_storage_node() - handles storage trie node lookups
- Add process_blinded_account_node() - handles account trie node lookups

Worker methods now delegate to these helpers, keeping them simple and focused
on tracing/metrics while the business logic lives in ProofTaskTx. This reduces
code duplication and follows established patterns in the codebase.
yongkangc added a commit that referenced this pull request Oct 28, 2025
Following the pattern from PR #19060, extract common blinded node processing
logic into helper methods on ProofTaskTx:

- Add process_blinded_storage_node() - handles storage trie node lookups
- Add process_blinded_account_node() - handles account trie node lookups

Worker methods now delegate to these helpers, keeping them simple and focused
on tracing/metrics while the business logic lives in ProofTaskTx. This reduces
code duplication and follows established patterns in the codebase.
@yongkangc yongkangc closed this Oct 28, 2025
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Reth Tracker Oct 28, 2025
@jenpaff jenpaff moved this from Done to Out of Scope in Reth Tracker Oct 30, 2025
@mediocregopher mediocregopher deleted the yk/cleanup_proof branch February 25, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Out of Scope

Development

Successfully merging this pull request may close these issues.

refactor: add struct to loop and ProofTaskMangerHandle

4 participants