refactor: dedupe batch header / blob helpers, single-source on common/batch#945
Merged
curryxbo merged 3 commits intofeat/multi_batchfrom May 7, 2026
Merged
Conversation
…oding This fixes correctness bugs that have been quietly diverging between the sequencer-side fork (common/batch) and the derivation-side fork (node/types). The fixes are ported from node/types so that, in the next commit, derivation can switch to common/batch without losing them. batch_header.go: - Add expectedLengthV2 = 257 (V2 reuses the V1 wire layout but the field at offset 57 holds an aggregated blob hash, not a single versioned hash). - Make BlobVersionedHash() reject V2 batches; previously it silently returned the aggregated hash as if it were a versioned hash, which is wrong and would propagate the wrong value to any caller that hadn't noticed the V2 semantic shift. - Add BlobHashesHash() for V2+ which exposes keccak256(blobhash(0)||...|| blobhash(N-1)); V0/V1 are explicitly rejected so callers can't pick the wrong accessor. blob.go (DecodeTxsFromBytes): - Split the old switch into decodeTypedTx (AccessList/DynamicFee/SetCode) and decodeMorphTx. The old code unconditionally consumed one byte after the MorphTx type byte and treated it as the RLP prefix, which is wrong for the V1 MorphTx wire format `type(0x7f)||version(0x01)||RLP(...)`: the version byte (0x01) was being passed to extractInnerTxFullBytes as firstByte, where firstByte - 0xf7 underflows to 10 and the next read either fails out-of-range or allocates a multi-GB buffer. - Typed txs (AccessList/DynamicFee/SetCode) are now reconstructed via Transaction.UnmarshalBinary instead of rlp.DecodeBytes onto the inner type, matching the EIP-2718 envelope path used elsewhere. - Add `sizeByteLen > 4` guard in extractInnerTxFullBytes; firstByte=0x01 used to underflow to 10 and silently bypass any sanity check. batch_header_test.go: - New unit tests for the V2 routing: TestBatchHeaderV2 covers the V2 encoding, version dispatch, BlobVersionedHash rejection, BlobHashesHash acceptance, and the length check; TestBlobHashesHashUnavailableForLegacy guards V0/V1 against accidentally calling the V2 accessor. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Now that common/batch carries the V2 routing fix and the MorphTx V1 wire decode fix (previous commit), redirect every consumer of the duplicated node/types symbols to the common/batch versions. Three sites touched: - node/derivation/batch_info.go: BatchHeaderBytes, MaxBlobBytesSize, RetrieveBlobBytes, DecodeTxsFromBytes — the parent header parse and blob byte extraction now go through the V2-aware helpers. - node/derivation/batch_info_test.go: BatchHeaderV0/V1, EmptyVersionedHash, MaxBlobBytesSize, MakeBlobCanonical, RetrieveBlobBytes — the file references EmptyVersionedHash and MakeBlobCanonical which main's PR #939 deleted from node/types, so this also unbreaks `go test ./node/derivation/...`. - oracle/oracle/batch.go: BatchHeaderBytes — its only nodetypes use; the nodetypes import is dropped here. tx-submitter/utils/utils.go is intentionally NOT touched in this PR even though it carries the same `node/types.BatchHeaderBytes` import: a naive switch to common/batch closes a cycle (common/batch → tx-submitter/db → tx-submitter/utils → common/batch). Untangling that cycle requires moving BatchCache / batch_storage / batch_query out of common/batch and down to tx-submitter/, which is a tx-submitter-owned refactor and out of scope here. node/types/batch_header.go is therefore kept alive in the next commit so that tx-submitter/utils continues to build; that file carries a DEPRECATED header documenting the cleanup path. types.WrappedBlock continues to come from node/types in batch_info.go — it's a node-only consensus type, not part of the duplicated batch encoding surface. Co-authored-by: Cursor <cursoragent@cursor.com>
…precated - Delete node/types/blob.go: nothing under tx-submitter/* or anywhere else still imports MaxBlobBytesSize / RetrieveBlobBytes / DecodeTxsFromBytes / MakeBlobCanonical from node/types after the previous commit, so it can go. - Delete node/types/batch_test.go: it referenced EmptyVersionedHash (which main's PR #939 already removed from node/types) and only exercised V0/V1 layout — the V2-aware unit tests now live in common/batch/batch_header_test.go. - Keep node/types/batch_header.go but add a DEPRECATED file-header comment: tx-submitter/utils/utils.go still imports BatchHeaderBytes from here, and the obvious cleanup (turn this file into a thin re-export of common/batch.BatchHeaderBytes) would close an import cycle (common/batch → tx-submitter/db → tx-submitter/utils → common/batch via the shim). The header comment documents the proper cleanup path: move BatchCache out of common/batch into tx-submitter/, then redirect utils, then delete this file. After this commit, the duplicate is intentional and documented. The remaining cleanup is owned by the tx-submitter team. Co-authored-by: Cursor <cursoragent@cursor.com>
curryxbo
force-pushed
the
refactor/dedup-batch-types-into-common
branch
from
d6c1f33 to
acd83f1
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Eliminate the duplicated batch-header / blob helpers that lived in both
common/batchandnode/types. The two copies had silently diverged:the
common/batchversions were missing the V2 header routing fix, theMorphTx V1 wire-format decoding, and the
sizeByteLen > 4guard. ThisPR backports those fixes to
common/batch, switchesderivationandoracleconsumers to import fromcommon/batch, and removes thenode/typesduplicates that are no longer used.Commits
fix(common/batch): backport V2 header routing and MorphTx V1 wire decodingBlobVersionedHash()now panics for V2+ headers (useBlobHashesHash()instead) so misuse stops returning silentlywrong data.
DecodeTxsFromBytesrecognises the MorphTx V0/V1 wire envelopesproduced by the sequencer; otherwise legacy / typed-tx bytes go
through the standard RLP path.
extractInnerTxFullBytessizeByteLen > 4guard.common/batch/batch_header_test.gocovers V2 routing and theaccessor-misuse panic.
refactor: switch derivation and oracle batch helpers to common/batchnode/derivation/batch_info.go,node/derivation/batch_info_test.go,oracle/oracle/batch.gonowimport
morph-l2/common/batchforBatchHeaderBytes,BatchHeaderV0/V1,EmptyVersionedHash,MaxBlobBytesSize,MakeBlobCanonical,RetrieveBlobBytes,DecodeTxsFromBytes.tx-submitter/— see note below.refactor(node/types): drop unused duplicates, mark batch_header as deprecatednode/types/blob.goandnode/types/batch_test.go(functionality and coverage now lives in
common/batch).node/types/batch_header.gowith a top-of-fileDEPRECATEDcomment. It is preserved purely becausetx-submitter/utils/utils.gostill importsBatchHeaderBytesfromhere, and switching that one site to
common/batchwould close animport cycle (
common/batch→tx-submitter/db(viaBatchCache) →tx-submitter/utils→common/batch).Follow-up owned by tx-submitter team
The remaining duplicate (
node/types/batch_header.go) goes away oncetx-submitterdoes the architectural cleanup the comment describes:common/batch/batch_cache.go,batch_storage.go,batch_query.godown intotx-submitter/batch/socommon/batchbecomes a true leaf (depends on nothing under
tx-submitter/).tx-submitter/utils/utils.goto importmorph-l2/common/batch.BatchHeaderBytes.node/types/batch_header.go.That work is intentionally out of scope here.
Test plan
cd common && go build ./... && go test ./batch/...cd node && go build ./... && go test ./derivation/...cd oracle && go build ./...cd tx-submitter && go build ./...— unchanged in this PR; thetx-submitter team owns the cycle-breaking cleanup above.