Skip to content

ProofAggregationCoordinatorService: remove dependency on AggregationsRepository and ForcedTransactionsDao#2590

Merged
gauravahuja merged 2 commits intomainfrom
refactor_ProofAggregationCoordinatorService
Mar 20, 2026
Merged

ProofAggregationCoordinatorService: remove dependency on AggregationsRepository and ForcedTransactionsDao#2590
gauravahuja merged 2 commits intomainfrom
refactor_ProofAggregationCoordinatorService

Conversation

@gauravahuja
Copy link
Contributor

@gauravahuja gauravahuja commented Mar 16, 2026

Part 1 of #2241
This PR implements issue(s) #2241

Checklist

  • I wrote new tests for my new core changes.
  • I have successfully ran tests, style checker and build against my new changes locally.
  • If this change is deployed to any environment (including Devnet), E2E test coverage exists or is included in this
    PR.
  • I have informed the team of any breaking changes if there are any.

Note

Medium Risk
Medium risk because it changes how proven aggregations are persisted and how invalidity proofs are fetched by introducing new injected handlers/providers; incorrect wiring could break aggregation finalization or metrics updates.

Overview
Refactors ProofAggregationCoordinatorService to remove direct dependencies on AggregationsRepository and ForcedTransactionsDao by introducing injected abstractions: AggregationProofHandler (persists proven aggregations and updates highest/consecutive aggregation trackers) and InvalidityProofProvider (supplies invalidity proof indexes).

Updates ConflationApp wiring to construct and pass AggregationProofHandlerImpl, InvalidityProofProviderImpl, and an externally-created AggregationL2StateProviderImpl (and reuses a single l2MessageService instance). Tests are adjusted to mock the new interfaces and validate the aggregation flow with the new injection points.

Written by Cursor Bugbot for commit d87df61. This will update automatically on new commits. Configure here.

@codecov-commenter
Copy link

codecov-commenter commented Mar 16, 2026

Codecov Report

❌ Patch coverage is 5.71429% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.70%. Comparing base (f23c478) to head (d87df61).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...oordination/aggregation/AggregationProofHandler.kt 0.00% 31 Missing ⚠️
.../zkevm/coordinator/app/conflation/ConflationApp.kt 0.00% 21 Missing ⚠️
...oordination/aggregation/InvalidityProofProvider.kt 0.00% 11 Missing ⚠️
.../aggregation/ProofAggregationCoordinatorService.kt 57.14% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              main    #2590       +/-   ##
============================================
+ Coverage     9.70%   58.70%   +48.99%     
- Complexity       0     1611     +1611     
============================================
  Files          454      456        +2     
  Lines        18530    18541       +11     
  Branches      2014     2016        +2     
============================================
+ Hits          1799    10884     +9085     
+ Misses       16731     6989     -9742     
- Partials         0      668      +668     
Flag Coverage Δ *Carryforward flag
hardhat 96.53% <ø> (ø) Carriedforward from bf1e1a7
kotlin 54.52% <5.71%> (+54.41%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
.../aggregation/ProofAggregationCoordinatorService.kt 64.83% <57.14%> (+64.83%) ⬆️
...oordination/aggregation/InvalidityProofProvider.kt 0.00% <0.00%> (ø)
.../zkevm/coordinator/app/conflation/ConflationApp.kt 0.00% <0.00%> (ø)
...oordination/aggregation/AggregationProofHandler.kt 0.00% <0.00%> (ø)

... and 292 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gauravahuja gauravahuja force-pushed the refactor_ProofAggregationCoordinatorService branch from d436dd3 to 1a647a1 Compare March 16, 2026 05:42
@gauravahuja gauravahuja requested a review from a team March 17, 2026 07:17
@gauravahuja gauravahuja force-pushed the refactor_ProofAggregationCoordinatorService branch from 1a647a1 to aee2178 Compare March 19, 2026 04:25
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

…dependency on AggregationsRepository and ForcedTransactionsDao
@gauravahuja gauravahuja force-pushed the refactor_ProofAggregationCoordinatorService branch from aee2178 to bf1e1a7 Compare March 19, 2026 05:13
@fluentcrafter
Copy link
Collaborator

@claude-review

@gauravahuja gauravahuja merged commit 02862e9 into main Mar 20, 2026
40 checks passed
@gauravahuja gauravahuja deleted the refactor_ProofAggregationCoordinatorService branch March 20, 2026 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants