[don't merge]fix(rollup): reject empty block span in commitBatch (#996)

[curryxbo]

What

Closes #996. Fixes the on-chain root cause of the batch-parse DoS mitigated node-side in #994 / #995.

Rollup.sol::_commitBatch accepted an unconstrained lastBlockNumber from calldata — there was no check that the committed batch's lastBlockNumber is strictly greater than the parent's (i.e. that blockCount >= 1). Because the BLS signature path is still a stub (_getBLSMsgHash returns bytes32(0)), the field is not signature-bound, so a single active staker (byzantine or buggy) can commit lastBlockNumber == parent.lastBlockNumber (empty span) on L1. Derivation nodes in layer1 verify mode then panic (pre-#995) or stall permanently (post-#995) on that batch, since it cannot be skipped without diverging from the L1-canonical chain.

Fix

In _commitBatchWithBatchData (the shared path for commitBatch / commitState / commitBatchWithProof), after loading the parent header and before computing the data hash:

if (BatchHeaderCodecV0.getVersion(_batchPtr) >= 1) {
    require(
        batchDataInput.lastBlockNumber > BatchHeaderCodecV1.getLastBlockNumber(_batchPtr),
        "empty block span"
    );
}

This mirrors the node's blockCount = lastBlockNumber - parentLastBlockNumber derivation. The check only applies when the parent header carries a lastBlockNumber (V1+). The V0→V1 transition (parent version 0) carries no lastBlockNumber and is a historical one-time event, so it is not guarded here — matching the node's parentVersion == 0 branch.

Tests

New tests in RollupCommitBatchWithProofTest:

• test_commitBatchWithProof_reverts_on_empty_block_span — lastBlockNumber == parent (the off-by-one the original underflow guard missed) → reverts "empty block span".
• test_commitBatchWithProof_allows_monotonic_block_span — lastBlockNumber > parent → still commits. This is the only happy-path coverage for a V1 parent; the existing suite is all V0, which skips the guard.

Full Rollup.t.sol suite (39 tests across 5 contracts) passes, including the existing test_commitBatches_succeeds / test_revertBatch_succeeds multi-batch paths.

Scope / follow-up (out of scope)

• _getBLSMsgHash is still a bytes32(0) stub — until batch fields (incl. lastBlockNumber) are bound to a real BLS signature over the sequencer set (_getValidSequencerSet is also a TODO), the data hash is authenticated only by a single submitter, not by consensus. This is the deeper systemic fix and should be tracked separately.
• An upper-bound span cap (MAX_BLOCKS_PER_BATCH) as defense-in-depth; the node side (fix(derivation): reject invalid blockCount to prevent batch-parse DoS #995) already bounds allocation by payload length, so it is not required for this DoS.

Notes

• This is an L1 contract change and requires the normal upgrade path (audit + timelock/governance). fix(derivation): reject invalid blockCount to prevent batch-parse DoS #995 (node-side mitigation) ships independently and faster; this PR is the source-level closure.
• The commit is unsigned (GPG signing was unavailable in the authoring session) — re-sign on merge if branch protection requires it.

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a monotonicity check in _commitBatchWithBatchData for V1 batch headers: if the parent header's version is ≥1, the incoming batch's lastBlockNumber must be strictly greater than the parent's, reverting with "empty block span" otherwise. Three new tests cover the equal, decreasing, and strictly increasing cases.

Changes

Empty Block Span Validation for V1 Batch Headers

Layer / File(s)	Summary
Empty block span require in _commitBatchWithBatchData contracts/contracts/l1/rollup/Rollup.sol	For parent headers with version ≥1, reads parentLastBlockNumber via BatchHeaderCodecV1.getLastBlockNumber and reverts with "empty block span" when batchDataInput.lastBlockNumber is not strictly greater.
V1 header builder and first-batch helper contracts/contracts/test/Rollup.t.sol	_createMatchingBatchHeaderV1 constructs a 257-byte V1 header via assembly with all required fields including lastBlockNumber; _commitFirstV1Batch mocks verifier and message-queue, warps time, commits the genesis V1 batch, and asserts the initial batch index.
Block span revert and success tests contracts/contracts/test/Rollup.t.sol	Three tests cover: revert on equal lastBlockNumber (empty span), revert on decreasing lastBlockNumber (underflow), and success on strictly increasing lastBlockNumber with assertion that lastCommittedBatchIndex() advances to 2.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• security(rollup): _commitBatch does not validate lastBlockNumber > parent (empty block span → node DoS) #996: This PR implements the exact fix described in the issue — adding a require(lastBlockNumber > parentLastBlockNumber) guard in _commitBatchWithBatchData for V1 headers to prevent empty block spans.
• derivation: malformed L1 commitBatch panics layer1-verify nodes (blockCount==0 nil deref + blockCount*60 overflow) #994: This PR directly addresses the DoS vulnerability described in this issue by enforcing lastBlockNumber strictly increases, preventing blockCount == 0 batches from being committed.

Possibly related PRs

• morph-l2/morph#906: Modifies the same _commitBatchWithBatchData internals and commitBatchWithProof call sites, making it directly adjacent to this PR's new validation block.

Suggested reviewers

• tomatoishealthy
• SegueII
• Kukoomomo

Poem

🐰 Hoppity hop, I check each block span,
No empty batches shall pass — that's the plan!
V1 headers must grow, never shrink, never stay,
A require stands guard on the rollup highway.
The bunny enforces monotonic law today! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically identifies the main change: rejecting empty block spans in the commitBatch function to fix a DoS vulnerability.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/rollup-validate-lastblocknumber

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}