-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
op-batcher: Refactor frame & tx data handling #5083
Conversation
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #5083 +/- ##
===========================================
- Coverage 41.19% 36.32% -4.88%
===========================================
Files 349 209 -140
Lines 21317 17241 -4076
Branches 776 0 -776
===========================================
- Hits 8782 6262 -2520
+ Misses 11866 10347 -1519
+ Partials 669 632 -37
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ab8d55a
to
81047fd
Compare
c9c3502
to
4e9f5cd
Compare
4e9f5cd
to
1b5f85d
Compare
1b5f85d
to
448a3b3
Compare
Hey @sebastianst! This PR has merge conflicts. Please fix them before continuing review. |
448a3b3
to
6d862ab
Compare
Introduces two separate types for frame data and tx data to make a clear distinction between the two. Also prepares the batcher for future changes when it will support sending multiple frames per tx.
Also adds a function RandomBlockPrependTxs to package op-node/testutils to avoid code duplication with RandomBlock.
Regression test for #4937
6d862ab
to
4df9505
Compare
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.
lgtm
This PR has been added to the merge queue, and will be merged soon. |
Hey @sebastianst, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by commenting with |
Description
Introduces two separate types for frame data and tx data to make a clear
distinction between the two. Also prepares the batcher for future changes
when it will support sending multiple frames per tx.
Tests
Added a regression test to validate that requeued transactions have the same data (and don't prepend 0 version bytes) and that transaction data can be parsed correctly.
I added a random L2 block generator. Because of dependency cycles, I couldn't put it into package
op-node/testutils
since it uses a function from thederive
package. Hence added the random generator to a new packageop-node/rollup/derive/test
, following the pattern to create testing code dependent on a package in a.../test
sub-package.Additional context
Cleaner solution of #5050