[Pushers L03] BlockHashesPushedis emitted when storage is not written.

[luiz-lvj]

The IBuffer.receiveHashes NatSpec comment specifies that "the last block in the buffer must be less than the last block being pushed", implying strictly monotonic pushes. However, BaseBuffer._receiveHashes silently skips writes for any blockNumber <= existingBlockNumber and unconditionally emits BlockHashesPushed(firstBlockNumber, lastBlockNumber) regardless of whether any hashes were stored.

Since pushers are permissionless, third parties can enqueue duplicate or out-of-order batches. If the buffer contains hashes up to block 100 and a message arrives for blocks 90-99, all writes are skipped yet BlockHashesPushed(90, 99) is emitted. Off-chain systems monitoring these events may misinterpret them as progress. In addition, block 0 is permanently unwriteable because _blockNumberBuffer defaults to zero and the skip condition always triggers.

This PR updates the behavior by verifying if at least one block hash is written in order to emit the event.

Summary by CodeRabbit

• New Features

  • Added validation to prevent zero values for the first block number parameter in block hash operations.

• Bug Fixes

  • Improved event reliability: BlockHashesPushed events now emit only when new hashes are successfully written, preventing duplicate submissions from generating spurious events that could cause downstream issues.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR enhances the BaseBuffer contract with validation to reject zero-valued firstBlockNumber and implements conditional event emission—BlockHashesPushed is now emitted only when new hashes are actually written, preventing spurious events during duplicate or empty writes.

Changes

Cohort / File(s)	Summary
Core Buffer Logic src/contracts/block-hash-pusher/BaseBuffer.sol, src/contracts/block-hash-pusher/interfaces/IBuffer.sol	Added firstBlockNumber non-zero validation guard, introduced hashesWritten flag to track if any new hashes were written, and made lastBlockNumber updates and BlockHashesPushed event emission conditional on hashesWritten being true. Added new InvalidFirstBlockNumber() error to IBuffer interface.
Buffer Implementation Tests test/block-hash-pusher/linea/LineaBuffer.t.sol, test/block-hash-pusher/scroll/ScrollBuffer.t.sol, test/block-hash-pusher/zksync/ZkSyncBuffer.t.sol	Updated fuzz tests to expect InvalidFirstBlockNumber revert when firstBlockNumber is 0; otherwise expect BlockHashesPushed emission. Added new test_receiveHashes_does_not_emit_event_when_no_hashes_written() to verify that duplicate pushes with identical hashes do not emit BlockHashesPushed and that newestBlockNumber remains unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• Change storage layout to allow mapping #62: Overhauls BaseBuffer storage to a mapping and changes hash-write/eviction and newestBlockNumber update semantics, directly intersecting with this PR's conditional-update logic.
• Add Linea pusher and buffer #59: Introduces LineaBuffer.receiveHashes and error handling that directly depend on the InvalidFirstBlockNumber error and conditional-emit behavior added here.
• Add Scroll pusher and buffer  #57: Adds ScrollBuffer/ScrollPusher implementation that relies on the receiveHashes behavior and validation modified in this PR.

Suggested reviewers

• frangio
• pepebndc
• nahimterrazas

Poem

🐰 Hop-hop, the buffer now stands guard so true,
Zero-blocked at the gate, no slipping through!
Events only when hashes are penned to the ledger,
Duplicate pushes meet silence—a clever dodger! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the main change: preventing BlockHashesPushed emission when no hashes are written to storage, which is the core fix for the L03 issue.
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 fix/pushers-L03

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

src/contracts/block-hash-pusher/BaseBuffer.sol (1)

67-76: Event emits the full batch range even when only a subset of hashes were written.

When hashesWritten is true, the event is emitted with (firstBlockNumber, lastBlockNumber) covering the entire requested batch, even if some individual hashes within the range were skipped (e.g., due to existing higher-numbered entries at those buffer indices). This means off-chain consumers could see BlockHashesPushed(1, 10) when, say, only blocks 1–4 and 6–10 were actually stored.

This is likely acceptable since the event describes the batch boundaries and the buffer is documented as sparse. But if precise write-range reporting matters to downstream indexers, consider documenting this behavior explicitly in the event's NatSpec or adjusting firstBlockNumber/lastBlockNumber to reflect the actual written range.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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