refactor: cleanup abstractions in multiproofs #19308
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yongkangc
commented
Oct 27, 2025
| @@ -1,11 +1,7 @@ | |||
| //! Multiproof task related functionality. | |||
There was a problem hiding this comment.
The module in multiproof.rs is now organised as
//! This module is organized as follows:
//! - Main Types: [MultiproofManager], [MultiProofTask]
//! - Metrics: [MultiProofTaskMetrics]
//! - Supporting Types: [SparseTrieUpdate], [MultiProofConfig], [MultiProofMessage],
//! [StateHookSender]
//! - Internal Types: [ProofSequencer], [MultiProofTaskState], [MultiproofInput]
//! - Helper Functions: [evm_state_to_hashed_post_state], [get_proof_targets]
- Introduced `ProofWorkerHandle` for type-safe access to storage and account worker pools. - Replaced `ProofResultContext` with direct senders for improved performance. - Updated worker loops to handle proof tasks more efficiently, including better tracing and error handling. - Added methods for dispatching storage and account multiproof computations, improving interleaved parallelism. - Enhanced metrics tracking for storage and account processing.
yongkangc
commented
Oct 27, 2025
|
|
||
| type StorageProofResult = Result<DecodedStorageMultiProof, ParallelStateRootError>; | ||
| type TrieNodeProviderResult = Result<Option<RevealedNode>, SparseTrieError>; | ||
| type AccountMultiproofResult = |
There was a problem hiding this comment.
removed AccountMultiproofResult as it is redundant. Both account and storage results now come as ProofResult, with the account case carried by ProofResult::AccountMultiproof(multiproof, stats).
yongkangc
commented
Oct 27, 2025
yongkangc
commented
Oct 27, 2025
Comment on lines
-218
to
-225
| /// A pending multiproof task, either [`StorageMultiproofInput`] or [`MultiproofInput`]. | ||
| #[derive(Debug)] | ||
| enum PendingMultiproofTask { | ||
| /// A storage multiproof task input. | ||
| Storage(StorageMultiproofInput), | ||
| /// A regular multiproof task input. | ||
| Regular(MultiproofInput), | ||
| } |
There was a problem hiding this comment.
removed this
After we removed the old storage/account enum and always pass a single MultiproofInput value into MultiproofManager::dispatch, the alias was redundant.
Removing it lets thecall sites work directly with MultiproofInput, without indirection
…nversion - Updated the `dispatch` method to accept `MultiproofInput` directly, simplifying the handling of proof targets. - Removed the separate handling for `PendingMultiproofTask`, consolidating the dispatch logic. - Introduced `into_multiproof` method in `ProofResult` for converting proof results into `DecodedMultiProof`, enhancing clarity and usability.
- Updated the extraction of storage proofs to directly match against `ProofResult`, improving clarity and reducing unnecessary complexity. - Replaced error handling for mismatched addresses with `debug_assert_eq!` for better debugging during development. - Ensured that unreachable code paths are clearly defined for account multiproof handling, enhancing code safety and maintainability.
yongkangc
requested review from
Rjected,
fgimenez,
mattsse and
shekhirin
as code owners
|
splitting this up into smaller prs to review :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactors multiproof computation code to close #19182
Changes
ProofResultenum to eliminate unnecessary allocations when returning storage proofsDecodedMultiProofstructureStorageMultiproofInput(never constructed)PendingMultiproofTask(unnecessary wrapper)AccountMultiproofResult(unused alias)unreachable!()anddebug_assert_eq!()for impossible conditions with storage proofs.proof_task.rsandmultiproof.rsfor logical flow. i.e main types at the startFuture PR
DecodedMultiProof. This is a redudant abstraction that we should remove. tracking:refactor: Eliminate ProofResult Conversion Layer #19309
Testing