consensus/bor: refactor Finalize to add better checks for state-sync#2185
consensus/bor: refactor Finalize to add better checks for state-sync#2185
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
Refactors the consensus Engine.Finalize pipeline to return errors (instead of silently failing) and introduces clearer Bor state-sync error categorization so callers can surface the real root cause (processing failure vs. local/producer mismatch).
Changes:
- Update
consensus.Engine.Finalizeto return([]*types.Receipt, error)and propagate errors through state processors. - Introduce
core.ErrStateSyncMismatchand tighten post-Madhugiri state-sync tx/receipt validation. - Expand Bor tests to cover state-sync mismatch and processing error scenarios.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/bor/bor_test.go | Updates expected error type for invalid state-sync tx in block body. |
| core/state_processor.go | Propagates Finalize errors; improves state-sync receipt count mismatch error. |
| core/parallel_state_processor.go | Propagates Finalize errors; improves state-sync receipt count mismatch error. |
| core/error.go | Adds ErrStateSyncMismatch and clarifies Bor state-sync error semantics. |
| consensus/consensus.go | Changes Engine.Finalize signature to return receipts + error. |
| consensus/ethash/consensus.go | Adapts Ethash Finalize to new signature and updates FinalizeAndAssemble. |
| consensus/clique/clique.go | Adapts Clique Finalize/FinalizeAndAssemble to new signature. |
| consensus/beacon/consensus.go | Adapts Beacon Finalize/FinalizeAndAssemble to new signature. |
| consensus/bor/bor.go | Implements stricter Bor state-sync validation and returns structured errors from Finalize. |
| consensus/bor/bor_test.go | Updates existing tests for new Finalize signature; adds state-sync validation tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (66.66%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #2185 +/- ##
===========================================
+ Coverage 51.93% 51.96% +0.02%
===========================================
Files 884 884
Lines 155446 155453 +7
===========================================
+ Hits 80731 80780 +49
+ Misses 69509 69465 -44
- Partials 5206 5208 +2
... and 22 files with indirect coverage changes
🚀 New features to boost your workflow:
|
|
Description
The Finalize method being used in bor consensus to apply state-sync didn't return an error. The way errors are handled isn't very straight-forward as it puts everything on the caller to validate and handle appropriate errors. This isn't very neat as the root cause is completely hidden and the final error bubbled up is completely different. E.g. if we fail applying state-sync events, final error is invalid state root. Ideally the original error should be reported as it can have some nuances on why it failed.
This PR
Changes
Breaking changes
Please complete this section if any breaking changes have been made, otherwise delete it
Nodes audience
In case this PR includes changes that must be applied only to a subset of nodes, please specify how you handled it (e.g. by adding a flag with a default value...)
Checklist
Cross repository changes
Testing
Manual tests
Please complete this section with the steps you performed if you ran manual tests for this functionality, otherwise delete it
Additional comments
Please post additional comments in this section if you have them, otherwise delete it