fix: ensure L1 messages are stored in db consistently #679
Conversation
This file contains 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
0xmountaintop
approved these changes
Mar 26, 2024
colinlyguo
approved these changes
Mar 26, 2024
|
To see if I understand correctly: the check of db error would not likely happen after adding failure check and retry in sync service. And it's added to the completeness of db corrupt detection. |
Yes, the DB error should never happen, but I added this check for completeness' sake. |
0xmountaintop
pushed a commit
that referenced
this pull request
Jun 10, 2024
* fix: ensure L1 messages are stored in db consistently * check db iterator errors
0xmountaintop
pushed a commit
that referenced
this pull request
Jun 13, 2024
* fix: ensure L1 messages are stored in db consistently * check db iterator errors
0xmountaintop
added a commit
that referenced
this pull request
Jun 13, 2024
* fix: ensure L1 messages are stored in db consistently (#679) * fix: ensure L1 messages are stored in db consistently * check db iterator errors * fix(sync-service): only add queue index when message index is not zero (#682) * fix(sync-service): increase queue index only when L1 message index is not zero --------- Co-authored-by: Péter Garamvölgyi <peter@scroll.io> fix --------- Co-authored-by: Péter Garamvölgyi <peter@scroll.io> Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com>
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.
1. Purpose or design rationale of this PR
In
fetchMessagesInRangewe did not checkit.Error(). If there is a transient error in the L1 node, we could miss some L1 messages and end up with an inconsistent database. The correct behavior is to detect error and retry.Since L1 message consistency (that means that we have all L1 messages with no gap in queue index) was taken for granted, the only consistency check was in
ReadL1MessagesFrom. This PR adds a consistency check inSyncService.fetchMessagesto allow us to detect inconsistencies early and avoid corrupting the node's local database.Fixes #592.
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?