fix(protocol/derive): fix singular batch extraction error handling#3059
fix(protocol/derive): fix singular batch extraction error handling#3059
Conversation
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Sentry. |
a315f6c to
f13c6ec
Compare
b6e55ee to
b229171
Compare
390f77c to
bc7f8e5
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes singular batch extraction error handling in the protocol derivation pipeline for post-Holocene batch validation. It addresses a scenario where overlapping span batches can pass prefix checks but contain batches with outdated L1 origins after the safe head. When such invalid batches are detected during extraction, the stage is now properly flushed and the error is handled consistently with dropped batches during prefix checks.
Key Changes:
- Added validation check to detect and reject batches with L1 origins before the safe head L1 origin
- Improved error handling in
BatchStreamto flush the stage when singular batch extraction fails - Refactored batch validation logic with clearer variable naming (
batch_timestamp,batch_epoch)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
crates/protocol/protocol/src/batch/errors.rs |
Adds L1OriginBeforeSafeHead error variant for detecting outdated batch origins |
crates/protocol/protocol/src/batch/span.rs |
Implements validation check in both get_singular_batches and check_batch methods, improves variable naming consistency, adds comprehensive test coverage |
crates/protocol/derive/src/stages/batch/batch_stream.rs |
Updates error handling to properly catch extraction failures, flush the stage, and propagate errors; consolidates metrics code under feature flag |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bc7f8e5 to
08102f1
Compare
|
@sebastianst there are still a couple of tests failing. Can we fix those before we merge this PR? |
08102f1 to
0328a23
Compare
0328a23 to
fe938c7
Compare
|
It seems that CI is stuck for this one. I am going to force merge it. I need to get this in asap |
…p-rs/kona#3059) This is the same fix as #18283 for Kona. Still WIP. Only fixed for the post-Holocene `BatchValidator` so far, old `BatchQueue` still todo, but should be easy. Tests also still todo. The derivation rule was technically in the specs already, but they get clarified with ethereum-optimism/specs#863. Co-authored-by: theo <80177219+theochap@users.noreply.github.com>
…p-rs/kona#3059) This is the same fix as #18283 for Kona. Still WIP. Only fixed for the post-Holocene `BatchValidator` so far, old `BatchQueue` still todo, but should be easy. Tests also still todo. The derivation rule was technically in the specs already, but they get clarified with ethereum-optimism/specs#863. Co-authored-by: theo <80177219+theochap@users.noreply.github.com>
This is the same fix as ethereum-optimism/optimism#18283 for Kona.
Fixed for the post-Holocene
BatchValidatorand oldBatchQueue. Unit tests added.The derivation rule was technically in the specs already, but they get clarified with ethereum-optimism/specs#863.