-
Notifications
You must be signed in to change notification settings - Fork 0
Add Scroll pusher and buffer #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis PR adds Scroll Layer 2 integration to the block-hash-pusher system. It introduces ScrollBuffer (L2) and ScrollPusher (L1) contracts enabling cross-chain block hash transmission via Scroll's native messaging. Includes mock implementations, comprehensive test coverage, and documentation updates. The ZkSync L2Transaction struct is also renamed for clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant L1User as L1: User/Contract
participant L1Pusher as L1: ScrollPusher
participant L1Messenger as L1: ScrollMessenger
participant L2Messenger as L2: ScrollMessenger
participant L2Buffer as L2: ScrollBuffer
L1User->>L1Pusher: pushHashes(firstBlockNumber, batchSize, l2TxData)
activate L1Pusher
L1Pusher->>L1Pusher: Build block hash array
L1Pusher->>L1Pusher: Encode receiveHashes() call
L1Pusher->>L1Pusher: Decode ScrollL2Transaction
L1Pusher->>L1Messenger: sendMessage(L2Buffer, encoded call, gasLimit, refundAddress)
deactivate L1Pusher
activate L1Messenger
L1Messenger->>L1Messenger: Emit SentMessage event
deactivate L1Messenger
Note over L1Messenger,L2Messenger: Cross-chain message transmission
L2Messenger->>L2Buffer: relayMessage(from=L1Pusher, receiveHashes call)
activate L2Buffer
L2Buffer->>L2Buffer: Verify sender is L2Messenger
L2Buffer->>L2Buffer: Verify xDomainMessageSender == pusher
L2Buffer->>L2Buffer: Store block hashes
L2Buffer->>L2Buffer: Emit BlockHashesReceived
deactivate L2Buffer
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
…caster into scroll-pusher
…caster into scroll-pusher
…caster into scroll-pusher
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/contracts/block-hash-pusher/scroll/ScrollPusher.sol`:
- Around line 21-32: The constructor currently assigns _l1ScrollMessenger and
_bufferAddress without validation; add require checks in the constructor to
ensure l1ScrollMessenger_ and bufferAddress_ are non‑zero (address(0)) and
revert with clear error messages (e.g., "InvalidL1ScrollMessenger" /
"InvalidBufferAddress") before assigning to _l1ScrollMessenger and
_bufferAddress to prevent invalid deployments; update the constructor function
signature and body accordingly so the checks run at deployment time.
In `@test/block-hash-pusher/scroll/ScrollBuffer.t.sol`:
- Around line 57-66: The test
test_receiveHashes_reverts_if_xDomainMessageSender_does_not_match_pusher should
explicitly set the expected pusher before invoking the relay to ensure the
failure is due to a sender mismatch; call the ScrollBuffer method
setPusherAddress (or equivalent setter used in the contract) on the buffer
instance to set pusher to the intended address (e.g., pusher) before preparing
l2Calldata and calling mockL2ScrollMessenger.relayMessage, then proceed with
vm.prank(relayer) and the expectEmit check so the test isolates the
xDomainMessageSender mismatch rather than an unset pusher.
In `@test/block-hash-pusher/scroll/ScrollPusher.t.sol`:
- Around line 14-23: The test_pushHashes_fork uses a Sepolia-specific
l1ScrollMessengerAddress but calls vm.createSelectFork with a generic
ETHEREUM_RPC_URL which can point to mainnet; update the fork creation or add an
explicit chain check to ensure the fork is Sepolia: change vm.createSelectFork
to use a Sepolia RPC (e.g., SEPOLIA_RPC_URL) or call vm.createSelectFork with
the Sepolia identifier and/or immediately assert the chain id (e.g.,
require(block.chainid == 11155111)) so that test_pushHashes_fork and the
Sepolia-specific l1ScrollMessengerAddress always run against Sepolia; reference
symbols: l1ScrollMessengerAddress, test_pushHashes_fork, vm.createSelectFork,
ETHEREUM_RPC_URL.
🧹 Nitpick comments (1)
test/block-hash-pusher/scroll/ScrollPusher.t.sol (1)
39-54: Keep invalid batch-size fuzzing aligned with the contract limit.The valid test assumes
<=256, while invalid only covers>8191. Consider driving the invalid range from the contract’s max (if exposed) so tests don’t drift when limits change.♻️ Example (if a max constant is available)
- vm.assume(batchSize == 0 || batchSize > 8191); + uint16 maxBatch = ScrollPusher.MAX_BATCH_SIZE(); + vm.assume(batchSize == 0 || batchSize > maxBatch);
| /// @notice Parameters for the L2 transaction that will be executed on Scroll. | ||
| /// @param gasLimit The gas limit for the L2 transaction. | ||
| /// @param refundAddress The address to receive any refunds. | ||
| struct ScrollL2Transaction { | ||
| uint256 gasLimit; | ||
| address refundAddress; | ||
| } | ||
|
|
||
| constructor(address l1ScrollMessenger_, address bufferAddress_) { | ||
| _l1ScrollMessenger = l1ScrollMessenger_; | ||
| _bufferAddress = bufferAddress_; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate messenger and buffer addresses in the constructor.
Without non‑zero checks, a misconfigured deployment can silently “succeed” but never deliver messages.
🛠️ Suggested fix
contract ScrollPusher is BlockHashArrayBuilder, IPusher {
+ error InvalidL1ScrollMessenger();
+ error InvalidBufferAddress();
+
@@
constructor(address l1ScrollMessenger_, address bufferAddress_) {
+ if (l1ScrollMessenger_ == address(0)) revert InvalidL1ScrollMessenger();
+ if (bufferAddress_ == address(0)) revert InvalidBufferAddress();
_l1ScrollMessenger = l1ScrollMessenger_;
_bufferAddress = bufferAddress_;
}🤖 Prompt for AI Agents
In `@src/contracts/block-hash-pusher/scroll/ScrollPusher.sol` around lines 21 -
32, The constructor currently assigns _l1ScrollMessenger and _bufferAddress
without validation; add require checks in the constructor to ensure
l1ScrollMessenger_ and bufferAddress_ are non‑zero (address(0)) and revert with
clear error messages (e.g., "InvalidL1ScrollMessenger" / "InvalidBufferAddress")
before assigning to _l1ScrollMessenger and _bufferAddress to prevent invalid
deployments; update the constructor function signature and body accordingly so
the checks run at deployment time.
| function test_receiveHashes_reverts_if_xDomainMessageSender_does_not_match_pusher(address notPusher) public { | ||
| vm.assume(notPusher != pusher); | ||
| ScrollBuffer buffer = new ScrollBuffer(address(mockL2ScrollMessenger), owner); | ||
|
|
||
| bytes memory l2Calldata = abi.encodeCall(buffer.receiveHashes, (1, new bytes32[](1))); | ||
|
|
||
| vm.expectEmit(); | ||
| emit IScrollMessenger.FailedRelayedMessage(keccak256(l2Calldata)); | ||
| vm.prank(relayer); | ||
| mockL2ScrollMessenger.relayMessage(notPusher, address(buffer), 0, 0, l2Calldata); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set pusher before testing xDomain mismatch.
The test currently never calls setPusherAddress, so a failure could be due to an unset pusher rather than a true mismatch. Set the pusher first to isolate the intended condition.
🔧 Suggested adjustment
ScrollBuffer buffer = new ScrollBuffer(address(mockL2ScrollMessenger), owner);
+ vm.prank(owner);
+ buffer.setPusherAddress(pusher);
+
bytes memory l2Calldata = abi.encodeCall(buffer.receiveHashes, (1, new bytes32[](1)));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function test_receiveHashes_reverts_if_xDomainMessageSender_does_not_match_pusher(address notPusher) public { | |
| vm.assume(notPusher != pusher); | |
| ScrollBuffer buffer = new ScrollBuffer(address(mockL2ScrollMessenger), owner); | |
| bytes memory l2Calldata = abi.encodeCall(buffer.receiveHashes, (1, new bytes32[](1))); | |
| vm.expectEmit(); | |
| emit IScrollMessenger.FailedRelayedMessage(keccak256(l2Calldata)); | |
| vm.prank(relayer); | |
| mockL2ScrollMessenger.relayMessage(notPusher, address(buffer), 0, 0, l2Calldata); | |
| function test_receiveHashes_reverts_if_xDomainMessageSender_does_not_match_pusher(address notPusher) public { | |
| vm.assume(notPusher != pusher); | |
| ScrollBuffer buffer = new ScrollBuffer(address(mockL2ScrollMessenger), owner); | |
| vm.prank(owner); | |
| buffer.setPusherAddress(pusher); | |
| bytes memory l2Calldata = abi.encodeCall(buffer.receiveHashes, (1, new bytes32[](1))); | |
| vm.expectEmit(); | |
| emit IScrollMessenger.FailedRelayedMessage(keccak256(l2Calldata)); | |
| vm.prank(relayer); | |
| mockL2ScrollMessenger.relayMessage(notPusher, address(buffer), 0, 0, l2Calldata); | |
| } |
🤖 Prompt for AI Agents
In `@test/block-hash-pusher/scroll/ScrollBuffer.t.sol` around lines 57 - 66, The
test test_receiveHashes_reverts_if_xDomainMessageSender_does_not_match_pusher
should explicitly set the expected pusher before invoking the relay to ensure
the failure is due to a sender mismatch; call the ScrollBuffer method
setPusherAddress (or equivalent setter used in the contract) on the buffer
instance to set pusher to the intended address (e.g., pusher) before preparing
l2Calldata and calling mockL2ScrollMessenger.relayMessage, then proceed with
vm.prank(relayer) and the expectEmit check so the test isolates the
xDomainMessageSender mismatch rather than an unset pusher.
| address public l1ScrollMessengerAddress = 0x50c7d3e7f7c656493D1D76aaa1a836CedfCBB16A; // Address for Ethereum Sepolia | ||
|
|
||
| function setUp() public { | ||
| mockL1ScrollMessenger = address(new MockL1ScrollMessenger()); | ||
| } | ||
|
|
||
| function test_pushHashes_fork() public { | ||
| vm.createSelectFork(vm.envString("ETHEREUM_RPC_URL")); | ||
|
|
||
| ScrollPusher scrollPusher = new ScrollPusher(l1ScrollMessengerAddress, buffer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fork network must match the Sepolia messenger address.
l1ScrollMessengerAddress is Sepolia-specific, but the fork uses ETHEREUM_RPC_URL, which can easily point to mainnet and make this test fail. Make the fork explicit (or assert the chain ID) to avoid flaky CI failures.
🔧 Suggested adjustment
- vm.createSelectFork(vm.envString("ETHEREUM_RPC_URL"));
+ vm.createSelectFork(vm.envString("SEPOLIA_RPC_URL"));
+ assertEq(block.chainid, 11155111, "fork is not Sepolia");🤖 Prompt for AI Agents
In `@test/block-hash-pusher/scroll/ScrollPusher.t.sol` around lines 14 - 23, The
test_pushHashes_fork uses a Sepolia-specific l1ScrollMessengerAddress but calls
vm.createSelectFork with a generic ETHEREUM_RPC_URL which can point to mainnet;
update the fork creation or add an explicit chain check to ensure the fork is
Sepolia: change vm.createSelectFork to use a Sepolia RPC (e.g., SEPOLIA_RPC_URL)
or call vm.createSelectFork with the Sepolia identifier and/or immediately
assert the chain id (e.g., require(block.chainid == 11155111)) so that
test_pushHashes_fork and the Sepolia-specific l1ScrollMessengerAddress always
run against Sepolia; reference symbols: l1ScrollMessengerAddress,
test_pushHashes_fork, vm.createSelectFork, ETHEREUM_RPC_URL.
Add Scroll Chain Support to Block Hash Pusher
This PR adds support for Scroll L2 to the block hash pusher system, enabling Ethereum L1 block hashes to be pushed to Scroll L2.
Contracts:
ScrollPusher- L1 contract that pushes block hashes to Scroll L2 viaL1ScrollMessenger.sendMessage()ScrollBuffer- L2 contract that receives and stores block hashes with access control viaL2ScrollMessengerPS: this PR must be merged after #56
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.