-
Notifications
You must be signed in to change notification settings - Fork 82
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(batcher): include checksum in zlib-compressed channel encoding #395
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
b3c6d6f
to
808998f
Compare
808998f
to
c27a652
Compare
c27a652
to
29cbab7
Compare
I think the batch derivation process involves a read through zlib.Reader and that includes checksum verification. |
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
I have confirmed that the zlib.Reader in Go does not always verify the checksum in all cases. Therefore, in BatchReader, it performs decompression without checking the checksum, regardless of its presence. |
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
Description
This PR addresses an issue in the batcher where the checksum was not being included during the compression of rlp_spanbatches using zlib. The checksum is now correctly included in the compressed data.
This work originated from an investigation into a derivation failure observed in Kona, during which it was discovered that the checksum was missing in the compressed output.
Additionally, a related upstream PR #10002, primarily focused on optimization and refactoring, also resolved this checksum issue as part of its broader changes.