add fn selectors

comments comments fix fix comments comments comments test test comment ethereum/solidity#14430 comments test comments liberryfy test asdf add manual test asdf test fix asdf asdf comments comments
AztecProtocol · Mar 11, 2024 · 1c1b87e · 1c1b87e
1 parent 980c711
commit 1c1b87e
Show file tree

Hide file tree

Showing 8 changed files with 253 additions and 106 deletions.
diff --git a/l1-contracts/foundry.toml b/l1-contracts/foundry.toml
@@ -2,7 +2,7 @@
 src = 'src'
 out = 'out'
 libs = ['lib']
-solc = "0.8.21"
+solc = "0.8.23"
 
 remappings = [
   "@oz/=lib/openzeppelin-contracts/contracts/"

diff --git a/l1-contracts/src/core/interfaces/messagebridge/INewOutbox.sol b/l1-contracts/src/core/interfaces/messagebridge/INewOutbox.sol
@@ -14,12 +14,12 @@ import {DataStructures} from "../../libraries/DataStructures.sol";
 interface INewOutbox {
   event RootAdded(uint256 indexed l2BlockNumber, bytes32 indexed root, uint256 height);
   event MessageConsumed(
-    uint256 indexed l2BlockNumber, bytes32 indexed root, bytes32 indexed messageHash
+    uint256 indexed l2BlockNumber, bytes32 indexed root, bytes32 indexed messageHash, uint256 leafIndex
   );
 
   /*
    * @notice Inserts the root of a merkle tree containing all of the L2 to L1 messages in
-   * a block specified by _blockNumber.
+   * a block specified by _l2BlockNumber.
    * @dev Only callable by the state transitioner (rollup contract)
    * @dev Emits `RootAdded` upon inserting the root successfully
    * @param _l2BlockNumber - The L2 Block Number in which the L2 to L1 messages reside
@@ -30,12 +30,14 @@ interface INewOutbox {
 
   /*
    * @notice Consumes an entry from the Outbox
-   * @dev Only meaningfully callable by portals / recipients of messages
+   * @dev Only useable by portals / recipients of messages
    * @dev Emits `MessageConsumed` when consuming messages
    * @param _l2BlockNumber - The block number specifying the block that contains the message we want to consume
    * @param _leafIndex - The index inside the merkle tree where the message is located
    * @param _message - The L2 to L1 message
-   * @param _path - The sibling path used to prove inclusion of the message
+   * @param _path - The sibling path used to prove inclusion of the message, the _path length directly depends
+   * on the total amount of L2 to L1 messages in the block. i.e. the length of _path is equal to the depth of the
+   * L1 to L2 message tree.
    */
   function consume(
     uint256 _l2BlockNumber,

diff --git a/l1-contracts/src/core/libraries/Errors.sol b/l1-contracts/src/core/libraries/Errors.sol
@@ -46,12 +46,12 @@ library Errors {
     uint32 storedDeadline,
     uint32 deadlinePassed
   ); // 0x5e789f34
-  error Outbox__InvalidPathLength(uint256 expected, uint256 actual);
-  error Outbox__InvalidRoot(bytes32 expected, bytes32 actual);
-  error Outbox__RootAlreadySet(uint256 l2BlockNumber);
-  error Outbox__InvalidRecipient(address expected, address actual);
-  error Outbox__AlreadyNullified(uint256 l2BlockNumber, uint256 leafIndex);
-  error Outbox__NothingToConsumeAtBlock(uint256 l2BlockNumber);
+  error Outbox__InvalidPathLength(uint256 expected, uint256 actual); // 0x481bcd9c
+  error Outbox__InsertingInvalidRoot(); // 0x73c2daca
+  error Outbox__RootAlreadySetAtBlock(uint256 l2BlockNumber); // 0x3eccfd3e
+  error Outbox__InvalidRecipient(address expected, address actual); // 0x57aad581
+  error Outbox__AlreadyNullified(uint256 l2BlockNumber, uint256 leafIndex); // 0xfd71c2d4
+  error Outbox__NothingToConsumeAtBlock(uint256 l2BlockNumber); // 0xa4508f22
 
   // Rollup
   error Rollup__InvalidArchive(bytes32 expected, bytes32 actual); // 0xb682a40e
@@ -68,4 +68,7 @@ library Errors {
 
   // HeaderLib
   error HeaderLib__InvalidHeaderSize(uint256 expected, uint256 actual); // 0xf3ccb247
+
+  // MerkleLib
+  error MerkleLib__InvalidRoot(bytes32 expected, bytes32 actual); // 0xb77e99
 }
diff --git a/l1-contracts/src/core/libraries/Merkle.sol b/l1-contracts/src/core/libraries/Merkle.sol
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2024 Aztec Labs.
+pragma solidity >=0.8.18;
+
+import {Errors} from "../libraries/Errors.sol";
+
+/**
+ * @title
+ * @author Aztec Labs
+ * @notice
+ */
+library Merkle {  
+  /*
+    * @notice Verifies the membership of a leaf and path against an expected root.
+    * @dev In the case of a mismatched root, and subsequent inability to verify membership, this function throws.
+    * @param _path - The sibling path of the message as a leaf, used to prove message inclusion
+    * @param _leaf - The hash of the message we are trying to prove inclusion for
+    * @param _index - The index of the message inside the L2 to L1 message tree
+    * @param _expectedRoot - The expected root to check the validity of the message and sibling path with.
+    * @remark -
+    * E.g. A sibling path for a leaf at index 3 in a tree of depth 3 (between 5 and 8 leafs) consists of the 3 elements denoted as *'s
+    * d0:                                            [ root ]
+    * d1:                      [ ]                                               [*]
+    * d2:         [*]                      [ ]                       [ ]                     [ ]
+    * d3:   [ ]         [ ]          [*]         [ ]           [ ]         [ ]          [ ]        [ ].
+    * And the elements would be ordered as: [ d3_index_2, d2_index_0, d1_index_1 ].
+    * @remark - We use the indexAtHeight to see whether our child of the next subtree is at the left or the right side, as any odd indexes are right-sided children.
+    * This is important and we have assigned it into its own variable isRight, because this affects the way we concat our two children to then hash and calculate the root.
+    */
+  function _verifyMembership(
+    bytes32[] memory _path,
+    bytes32 _leaf,
+    uint256 _index,
+    bytes32 _expectedRoot
+  ) internal pure {
+    bytes32 subtreeRoot = _leaf;
+    uint256 indexAtHeight = _index;
+
+    for (uint256 height = 0; height < _path.length; height++) {
+      bool isRight = (indexAtHeight & 1) == 1;
+
+      subtreeRoot = isRight
+        ? sha256(bytes.concat(_path[height], subtreeRoot))
+        : sha256(bytes.concat(subtreeRoot, _path[height]));
+
+      indexAtHeight /= 2;
+    }
+
+    if (subtreeRoot != _expectedRoot) {
+      revert Errors.MerkleLib__InvalidRoot(_expectedRoot, subtreeRoot);
+    }
+  }
+}
diff --git a/l1-contracts/src/core/messagebridge/NewOutbox.sol b/l1-contracts/src/core/messagebridge/NewOutbox.sol
@@ -5,6 +5,7 @@ pragma solidity >=0.8.18;
 // Libraries
 import {DataStructures} from "../libraries/DataStructures.sol";
 import {Errors} from "../libraries/Errors.sol";
+import {Merkle} from "../libraries/Merkle.sol";
 import {Hash} from "../libraries/Hash.sol";
 import {INewOutbox} from "../interfaces/messagebridge/INewOutbox.sol";
 
@@ -32,7 +33,7 @@ contract NewOutbox is INewOutbox {
 
   /*
    * @notice Inserts the root of a merkle tree containing all of the L2 to L1 messages in
-   * a block specified by _blockNumber.
+   * a block specified by _l2BlockNumber.
    * @dev Only callable by the state transitioner (rollup contract)
    * @dev Emits `RootAdded` upon inserting the root successfully
    * @param _l2BlockNumber - The L2 Block Number in which the L2 to L1 messages reside
@@ -48,7 +49,11 @@ contract NewOutbox is INewOutbox {
     }
 
     if (roots[_l2BlockNumber].root != bytes32(0)) {
-      revert Errors.Outbox__RootAlreadySet(_l2BlockNumber);
+      revert Errors.Outbox__RootAlreadySetAtBlock(_l2BlockNumber);
+    }
+
+    if (_root == bytes32(0)) {
+      revert Errors.Outbox__InsertingInvalidRoot();
     }
 
     roots[_l2BlockNumber].root = _root;
@@ -59,19 +64,21 @@ contract NewOutbox is INewOutbox {
 
   /*
    * @notice Consumes an entry from the Outbox
-   * @dev Only meaningfully callable by portals / recipients of messages
+   * @dev Only useable by portals / recipients of messages
    * @dev Emits `MessageConsumed` when consuming messages
    * @param _l2BlockNumber - The block number specifying the block that contains the message we want to consume
    * @param _leafIndex - The index inside the merkle tree where the message is located
    * @param _message - The L2 to L1 message
-   * @param _path - The sibling path used to prove inclusion of the message
+   * @param _path - The sibling path used to prove inclusion of the message, the _path length directly depends
+   * on the total amount of L2 to L1 messages in the block. i.e. the length of _path is equal to the depth of the
+   * L1 to L2 message tree.
    */
   function consume(
     uint256 _l2BlockNumber,
     uint256 _leafIndex,
     DataStructures.L2ToL1Msg memory _message,
     bytes32[] memory _path
-  ) public override(INewOutbox) {
+  ) external override(INewOutbox) {
     if (msg.sender != _message.recipient.actor) {
       revert Errors.Outbox__InvalidRecipient(_message.recipient.actor, msg.sender);
     }
@@ -96,49 +103,45 @@ contract NewOutbox is INewOutbox {
       revert Errors.Outbox__InvalidPathLength(expectedHeight, _path.length);
     }
 
-    bytes32 messageHash = _verifyMembership(_path, _message, _leafIndex, expectedRoot);
+    bytes32 messageHash = _message.sha256ToField();
+
+    Merkle._verifyMembership(_path, messageHash, _leafIndex, expectedRoot);
 
     roots[_l2BlockNumber].nullified[_leafIndex] = true;
 
-    emit MessageConsumed(_l2BlockNumber, expectedRoot, messageHash);
+    emit MessageConsumed(_l2BlockNumber, expectedRoot, messageHash, _leafIndex);
   }
 
   /*
    * @notice Verifies the membership of an L2 to L1 message against an expected root.
    * @dev This function assumes a valid path height, as well as sane inputs.
-   * @dev In the case of a mismatched root, and subsequent inability to verify membership, this function throws
-   * and does not return anything.
+   * @dev In the case of a mismatched root, and subsequent inability to verify membership, this function throws.
    * @param _path - The sibling path of the message as a leaf, used to prove message inclusion
-   * @param _message - The message we are trying to prove inclusion for
+   * @param _leaf - The hash of the message we are trying to prove inclusion for
    * @param _index - The index of the message inside the L2 to L1 message tree
    * @param _expectedRoot - The expected root to check the validity of the message and sibling path with.
-   * @returns - The leaf of the message in the L2 to L1 tree, which is the L2 to L1 message hashed via SHA256.
    */
   function _verifyMembership(
     bytes32[] memory _path,
-    DataStructures.L2ToL1Msg memory _message,
+    bytes32 _leaf,
     uint256 _index,
     bytes32 _expectedRoot
-  ) internal pure returns (bytes32) {
-    bytes32 leaf = _message.sha256ToField();
+  ) internal pure {
+    bytes32 subtreeRoot = _leaf;
+    uint256 indexAtHeight = _index;
 
-    bytes32 root;
-    uint256 index = _index;
-
-    for (uint256 i = 0; i < _path.length; i++) {
-      bool isRight = (index & 1) == 1;
+    for (uint256 height = 0; height < _path.length; height++) {
+      bool isRight = (indexAtHeight & 1) == 1;
 
-      root = isRight
-        ? sha256(bytes.concat(_path[i], i == 0 ? leaf : root))
-        : sha256(bytes.concat(i == 0 ? leaf : root, _path[i]));
+      subtreeRoot = isRight
+        ? sha256(bytes.concat(_path[height], subtreeRoot))
+        : sha256(bytes.concat(subtreeRoot, _path[height]));
 
-      index /= 2;
+      indexAtHeight /= 2;
     }
 
-    if (root != _expectedRoot) {
-      revert Errors.Outbox__InvalidRoot(_expectedRoot, root);
+    if (subtreeRoot != _expectedRoot) {
+      revert Errors.MerkleLib__InvalidRoot(_expectedRoot, subtreeRoot);
     }
-
-    return leaf;
   }
 }