[Pushers N02] Custom Errors in require Statements

[pepebndc]

Summary by CodeRabbit

• Refactor
  • Standardized input validation across Block Hash Pusher components, replacing conditional revert branches with unified assertion-style checks.
  • Unified error handling for invalid buffers, senders, batch ranges, empty inputs, and zero-valued fields for clearer, consistent failures.
  • Applied updates across core and network-specific modules (Linea, Scroll, ZkSync) with no changes to public interfaces or external behavior.

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Replaced explicit if-revert branches with require(...) validations across multiple block-hash pusher buffer and pusher contracts. Error types and public interfaces remain unchanged; only internal validation style was unified.

Changes

Cohort / File(s)	Summary
Core Buffer Base src/contracts/block-hash-pusher/BaseBuffer.sol	Replaced conditional revert branches with require() checks in parentChainBlockHash and _receiveHashes; behavior unchanged.
Array Builder src/contracts/block-hash-pusher/BlockHashArrayBuilder.sol	Consolidated batch/range validation into single require() and switched block-hash nonzero check to require(); no interface changes.
Linea L2 src/contracts/block-hash-pusher/linea/LineaBuffer.sol, src/contracts/block-hash-pusher/linea/LineaPusher.sol	Replaced constructor, sender and buffer validations with require() calls using existing custom errors; signatures unchanged.
Scroll L2 src/contracts/block-hash-pusher/scroll/ScrollBuffer.sol, src/contracts/block-hash-pusher/scroll/ScrollPusher.sol	Swapped conditional reverts for require() in constructors, receiveHashes, and buffer checks; semantics preserved.
ZkSync L2 src/contracts/block-hash-pusher/zksync/ZkSyncBuffer.sol, src/contracts/block-hash-pusher/zksync/ZkSyncPusher.sol	Added/converted pusher/buffer/tx-hash validations to require() style and updated access-control check to NotPusher() via require(); no external API changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Add Scroll pusher and buffer  #57 — Modifies ScrollBuffer/ScrollPusher; touches the same Scroll validation paths updated here.
• Add Pusher and Buffer contracts #56 — Introduced Buffer/Pusher contracts whose validation style is adjusted in this PR.
• [Pushers N02] Custom Errors in require Statements #73 — Contains similar require(...) style changes across buffer/pusher contracts as seen here.

Suggested reviewers

• luiz-lvj
• frangio

Poem

"I hop through lines of Solidity bright,
swapping reverts for requires at night.
Buffers snug, pushers in tune,
hashes hum beneath the moon.
Thump-thump—clean checks, all right!" 🐇

🚥 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 accurately describes the primary change: replacing explicit revert patterns with require statements that use custom errors across multiple pusher contract files.
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-N02

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

[luiz-lvj]

LGTM

I did some small changes to solve conflicts with the main branch

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/contracts/block-hash-pusher/zksync/ZkSyncBuffer.sol`:
- Around line 20-24: Move the require validation so it runs before assigning the
value to the immutable: in the constructor function (constructor(address
pusher_)), validate pusher_ with require(pusher_ != address(0),
InvalidPusherAddress()) before setting _pusher = pusher_ to follow the defensive
ordering used by other buffer constructors.

🧹 Nitpick comments (3)

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

47-49: Inconsistency: constructor still uses if-revert pattern.

The constructor validation for zkSyncDiamond_ was not converted to require, unlike constructors in LineaBuffer, ScrollBuffer, and ZkSyncBuffer which were migrated in this PR. Consider converting for consistency.

Suggested fix

     constructor(address zkSyncDiamond_) {
-        if (zkSyncDiamond_ == address(0)) {
-            revert InvalidZkSyncDiamondAddress();
-        }
+        require(zkSyncDiamond_ != address(0), InvalidZkSyncDiamondAddress());
 
         _zkSyncDiamond = zkSyncDiamond_;
     }

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

29-35: Inconsistency: constructor still uses if-revert pattern.

Same as ZkSyncPusher and LineaPusher — this constructor was not migrated while buffer constructors were. Consider converting for consistency across the PR.

Suggested fix

     constructor(address l1ScrollMessenger_) {
-        if (l1ScrollMessenger_ == address(0)) {
-            revert InvalidL1ScrollMessengerAddress();
-        }
+        require(l1ScrollMessenger_ != address(0), InvalidL1ScrollMessengerAddress());
 
         _l1ScrollMessenger = l1ScrollMessenger_;
     }

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

27-33: Inconsistency: constructor still uses if-revert pattern.

Same pattern as the other pusher constructors — not migrated while buffer constructors were.

Suggested fix

     constructor(address rollup_) {
-        if (rollup_ == address(0)) {
-            revert InvalidLineaRollupAddress();
-        }
+        require(rollup_ != address(0), InvalidLineaRollupAddress());
 
         _lineaRollup = rollup_;
     }