-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Modify Relayer service to order Events from L1 by block index #1779
Merged
Merged
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
02ec8cd
Change mock db to maintain order
MitchTurner d68bd4b
Add failing test
MitchTurner 2120be1
Get ordered tx test passing, start message test
MitchTurner b200848
WIP fixing message test
MitchTurner c39961f
Fix broken test
MitchTurner 307cf7c
Mark hung test to keep CI from running forever
MitchTurner 65491a2
Fix sad path test, fix other test definition
MitchTurner fe2c0f0
Make iterator combinator easier to read
MitchTurner bb33d12
Merge branch 'master' into ensure-relayer-message-order
MitchTurner 5d9f37a
Update CHANGELOG
MitchTurner a977697
Add missing Options
MitchTurner cfc9939
Add more missing `Option`s
MitchTurner 3db8add
Appease Clippy-sama
MitchTurner 3ceacf7
Redesign mock to fix pagination tests
MitchTurner 2f82803
Fix more tests
MitchTurner 8427203
Remove `Option`, ignore Clippy-sama
MitchTurner e831dd0
Remove all the formatting changes to CHANGELOG and move to `Breaking`
MitchTurner 954607c
Refactor some service stuff
MitchTurner 46bba1c
Reverted changes to `MockMiddleware` and use timeout in test instead
MitchTurner 502a1f8
Configure task to allow single runs in test
MitchTurner f0a8297
Fix broken constructors
MitchTurner daabb9d
Merge branch 'master' into ensure-relayer-message-order
xgreenx b249ebb
Return `Err` instead of `Ok` when `retry_on_error`
MitchTurner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
The alternative here would be set to
0
ifNone
.I think I prefer the error, but there are still ways for this to be non-deterministic, e.g. if there are more than one log with the same index.
So, maybe we need to have additional checks that there are no duplicates? And do we want to accept batches with missing log indices?
If we don't want to worry about all the edge cases, then we could just do
.unwrap_or(0)
instead of.ok_or
?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.
Talked to the team about this offline. It sounds like we are okay with throwing an error here and not including the batch. In practice, this value should always be
Some
.In a similar vein, we can probably assume they will always be in order and all unique.
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.
Are all events coming through this sort helper
fn
guaranteed to be grouped by da_height?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.
They get sorted into blocks after they are sorted by index. So it will maintain order once sorted by block.