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

# Description Fixes #359 and #624. # Checklist: - [ ] I have reviewed my diff in github, line by line. - [ ] Every change is related to the PR description. - [ ] I have [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this pull request to the issue(s) that it resolves. - [ ] There are no unexpected formatting changes, superfluous debug logs, or commented-out code. - [ ] The branch has been merged or rebased against the head of its merge target. - [ ] I'm happy for the PR to be merged at the reviewer's next convenience.
AztecProtocol · Jul 29, 2023 · abcf81c · abcf81c
1 parent eb4aa93
commit abcf81c
Show file tree

Hide file tree

Showing 38 changed files with 330 additions and 97 deletions.
diff --git a/l1-contracts/src/core/Rollup.sol b/l1-contracts/src/core/Rollup.sol
@@ -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);
 

diff --git a/l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol b/l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol
@@ -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);

diff --git a/l1-contracts/src/core/libraries/DataStructures.sol b/l1-contracts/src/core/libraries/DataStructures.sol
@@ -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;
   }
 

diff --git a/l1-contracts/src/core/libraries/Errors.sol b/l1-contracts/src/core/libraries/Errors.sol
@@ -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
@@ -22,6 +23,8 @@ library Errors {
     bytes32 entryKey,
     uint64 storedFee,
     uint64 feePassed,
+    uint32 storedVersion,
+    uint32 versionPassed,
     uint32 storedDeadline,
     uint32 deadlinePassed
   ); // 0xd483d8f2
@@ -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
@@ -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
 }
diff --git a/l1-contracts/src/core/libraries/MessageBox.sol b/l1-contracts/src/core/libraries/MessageBox.sol
@@ -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;
   }

diff --git a/l1-contracts/src/core/messagebridge/Inbox.sol b/l1-contracts/src/core/messagebridge/Inbox.sol
@@ -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);
   }
@@ -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,
@@ -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));
     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);
@@ -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
     );
   }
 }
diff --git a/l1-contracts/src/core/messagebridge/Outbox.sol b/l1-contracts/src/core/messagebridge/Outbox.sol
@@ -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);
   }
@@ -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]);
     }
   }
@@ -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);
   }
@@ -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
     );
   }
 }
diff --git a/l1-contracts/src/core/messagebridge/Registry.sol b/l1-contracts/src/core/messagebridge/Registry.sol
@@ -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
@@ -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));
   }
 
   /**
@@ -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)
@@ -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);
+
+    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);
+  }
 }