-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix intersecting l2 commitment blocks #853
Fix intersecting l2 commitment blocks #853
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since compare_commitments_from_db
isn't really just comparing anymore, i suggest calling it, adjust_commitments_from_db
or moving the set logic outside of the compare_
method.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
Question: We re-initialize the sequencer ledger db on startup from postgres... i believe the proper bug fix would be for the sequencer ledger DB to not be out of sync to begin with. Why does postgres have information ledger db doesn't have? |
@rakanalh what would you say to naming it |
Yea that works i guess. |
If you restart a sequencer, there is no problem. The problem is when full node is upgraded to be a sequencer. Full node does not track commitment related stuff in its LedgerDB. That is why this type of sync is necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Approving.
Please wait for @eyusufatik to review as well before merging.
Description
The issue was caused by the fact that when the sequencer is restarted, the sync of ledger db from postgres was problematic. This PR fixes the
compare_commitments_from_db
method.Linked Issues
Testing
Existing test
sequencer_crash_and_replace_full_node
works now.