Skip to content

Commit

Permalink
feat: L1 to L2 message size checks (#1262)
Browse files Browse the repository at this point in the history
# Description

Fixes #1263
  • Loading branch information
benesjan authored Jul 28, 2023
1 parent a0b01e7 commit c6f9a8d
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 4 deletions.
2 changes: 1 addition & 1 deletion l1-contracts/src/core/interfaces/messagebridge/IInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ interface IInbox {
* @dev Will emit `MessageAdded` with data for easy access by the sequencer
* @dev msg.value - The fee provided to sequencer for including the entry
* @param _recipient - The recipient of the entry
* @param _deadline - The deadline to consume a message. Only after it, can a message be cancalled.
* @param _deadline - The deadline to consume a message. Only after it, can a message be cancelled.
* @param _content - The content of the entry (application specific)
* @param _secretHash - The secret hash of the entry (make it possible to hide when a specific entry is consumed on L2)
* @return The key of the entry in the set
Expand Down
1 change: 1 addition & 0 deletions l1-contracts/src/core/libraries/Constants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ library Constants {
// Prime field modulus
uint256 internal constant P =
21888242871839275222246405745257275088548364400416034343698204186575808495617;
uint256 internal constant MAX_FIELD_VALUE = P - 1;

// Constants used for decoding rollup blocks
// TODO(962): Make this constant consistent across the codebase.
Expand Down
3 changes: 3 additions & 0 deletions l1-contracts/src/core/libraries/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ library Errors {
uint32 storedDeadline,
uint32 deadlinePassed
); // 0xd483d8f2
error Inbox__ActorTooLarge(bytes32 actor); // 0xa776a06e
error Inbox__ContentTooLarge(bytes32 content); // 0x47452014
error Inbox__SecretHashTooLarge(bytes32 secretHash); // 0xecde7e2c

// Outbox
error Outbox__Unauthorized(); // 0x2c9490c2
Expand Down
10 changes: 10 additions & 0 deletions l1-contracts/src/core/messagebridge/Inbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
import {IRegistry} from "@aztec/core/interfaces/messagebridge/IRegistry.sol";

// Libraries
import {Constants} from "@aztec/core/libraries/Constants.sol";
import {DataStructures} from "@aztec/core/libraries/DataStructures.sol";
import {Errors} from "@aztec/core/libraries/Errors.sol";
import {Hash} from "@aztec/core/libraries/Hash.sol";
Expand Down Expand Up @@ -53,7 +54,16 @@ contract Inbox is IInbox {
bytes32 _content,
bytes32 _secretHash
) external payable override(IInbox) returns (bytes32) {
if (uint256(_recipient.actor) > Constants.MAX_FIELD_VALUE) {
revert Errors.Inbox__ActorTooLarge(_recipient.actor);
}
if (_deadline <= block.timestamp) revert Errors.Inbox__DeadlineBeforeNow();
if (uint256(_content) > Constants.MAX_FIELD_VALUE) {
revert Errors.Inbox__ContentTooLarge(_content);
}
if (uint256(_secretHash) > Constants.MAX_FIELD_VALUE) {
revert Errors.Inbox__SecretHashTooLarge(_secretHash);
}
// `fee` is uint64 for slot packing of the Entry struct. uint64 caps at ~18.4 ETH which should be enough.
// we revert here to safely cast msg.value into uint64.
if (msg.value > type(uint64).max) revert Errors.Inbox__FeeTooHigh();
Expand Down
50 changes: 47 additions & 3 deletions l1-contracts/test/Inbox.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {Test} from "forge-std/Test.sol";
import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
import {Inbox} from "@aztec/core/messagebridge/Inbox.sol";
import {Registry} from "@aztec/core/messagebridge/Registry.sol";
import {Constants} from "@aztec/core/libraries/Constants.sol";
import {Errors} from "@aztec/core/libraries/Errors.sol";

import {DataStructures} from "@aztec/core/libraries/DataStructures.sol";
Expand Down Expand Up @@ -39,11 +40,11 @@ contract InboxTest is Test {
return DataStructures.L1ToL2Msg({
sender: DataStructures.L1Actor({actor: address(this), chainId: block.chainid}),
recipient: DataStructures.L2Actor({
actor: 0x2000000000000000000000000000000000000000000000000000000000000000,
actor: 0x1000000000000000000000000000000000000000000000000000000000000000,
version: 1
}),
content: 0x3000000000000000000000000000000000000000000000000000000000000000,
secretHash: 0x4000000000000000000000000000000000000000000000000000000000000000,
content: 0x2000000000000000000000000000000000000000000000000000000000000000,
secretHash: 0x3000000000000000000000000000000000000000000000000000000000000000,
fee: 5,
deadline: uint32(block.timestamp + 100)
});
Expand All @@ -52,9 +53,15 @@ contract InboxTest is Test {
function testFuzzSendL2Msg(DataStructures.L1ToL2Msg memory _message) public {
// fix message.sender and deadline:
_message.sender = DataStructures.L1Actor({actor: address(this), chainId: block.chainid});
// ensure actor fits in a field
_message.recipient.actor = bytes32(uint256(_message.recipient.actor) % Constants.P);
if (_message.deadline <= block.timestamp) {
_message.deadline = uint32(block.timestamp + 100);
}
// ensure content fits in a field
_message.content = bytes32(uint256(_message.content) % Constants.P);
// ensure secret hash fits in a field
_message.secretHash = bytes32(uint256(_message.secretHash) % Constants.P);
bytes32 expectedEntryKey = inbox.computeEntryKey(_message);
vm.expectEmit(true, true, true, true);
// event we expect
Expand Down Expand Up @@ -99,6 +106,37 @@ contract InboxTest is Test {
assertEq(inbox.get(entryKey1).deadline, message.deadline);
}

function testRevertIfActorTooLarge() public {
DataStructures.L1ToL2Msg memory message = _fakeMessage();
message.recipient.actor = bytes32(Constants.MAX_FIELD_VALUE + 1);
vm.expectRevert(
abi.encodeWithSelector(Errors.Inbox__ActorTooLarge.selector, message.recipient.actor)
);
inbox.sendL2Message{value: message.fee}(
message.recipient, message.deadline, message.content, message.secretHash
);
}

function testRevertIfContentTooLarge() public {
DataStructures.L1ToL2Msg memory message = _fakeMessage();
message.content = bytes32(Constants.MAX_FIELD_VALUE + 1);
vm.expectRevert(abi.encodeWithSelector(Errors.Inbox__ContentTooLarge.selector, message.content));
inbox.sendL2Message{value: message.fee}(
message.recipient, message.deadline, message.content, message.secretHash
);
}

function testRevertIfSecretHashTooLarge() public {
DataStructures.L1ToL2Msg memory message = _fakeMessage();
message.secretHash = bytes32(Constants.MAX_FIELD_VALUE + 1);
vm.expectRevert(
abi.encodeWithSelector(Errors.Inbox__SecretHashTooLarge.selector, message.secretHash)
);
inbox.sendL2Message{value: message.fee}(
message.recipient, message.deadline, message.content, message.secretHash
);
}

function testRevertIfCancellingMessageFromDifferentAddress() public {
DataStructures.L1ToL2Msg memory message = _fakeMessage();
inbox.sendL2Message{value: message.fee}(
Expand Down Expand Up @@ -220,9 +258,15 @@ contract InboxTest is Test {
DataStructures.L1ToL2Msg memory message = _messages[i];
// fix message.sender and deadline to be more than current time:
message.sender = DataStructures.L1Actor({actor: address(this), chainId: block.chainid});
// ensure actor fits in a field
message.recipient.actor = bytes32(uint256(message.recipient.actor) % Constants.P);
if (message.deadline <= block.timestamp) {
message.deadline = uint32(block.timestamp + 100);
}
// ensure content fits in a field
message.content = bytes32(uint256(message.content) % Constants.P);
// ensure secret hash fits in a field
message.secretHash = bytes32(uint256(message.secretHash) % Constants.P);
expectedTotalFee += message.fee;
entryKeys[i] = inbox.sendL2Message{value: message.fee}(
message.recipient, message.deadline, message.content, message.secretHash
Expand Down

0 comments on commit c6f9a8d

Please sign in to comment.