fix: check v7 batch compressed data compatibility #52
Conversation
This file contains hidden or 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
## Walkthrough
A new function, `checkCompressedDataCompatibilityV7`, was introduced to perform compatibility checks specific to version 7 of the compression format. The `DACodecV7` struct now uses this new function instead of the generic compatibility checker. The `checkCompressedDataCompatibility` method gained a `checkLength` flag to control length-based compression validation. The previously unimplemented `CheckChunkCompressedDataCompatibility` was implemented by delegating to batch compatibility checks. Related test cases involving repeated byte patterns were removed from the test suite.
## Changes
| File(s) | Change Summary |
|---------------------------|------------------------------------------------------------------------------------------------------------------|
| encoding/da.go | Added `checkCompressedDataCompatibilityV7` for v7-specific compressed data compatibility checks; updated existing compatibility check to ensure last block marker is set. |
| encoding/codecv7.go | Modified `checkCompressedDataCompatibility` to accept `checkLength` flag and use v7-specific checker; implemented `CheckChunkCompressedDataCompatibility` by delegating to batch check; updated method signatures accordingly. |
| encoding/codecv7_test.go | Removed `repeat` helper and associated repeated byte pattern test cases; updated calls to `checkCompressedDataCompatibility` with new `checkLength` argument. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant DACodecV7
participant checkCompressedDataCompatibilityV7
User->>DACodecV7: Call checkCompressedDataCompatibility(payloadBytes, checkLength)
DACodecV7->>checkCompressedDataCompatibilityV7: Validate compressed data format
checkCompressedDataCompatibilityV7-->>DACodecV7: Return error or success
DACodecV7->>DACodecV7: If checkLength true, compare compressed vs original length
DACodecV7-->>User: Return compatibility status and possibly compressed dataPossibly related PRs
Suggested reviewers
Poem
|
noel2004
reviewed
May 12, 2025
noel2004
previously approved these changes
May 12, 2025
3 tasks
added 2 commits
May 14, 2025 01:04
…length check in some cases
noel2004
previously approved these changes
May 15, 2025
jonastheis
reviewed
May 16, 2025
jonastheis
approved these changes
May 19, 2025
noel2004
approved these changes
May 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Purpose or design rationale of this PR
For v7 batches, uncompressed cases are resolved. This PR removes the uncompressed case checks and only keeps sanity checks. If sanity check fails, the circuit needs manual fix, and rollup-relayer can still commit uncompressed data.
Some unit tests related to fallback to
uncompressed modeare removed in this PR as well.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:
Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit