Fix commitBatchWithProof by any

[Kukoomomo]

Summary by CodeRabbit

• New Features
  • Non-stakeholder participants can now submit batches once the required delay period is satisfied.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes refactor the batch commit process to gather the submitter bitmap externally in commitBatch via IL1Staking(l1StakingContract).getStakerBitmap(_msgSender()) and pass it as a new parameter to the internal _commitBatchWithBatchData function, rather than computing it inside that function. A new test verifies that non-stakeholders can successfully call commitBatchWithProof when delay conditions are met.

Changes

Cohort / File(s)	Summary
Core Rollup Logic contracts/contracts/l1/rollup/Rollup.sol	Added submitterBitmap parameter to internal _commitBatchWithBatchData function. Updated commitBatch to retrieve the bitmap via IL1Staking and pass it to the function. Updated commitBatchWithProof to pass a 0 bitmap placeholder. Changes the data dependency and control flow for signer bitmap handling.
Test Coverage contracts/contracts/test/Rollup.t.sol	Added new test function test_commitBatchWithProof_anyone_can_call_when_delay_met() to verify that non-stakeholders can execute commitBatchWithProof when delay conditions are satisfied, ensuring batch commitment and state root consistency.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

enhancement

Suggested reviewers

• tomatoishealthy
• chengwenxi
• panos-xyz

Poem

🐰 A bitmap now journeys from staking's great hall,
Through parameter paths, answering the call,
Where batches commit with renewed clarity,
And anyone can prove with steadfast rarity! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Fix commitBatchWithProof by any' is incomplete and grammatically unclear, making it difficult to understand the intended change.	Complete the title to clearly express the fix, such as 'Allow anyone to call commitBatchWithProof when delay is met' or 'Fix commitBatchWithProof to allow any caller after delay.'
Linked Issues check	❓ Inconclusive	No linked issues or requirements are provided in the pull request metadata to assess against.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Description check	✅ Passed	The pull request includes detailed summaries explaining the signature change to _commitBatchWithBatchData, the addition of submitterBitmap parameter threading, and the new test case validating the fix.
Out of Scope Changes check	✅ Passed	All changes are focused on the commitBatchWithProof functionality and its related test coverage; no out-of-scope modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch batch_any

---

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.}

[coderabbitai]

🧹 Nitpick comments (2)

contracts/contracts/l1/rollup/Rollup.sol (1)

247-249: Avoid overloading signedSequencersBitmap with a sentinel 0.

commitBatchWithProof() now stores 0 in batchDataStore[_batchIndex].signedSequencersBitmap, while _challengerWin() still treats that field as the slash target. That makes this path depend on proof-backed batches never flowing through any punishment logic later. Splitting submitter accountability from signer/slash metadata would make this invariant explicit and much safer to maintain.

Also applies to: 323-330, 351-372

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@contracts/contracts/l1/rollup/Rollup.sol` around lines 247 - 249, The code
currently overloads batchDataStore[_batchIndex].signedSequencersBitmap by
storing sentinel 0 in commitBatchWithProof while _challengerWin() and punishment
logic treat signedSequencersBitmap as the slash target; separate the concepts by
adding a new explicit storage field (e.g., submitterBitmap or
submitterAccountabilityBitmap) to the Batch struct and use that for submitter
identity in commitBatchWithProof (leaving signedSequencersBitmap strictly for
signer/slash metadata), then update all callers and checks in _challengerWin(),
any punishment logic, and related helpers that reference signedSequencersBitmap
(also adjust handling in functions around lines noted: the other
commit/validation paths) to reference the new field so no sentinel 0 value is
overloaded. Ensure BatchSignatureInput/commitBatchWithProof write the new
submitter field and _challengerWin()/slash logic reads signedSequencersBitmap
only for signer info.

contracts/contracts/test/Rollup.t.sol (1)

542-577: Assert the bob precondition explicitly.

This test assumes bob is not an active staker, but it never proves that. A precondition assertion will keep the test from silently becoming another “staker can call” case if the shared fixture changes.

Proposed test hardening

         bytes memory batchHeader1 = _createMatchingBatchHeader(1, 0, prevStateRoot, postStateRoot, withdrawalRoot);

         // Caller is bob: not a staker, not owner. Anyone can submit when delay is met.
+        assertFalse(l1Staking.isActiveStaker(bob));
         hevm.prank(bob);
         rollup.commitBatchWithProof(
             batchDataInput,
             batchSignatureInput,
             batchHeader1,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@contracts/contracts/test/Rollup.t.sol` around lines 542 - 577, Add an
explicit precondition that bob is not an active staker before hevm.prank(bob) to
prevent the test from passing accidentally if fixtures change: insert an
assertion using the rollup contract and the test's bob variable (for example
assertFalse(rollup.isStaker(bob)) or assertEq(rollup.stakers(bob), 0) depending
on the rollup interface) immediately before the prank call so the test
guarantees “bob is not a staker” when calling rollup.commitBatchWithProof.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@contracts/contracts/l1/rollup/Rollup.sol`:
- Around line 247-249: The code currently overloads
batchDataStore[_batchIndex].signedSequencersBitmap by storing sentinel 0 in
commitBatchWithProof while _challengerWin() and punishment logic treat
signedSequencersBitmap as the slash target; separate the concepts by adding a
new explicit storage field (e.g., submitterBitmap or
submitterAccountabilityBitmap) to the Batch struct and use that for submitter
identity in commitBatchWithProof (leaving signedSequencersBitmap strictly for
signer/slash metadata), then update all callers and checks in _challengerWin(),
any punishment logic, and related helpers that reference signedSequencersBitmap
(also adjust handling in functions around lines noted: the other
commit/validation paths) to reference the new field so no sentinel 0 value is
overloaded. Ensure BatchSignatureInput/commitBatchWithProof write the new
submitter field and _challengerWin()/slash logic reads signedSequencersBitmap
only for signer info.

In `@contracts/contracts/test/Rollup.t.sol`:
- Around line 542-577: Add an explicit precondition that bob is not an active
staker before hevm.prank(bob) to prevent the test from passing accidentally if
fixtures change: insert an assertion using the rollup contract and the test's
bob variable (for example assertFalse(rollup.isStaker(bob)) or
assertEq(rollup.stakers(bob), 0) depending on the rollup interface) immediately
before the prank call so the test guarantees “bob is not a staker” when calling
rollup.commitBatchWithProof.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2776f114-13ba-4786-900d-f57f09f67bb0

📥 Commits

Reviewing files that changed from the base of the PR and between 895120c and 4533559.

📒 Files selected for processing (2)

• contracts/contracts/l1/rollup/Rollup.sol
• contracts/contracts/test/Rollup.t.sol