Skip to content
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

Feat: Enforce versioning when rollup push/consume messages #1245

Merged
merged 7 commits into from
Jul 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions l1-contracts/src/core/Rollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,9 @@ contract Rollup is IRollup {
lastBlockTs = block.timestamp;

// @todo (issue #605) handle fee collector
// @todo: (issue #624) handle different versions
IInbox inbox = REGISTRY.getInbox();
inbox.batchConsume(l1ToL2Msgs, msg.sender);

// @todo: (issue #624) handle different versions
IOutbox outbox = REGISTRY.getOutbox();
outbox.sendL1Messages(l2ToL1Msgs);

Expand Down
4 changes: 3 additions & 1 deletion l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import {IInbox} from "./IInbox.sol";
import {IOutbox} from "./IOutbox.sol";

interface IRegistry {
function upgrade(address _rollup, address _inbox, address _outbox) external;
function upgrade(address _rollup, address _inbox, address _outbox) external returns (uint256);

function getRollup() external view returns (IRollup);

function getVersionFor(address _rollup) external view returns (uint256);

function getInbox() external view returns (IInbox);

function getOutbox() external view returns (IOutbox);
Expand Down
7 changes: 5 additions & 2 deletions l1-contracts/src/core/libraries/DataStructures.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@ pragma solidity >=0.8.18;
library DataStructures {
/**
* @notice Entry struct - Done as struct to easily support extensions if needed
* @param count - The occurrence of the entry in the dataset
* @param fee - The fee provided to sequencer for including in the inbox. 0 if Outbox (as not applicable).
* @param count - The occurrence of the entry in the dataset
* @param version - The version of the entry
* @param deadline - The deadline to consume a message. Only after it, can a message be cancelled.
*/
struct Entry {
uint64 count;
uint64 fee;
uint32 count;
uint32 version;
uint32 deadline;
}

Expand Down
12 changes: 11 additions & 1 deletion l1-contracts/src/core/libraries/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ library Errors {
error Inbox__DeadlineBeforeNow(); // 0xbf94a5dc
error Inbox__NotPastDeadline(); //0x3218ad9e
error Inbox__PastDeadline(); // 0x1eb114ea
error Inbox__InvalidVersion(uint256 entry, uint256 rollup); // 0x60be5dca
error Inbox__FeeTooHigh(); // 0x6f478f42
error Inbox__FailedToWithdrawFees(); // 0xbc66d464
error Inbox__Unauthorized(); // 0xe5336a6b
Expand All @@ -22,6 +23,8 @@ library Errors {
bytes32 entryKey,
uint64 storedFee,
uint64 feePassed,
uint32 storedVersion,
uint32 versionPassed,
uint32 storedDeadline,
uint32 deadlinePassed
); // 0xd483d8f2
Expand All @@ -32,14 +35,17 @@ library Errors {
// Outbox
error Outbox__Unauthorized(); // 0x2c9490c2
error Outbox__InvalidChainId(); // 0x577ec7c4
error Outbox__InvalidVersion(uint256 entry, uint256 message); // 0x7915cac3
error Outbox__NothingToConsume(bytes32 entryKey); // 0xfb4fb506
error Outbox__IncompatibleEntryArguments(
bytes32 entryKey,
uint64 storedFee,
uint64 feePassed,
uint32 storedVersion,
uint32 versionPassed,
uint32 storedDeadline,
uint32 deadlinePassed
); // 0xad1b401b
); // 0x5e789f34

// Rollup
error Rollup__InvalidStateHash(bytes32 expected, bytes32 actual); // 0xa3cfaab3
Expand All @@ -48,4 +54,8 @@ library Errors {
error Rollup__InvalidVersion(uint256 expected, uint256 actual); // 0x9ef30794
error Rollup__TimestampInFuture(); // 0xbc1ce916
error Rollup__TimestampTooOld(); // 0x72ed9c81

// Registry
error Registry__RollupNotRegistered(address rollup); // 0xa1fee4cf
error Registry__RollupAlreadyRegistered(address rollup); // 0x3c34eabf
}
9 changes: 7 additions & 2 deletions l1-contracts/src/core/libraries/MessageBox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,30 @@ library MessageBox {
mapping(bytes32 entryKey => DataStructures.Entry entry) storage _self,
bytes32 _entryKey,
uint64 _fee,
uint32 _version,
uint32 _deadline,
function(
bytes32,
uint64,
uint64,
uint32,
uint32,
uint32,
uint32
) pure _err
) internal {
DataStructures.Entry memory entry = _self[_entryKey];
if (
(entry.fee != 0 && entry.fee != _fee) || (entry.deadline != 0 && entry.deadline != _deadline)
|| (entry.version != 0 && entry.version != _version)
) {
// this should never happen as it is trying to overwrite `fee` and `deadline` with different values
// this should never happen as it is trying to overwrite `fee`, `version` and `deadline` with different values
// even though the entryKey (a hash) is the same! Pass all arguments to the error message for debugging.
_err(_entryKey, entry.fee, _fee, entry.deadline, _deadline);
_err(_entryKey, entry.fee, _fee, entry.version, _version, entry.deadline, _deadline);
}
entry.count += 1;
entry.fee = _fee;
entry.version = _version;
entry.deadline = _deadline;
_self[_entryKey] = entry;
}
Expand Down
27 changes: 18 additions & 9 deletions l1-contracts/src/core/messagebridge/Inbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ contract Inbox is IInbox {
mapping(bytes32 entryKey => DataStructures.Entry entry) internal entries;
mapping(address account => uint256 balance) public feesAccrued;

modifier onlyRollup() {
// @todo: (issue #624) handle different versions
if (msg.sender != address(REGISTRY.getRollup())) revert Errors.Inbox__Unauthorized();
_;
}

constructor(address _registry) {
REGISTRY = IRegistry(_registry);
}
Expand Down Expand Up @@ -78,7 +72,8 @@ contract Inbox is IInbox {
});

bytes32 key = computeEntryKey(message);
entries.insert(key, fee, _deadline, _errIncompatibleEntryArguments);
// Unsafe cast to uint32, but as we increment by 1 for versions to lookup the snapshots, we should be fine.
entries.insert(key, fee, uint32(_recipient.version), _deadline, _errIncompatibleEntryArguments);

emit MessageAdded(
key,
Expand Down Expand Up @@ -127,12 +122,16 @@ contract Inbox is IInbox {
function batchConsume(bytes32[] memory _entryKeys, address _feeCollector)
external
override(IInbox)
onlyRollup
{
uint256 totalFee = 0;
// This MUST revert if not called by a listed rollup contract
uint32 expectedVersion = uint32(REGISTRY.getVersionFor(msg.sender));
rahul-kothari marked this conversation as resolved.
Show resolved Hide resolved
for (uint256 i = 0; i < _entryKeys.length; i++) {
if (_entryKeys[i] == bytes32(0)) continue;
DataStructures.Entry memory entry = get(_entryKeys[i]);
if (entry.version != expectedVersion) {
revert Errors.Inbox__InvalidVersion(entry.version, expectedVersion);
}
// cant consume if we are already past deadline.
if (block.timestamp > entry.deadline) revert Errors.Inbox__PastDeadline();
entries.consume(_entryKeys[i], _errNothingToConsume);
Expand Down Expand Up @@ -205,18 +204,28 @@ contract Inbox is IInbox {
* @param _entryKey - The key to lookup
* @param _storedFee - The fee stored in the entry
* @param _feePassed - The fee passed into the insertion
* @param _storedVersion - The version stored in the entry
* @param _versionPassed - The version passed into the insertion
* @param _storedDeadline - The deadline stored in the entry
* @param _deadlinePassed - The deadline passed into the insertion
*/
function _errIncompatibleEntryArguments(
bytes32 _entryKey,
uint64 _storedFee,
uint64 _feePassed,
uint32 _storedVersion,
uint32 _versionPassed,
uint32 _storedDeadline,
uint32 _deadlinePassed
) internal pure {
revert Errors.Inbox__IncompatibleEntryArguments(
_entryKey, _storedFee, _feePassed, _storedDeadline, _deadlinePassed
_entryKey,
_storedFee,
_feePassed,
_storedVersion,
_versionPassed,
_storedDeadline,
_deadlinePassed
);
}
}
29 changes: 20 additions & 9 deletions l1-contracts/src/core/messagebridge/Outbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ contract Outbox is IOutbox {

mapping(bytes32 entryKey => DataStructures.Entry entry) internal entries;

modifier onlyRollup() {
// @todo: (issue #624) handle different versions
if (msg.sender != address(REGISTRY.getRollup())) revert Errors.Outbox__Unauthorized();
_;
}

constructor(address _registry) {
REGISTRY = IRegistry(_registry);
}
Expand All @@ -41,10 +35,12 @@ contract Outbox is IOutbox {
* @dev Only callable by the rollup contract
* @param _entryKeys - Array of entry keys (hash of the message) - computed by the L2 counterpart and sent to L1 via rollup block
*/
function sendL1Messages(bytes32[] memory _entryKeys) external override(IOutbox) onlyRollup {
function sendL1Messages(bytes32[] memory _entryKeys) external override(IOutbox) {
// This MUST revert if not called by a listed rollup contract
uint32 version = uint32(REGISTRY.getVersionFor(msg.sender));
for (uint256 i = 0; i < _entryKeys.length; i++) {
if (_entryKeys[i] == bytes32(0)) continue;
entries.insert(_entryKeys[i], 0, 0, _errIncompatibleEntryArguments);
entries.insert(_entryKeys[i], 0, version, 0, _errIncompatibleEntryArguments);
emit MessageAdded(_entryKeys[i]);
}
}
Expand All @@ -65,6 +61,11 @@ contract Outbox is IOutbox {
if (block.chainid != _message.recipient.chainId) revert Errors.Outbox__InvalidChainId();

entryKey = computeEntryKey(_message);
DataStructures.Entry memory entry = entries.get(entryKey, _errNothingToConsume);
if (entry.version != _message.sender.version) {
revert Errors.Outbox__InvalidVersion(entry.version, _message.sender.version);
}

entries.consume(entryKey, _errNothingToConsume);
emit MessageConsumed(entryKey, msg.sender);
}
Expand Down Expand Up @@ -121,18 +122,28 @@ contract Outbox is IOutbox {
* @param _entryKey - The key to lookup
* @param _storedFee - The fee stored in the entry
* @param _feePassed - The fee passed into the insertion
* @param _storedVersion - The version stored in the entry
* @param _versionPassed - The version passed into the insertion
* @param _storedDeadline - The deadline stored in the entry
* @param _deadlinePassed - The deadline passed into the insertion
*/
function _errIncompatibleEntryArguments(
bytes32 _entryKey,
uint64 _storedFee,
uint64 _feePassed,
uint32 _storedVersion,
uint32 _versionPassed,
uint32 _storedDeadline,
uint32 _deadlinePassed
) internal pure {
revert Errors.Outbox__IncompatibleEntryArguments(
_entryKey, _storedFee, _feePassed, _storedDeadline, _deadlinePassed
_entryKey,
_storedFee,
_feePassed,
_storedVersion,
_versionPassed,
_storedDeadline,
_deadlinePassed
);
}
}
61 changes: 49 additions & 12 deletions l1-contracts/src/core/messagebridge/Registry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {IOutbox} from "@aztec/core/interfaces/messagebridge/IOutbox.sol";

// Libraries
import {DataStructures} from "@aztec/core/libraries/DataStructures.sol";
import {Errors} from "@aztec/core/libraries/Errors.sol";

/**
* @title Registry
Expand All @@ -22,19 +23,12 @@ contract Registry is IRegistry {
uint256 public numberOfVersions;
DataStructures.RegistrySnapshot internal currentSnapshot;
mapping(uint256 version => DataStructures.RegistrySnapshot snapshot) internal snapshots;
mapping(address rollup => uint256 version) internal rollupToVersion;

/**
* @notice Creates a new snapshot of the registry
* todo: this function must be permissioned with some kind of governance/voting/authority
* @param _rollup - The address of the rollup contract
* @param _inbox - The address of the inbox contract
* @param _outbox - The address of the outbox contract
*/
function upgrade(address _rollup, address _inbox, address _outbox) external override(IRegistry) {
DataStructures.RegistrySnapshot memory newSnapshot =
DataStructures.RegistrySnapshot(_rollup, _inbox, _outbox, block.number);
currentSnapshot = newSnapshot;
snapshots[numberOfVersions++] = newSnapshot;
constructor() {
// Inserts a "dead" rollup and message boxes at version 0
// This is simply done to make first version 1, which fits better with the rest of the system
upgrade(address(0xdead), address(0xdead), address(0xdead));
}

/**
Expand All @@ -45,6 +39,17 @@ contract Registry is IRegistry {
return IRollup(currentSnapshot.rollup);
}

/**
* @notice Returns the version for a specific rollup contract or reverts if not listed
* @param _rollup - The address of the rollup contract
* @return The version of the rollup contract
*/
function getVersionFor(address _rollup) external view override(IRegistry) returns (uint256) {
(uint256 version, bool exists) = _getVersionFor(_rollup);
if (!exists) revert Errors.Registry__RollupNotRegistered(_rollup);
return version;
}

/**
* @notice Returns the inbox contract
* @return The inbox contract (of type IInbox)
Expand Down Expand Up @@ -88,4 +93,36 @@ contract Registry is IRegistry {
{
return currentSnapshot;
}

/**
* @notice Creates a new snapshot of the registry
* @dev Reverts if the rollup is already registered
* todo: this function must be permissioned with some kind of governance/voting/authority
* @param _rollup - The address of the rollup contract
* @param _inbox - The address of the inbox contract
* @param _outbox - The address of the outbox contract
* @return The version of the new snapshot
*/
function upgrade(address _rollup, address _inbox, address _outbox)
public
override(IRegistry)
returns (uint256)
{
(, bool exists) = _getVersionFor(_rollup);
if (exists) revert Errors.Registry__RollupAlreadyRegistered(_rollup);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just add a dumb test to ensure this never gets deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a registry test file 👍


DataStructures.RegistrySnapshot memory newSnapshot =
DataStructures.RegistrySnapshot(_rollup, _inbox, _outbox, block.number);
currentSnapshot = newSnapshot;
uint256 version = numberOfVersions++;
snapshots[version] = newSnapshot;
rollupToVersion[_rollup] = version;

return version;
}

function _getVersionFor(address _rollup) internal view returns (uint256 version, bool exists) {
version = rollupToVersion[_rollup];
return (version, version > 0);
}
}
Loading