From e2c9d1eb2ca6ca2eb1e680f0425143d66ff4a879 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 15:37:13 +0000
Subject: [PATCH 01/48] refactor: mocing types consts to constants.nr


From 92a39edbe12aeced020b8fb27b1b607d464296fa Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Fri, 1 Mar 2024 11:22:29 +0000
Subject: [PATCH 02/48] feat: new Inbox

---
 .../src/core/messagebridge/NewInbox.sol       | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 l1-contracts/src/core/messagebridge/NewInbox.sol

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
new file mode 100644
index 00000000000..b88d4c00971
--- /dev/null
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2023 Aztec Labs.
+pragma solidity >=0.8.18;
+
+// Interfaces
+import {IInbox} from "../interfaces/messagebridge/IInbox.sol";
+import {IRegistry} from "../interfaces/messagebridge/IRegistry.sol";
+
+// Libraries
+import {Constants} from "../libraries/ConstantsGen.sol";
+import {DataStructures} from "../libraries/DataStructures.sol";
+import {Errors} from "../libraries/Errors.sol";
+import {Hash} from "../libraries/Hash.sol";
+import {MessageBox} from "../libraries/MessageBox.sol";
+
+/**
+ * @title Inbox
+ * @author Aztec Labs
+ * @notice Lives on L1 and is used to pass messages into the rollup, e.g., L1 -> L2 messages.
+ */
+// TODO: rename to Inbox once all the pieces of the new message model are in place.
+contract NewInbox {
+  IRegistry public immutable REGISTRY;
+
+  event LeafInserted(uint256 indexed treeNumber, uint256 indexed index, bytes32 value);
+
+  constructor(address _registry) {
+    REGISTRY = IRegistry(_registry);
+  }
+}

From 418e1c44cc645d3171995a046ec748ebf5a419e4 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Fri, 1 Mar 2024 13:12:11 +0000
Subject: [PATCH 03/48] WIP

---
 .../src/core/messagebridge/NewInbox.sol       | 69 ++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index b88d4c00971..828a7add95d 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -22,9 +22,76 @@ import {MessageBox} from "../libraries/MessageBox.sol";
 contract NewInbox {
   IRegistry public immutable REGISTRY;
 
+  uint256 public immutable HEIGHT;
+  uint256 public immutable SIZE;
+  bytes32 private immutable ZERO;
+
+  uint256 private toInclude = 0;
+  uint256 private inProgress = 1;
+
+  mapping(uint256 treeNumber => address tree) public frontier;
+
   event LeafInserted(uint256 indexed treeNumber, uint256 indexed index, bytes32 value);
 
-  constructor(address _registry) {
+  constructor(address _registry, uint256 _height, bytes32 _zero) {
     REGISTRY = IRegistry(_registry);
+
+    HEIGHT = _height;
+    SIZE = 2**_height;
+    ZERO = _zero;
   }
 }
+
+// class Inbox:
+//   STATE_TRANSITIONER: immutable(address)
+//   ZERO: immutable(bytes32)
+
+//   HEIGHT: immutable(uint256)
+//   SIZE: immutable(uint256)
+
+//   trees: HashMap[uint256, FrontierTree]
+
+//   to_include: uint256 = 0
+//   in_progress: uint256 = 1
+
+//   def __init__(self, _height: uint256, _zero: bytes32, _state_transitioner: address):
+//     self.HEIGHT = _height
+//     self.SIZE = 2**_height
+//     self.ZERO = _zero
+//     self.STATE_TRANSITIONER = _state_transitioner
+
+//     self.trees[1] = FrontierTree(self.HEIGHT)
+ 
+//   def insert(self, message: L1ToL2Message) -> bytes32:
+//     '''
+//     Insert into the next FrontierTree. If the tree is full, creates a new one
+//     '''
+//     if self.trees[self.in_progress].next_index == 2**self.HEIGHT:
+//       self.in_progress += 1
+//       self.trees[self.in_progress] = FrontierTree(self.HEIGHT)
+
+//     message.sender.actor = msg.sender
+//     message.sender.chain_id = block.chainid
+
+//     leaf = message.hash_to_field()
+//     self.trees[self.in_progress].insert(leaf)
+//     return leaf
+
+//   def consume(self) -> bytes32:
+//     '''
+//     Consumes the current tree, and starts a new one if needed
+//     '''
+//     assert msg.sender == self.STATE_TRANSITIONER
+
+//     root = self.ZERO
+//     if self.to_include > 0:
+//       root = self.trees[self.to_include].root()
+
+//     # If we are "catching up" we can skip the creation as it is already there
+//     if self.to_include + 1 == self.in_progress:
+//       self.in_progress += 1
+//       self.trees[self.in_progress] = FrontierTree(self.HEIGHT)
+
+//     self.to_include += 1
+
+//     return root
\ No newline at end of file

From 1ba2bd68ca5cf42c19971beff30996372b2ba87e Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Fri, 1 Mar 2024 14:53:11 +0000
Subject: [PATCH 04/48] unifying naming with inbox and yellow paper

---
 .../messagebridge/frontier_tree/Frontier.sol     | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
index d20e053a58c..4b5a4b2df65 100644
--- a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
+++ b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
@@ -5,7 +5,7 @@ pragma solidity >=0.8.18;
 import {IFrontier} from "../../interfaces/messagebridge/IFrontier.sol";
 
 contract FrontierMerkle is IFrontier {
-  uint256 public immutable DEPTH;
+  uint256 public immutable HEIGHT;
   uint256 public immutable SIZE;
 
   uint256 internal nextIndex = 0;
@@ -16,12 +16,12 @@ contract FrontierMerkle is IFrontier {
   // for the zeros at each level. This would save gas on computations
   mapping(uint256 level => bytes32 zero) public zeros;
 
-  constructor(uint256 _depth) {
-    DEPTH = _depth;
-    SIZE = 2 ** _depth;
+  constructor(uint256 _height) {
+    HEIGHT = _height;
+    SIZE = 2 ** _height;
 
     zeros[0] = bytes32(0);
-    for (uint256 i = 1; i <= DEPTH; i++) {
+    for (uint256 i = 1; i <= HEIGHT; i++) {
       zeros[i] = sha256(bytes.concat(zeros[i - 1], zeros[i - 1]));
     }
   }
@@ -39,10 +39,10 @@ contract FrontierMerkle is IFrontier {
   function root() external view override(IFrontier) returns (bytes32) {
     uint256 next = nextIndex;
     if (next == 0) {
-      return zeros[DEPTH];
+      return zeros[HEIGHT];
     }
     if (next == SIZE) {
-      return frontier[DEPTH];
+      return frontier[HEIGHT];
     }
 
     uint256 index = next - 1;
@@ -52,7 +52,7 @@ contract FrontierMerkle is IFrontier {
     bytes32 temp = frontier[level];
 
     uint256 bits = index >> level;
-    for (uint256 i = level; i < DEPTH; i++) {
+    for (uint256 i = level; i < HEIGHT; i++) {
       bool isRight = bits & 1 == 1;
       if (isRight) {
         if (frontier[i] == temp) {

From 8019330fac374fa140dca7073cb3f23cd8bee978 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Fri, 1 Mar 2024 15:50:23 +0000
Subject: [PATCH 05/48] initial impl

---
 .../interfaces/messagebridge/IFrontier.sol    |   4 +-
 .../src/core/messagebridge/NewInbox.sol       | 138 ++++++++++--------
 .../messagebridge/frontier_tree/Frontier.sol  |   8 +-
 3 files changed, 88 insertions(+), 62 deletions(-)

diff --git a/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol b/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
index 5b17cab8dc4..17354e1db73 100644
--- a/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
+++ b/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
@@ -3,7 +3,9 @@
 pragma solidity >=0.8.18;
 
 interface IFrontier {
-  function insertLeaf(bytes32 _leaf) external;
+  function insertLeaf(bytes32 _leaf) external returns (uint256);
 
   function root() external view returns (bytes32);
+
+  function isFull() external view returns (bool);
 }
diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 828a7add95d..7a3b819bf91 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -3,7 +3,7 @@
 pragma solidity >=0.8.18;
 
 // Interfaces
-import {IInbox} from "../interfaces/messagebridge/IInbox.sol";
+import {IFrontier} from "../interfaces/messagebridge/IFrontier.sol";
 import {IRegistry} from "../interfaces/messagebridge/IRegistry.sol";
 
 // Libraries
@@ -11,7 +11,9 @@ import {Constants} from "../libraries/ConstantsGen.sol";
 import {DataStructures} from "../libraries/DataStructures.sol";
 import {Errors} from "../libraries/Errors.sol";
 import {Hash} from "../libraries/Hash.sol";
-import {MessageBox} from "../libraries/MessageBox.sol";
+
+// Contracts
+import {FrontierMerkle} from "./frontier_tree/Frontier.sol";
 
 /**
  * @title Inbox
@@ -20,7 +22,9 @@ import {MessageBox} from "../libraries/MessageBox.sol";
  */
 // TODO: rename to Inbox once all the pieces of the new message model are in place.
 contract NewInbox {
-  IRegistry public immutable REGISTRY;
+  using Hash for DataStructures.L1ToL2Msg;
+
+  address public immutable ROLLUP;
 
   uint256 public immutable HEIGHT;
   uint256 public immutable SIZE;
@@ -29,69 +33,83 @@ contract NewInbox {
   uint256 private toInclude = 0;
   uint256 private inProgress = 1;
 
-  mapping(uint256 treeNumber => address tree) public frontier;
+  mapping(uint256 treeNumber => IFrontier tree) public frontier;
 
   event LeafInserted(uint256 indexed treeNumber, uint256 indexed index, bytes32 value);
 
-  constructor(address _registry, uint256 _height, bytes32 _zero) {
-    REGISTRY = IRegistry(_registry);
+  constructor(address _rollup, uint256 _height, bytes32 _zero) {
+    ROLLUP = _rollup;
 
     HEIGHT = _height;
-    SIZE = 2**_height;
+    SIZE = 2 ** _height;
     ZERO = _zero;
-  }
-}
-
-// class Inbox:
-//   STATE_TRANSITIONER: immutable(address)
-//   ZERO: immutable(bytes32)
-
-//   HEIGHT: immutable(uint256)
-//   SIZE: immutable(uint256)
-
-//   trees: HashMap[uint256, FrontierTree]
-
-//   to_include: uint256 = 0
-//   in_progress: uint256 = 1
-
-//   def __init__(self, _height: uint256, _zero: bytes32, _state_transitioner: address):
-//     self.HEIGHT = _height
-//     self.SIZE = 2**_height
-//     self.ZERO = _zero
-//     self.STATE_TRANSITIONER = _state_transitioner
 
-//     self.trees[1] = FrontierTree(self.HEIGHT)
- 
-//   def insert(self, message: L1ToL2Message) -> bytes32:
-//     '''
-//     Insert into the next FrontierTree. If the tree is full, creates a new one
-//     '''
-//     if self.trees[self.in_progress].next_index == 2**self.HEIGHT:
-//       self.in_progress += 1
-//       self.trees[self.in_progress] = FrontierTree(self.HEIGHT)
-
-//     message.sender.actor = msg.sender
-//     message.sender.chain_id = block.chainid
-
-//     leaf = message.hash_to_field()
-//     self.trees[self.in_progress].insert(leaf)
-//     return leaf
-
-//   def consume(self) -> bytes32:
-//     '''
-//     Consumes the current tree, and starts a new one if needed
-//     '''
-//     assert msg.sender == self.STATE_TRANSITIONER
-
-//     root = self.ZERO
-//     if self.to_include > 0:
-//       root = self.trees[self.to_include].root()
-
-//     # If we are "catching up" we can skip the creation as it is already there
-//     if self.to_include + 1 == self.in_progress:
-//       self.in_progress += 1
-//       self.trees[self.in_progress] = FrontierTree(self.HEIGHT)
+    // We deploy the first tree
+    frontier[inProgress] = IFrontier(new FrontierMerkle(_height));
+  }
 
-//     self.to_include += 1
+  /**
+   * @notice Inserts an entry into the Inbox
+   * @dev Will emit `LeafInserted` with data for easy access by the sequencer
+   * @param _recipient - The recipient of the entry
+   * @param _deadline - The deadline to consume a message. Only after it, can a message be cancelled.
+   * it is uint32 to for slot packing of the Entry struct. Should work until Feb 2106.
+   * @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
+   */
+  function insert(
+    DataStructures.L2Actor memory _recipient,
+    uint32 _deadline,
+    bytes32 _content,
+    bytes32 _secretHash
+  ) external returns (bytes32) {
+    IFrontier currentTree = frontier[inProgress];
+    if (currentTree.isFull()) {
+      inProgress += 1;
+      currentTree = IFrontier(new FrontierMerkle(HEIGHT));
+      frontier[inProgress] = currentTree;
+    }
+
+    DataStructures.L1ToL2Msg memory message = DataStructures.L1ToL2Msg({
+      sender: DataStructures.L1Actor(msg.sender, block.chainid),
+      recipient: _recipient,
+      content: _content,
+      secretHash: _secretHash,
+      deadline: _deadline,
+      fee: 0 // TODO: nuke this (there will no longer be fees for messages)
+    });
+
+    bytes32 leaf = message.sha256ToField();
+    uint256 nextIndex = currentTree.insertLeaf(leaf);
+    emit LeafInserted(inProgress, nextIndex, leaf);
+
+    return leaf;
+  }
 
-//     return root
\ No newline at end of file
+  /**
+   * @notice Consumes the current tree, and starts a new one if needed
+   * @dev Only callable by the rollup contract
+   * @return The root of the consumed tree
+   */
+  function consume() external returns (bytes32) {
+    if (msg.sender != ROLLUP) {
+      revert Errors.Inbox__Unauthorized();
+    }
+
+    bytes32 root = ZERO;
+    if (toInclude > 0) {
+      root = frontier[toInclude].root();
+    }
+
+    // If we are "catching up" we can skip the creation as it is already there
+    if (toInclude + 1 == inProgress) {
+      inProgress += 1;
+      frontier[inProgress] = IFrontier(new FrontierMerkle(HEIGHT));
+    }
+
+    toInclude += 1;
+
+    return root;
+  }
+}
diff --git a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
index 4b5a4b2df65..c81b38afc07 100644
--- a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
+++ b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
@@ -26,7 +26,7 @@ contract FrontierMerkle is IFrontier {
     }
   }
 
-  function insertLeaf(bytes32 _leaf) external override(IFrontier) {
+  function insertLeaf(bytes32 _leaf) external override(IFrontier) returns (uint256) {
     uint256 level = _computeLevel(nextIndex);
     bytes32 right = _leaf;
     for (uint256 i = 0; i < level; i++) {
@@ -34,6 +34,8 @@ contract FrontierMerkle is IFrontier {
     }
     frontier[level] = right;
     nextIndex++;
+
+    return nextIndex;
   }
 
   function root() external view override(IFrontier) returns (bytes32) {
@@ -71,6 +73,10 @@ contract FrontierMerkle is IFrontier {
     return temp;
   }
 
+  function isFull() external view override(IFrontier) returns (bool) {
+    return nextIndex == SIZE;
+  }
+
   function _computeLevel(uint256 _leafIndex) internal pure returns (uint256) {
     // The number of trailing ones is how many times in a row we are the right child.
     // e.g., each time this happens we go another layer up to update the parent.

From b79e97f7afb1817c577a0c66756e3e59c799f0d8 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Fri, 1 Mar 2024 16:29:52 +0000
Subject: [PATCH 06/48] test setup

---
 .../src/core/messagebridge/NewInbox.sol       | 14 +++----
 l1-contracts/test/NewInbox.t.sol              | 40 +++++++++++++++++++
 2 files changed, 45 insertions(+), 9 deletions(-)
 create mode 100644 l1-contracts/test/NewInbox.t.sol

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 7a3b819bf91..9a30593337b 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -52,18 +52,14 @@ contract NewInbox {
    * @notice Inserts an entry into the Inbox
    * @dev Will emit `LeafInserted` with data for easy access by the sequencer
    * @param _recipient - The recipient of the entry
-   * @param _deadline - The deadline to consume a message. Only after it, can a message be cancelled.
-   * it is uint32 to for slot packing of the Entry struct. Should work until Feb 2106.
    * @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
    */
-  function insert(
-    DataStructures.L2Actor memory _recipient,
-    uint32 _deadline,
-    bytes32 _content,
-    bytes32 _secretHash
-  ) external returns (bytes32) {
+  function insert(DataStructures.L2Actor memory _recipient, bytes32 _content, bytes32 _secretHash)
+    external
+    returns (bytes32)
+  {
     IFrontier currentTree = frontier[inProgress];
     if (currentTree.isFull()) {
       inProgress += 1;
@@ -76,7 +72,7 @@ contract NewInbox {
       recipient: _recipient,
       content: _content,
       secretHash: _secretHash,
-      deadline: _deadline,
+      deadline: 0, // TODO: nuke this (there will no longer be fees for messages)
       fee: 0 // TODO: nuke this (there will no longer be fees for messages)
     });
 
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
new file mode 100644
index 00000000000..3a8388535fe
--- /dev/null
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2023 Aztec Labs.
+pragma solidity >=0.8.18;
+
+import {Test} from "forge-std/Test.sol";
+import {NewInbox} from "../src/core/messagebridge/NewInbox.sol";
+import {Constants} from "../src/core/libraries/ConstantsGen.sol";
+import {Errors} from "../src/core/libraries/Errors.sol";
+
+import {DataStructures} from "../src/core/libraries/DataStructures.sol";
+
+contract NewInboxTest is Test {
+  NewInbox internal inbox;
+  uint256 internal version = 0;
+
+  function setUp() public {
+    address rollup = address(this);
+    inbox = new NewInbox(rollup, 10, 0);
+  }
+
+  function _fakeMessage() internal view returns (DataStructures.L1ToL2Msg memory) {
+    return DataStructures.L1ToL2Msg({
+      sender: DataStructures.L1Actor({actor: address(this), chainId: block.chainid}),
+      recipient: DataStructures.L2Actor({
+        actor: 0x1000000000000000000000000000000000000000000000000000000000000000,
+        version: version
+      }),
+      content: 0x2000000000000000000000000000000000000000000000000000000000000000,
+      secretHash: 0x3000000000000000000000000000000000000000000000000000000000000000,
+      fee: 0,
+      deadline: 0
+    });
+  }
+
+  function testRevertIfNotConsumingFromRollup() public {
+    vm.prank(address(0x1));
+    vm.expectRevert(Errors.Inbox__Unauthorized.selector);
+    inbox.consume();
+  }
+}

From 23828d4e4aa18aaf3f656a7b1dd05937594e5cee Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Fri, 1 Mar 2024 17:19:48 +0000
Subject: [PATCH 07/48] WIP

---
 .../src/core/messagebridge/NewInbox.sol       |  8 +++--
 l1-contracts/test/NewInbox.t.sol              | 29 +++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 9a30593337b..746c81c4c59 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -35,7 +35,7 @@ contract NewInbox {
 
   mapping(uint256 treeNumber => IFrontier tree) public frontier;
 
-  event LeafInserted(uint256 indexed treeNumber, uint256 indexed index, bytes32 value);
+  event LeafInserted(uint256 treeNumber, uint256 index, bytes32 value);
 
   constructor(address _rollup, uint256 _height, bytes32 _zero) {
     ROLLUP = _rollup;
@@ -72,14 +72,16 @@ contract NewInbox {
       recipient: _recipient,
       content: _content,
       secretHash: _secretHash,
-      deadline: 0, // TODO: nuke this (there will no longer be fees for messages)
-      fee: 0 // TODO: nuke this (there will no longer be fees for messages)
+      // TODO: nuke the following 2 values from the struct once the new message model is in place
+      deadline: 0,
+      fee: 0
     });
 
     bytes32 leaf = message.sha256ToField();
     uint256 nextIndex = currentTree.insertLeaf(leaf);
     emit LeafInserted(inProgress, nextIndex, leaf);
 
+    // TODO: do we really need to return this?
     return leaf;
   }
 
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 3a8388535fe..0018cc66e3f 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -6,13 +6,18 @@ import {Test} from "forge-std/Test.sol";
 import {NewInbox} from "../src/core/messagebridge/NewInbox.sol";
 import {Constants} from "../src/core/libraries/ConstantsGen.sol";
 import {Errors} from "../src/core/libraries/Errors.sol";
+import {Hash} from "../src/core/libraries/Hash.sol";
 
 import {DataStructures} from "../src/core/libraries/DataStructures.sol";
 
 contract NewInboxTest is Test {
+  using Hash for DataStructures.L1ToL2Msg;
+
   NewInbox internal inbox;
   uint256 internal version = 0;
 
+  event LeafInserted(uint256 indexed treeNumber, uint256 indexed index, bytes32 value);
+
   function setUp() public {
     address rollup = address(this);
     inbox = new NewInbox(rollup, 10, 0);
@@ -37,4 +42,28 @@ contract NewInboxTest is Test {
     vm.expectRevert(Errors.Inbox__Unauthorized.selector);
     inbox.consume();
   }
+
+  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);
+    // 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);
+
+    // TODO: nuke the following 2 values from the struct once the new message model is in place
+    _message.deadline = 0;
+    _message.fee = 0;
+
+    bytes32 leaf = _message.sha256ToField();
+    vm.expectEmit(true, true, true, true);
+    // event we expect
+    emit LeafInserted(1, 1, leaf);
+    // event we will get
+    bytes32 insertedLeaf = inbox.insert(_message.recipient, _message.content, _message.secretHash);
+
+    assertEq(insertedLeaf, leaf);
+  }
 }

From 5b4cfb5710429901c2523876376cb3017b1f58fa Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 08:15:58 +0000
Subject: [PATCH 08/48] fix

---
 l1-contracts/test/NewInbox.t.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 0018cc66e3f..fe98f5fb3c7 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -16,7 +16,7 @@ contract NewInboxTest is Test {
   NewInbox internal inbox;
   uint256 internal version = 0;
 
-  event LeafInserted(uint256 indexed treeNumber, uint256 indexed index, bytes32 value);
+  event LeafInserted(uint256 treeNumber, uint256 index, bytes32 value);
 
   function setUp() public {
     address rollup = address(this);

From 0eedc351d905cf13fb23e312aecdaabd511f0ab3 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 08:32:16 +0000
Subject: [PATCH 09/48] testSendMultipleSameL2Messages

---
 l1-contracts/test/NewInbox.t.sol | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index fe98f5fb3c7..5b043e7bb65 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -66,4 +66,18 @@ contract NewInboxTest is Test {
 
     assertEq(insertedLeaf, leaf);
   }
+
+  function testSendMultipleSameL2Messages() public {
+    DataStructures.L1ToL2Msg memory message = _fakeMessage();
+    bytes32 leaf1 = inbox.insert(message.recipient, message.content, message.secretHash);
+    bytes32 leaf2 = inbox.insert(message.recipient, message.content, message.secretHash);
+    bytes32 leaf3 = inbox.insert(message.recipient, message.content, message.secretHash);
+
+    assertEq(address(inbox.frontier(0)), address(0));
+    assertNotEq(address(inbox.frontier(1)), address(0));
+    assertEq(address(inbox.frontier(2)), address(0));
+
+    assertEq(leaf1, leaf2);
+    assertEq(leaf2, leaf3);
+  }
 }

From 5de257d295999bf172da44a30cded687bce696d9 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 08:42:41 +0000
Subject: [PATCH 10/48] WIP

---
 .../src/core/messagebridge/NewInbox.sol       | 10 +++++++
 l1-contracts/test/NewInbox.t.sol              | 28 +++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 746c81c4c59..43ff28dd163 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -60,6 +60,16 @@ contract NewInbox {
     external
     returns (bytes32)
   {
+    if (uint256(_recipient.actor) > Constants.MAX_FIELD_VALUE) {
+      revert Errors.Inbox__ActorTooLarge(_recipient.actor);
+    }
+    if (uint256(_content) > Constants.MAX_FIELD_VALUE) {
+      revert Errors.Inbox__ContentTooLarge(_content);
+    }
+    if (uint256(_secretHash) > Constants.MAX_FIELD_VALUE) {
+      revert Errors.Inbox__SecretHashTooLarge(_secretHash);
+    }
+
     IFrontier currentTree = frontier[inProgress];
     if (currentTree.isFull()) {
       inProgress += 1;
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 5b043e7bb65..a874016ebcb 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -9,6 +9,7 @@ import {Errors} from "../src/core/libraries/Errors.sol";
 import {Hash} from "../src/core/libraries/Hash.sol";
 
 import {DataStructures} from "../src/core/libraries/DataStructures.sol";
+import {IFrontier} from "../src/core/interfaces/messagebridge/IFrontier.sol";
 
 contract NewInboxTest is Test {
   using Hash for DataStructures.L1ToL2Msg;
@@ -73,11 +74,38 @@ contract NewInboxTest is Test {
     bytes32 leaf2 = inbox.insert(message.recipient, message.content, message.secretHash);
     bytes32 leaf3 = inbox.insert(message.recipient, message.content, message.secretHash);
 
+    // Only 1 tree should be non-zero
     assertEq(address(inbox.frontier(0)), address(0));
     assertNotEq(address(inbox.frontier(1)), address(0));
     assertEq(address(inbox.frontier(2)), address(0));
 
+    // All the leaves should be the same
     assertEq(leaf1, leaf2);
     assertEq(leaf2, leaf3);
   }
+
+  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.insert(message.recipient, 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.insert(message.recipient, 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.insert(message.recipient, message.content, message.secretHash);
+  }
 }

From 00f41656022ad572989e36cc03c4140ecea3bb98 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 09:58:37 +0000
Subject: [PATCH 11/48] WIP

---
 l1-contracts/test/NewInbox.t.sol | 74 ++++++++++++++++++++++++++++++--
 1 file changed, 70 insertions(+), 4 deletions(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index a874016ebcb..adc7f0c9480 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -11,17 +11,22 @@ import {Hash} from "../src/core/libraries/Hash.sol";
 import {DataStructures} from "../src/core/libraries/DataStructures.sol";
 import {IFrontier} from "../src/core/interfaces/messagebridge/IFrontier.sol";
 
+import "forge-std/console.sol";
+
 contract NewInboxTest is Test {
   using Hash for DataStructures.L1ToL2Msg;
 
   NewInbox internal inbox;
   uint256 internal version = 0;
+  bytes32 internal emptyTreeRoot;
 
   event LeafInserted(uint256 treeNumber, uint256 index, bytes32 value);
 
   function setUp() public {
     address rollup = address(this);
-    inbox = new NewInbox(rollup, 10, 0);
+    // We set low depth (5) to ensure we sufficiently test tree transitions
+    inbox = new NewInbox(rollup, 5, 0);
+    emptyTreeRoot = inbox.frontier(1).root();
   }
 
   function _fakeMessage() internal view returns (DataStructures.L1ToL2Msg memory) {
@@ -38,6 +43,18 @@ contract NewInboxTest is Test {
     });
   }
 
+  function _getNumTrees() internal view returns (uint256) {
+    uint256 treeNumber = 1;
+    while (address(inbox.frontier(treeNumber)) != address(0)) {
+      treeNumber++;
+    }
+    return treeNumber - 1;
+  }
+
+  function _divideAndRoundUp(uint256 a, uint256 b) internal pure returns (uint256) {
+    return (a + b - 1) / b;
+  }
+
   function testRevertIfNotConsumingFromRollup() public {
     vm.prank(address(0x1));
     vm.expectRevert(Errors.Inbox__Unauthorized.selector);
@@ -75,9 +92,7 @@ contract NewInboxTest is Test {
     bytes32 leaf3 = inbox.insert(message.recipient, message.content, message.secretHash);
 
     // Only 1 tree should be non-zero
-    assertEq(address(inbox.frontier(0)), address(0));
-    assertNotEq(address(inbox.frontier(1)), address(0));
-    assertEq(address(inbox.frontier(2)), address(0));
+    assertEq(_getNumTrees(), 1);
 
     // All the leaves should be the same
     assertEq(leaf1, leaf2);
@@ -108,4 +123,55 @@ contract NewInboxTest is Test {
     );
     inbox.insert(message.recipient, message.content, message.secretHash);
   }
+
+  function testFuzzConsume(DataStructures.L1ToL2Msg[] memory _messages, uint256 _numTreesToConsume)
+    public
+  {
+    // insert messages:
+    for (uint256 i = 0; i < _messages.length; i++) {
+      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);
+      // update version
+      message.recipient.version = version;
+
+      // TODO: nuke the following 2 values from the struct once the new message model is in place
+      message.deadline = 0;
+      message.fee = 0;
+
+      inbox.insert(message.recipient, message.content, message.secretHash);
+    }
+
+    uint256 expectedNumTrees = _divideAndRoundUp(_messages.length, inbox.SIZE());
+    if (expectedNumTrees == 0) {
+      // This occurs when there are no messages but we initialize the first tree in the constructor so there are never
+      // zero trees
+      expectedNumTrees = 1;
+    }
+    uint256 numTrees = _getNumTrees();
+    assertEq(numTrees, expectedNumTrees);
+
+    // We use (numTrees * 2) as upper bound here because we want to test the case where we go beyond the currently
+    // initalized number of trees. When consuming the newly initialized trees we should get zero roots.
+    uint256 numTreesToConsume = bound(_numTreesToConsume, 1, numTrees * 2);
+
+    // Now we consume the trees
+    for (uint256 i = 0; i < numTreesToConsume; i++) {
+      bytes32 root = inbox.consume();
+      if (i < numTrees) {
+        assertNotEq(root, emptyTreeRoot, "Root should not be zero");
+      } else {
+        assertEq(root, emptyTreeRoot, "Root should be zero");
+      }
+    }
+  }
 }

From e76919639e3562796758b8dca5403988026281d7 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 10:42:24 +0000
Subject: [PATCH 12/48] fix

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 9 +++------
 l1-contracts/test/NewInbox.t.sol                 | 7 ++-----
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 43ff28dd163..6c7d895f344 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -30,7 +30,7 @@ contract NewInbox {
   uint256 public immutable SIZE;
   bytes32 private immutable ZERO;
 
-  uint256 private toInclude = 0;
+  uint256 private toInclude = 1;
   uint256 private inProgress = 1;
 
   mapping(uint256 treeNumber => IFrontier tree) public frontier;
@@ -105,13 +105,10 @@ contract NewInbox {
       revert Errors.Inbox__Unauthorized();
     }
 
-    bytes32 root = ZERO;
-    if (toInclude > 0) {
-      root = frontier[toInclude].root();
-    }
+    bytes32 root = frontier[toInclude].root();
 
     // If we are "catching up" we can skip the creation as it is already there
-    if (toInclude + 1 == inProgress) {
+    if (toInclude == inProgress) {
       inProgress += 1;
       frontier[inProgress] = IFrontier(new FrontierMerkle(HEIGHT));
     }
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index adc7f0c9480..74b5f07d42f 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -3,15 +3,12 @@
 pragma solidity >=0.8.18;
 
 import {Test} from "forge-std/Test.sol";
+
 import {NewInbox} from "../src/core/messagebridge/NewInbox.sol";
 import {Constants} from "../src/core/libraries/ConstantsGen.sol";
 import {Errors} from "../src/core/libraries/Errors.sol";
 import {Hash} from "../src/core/libraries/Hash.sol";
-
 import {DataStructures} from "../src/core/libraries/DataStructures.sol";
-import {IFrontier} from "../src/core/interfaces/messagebridge/IFrontier.sol";
-
-import "forge-std/console.sol";
 
 contract NewInboxTest is Test {
   using Hash for DataStructures.L1ToL2Msg;
@@ -167,7 +164,7 @@ contract NewInboxTest is Test {
     // Now we consume the trees
     for (uint256 i = 0; i < numTreesToConsume; i++) {
       bytes32 root = inbox.consume();
-      if (i < numTrees) {
+      if (i < numTrees && _messages.length > 0) {
         assertNotEq(root, emptyTreeRoot, "Root should not be zero");
       } else {
         assertEq(root, emptyTreeRoot, "Root should be zero");

From 2adfe77d8480c9120a50de88e89d006e0edef83c Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 10:43:01 +0000
Subject: [PATCH 13/48] slither

---
 l1-contracts/slither_output.md | 110 ++++++++++++++++++++++++---------
 1 file changed, 81 insertions(+), 29 deletions(-)

diff --git a/l1-contracts/slither_output.md b/l1-contracts/slither_output.md
index 420d9f75330..f34fed2da29 100644
--- a/l1-contracts/slither_output.md
+++ b/l1-contracts/slither_output.md
@@ -3,16 +3,17 @@ Summary
  - [uninitialized-local](#uninitialized-local) (2 results) (Medium)
  - [unused-return](#unused-return) (1 results) (Medium)
  - [pess-dubious-typecast](#pess-dubious-typecast) (8 results) (Medium)
- - [reentrancy-events](#reentrancy-events) (1 results) (Low)
+ - [missing-zero-check](#missing-zero-check) (1 results) (Low)
+ - [reentrancy-events](#reentrancy-events) (2 results) (Low)
  - [timestamp](#timestamp) (4 results) (Low)
- - [pess-public-vs-external](#pess-public-vs-external) (5 results) (Low)
+ - [pess-public-vs-external](#pess-public-vs-external) (6 results) (Low)
  - [assembly](#assembly) (2 results) (Informational)
  - [dead-code](#dead-code) (5 results) (Informational)
  - [solc-version](#solc-version) (1 results) (Informational)
  - [low-level-calls](#low-level-calls) (1 results) (Informational)
  - [similar-names](#similar-names) (3 results) (Informational)
  - [constable-states](#constable-states) (1 results) (Optimization)
- - [pess-multiple-storage-read](#pess-multiple-storage-read) (2 results) (Optimization)
+ - [pess-multiple-storage-read](#pess-multiple-storage-read) (6 results) (Optimization)
 ## pess-unprotected-setter
 Impact: High
 Confidence: Medium
@@ -131,6 +132,16 @@ src/core/messagebridge/Inbox.sol#L122-L143
 Impact: Low
 Confidence: Medium
  - [ ] ID-12
+[NewInbox.constructor(address,uint256,bytes32)._rollup](src/core/messagebridge/NewInbox.sol#L40) lacks a zero-check on :
+		- [ROLLUP = _rollup](src/core/messagebridge/NewInbox.sol#L41)
+
+src/core/messagebridge/NewInbox.sol#L40
+
+
+## reentrancy-events
+Impact: Low
+Confidence: Medium
+ - [ ] ID-13
 Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L53-L91):
 	External calls:
 	- [inbox.batchConsume(l1ToL2Msgs,msg.sender)](src/core/Rollup.sol#L85)
@@ -141,10 +152,20 @@ Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L5
 src/core/Rollup.sol#L53-L91
 
 
+ - [ ] ID-14
+Reentrancy in [NewInbox.insert(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L59-L96):
+	External calls:
+	- [nextIndex = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L91)
+	Event emitted after the call(s):
+	- [LeafInserted(inProgress,nextIndex,leaf)](src/core/messagebridge/NewInbox.sol#L92)
+
+src/core/messagebridge/NewInbox.sol#L59-L96
+
+
 ## timestamp
 Impact: Low
 Confidence: Medium
- - [ ] ID-13
+ - [ ] ID-15
 [Inbox.batchConsume(bytes32[],address)](src/core/messagebridge/Inbox.sol#L122-L143) uses timestamp for comparisons
 	Dangerous comparisons:
 	- [block.timestamp > entry.deadline](src/core/messagebridge/Inbox.sol#L136)
@@ -152,7 +173,7 @@ Confidence: Medium
 src/core/messagebridge/Inbox.sol#L122-L143
 
 
- - [ ] ID-14
+ - [ ] ID-16
 [HeaderLib.validate(HeaderLib.Header,uint256,uint256,bytes32)](src/core/libraries/HeaderLib.sol#L108-L138) uses timestamp for comparisons
 	Dangerous comparisons:
 	- [_header.globalVariables.timestamp > block.timestamp](src/core/libraries/HeaderLib.sol#L122)
@@ -160,7 +181,7 @@ src/core/messagebridge/Inbox.sol#L122-L143
 src/core/libraries/HeaderLib.sol#L108-L138
 
 
- - [ ] ID-15
+ - [ ] ID-17
 [Inbox.sendL2Message(DataStructures.L2Actor,uint32,bytes32,bytes32)](src/core/messagebridge/Inbox.sol#L45-L91) uses timestamp for comparisons
 	Dangerous comparisons:
 	- [_deadline <= block.timestamp](src/core/messagebridge/Inbox.sol#L54)
@@ -168,7 +189,7 @@ src/core/libraries/HeaderLib.sol#L108-L138
 src/core/messagebridge/Inbox.sol#L45-L91
 
 
- - [ ] ID-16
+ - [ ] ID-18
 [Inbox.cancelL2Message(DataStructures.L1ToL2Msg,address)](src/core/messagebridge/Inbox.sol#L102-L113) uses timestamp for comparisons
 	Dangerous comparisons:
 	- [block.timestamp <= _message.deadline](src/core/messagebridge/Inbox.sol#L108)
@@ -179,28 +200,35 @@ src/core/messagebridge/Inbox.sol#L102-L113
 ## pess-public-vs-external
 Impact: Low
 Confidence: Medium
- - [ ] ID-17
-The following public functions could be turned into external in [FrontierMerkle](src/core/messagebridge/frontier_tree/Frontier.sol#L7-L85) contract:
+ - [ ] ID-19
+The following public functions could be turned into external in [NewInbox](src/core/messagebridge/NewInbox.sol#L24-L120) contract:
+	[NewInbox.constructor(address,uint256,bytes32)](src/core/messagebridge/NewInbox.sol#L40-L49)
+
+src/core/messagebridge/NewInbox.sol#L24-L120
+
+
+ - [ ] ID-20
+The following public functions could be turned into external in [FrontierMerkle](src/core/messagebridge/frontier_tree/Frontier.sol#L7-L91) contract:
 	[FrontierMerkle.constructor(uint256)](src/core/messagebridge/frontier_tree/Frontier.sol#L19-L27)
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L7-L85
+src/core/messagebridge/frontier_tree/Frontier.sol#L7-L91
 
 
- - [ ] ID-18
+ - [ ] ID-21
 The following public functions could be turned into external in [Registry](src/core/messagebridge/Registry.sol#L22-L129) contract:
 	[Registry.constructor()](src/core/messagebridge/Registry.sol#L29-L33)
 
 src/core/messagebridge/Registry.sol#L22-L129
 
 
- - [ ] ID-19
+ - [ ] ID-22
 The following public functions could be turned into external in [Rollup](src/core/Rollup.sol#L27-L100) contract:
 	[Rollup.constructor(IRegistry,IAvailabilityOracle)](src/core/Rollup.sol#L39-L44)
 
 src/core/Rollup.sol#L27-L100
 
 
- - [ ] ID-20
+ - [ ] ID-23
 The following public functions could be turned into external in [Outbox](src/core/messagebridge/Outbox.sol#L21-L148) contract:
 	[Outbox.constructor(address)](src/core/messagebridge/Outbox.sol#L29-L31)
 	[Outbox.get(bytes32)](src/core/messagebridge/Outbox.sol#L77-L84)
@@ -209,7 +237,7 @@ The following public functions could be turned into external in [Outbox](src/cor
 src/core/messagebridge/Outbox.sol#L21-L148
 
 
- - [ ] ID-21
+ - [ ] ID-24
 The following public functions could be turned into external in [Inbox](src/core/messagebridge/Inbox.sol#L21-L231) contract:
 	[Inbox.constructor(address)](src/core/messagebridge/Inbox.sol#L30-L32)
 	[Inbox.contains(bytes32)](src/core/messagebridge/Inbox.sol#L174-L176)
@@ -238,31 +266,31 @@ src/core/libraries/decoders/TxsDecoder.sol#L291-L310
 ## dead-code
 Impact: Informational
 Confidence: Medium
- - [ ] ID-24
+ - [ ] ID-27
 [Inbox._errIncompatibleEntryArguments(bytes32,uint64,uint64,uint32,uint32,uint32,uint32)](src/core/messagebridge/Inbox.sol#L212-L230) is never used and should be removed
 
 src/core/messagebridge/Inbox.sol#L212-L230
 
 
- - [ ] ID-25
+ - [ ] ID-28
 [Outbox._errNothingToConsume(bytes32)](src/core/messagebridge/Outbox.sol#L114-L116) is never used and should be removed
 
 src/core/messagebridge/Outbox.sol#L114-L116
 
 
- - [ ] ID-26
+ - [ ] ID-29
 [Hash.sha256ToField(bytes32)](src/core/libraries/Hash.sol#L59-L61) is never used and should be removed
 
 src/core/libraries/Hash.sol#L59-L61
 
 
- - [ ] ID-27
+ - [ ] ID-30
 [Inbox._errNothingToConsume(bytes32)](src/core/messagebridge/Inbox.sol#L197-L199) is never used and should be removed
 
 src/core/messagebridge/Inbox.sol#L197-L199
 
 
- - [ ] ID-28
+ - [ ] ID-31
 [Outbox._errIncompatibleEntryArguments(bytes32,uint64,uint64,uint32,uint32,uint32,uint32)](src/core/messagebridge/Outbox.sol#L129-L147) is never used and should be removed
 
 src/core/messagebridge/Outbox.sol#L129-L147
@@ -271,13 +299,13 @@ src/core/messagebridge/Outbox.sol#L129-L147
 ## solc-version
 Impact: Informational
 Confidence: High
- - [ ] ID-29
+ - [ ] ID-32
 solc-0.8.21 is not recommended for deployment
 
 ## low-level-calls
 Impact: Informational
 Confidence: High
- - [ ] ID-30
+ - [ ] ID-33
 Low level call in [Inbox.withdrawFees()](src/core/messagebridge/Inbox.sol#L148-L153):
 	- [(success) = msg.sender.call{value: balance}()](src/core/messagebridge/Inbox.sol#L151)
 
@@ -299,7 +327,7 @@ Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol
 src/core/libraries/ConstantsGen.sol#L109
 
 
- - [ ] ID-33
+ - [ ] ID-36
 Variable [Rollup.AVAILABILITY_ORACLE](src/core/Rollup.sol#L30) is too similar to [Rollup.constructor(IRegistry,IAvailabilityOracle)._availabilityOracle](src/core/Rollup.sol#L39)
 
 src/core/Rollup.sol#L30
@@ -308,7 +336,7 @@ src/core/Rollup.sol#L30
 ## constable-states
 Impact: Optimization
 Confidence: High
- - [ ] ID-34
+ - [ ] ID-37
 [Rollup.lastWarpedBlockTs](src/core/Rollup.sol#L37) should be constant 
 
 src/core/Rollup.sol#L37
@@ -317,15 +345,39 @@ src/core/Rollup.sol#L37
 ## pess-multiple-storage-read
 Impact: Optimization
 Confidence: High
- - [ ] ID-35
-In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L39-L72) variable [FrontierMerkle.DEPTH](src/core/messagebridge/frontier_tree/Frontier.sol#L8) is read multiple times
+ - [ ] ID-38
+In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74) variable [FrontierMerkle.HEIGHT](src/core/messagebridge/frontier_tree/Frontier.sol#L8) is read multiple times
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L39-L72
+src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
 
 
- - [ ] ID-36
-In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L39-L72) variable [FrontierMerkle.frontier](src/core/messagebridge/frontier_tree/Frontier.sol#L13) is read multiple times
+ - [ ] ID-39
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L103-L119) variable [NewInbox.toInclude](src/core/messagebridge/NewInbox.sol#L33) is read multiple times
+
+src/core/messagebridge/NewInbox.sol#L103-L119
+
+
+ - [ ] ID-40
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L103-L119) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
+
+src/core/messagebridge/NewInbox.sol#L103-L119
+
+
+ - [ ] ID-41
+In a function [FrontierMerkle.insertLeaf(bytes32)](src/core/messagebridge/frontier_tree/Frontier.sol#L29-L39) variable [FrontierMerkle.nextIndex](src/core/messagebridge/frontier_tree/Frontier.sol#L11) is read multiple times
+
+src/core/messagebridge/frontier_tree/Frontier.sol#L29-L39
+
+
+ - [ ] ID-42
+In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74) variable [FrontierMerkle.frontier](src/core/messagebridge/frontier_tree/Frontier.sol#L13) is read multiple times
+
+src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
+
+
+ - [ ] ID-43
+In a function [NewInbox.insert(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L59-L96) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L39-L72
+src/core/messagebridge/NewInbox.sol#L59-L96
 
 

From 8bac58c3f3b3edcce1890dc93940780997091843 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 11:17:16 +0000
Subject: [PATCH 14/48] Re-introducing lag

---
 .../src/core/messagebridge/NewInbox.sol       | 20 ++++++++++++-------
 l1-contracts/test/NewInbox.t.sol              | 11 +++++++---
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 6c7d895f344..c24af5cfb26 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -28,24 +28,26 @@ contract NewInbox {
 
   uint256 public immutable HEIGHT;
   uint256 public immutable SIZE;
-  bytes32 private immutable ZERO;
+  bytes32 private immutable EMPTY_ROOT; // The root of an empty frontier tree
 
-  uint256 private toInclude = 1;
+  uint256 private toInclude = 0;
   uint256 private inProgress = 1;
 
   mapping(uint256 treeNumber => IFrontier tree) public frontier;
 
   event LeafInserted(uint256 treeNumber, uint256 index, bytes32 value);
 
-  constructor(address _rollup, uint256 _height, bytes32 _zero) {
+  constructor(address _rollup, uint256 _height) {
     ROLLUP = _rollup;
 
     HEIGHT = _height;
     SIZE = 2 ** _height;
-    ZERO = _zero;
 
     // We deploy the first tree
-    frontier[inProgress] = IFrontier(new FrontierMerkle(_height));
+    IFrontier firstTree = IFrontier(new FrontierMerkle(_height));
+    frontier[inProgress] = firstTree;
+
+    EMPTY_ROOT = firstTree.root();
   }
 
   /**
@@ -91,13 +93,14 @@ contract NewInbox {
     uint256 nextIndex = currentTree.insertLeaf(leaf);
     emit LeafInserted(inProgress, nextIndex, leaf);
 
-    // TODO: do we really need to return this?
     return leaf;
   }
 
   /**
    * @notice Consumes the current tree, and starts a new one if needed
    * @dev Only callable by the rollup contract
+   * @dev In the first iteration we return empty tree root because first block's messages tree is always
+   * empty because there has to be a 1 block lag to prevent sequencer DOS attacks.
    * @return The root of the consumed tree
    */
   function consume() external returns (bytes32) {
@@ -105,7 +108,10 @@ contract NewInbox {
       revert Errors.Inbox__Unauthorized();
     }
 
-    bytes32 root = frontier[toInclude].root();
+    bytes32 root = EMPTY_ROOT;
+    if (toInclude > 0) {
+      root = frontier[toInclude].root();
+    }
 
     // If we are "catching up" we can skip the creation as it is already there
     if (toInclude == inProgress) {
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 74b5f07d42f..c1245676944 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -21,8 +21,8 @@ contract NewInboxTest is Test {
 
   function setUp() public {
     address rollup = address(this);
-    // We set low depth (5) to ensure we sufficiently test tree transitions
-    inbox = new NewInbox(rollup, 5, 0);
+    // We set low depth (5) to ensure we sufficiently test the tree transitions
+    inbox = new NewInbox(rollup, 5);
     emptyTreeRoot = inbox.frontier(1).root();
   }
 
@@ -164,7 +164,12 @@ contract NewInboxTest is Test {
     // Now we consume the trees
     for (uint256 i = 0; i < numTreesToConsume; i++) {
       bytes32 root = inbox.consume();
-      if (i < numTrees && _messages.length > 0) {
+
+      // Notes:
+      // 1) For i = 0 we should always get empty tree root because first block's messages tree is always
+      // empty because we introduced a 1 block lag to prevent sequencer DOS attacks.
+      // 2) For _messages.length = 0 and i = 1 we get empty root as well
+      if (i != 0 && i <= numTrees && _messages.length > 0) {
         assertNotEq(root, emptyTreeRoot, "Root should not be zero");
       } else {
         assertEq(root, emptyTreeRoot, "Root should be zero");

From 3c16058e21789e2906584a43da3aeebc1e08e6e7 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 11:25:32 +0000
Subject: [PATCH 15/48] updated docs

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index c24af5cfb26..6271509c4a3 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -51,12 +51,12 @@ contract NewInbox {
   }
 
   /**
-   * @notice Inserts an entry into the Inbox
-   * @dev Will emit `LeafInserted` with data for easy access by the sequencer
-   * @param _recipient - The recipient of the entry
-   * @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
+   * @notice Inserts a new message into the Inbox
+   * @dev Emits `LeafInserted` with data for easy access by the sequencer
+   * @param _recipient - The recipient of the message
+   * @param _content - The content of the message (application specific)
+   * @param _secretHash - The secret hash of the message (make it possible to hide when a specific message is consumed on L2)
+   * @return The key of the message in the set
    */
   function insert(DataStructures.L2Actor memory _recipient, bytes32 _content, bytes32 _secretHash)
     external
@@ -100,7 +100,7 @@ contract NewInbox {
    * @notice Consumes the current tree, and starts a new one if needed
    * @dev Only callable by the rollup contract
    * @dev In the first iteration we return empty tree root because first block's messages tree is always
-   * empty because there has to be a 1 block lag to prevent sequencer DOS attacks.
+   * empty because there has to be a 1 block lag to prevent sequencer DOS attacks
    * @return The root of the consumed tree
    */
   function consume() external returns (bytes32) {
@@ -113,7 +113,7 @@ contract NewInbox {
       root = frontier[toInclude].root();
     }
 
-    // If we are "catching up" we can skip the creation as it is already there
+    // If we are "catching up" we skip the tree creation as it is already there
     if (toInclude == inProgress) {
       inProgress += 1;
       frontier[inProgress] = IFrontier(new FrontierMerkle(HEIGHT));

From 164a7ea93afcbfe0062bbaa4afa58b0cd0f8035a Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 14:25:09 +0000
Subject: [PATCH 16/48] setting deadline to max

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 2 +-
 l1-contracts/test/NewInbox.t.sol                 | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 6271509c4a3..79dc83b1b72 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -85,7 +85,7 @@ contract NewInbox {
       content: _content,
       secretHash: _secretHash,
       // TODO: nuke the following 2 values from the struct once the new message model is in place
-      deadline: 0,
+      deadline: 2 ** 32 - 1,
       fee: 0
     });
 
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index c1245676944..e0b0030962d 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -58,7 +58,7 @@ contract NewInboxTest is Test {
     inbox.consume();
   }
 
-  function testFuzzSendL2Msg(DataStructures.L1ToL2Msg memory _message) public {
+  function testFuzzInsert(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
@@ -69,7 +69,7 @@ contract NewInboxTest is Test {
     _message.secretHash = bytes32(uint256(_message.secretHash) % Constants.P);
 
     // TODO: nuke the following 2 values from the struct once the new message model is in place
-    _message.deadline = 0;
+    _message.deadline = 2 ** 32 - 1;
     _message.fee = 0;
 
     bytes32 leaf = _message.sha256ToField();
@@ -142,7 +142,7 @@ contract NewInboxTest is Test {
       message.recipient.version = version;
 
       // TODO: nuke the following 2 values from the struct once the new message model is in place
-      message.deadline = 0;
+      message.deadline = 2 ** 32 - 1;
       message.fee = 0;
 
       inbox.insert(message.recipient, message.content, message.secretHash);

From a54fee9571f667f9def9c865e1bcfc2d68b2bc78 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 15:36:24 +0000
Subject: [PATCH 17/48] indexing by block num

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 10 +++++-----
 l1-contracts/test/NewInbox.t.sol                 | 16 +++++++++-------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 79dc83b1b72..1a8581db84f 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -30,12 +30,12 @@ contract NewInbox {
   uint256 public immutable SIZE;
   bytes32 private immutable EMPTY_ROOT; // The root of an empty frontier tree
 
-  uint256 private toInclude = 0;
-  uint256 private inProgress = 1;
+  uint256 private toInclude = 1;
+  uint256 private inProgress = 2;
 
-  mapping(uint256 treeNumber => IFrontier tree) public frontier;
+  mapping(uint256 blockNumber => IFrontier tree) public frontier;
 
-  event LeafInserted(uint256 treeNumber, uint256 index, bytes32 value);
+  event LeafInserted(uint256 indexed blockNumber, uint256 index, bytes32 value);
 
   constructor(address _rollup, uint256 _height) {
     ROLLUP = _rollup;
@@ -109,7 +109,7 @@ contract NewInbox {
     }
 
     bytes32 root = EMPTY_ROOT;
-    if (toInclude > 0) {
+    if (toInclude > Constants.INITIAL_L2_BLOCK_NUM) {
       root = frontier[toInclude].root();
     }
 
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index e0b0030962d..813636944ca 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -13,17 +13,19 @@ import {DataStructures} from "../src/core/libraries/DataStructures.sol";
 contract NewInboxTest is Test {
   using Hash for DataStructures.L1ToL2Msg;
 
+  uint256 internal constant FIRST_REAL_TREE_NUM = Constants.INITIAL_L2_BLOCK_NUM + 1;
+
   NewInbox internal inbox;
   uint256 internal version = 0;
   bytes32 internal emptyTreeRoot;
 
-  event LeafInserted(uint256 treeNumber, uint256 index, bytes32 value);
+  event LeafInserted(uint256 indexed blockNumber, uint256 index, bytes32 value);
 
   function setUp() public {
     address rollup = address(this);
     // We set low depth (5) to ensure we sufficiently test the tree transitions
     inbox = new NewInbox(rollup, 5);
-    emptyTreeRoot = inbox.frontier(1).root();
+    emptyTreeRoot = inbox.frontier(2).root();
   }
 
   function _fakeMessage() internal view returns (DataStructures.L1ToL2Msg memory) {
@@ -41,11 +43,11 @@ contract NewInboxTest is Test {
   }
 
   function _getNumTrees() internal view returns (uint256) {
-    uint256 treeNumber = 1;
-    while (address(inbox.frontier(treeNumber)) != address(0)) {
-      treeNumber++;
+    uint256 blockNumber = FIRST_REAL_TREE_NUM;
+    while (address(inbox.frontier(blockNumber)) != address(0)) {
+      blockNumber++;
     }
-    return treeNumber - 1;
+    return blockNumber - 2; // -2 because first real tree is included in block 2
   }
 
   function _divideAndRoundUp(uint256 a, uint256 b) internal pure returns (uint256) {
@@ -75,7 +77,7 @@ contract NewInboxTest is Test {
     bytes32 leaf = _message.sha256ToField();
     vm.expectEmit(true, true, true, true);
     // event we expect
-    emit LeafInserted(1, 1, leaf);
+    emit LeafInserted(FIRST_REAL_TREE_NUM, 1, leaf);
     // event we will get
     bytes32 insertedLeaf = inbox.insert(_message.recipient, _message.content, _message.secretHash);
 

From 3d79858b2e67fe2859d398aa8bf58a71576993c7 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 15:45:19 +0000
Subject: [PATCH 18/48] using type(uint32).max instead of 2**32 - 1

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 2 +-
 l1-contracts/test/NewInbox.t.sol                 | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 1a8581db84f..a281c38359e 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -85,7 +85,7 @@ contract NewInbox {
       content: _content,
       secretHash: _secretHash,
       // TODO: nuke the following 2 values from the struct once the new message model is in place
-      deadline: 2 ** 32 - 1,
+      deadline: type(uint32).max,
       fee: 0
     });
 
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 813636944ca..3442ebbbbda 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -38,7 +38,7 @@ contract NewInboxTest is Test {
       content: 0x2000000000000000000000000000000000000000000000000000000000000000,
       secretHash: 0x3000000000000000000000000000000000000000000000000000000000000000,
       fee: 0,
-      deadline: 0
+      deadline: type(uint32).max
     });
   }
 
@@ -71,7 +71,7 @@ contract NewInboxTest is Test {
     _message.secretHash = bytes32(uint256(_message.secretHash) % Constants.P);
 
     // TODO: nuke the following 2 values from the struct once the new message model is in place
-    _message.deadline = 2 ** 32 - 1;
+    _message.deadline = type(uint32).max;
     _message.fee = 0;
 
     bytes32 leaf = _message.sha256ToField();
@@ -144,7 +144,7 @@ contract NewInboxTest is Test {
       message.recipient.version = version;
 
       // TODO: nuke the following 2 values from the struct once the new message model is in place
-      message.deadline = 2 ** 32 - 1;
+      message.deadline = type(uint32).max;
       message.fee = 0;
 
       inbox.insert(message.recipient, message.content, message.secretHash);

From 0d970f28df5d4e68bc517a62af44bd578eb2654d Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 15:53:52 +0000
Subject: [PATCH 19/48] event index fix

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 2 +-
 l1-contracts/test/NewInbox.t.sol                 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index a281c38359e..2f8c4cf4a24 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -91,7 +91,7 @@ contract NewInbox {
 
     bytes32 leaf = message.sha256ToField();
     uint256 nextIndex = currentTree.insertLeaf(leaf);
-    emit LeafInserted(inProgress, nextIndex, leaf);
+    emit LeafInserted(inProgress, nextIndex - 1, leaf);
 
     return leaf;
   }
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 3442ebbbbda..27080130585 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -77,7 +77,7 @@ contract NewInboxTest is Test {
     bytes32 leaf = _message.sha256ToField();
     vm.expectEmit(true, true, true, true);
     // event we expect
-    emit LeafInserted(FIRST_REAL_TREE_NUM, 1, leaf);
+    emit LeafInserted(FIRST_REAL_TREE_NUM, 0, leaf);
     // event we will get
     bytes32 insertedLeaf = inbox.insert(_message.recipient, _message.content, _message.secretHash);
 

From 3228fb423bb6dbb05286c32f424306d2bf76c5ad Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 15:57:54 +0000
Subject: [PATCH 20/48] fix

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 2f8c4cf4a24..60f2551dd0c 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -114,7 +114,7 @@ contract NewInbox {
     }
 
     // If we are "catching up" we skip the tree creation as it is already there
-    if (toInclude == inProgress) {
+    if (toInclude + 1 == inProgress) {
       inProgress += 1;
       frontier[inProgress] = IFrontier(new FrontierMerkle(HEIGHT));
     }

From 83c9e3801c29bc912d3d998daece48d974e0f96f Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Mon, 4 Mar 2024 16:01:48 +0000
Subject: [PATCH 21/48] sendL2Message instead of insert

---
 .../src/core/messagebridge/NewInbox.sol       |   9 +-
 l1-contracts/test/NewInbox.t.sol              | 114 ++++++++++--------
 2 files changed, 66 insertions(+), 57 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 60f2551dd0c..ee3686e9443 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -58,10 +58,11 @@ contract NewInbox {
    * @param _secretHash - The secret hash of the message (make it possible to hide when a specific message is consumed on L2)
    * @return The key of the message in the set
    */
-  function insert(DataStructures.L2Actor memory _recipient, bytes32 _content, bytes32 _secretHash)
-    external
-    returns (bytes32)
-  {
+  function sendL2Message(
+    DataStructures.L2Actor memory _recipient,
+    bytes32 _content,
+    bytes32 _secretHash
+  ) external returns (bytes32) {
     if (uint256(_recipient.actor) > Constants.MAX_FIELD_VALUE) {
       revert Errors.Inbox__ActorTooLarge(_recipient.actor);
     }
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 27080130585..8b5f9779035 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -79,16 +79,17 @@ contract NewInboxTest is Test {
     // event we expect
     emit LeafInserted(FIRST_REAL_TREE_NUM, 0, leaf);
     // event we will get
-    bytes32 insertedLeaf = inbox.insert(_message.recipient, _message.content, _message.secretHash);
+    bytes32 insertedLeaf =
+      inbox.sendL2Message(_message.recipient, _message.content, _message.secretHash);
 
     assertEq(insertedLeaf, leaf);
   }
 
   function testSendMultipleSameL2Messages() public {
     DataStructures.L1ToL2Msg memory message = _fakeMessage();
-    bytes32 leaf1 = inbox.insert(message.recipient, message.content, message.secretHash);
-    bytes32 leaf2 = inbox.insert(message.recipient, message.content, message.secretHash);
-    bytes32 leaf3 = inbox.insert(message.recipient, message.content, message.secretHash);
+    bytes32 leaf1 = inbox.sendL2Message(message.recipient, message.content, message.secretHash);
+    bytes32 leaf2 = inbox.sendL2Message(message.recipient, message.content, message.secretHash);
+    bytes32 leaf3 = inbox.sendL2Message(message.recipient, message.content, message.secretHash);
 
     // Only 1 tree should be non-zero
     assertEq(_getNumTrees(), 1);
@@ -104,14 +105,14 @@ contract NewInboxTest is Test {
     vm.expectRevert(
       abi.encodeWithSelector(Errors.Inbox__ActorTooLarge.selector, message.recipient.actor)
     );
-    inbox.insert(message.recipient, message.content, message.secretHash);
+    inbox.sendL2Message(message.recipient, 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.insert(message.recipient, message.content, message.secretHash);
+    inbox.sendL2Message(message.recipient, message.content, message.secretHash);
   }
 
   function testRevertIfSecretHashTooLarge() public {
@@ -120,61 +121,68 @@ contract NewInboxTest is Test {
     vm.expectRevert(
       abi.encodeWithSelector(Errors.Inbox__SecretHashTooLarge.selector, message.secretHash)
     );
-    inbox.insert(message.recipient, message.content, message.secretHash);
+    inbox.sendL2Message(message.recipient, message.content, message.secretHash);
   }
 
   function testFuzzConsume(DataStructures.L1ToL2Msg[] memory _messages, uint256 _numTreesToConsume)
     public
   {
-    // insert messages:
-    for (uint256 i = 0; i < _messages.length; i++) {
-      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);
+    uint256 numTrees;
+
+    // Send the messages
+    {
+      for (uint256 i = 0; i < _messages.length; i++) {
+        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);
+        // update version
+        message.recipient.version = version;
+
+        // TODO: nuke the following 2 values from the struct once the new message model is in place
+        message.deadline = type(uint32).max;
+        message.fee = 0;
+
+        inbox.sendL2Message(message.recipient, message.content, message.secretHash);
       }
-      // 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);
-      // update version
-      message.recipient.version = version;
-
-      // TODO: nuke the following 2 values from the struct once the new message model is in place
-      message.deadline = type(uint32).max;
-      message.fee = 0;
-
-      inbox.insert(message.recipient, message.content, message.secretHash);
-    }
 
-    uint256 expectedNumTrees = _divideAndRoundUp(_messages.length, inbox.SIZE());
-    if (expectedNumTrees == 0) {
-      // This occurs when there are no messages but we initialize the first tree in the constructor so there are never
-      // zero trees
-      expectedNumTrees = 1;
+      uint256 expectedNumTrees = _divideAndRoundUp(_messages.length, inbox.SIZE());
+      if (expectedNumTrees == 0) {
+        // This occurs when there are no messages but we initialize the first tree in the constructor so there are never
+        // zero trees
+        expectedNumTrees = 1;
+      }
+      numTrees = _getNumTrees();
+      assertEq(numTrees, expectedNumTrees);
     }
-    uint256 numTrees = _getNumTrees();
-    assertEq(numTrees, expectedNumTrees);
-
-    // We use (numTrees * 2) as upper bound here because we want to test the case where we go beyond the currently
-    // initalized number of trees. When consuming the newly initialized trees we should get zero roots.
-    uint256 numTreesToConsume = bound(_numTreesToConsume, 1, numTrees * 2);
-
-    // Now we consume the trees
-    for (uint256 i = 0; i < numTreesToConsume; i++) {
-      bytes32 root = inbox.consume();
-
-      // Notes:
-      // 1) For i = 0 we should always get empty tree root because first block's messages tree is always
-      // empty because we introduced a 1 block lag to prevent sequencer DOS attacks.
-      // 2) For _messages.length = 0 and i = 1 we get empty root as well
-      if (i != 0 && i <= numTrees && _messages.length > 0) {
-        assertNotEq(root, emptyTreeRoot, "Root should not be zero");
-      } else {
-        assertEq(root, emptyTreeRoot, "Root should be zero");
+
+    // Consume the messages
+    {
+      // We use (numTrees * 2) as upper bound here because we want to test the case where we go beyond the currently
+      // initalized number of trees. When consuming the newly initialized trees we should get zero roots.
+      uint256 numTreesToConsume = bound(_numTreesToConsume, 1, numTrees * 2);
+
+      // Now we consume the trees
+      for (uint256 i = 0; i < numTreesToConsume; i++) {
+        bytes32 root = inbox.consume();
+
+        // Notes:
+        // 1) For i = 0 we should always get empty tree root because first block's messages tree is always
+        // empty because we introduced a 1 block lag to prevent sequencer DOS attacks.
+        // 2) For _messages.length = 0 and i = 1 we get empty root as well
+        if (i != 0 && i <= numTrees && _messages.length > 0) {
+          assertNotEq(root, emptyTreeRoot, "Root should not be zero");
+        } else {
+          assertEq(root, emptyTreeRoot, "Root should be zero");
+        }
       }
     }
   }

From 334b74f81901941c35320773e806a6fe5db03a10 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 07:39:28 +0000
Subject: [PATCH 22/48] slither

---
 l1-contracts/slither_output.md | 66 +++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/l1-contracts/slither_output.md b/l1-contracts/slither_output.md
index f34fed2da29..eb69989e98f 100644
--- a/l1-contracts/slither_output.md
+++ b/l1-contracts/slither_output.md
@@ -132,7 +132,7 @@ src/core/messagebridge/Inbox.sol#L122-L143
 Impact: Low
 Confidence: Medium
  - [ ] ID-12
-[NewInbox.constructor(address,uint256,bytes32)._rollup](src/core/messagebridge/NewInbox.sol#L40) lacks a zero-check on :
+[NewInbox.constructor(address,uint256)._rollup](src/core/messagebridge/NewInbox.sol#L40) lacks a zero-check on :
 		- [ROLLUP = _rollup](src/core/messagebridge/NewInbox.sol#L41)
 
 src/core/messagebridge/NewInbox.sol#L40
@@ -142,24 +142,24 @@ src/core/messagebridge/NewInbox.sol#L40
 Impact: Low
 Confidence: Medium
  - [ ] ID-13
-Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L53-L91):
+Reentrancy in [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L61-L98):
 	External calls:
-	- [inbox.batchConsume(l1ToL2Msgs,msg.sender)](src/core/Rollup.sol#L85)
-	- [outbox.sendL1Messages(l2ToL1Msgs)](src/core/Rollup.sol#L88)
+	- [nextIndex = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L94)
 	Event emitted after the call(s):
-	- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L90)
+	- [LeafInserted(inProgress,nextIndex - 1,leaf)](src/core/messagebridge/NewInbox.sol#L95)
 
-src/core/Rollup.sol#L53-L91
+src/core/messagebridge/NewInbox.sol#L61-L98
 
 
  - [ ] ID-14
-Reentrancy in [NewInbox.insert(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L59-L96):
+Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L53-L91):
 	External calls:
-	- [nextIndex = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L91)
+	- [inbox.batchConsume(l1ToL2Msgs,msg.sender)](src/core/Rollup.sol#L85)
+	- [outbox.sendL1Messages(l2ToL1Msgs)](src/core/Rollup.sol#L88)
 	Event emitted after the call(s):
-	- [LeafInserted(inProgress,nextIndex,leaf)](src/core/messagebridge/NewInbox.sol#L92)
+	- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L90)
 
-src/core/messagebridge/NewInbox.sol#L59-L96
+src/core/Rollup.sol#L53-L91
 
 
 ## timestamp
@@ -201,34 +201,27 @@ src/core/messagebridge/Inbox.sol#L102-L113
 Impact: Low
 Confidence: Medium
  - [ ] ID-19
-The following public functions could be turned into external in [NewInbox](src/core/messagebridge/NewInbox.sol#L24-L120) contract:
-	[NewInbox.constructor(address,uint256,bytes32)](src/core/messagebridge/NewInbox.sol#L40-L49)
-
-src/core/messagebridge/NewInbox.sol#L24-L120
-
-
- - [ ] ID-20
 The following public functions could be turned into external in [FrontierMerkle](src/core/messagebridge/frontier_tree/Frontier.sol#L7-L91) contract:
 	[FrontierMerkle.constructor(uint256)](src/core/messagebridge/frontier_tree/Frontier.sol#L19-L27)
 
 src/core/messagebridge/frontier_tree/Frontier.sol#L7-L91
 
 
- - [ ] ID-21
+ - [ ] ID-20
 The following public functions could be turned into external in [Registry](src/core/messagebridge/Registry.sol#L22-L129) contract:
 	[Registry.constructor()](src/core/messagebridge/Registry.sol#L29-L33)
 
 src/core/messagebridge/Registry.sol#L22-L129
 
 
- - [ ] ID-22
+ - [ ] ID-21
 The following public functions could be turned into external in [Rollup](src/core/Rollup.sol#L27-L100) contract:
 	[Rollup.constructor(IRegistry,IAvailabilityOracle)](src/core/Rollup.sol#L39-L44)
 
 src/core/Rollup.sol#L27-L100
 
 
- - [ ] ID-23
+ - [ ] ID-22
 The following public functions could be turned into external in [Outbox](src/core/messagebridge/Outbox.sol#L21-L148) contract:
 	[Outbox.constructor(address)](src/core/messagebridge/Outbox.sol#L29-L31)
 	[Outbox.get(bytes32)](src/core/messagebridge/Outbox.sol#L77-L84)
@@ -237,7 +230,7 @@ The following public functions could be turned into external in [Outbox](src/cor
 src/core/messagebridge/Outbox.sol#L21-L148
 
 
- - [ ] ID-24
+ - [ ] ID-23
 The following public functions could be turned into external in [Inbox](src/core/messagebridge/Inbox.sol#L21-L231) contract:
 	[Inbox.constructor(address)](src/core/messagebridge/Inbox.sol#L30-L32)
 	[Inbox.contains(bytes32)](src/core/messagebridge/Inbox.sol#L174-L176)
@@ -245,6 +238,13 @@ The following public functions could be turned into external in [Inbox](src/core
 src/core/messagebridge/Inbox.sol#L21-L231
 
 
+ - [ ] ID-24
+The following public functions could be turned into external in [NewInbox](src/core/messagebridge/NewInbox.sol#L24-L127) contract:
+	[NewInbox.constructor(address,uint256)](src/core/messagebridge/NewInbox.sol#L40-L51)
+
+src/core/messagebridge/NewInbox.sol#L24-L127
+
+
 ## assembly
 Impact: Informational
 Confidence: High
@@ -346,38 +346,38 @@ src/core/Rollup.sol#L37
 Impact: Optimization
 Confidence: High
  - [ ] ID-38
-In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74) variable [FrontierMerkle.HEIGHT](src/core/messagebridge/frontier_tree/Frontier.sol#L8) is read multiple times
+In a function [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L61-L98) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
+src/core/messagebridge/NewInbox.sol#L61-L98
 
 
  - [ ] ID-39
-In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L103-L119) variable [NewInbox.toInclude](src/core/messagebridge/NewInbox.sol#L33) is read multiple times
+In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74) variable [FrontierMerkle.HEIGHT](src/core/messagebridge/frontier_tree/Frontier.sol#L8) is read multiple times
 
-src/core/messagebridge/NewInbox.sol#L103-L119
+src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
 
 
  - [ ] ID-40
-In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L103-L119) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L107-L126) variable [NewInbox.toInclude](src/core/messagebridge/NewInbox.sol#L33) is read multiple times
 
-src/core/messagebridge/NewInbox.sol#L103-L119
+src/core/messagebridge/NewInbox.sol#L107-L126
 
 
  - [ ] ID-41
-In a function [FrontierMerkle.insertLeaf(bytes32)](src/core/messagebridge/frontier_tree/Frontier.sol#L29-L39) variable [FrontierMerkle.nextIndex](src/core/messagebridge/frontier_tree/Frontier.sol#L11) is read multiple times
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L107-L126) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L29-L39
+src/core/messagebridge/NewInbox.sol#L107-L126
 
 
  - [ ] ID-42
-In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74) variable [FrontierMerkle.frontier](src/core/messagebridge/frontier_tree/Frontier.sol#L13) is read multiple times
+In a function [FrontierMerkle.insertLeaf(bytes32)](src/core/messagebridge/frontier_tree/Frontier.sol#L29-L39) variable [FrontierMerkle.nextIndex](src/core/messagebridge/frontier_tree/Frontier.sol#L11) is read multiple times
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
+src/core/messagebridge/frontier_tree/Frontier.sol#L29-L39
 
 
  - [ ] ID-43
-In a function [NewInbox.insert(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L59-L96) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
+In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74) variable [FrontierMerkle.frontier](src/core/messagebridge/frontier_tree/Frontier.sol#L13) is read multiple times
 
-src/core/messagebridge/NewInbox.sol#L59-L96
+src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
 
 

From 9a6308d2bc35683b5ac1a662233a2a0cea4fd782 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 07:47:07 +0000
Subject: [PATCH 23/48] _boundMessage

---
 l1-contracts/test/NewInbox.t.sol | 51 ++++++++++++++------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 8b5f9779035..d32a3414321 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -54,14 +54,12 @@ contract NewInboxTest is Test {
     return (a + b - 1) / b;
   }
 
-  function testRevertIfNotConsumingFromRollup() public {
-    vm.prank(address(0x1));
-    vm.expectRevert(Errors.Inbox__Unauthorized.selector);
-    inbox.consume();
-  }
-
-  function testFuzzInsert(DataStructures.L1ToL2Msg memory _message) public {
-    // fix message.sender and deadline:
+  function _boundMessage(DataStructures.L1ToL2Msg memory _message)
+    internal
+    view
+    returns (DataStructures.L1ToL2Msg memory)
+  {
+    // 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);
@@ -69,18 +67,32 @@ contract NewInboxTest is Test {
     _message.content = bytes32(uint256(_message.content) % Constants.P);
     // ensure secret hash fits in a field
     _message.secretHash = bytes32(uint256(_message.secretHash) % Constants.P);
+    // update version
+    _message.recipient.version = version;
 
     // TODO: nuke the following 2 values from the struct once the new message model is in place
     _message.deadline = type(uint32).max;
     _message.fee = 0;
 
-    bytes32 leaf = _message.sha256ToField();
+    return _message;
+  }
+
+  function testRevertIfNotConsumingFromRollup() public {
+    vm.prank(address(0x1));
+    vm.expectRevert(Errors.Inbox__Unauthorized.selector);
+    inbox.consume();
+  }
+
+  function testFuzzInsert(DataStructures.L1ToL2Msg memory _message) public {
+    DataStructures.L1ToL2Msg memory message = _boundMessage(_message);
+
+    bytes32 leaf = message.sha256ToField();
     vm.expectEmit(true, true, true, true);
     // event we expect
     emit LeafInserted(FIRST_REAL_TREE_NUM, 0, leaf);
     // event we will get
     bytes32 insertedLeaf =
-      inbox.sendL2Message(_message.recipient, _message.content, _message.secretHash);
+      inbox.sendL2Message(message.recipient, message.content, message.secretHash);
 
     assertEq(insertedLeaf, leaf);
   }
@@ -132,24 +144,7 @@ contract NewInboxTest is Test {
     // Send the messages
     {
       for (uint256 i = 0; i < _messages.length; i++) {
-        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);
-        // update version
-        message.recipient.version = version;
-
-        // TODO: nuke the following 2 values from the struct once the new message model is in place
-        message.deadline = type(uint32).max;
-        message.fee = 0;
+        DataStructures.L1ToL2Msg memory message = _boundMessage(_messages[i]);
 
         inbox.sendL2Message(message.recipient, message.content, message.secretHash);
       }

From b4f1f3e4abc153c1afc533904be0f2c03c6c1204 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 08:59:42 +0000
Subject: [PATCH 24/48] NewInboxHarness

---
 .../src/core/messagebridge/NewInbox.sol       | 22 +++++------
 l1-contracts/test/NewInbox.t.sol              | 35 ++++++++---------
 .../test/harnesses/NewInboxHarness.sol        | 39 +++++++++++++++++++
 3 files changed, 67 insertions(+), 29 deletions(-)
 create mode 100644 l1-contracts/test/harnesses/NewInboxHarness.sol

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index ee3686e9443..fe6b85c9515 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -26,14 +26,14 @@ contract NewInbox {
 
   address public immutable ROLLUP;
 
-  uint256 public immutable HEIGHT;
-  uint256 public immutable SIZE;
-  bytes32 private immutable EMPTY_ROOT; // The root of an empty frontier tree
+  uint256 internal immutable HEIGHT;
+  uint256 internal immutable SIZE;
+  bytes32 internal immutable EMPTY_ROOT; // The root of an empty frontier tree
 
-  uint256 private toInclude = 1;
-  uint256 private inProgress = 2;
+  uint256 internal toInclude = 1;
+  uint256 internal inProgress = 2;
 
-  mapping(uint256 blockNumber => IFrontier tree) public frontier;
+  mapping(uint256 blockNumber => IFrontier tree) internal trees;
 
   event LeafInserted(uint256 indexed blockNumber, uint256 index, bytes32 value);
 
@@ -45,7 +45,7 @@ contract NewInbox {
 
     // We deploy the first tree
     IFrontier firstTree = IFrontier(new FrontierMerkle(_height));
-    frontier[inProgress] = firstTree;
+    trees[inProgress] = firstTree;
 
     EMPTY_ROOT = firstTree.root();
   }
@@ -73,11 +73,11 @@ contract NewInbox {
       revert Errors.Inbox__SecretHashTooLarge(_secretHash);
     }
 
-    IFrontier currentTree = frontier[inProgress];
+    IFrontier currentTree = trees[inProgress];
     if (currentTree.isFull()) {
       inProgress += 1;
       currentTree = IFrontier(new FrontierMerkle(HEIGHT));
-      frontier[inProgress] = currentTree;
+      trees[inProgress] = currentTree;
     }
 
     DataStructures.L1ToL2Msg memory message = DataStructures.L1ToL2Msg({
@@ -111,13 +111,13 @@ contract NewInbox {
 
     bytes32 root = EMPTY_ROOT;
     if (toInclude > Constants.INITIAL_L2_BLOCK_NUM) {
-      root = frontier[toInclude].root();
+      root = trees[toInclude].root();
     }
 
     // If we are "catching up" we skip the tree creation as it is already there
     if (toInclude + 1 == inProgress) {
       inProgress += 1;
-      frontier[inProgress] = IFrontier(new FrontierMerkle(HEIGHT));
+      trees[inProgress] = IFrontier(new FrontierMerkle(HEIGHT));
     }
 
     toInclude += 1;
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index d32a3414321..ad1c3d2ba1e 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -4,7 +4,7 @@ pragma solidity >=0.8.18;
 
 import {Test} from "forge-std/Test.sol";
 
-import {NewInbox} from "../src/core/messagebridge/NewInbox.sol";
+import {NewInboxHarness} from "./harnesses/NewInboxHarness.sol";
 import {Constants} from "../src/core/libraries/ConstantsGen.sol";
 import {Errors} from "../src/core/libraries/Errors.sol";
 import {Hash} from "../src/core/libraries/Hash.sol";
@@ -15,7 +15,7 @@ contract NewInboxTest is Test {
 
   uint256 internal constant FIRST_REAL_TREE_NUM = Constants.INITIAL_L2_BLOCK_NUM + 1;
 
-  NewInbox internal inbox;
+  NewInboxHarness internal inbox;
   uint256 internal version = 0;
   bytes32 internal emptyTreeRoot;
 
@@ -24,8 +24,8 @@ contract NewInboxTest is Test {
   function setUp() public {
     address rollup = address(this);
     // We set low depth (5) to ensure we sufficiently test the tree transitions
-    inbox = new NewInbox(rollup, 5);
-    emptyTreeRoot = inbox.frontier(2).root();
+    inbox = new NewInboxHarness(rollup, 5);
+    emptyTreeRoot = inbox.getEmptyRoot();
   }
 
   function _fakeMessage() internal view returns (DataStructures.L1ToL2Msg memory) {
@@ -42,14 +42,6 @@ contract NewInboxTest is Test {
     });
   }
 
-  function _getNumTrees() internal view returns (uint256) {
-    uint256 blockNumber = FIRST_REAL_TREE_NUM;
-    while (address(inbox.frontier(blockNumber)) != address(0)) {
-      blockNumber++;
-    }
-    return blockNumber - 2; // -2 because first real tree is included in block 2
-  }
-
   function _divideAndRoundUp(uint256 a, uint256 b) internal pure returns (uint256) {
     return (a + b - 1) / b;
   }
@@ -104,7 +96,7 @@ contract NewInboxTest is Test {
     bytes32 leaf3 = inbox.sendL2Message(message.recipient, message.content, message.secretHash);
 
     // Only 1 tree should be non-zero
-    assertEq(_getNumTrees(), 1);
+    assertEq(inbox.getNumTrees(), 1);
 
     // All the leaves should be the same
     assertEq(leaf1, leaf2);
@@ -136,9 +128,10 @@ contract NewInboxTest is Test {
     inbox.sendL2Message(message.recipient, message.content, message.secretHash);
   }
 
-  function testFuzzConsume(DataStructures.L1ToL2Msg[] memory _messages, uint256 _numTreesToConsume)
-    public
-  {
+  function testFuzzSendAndConsume(
+    DataStructures.L1ToL2Msg[] memory _messages,
+    uint256 _numTreesToConsume
+  ) public {
     uint256 numTrees;
 
     // Send the messages
@@ -146,16 +139,22 @@ contract NewInboxTest is Test {
       for (uint256 i = 0; i < _messages.length; i++) {
         DataStructures.L1ToL2Msg memory message = _boundMessage(_messages[i]);
 
+        bytes32 toIncludeRoot = inbox.getToIncludeRoot();
         inbox.sendL2Message(message.recipient, message.content, message.secretHash);
+        assertEq(
+          inbox.getToIncludeRoot(),
+          toIncludeRoot,
+          "Root of a tree waiting to be included should not change"
+        );
       }
 
-      uint256 expectedNumTrees = _divideAndRoundUp(_messages.length, inbox.SIZE());
+      uint256 expectedNumTrees = _divideAndRoundUp(_messages.length, inbox.getSize());
       if (expectedNumTrees == 0) {
         // This occurs when there are no messages but we initialize the first tree in the constructor so there are never
         // zero trees
         expectedNumTrees = 1;
       }
-      numTrees = _getNumTrees();
+      numTrees = inbox.getNumTrees();
       assertEq(numTrees, expectedNumTrees);
     }
 
diff --git a/l1-contracts/test/harnesses/NewInboxHarness.sol b/l1-contracts/test/harnesses/NewInboxHarness.sol
new file mode 100644
index 00000000000..e85aab3390e
--- /dev/null
+++ b/l1-contracts/test/harnesses/NewInboxHarness.sol
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2023 Aztec Labs.
+pragma solidity >=0.8.18;
+
+import {NewInbox} from "../../src/core/messagebridge/NewInbox.sol";
+
+// Libraries
+import {Constants} from "../../src/core/libraries/ConstantsGen.sol";
+
+// TODO: rename to InboxHarness once all the pieces of the new message model are in place.
+contract NewInboxHarness is NewInbox {
+  uint256 public constant FIRST_REAL_TREE_NUM = Constants.INITIAL_L2_BLOCK_NUM + 1;
+
+  constructor(address _rollup, uint256 _height) NewInbox(_rollup, _height) {}
+
+  function getSize() external view returns (uint256) {
+    return SIZE;
+  }
+
+  function getEmptyRoot() external view returns (bytes32) {
+    return EMPTY_ROOT;
+  }
+
+  function getToIncludeRoot() external view returns (bytes32) {
+    bytes32 root = EMPTY_ROOT;
+    if (toInclude > Constants.INITIAL_L2_BLOCK_NUM) {
+      root = trees[toInclude].root();
+    }
+    return root;
+  }
+
+  function getNumTrees() external view returns (uint256) {
+    uint256 blockNumber = FIRST_REAL_TREE_NUM;
+    while (address(trees[blockNumber]) != address(0)) {
+      blockNumber++;
+    }
+    return blockNumber - 2; // -2 because first real tree is included in block 2
+  }
+}

From 8864c2b7920ea59b59be0912270dc2153a5967a3 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 09:00:59 +0000
Subject: [PATCH 25/48] slither

---
 l1-contracts/slither_output.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/l1-contracts/slither_output.md b/l1-contracts/slither_output.md
index eb69989e98f..771ad9681ed 100644
--- a/l1-contracts/slither_output.md
+++ b/l1-contracts/slither_output.md
@@ -315,13 +315,13 @@ src/core/messagebridge/Inbox.sol#L148-L153
 ## similar-names
 Impact: Informational
 Confidence: Medium
- - [ ] ID-31
+ - [ ] ID-34
 Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L129) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L122)
 
 src/core/libraries/ConstantsGen.sol#L129
 
 
- - [ ] ID-32
+ - [ ] ID-35
 Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L109) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L110)
 
 src/core/libraries/ConstantsGen.sol#L109

From 6ae47b254287a22f0c2d39dbd794bddbb173be88 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 10:20:27 +0000
Subject: [PATCH 26/48] WIP

---
 l1-contracts/test/NewInbox.t.sol              | 88 ++++++++++++-------
 .../test/harnesses/NewInboxHarness.sol        |  8 ++
 2 files changed, 65 insertions(+), 31 deletions(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index ad1c3d2ba1e..a9a290cac85 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -129,50 +129,76 @@ contract NewInboxTest is Test {
   }
 
   function testFuzzSendAndConsume(
-    DataStructures.L1ToL2Msg[] memory _messages,
-    uint256 _numTreesToConsume
+    DataStructures.L1ToL2Msg[] memory _messagesFirstBatch,
+    DataStructures.L1ToL2Msg[] memory _messagesSecondBatch,
+    uint256 _numTreesToConsumeFirstBatch,
+    uint256 _numTreesToConsumeSecondBatch
   ) public {
-    uint256 numTrees;
-
-    // Send the messages
-    {
-      for (uint256 i = 0; i < _messages.length; i++) {
-        DataStructures.L1ToL2Msg memory message = _boundMessage(_messages[i]);
-
-        bytes32 toIncludeRoot = inbox.getToIncludeRoot();
-        inbox.sendL2Message(message.recipient, message.content, message.secretHash);
-        assertEq(
-          inbox.getToIncludeRoot(),
-          toIncludeRoot,
-          "Root of a tree waiting to be included should not change"
-        );
-      }
+    // Send first batch of messages
+    _send(_messagesFirstBatch);
+
+    // Consume first few trees
+    _consume(_numTreesToConsumeFirstBatch, _messagesFirstBatch.length);
+
+    // Send second batch of messages
+    _send(_messagesSecondBatch);
+
+    // Consume second batch of trees
+    _consume(_numTreesToConsumeSecondBatch, _messagesSecondBatch.length);
+  }
+
+  function _send(DataStructures.L1ToL2Msg[] memory _messages) internal {
+    bool isFirstRun = inbox.getInProgress() == inbox.FIRST_REAL_TREE_NUM();
+    bytes32 toIncludeRoot = inbox.getToIncludeRoot();
+
+    for (uint256 i = 0; i < _messages.length; i++) {
+      DataStructures.L1ToL2Msg memory message = _boundMessage(_messages[i]);
 
+      // We send the message and check that toInclude root did not change.
+      inbox.sendL2Message(message.recipient, message.content, message.secretHash);
+    }
+
+    // Root of a tree waiting to be included should not change because we introduced a 1 block lag to prevent sequencer
+    // DOS attacks
+    assertEq(
+      inbox.getToIncludeRoot(),
+      toIncludeRoot,
+      "Root of a tree waiting to be included should not change"
+    );
+
+    // We perform expected num trees check only after first batch because after second one the following accounting
+    // does not work because a tree might have been consumed when not full or we might have consumed an empty tree
+    if (isFirstRun) {
       uint256 expectedNumTrees = _divideAndRoundUp(_messages.length, inbox.getSize());
       if (expectedNumTrees == 0) {
-        // This occurs when there are no messages but we initialize the first tree in the constructor so there are never
-        // zero trees
+        // This occurs when there are no messages but we initialize the first tree in the constructor so there are
+        // never zero trees
         expectedNumTrees = 1;
       }
-      numTrees = inbox.getNumTrees();
-      assertEq(numTrees, expectedNumTrees);
+      assertEq(inbox.getNumTrees(), expectedNumTrees);
     }
+  }
+
+  function _consume(uint256 _numTreesToConsume, uint256 _numMessages) internal {
+    bool isFirstRun = inbox.getToInclude() == Constants.INITIAL_L2_BLOCK_NUM;
+    uint256 numTrees = inbox.getNumTrees();
 
-    // Consume the messages
-    {
-      // We use (numTrees * 2) as upper bound here because we want to test the case where we go beyond the currently
-      // initalized number of trees. When consuming the newly initialized trees we should get zero roots.
-      uint256 numTreesToConsume = bound(_numTreesToConsume, 1, numTrees * 2);
+    // We use (numTrees * 2) as upper bound here because we want to test the case where we go beyond the currently
+    // initalized number of trees. When consuming the newly initialized trees we should get zero roots.
+    uint256 numTreesToConsume = bound(_numTreesToConsume, 1, numTrees * 2);
 
-      // Now we consume the trees
-      for (uint256 i = 0; i < numTreesToConsume; i++) {
-        bytes32 root = inbox.consume();
+    // Now we consume the trees
+    for (uint256 i = 0; i < numTreesToConsume; i++) {
+      bytes32 root = inbox.consume();
 
+      // We perform empty roots check only after first batch because after second one the following simple accounting
+      // does not work
+      if (isFirstRun) {
         // Notes:
         // 1) For i = 0 we should always get empty tree root because first block's messages tree is always
         // empty because we introduced a 1 block lag to prevent sequencer DOS attacks.
-        // 2) For _messages.length = 0 and i = 1 we get empty root as well
-        if (i != 0 && i <= numTrees && _messages.length > 0) {
+        // 2) For _numMessages = 0 and i = 1 we get empty root as well
+        if (i != 0 && i <= numTrees && _numMessages > 0) {
           assertNotEq(root, emptyTreeRoot, "Root should not be zero");
         } else {
           assertEq(root, emptyTreeRoot, "Root should be zero");
diff --git a/l1-contracts/test/harnesses/NewInboxHarness.sol b/l1-contracts/test/harnesses/NewInboxHarness.sol
index e85aab3390e..22168b417f1 100644
--- a/l1-contracts/test/harnesses/NewInboxHarness.sol
+++ b/l1-contracts/test/harnesses/NewInboxHarness.sol
@@ -21,6 +21,14 @@ contract NewInboxHarness is NewInbox {
     return EMPTY_ROOT;
   }
 
+  function getToInclude() external view returns (uint256) {
+    return toInclude;
+  }
+
+  function getInProgress() external view returns (uint256) {
+    return inProgress;
+  }
+
   function getToIncludeRoot() external view returns (bytes32) {
     bytes32 root = EMPTY_ROOT;
     if (toInclude > Constants.INITIAL_L2_BLOCK_NUM) {

From 88acc557cc07ba3dda8baf9b1f7c604a4eee51d9 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 10:25:26 +0000
Subject: [PATCH 27/48] more consistent naming

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 10 +++++-----
 l1-contracts/test/NewInbox.t.sol                 | 14 +++++++-------
 l1-contracts/test/harnesses/NewInboxHarness.sol  | 10 +++++-----
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index fe6b85c9515..f0f9d1db897 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -30,7 +30,7 @@ contract NewInbox {
   uint256 internal immutable SIZE;
   bytes32 internal immutable EMPTY_ROOT; // The root of an empty frontier tree
 
-  uint256 internal toInclude = 1;
+  uint256 internal toConsume = 1;
   uint256 internal inProgress = 2;
 
   mapping(uint256 blockNumber => IFrontier tree) internal trees;
@@ -110,17 +110,17 @@ contract NewInbox {
     }
 
     bytes32 root = EMPTY_ROOT;
-    if (toInclude > Constants.INITIAL_L2_BLOCK_NUM) {
-      root = trees[toInclude].root();
+    if (toConsume > Constants.INITIAL_L2_BLOCK_NUM) {
+      root = trees[toConsume].root();
     }
 
     // If we are "catching up" we skip the tree creation as it is already there
-    if (toInclude + 1 == inProgress) {
+    if (toConsume + 1 == inProgress) {
       inProgress += 1;
       trees[inProgress] = IFrontier(new FrontierMerkle(HEIGHT));
     }
 
-    toInclude += 1;
+    toConsume += 1;
 
     return root;
   }
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index a9a290cac85..22adfe982f8 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -149,21 +149,21 @@ contract NewInboxTest is Test {
 
   function _send(DataStructures.L1ToL2Msg[] memory _messages) internal {
     bool isFirstRun = inbox.getInProgress() == inbox.FIRST_REAL_TREE_NUM();
-    bytes32 toIncludeRoot = inbox.getToIncludeRoot();
+    bytes32 toConsumeRoot = inbox.getToConsumeRoot();
 
     for (uint256 i = 0; i < _messages.length; i++) {
       DataStructures.L1ToL2Msg memory message = _boundMessage(_messages[i]);
 
-      // We send the message and check that toInclude root did not change.
+      // We send the message and check that toConsume root did not change.
       inbox.sendL2Message(message.recipient, message.content, message.secretHash);
     }
 
-    // Root of a tree waiting to be included should not change because we introduced a 1 block lag to prevent sequencer
+    // Root of a tree waiting to be consumed should not change because we introduced a 1 block lag to prevent sequencer
     // DOS attacks
     assertEq(
-      inbox.getToIncludeRoot(),
-      toIncludeRoot,
-      "Root of a tree waiting to be included should not change"
+      inbox.getToConsumeRoot(),
+      toConsumeRoot,
+      "Root of a tree waiting to be consumed should not change"
     );
 
     // We perform expected num trees check only after first batch because after second one the following accounting
@@ -180,7 +180,7 @@ contract NewInboxTest is Test {
   }
 
   function _consume(uint256 _numTreesToConsume, uint256 _numMessages) internal {
-    bool isFirstRun = inbox.getToInclude() == Constants.INITIAL_L2_BLOCK_NUM;
+    bool isFirstRun = inbox.getToConsume() == Constants.INITIAL_L2_BLOCK_NUM;
     uint256 numTrees = inbox.getNumTrees();
 
     // We use (numTrees * 2) as upper bound here because we want to test the case where we go beyond the currently
diff --git a/l1-contracts/test/harnesses/NewInboxHarness.sol b/l1-contracts/test/harnesses/NewInboxHarness.sol
index 22168b417f1..3640ebc1b40 100644
--- a/l1-contracts/test/harnesses/NewInboxHarness.sol
+++ b/l1-contracts/test/harnesses/NewInboxHarness.sol
@@ -21,18 +21,18 @@ contract NewInboxHarness is NewInbox {
     return EMPTY_ROOT;
   }
 
-  function getToInclude() external view returns (uint256) {
-    return toInclude;
+  function getToConsume() external view returns (uint256) {
+    return toConsume;
   }
 
   function getInProgress() external view returns (uint256) {
     return inProgress;
   }
 
-  function getToIncludeRoot() external view returns (bytes32) {
+  function getToConsumeRoot() external view returns (bytes32) {
     bytes32 root = EMPTY_ROOT;
-    if (toInclude > Constants.INITIAL_L2_BLOCK_NUM) {
-      root = trees[toInclude].root();
+    if (toConsume > Constants.INITIAL_L2_BLOCK_NUM) {
+      root = trees[toConsume].root();
     }
     return root;
   }

From abc32791dd427753c53f5c50bb338cd712ada4c0 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 10:34:56 +0000
Subject: [PATCH 28/48] checking invariant

---
 l1-contracts/test/NewInbox.t.sol                | 10 ++++++++++
 l1-contracts/test/harnesses/NewInboxHarness.sol |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 22adfe982f8..94b09e80c31 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -69,6 +69,12 @@ contract NewInboxTest is Test {
     return _message;
   }
 
+  // Since there is a 1 block lag between tree to be consumed and tree in progress the following invariant should never
+  // be violated
+  function _checkInvariant() internal {
+    assertLt(inbox.getToConsume(), inbox.getInProgress());
+  }
+
   function testRevertIfNotConsumingFromRollup() public {
     vm.prank(address(0x1));
     vm.expectRevert(Errors.Inbox__Unauthorized.selector);
@@ -156,6 +162,8 @@ contract NewInboxTest is Test {
 
       // We send the message and check that toConsume root did not change.
       inbox.sendL2Message(message.recipient, message.content, message.secretHash);
+      // We check that the "toConsume < inProgress" invariant is maintained
+      _checkInvariant();
     }
 
     // Root of a tree waiting to be consumed should not change because we introduced a 1 block lag to prevent sequencer
@@ -190,6 +198,8 @@ contract NewInboxTest is Test {
     // Now we consume the trees
     for (uint256 i = 0; i < numTreesToConsume; i++) {
       bytes32 root = inbox.consume();
+      // We check that the "toConsume < inProgress" invariant is maintained
+      _checkInvariant();
 
       // We perform empty roots check only after first batch because after second one the following simple accounting
       // does not work
diff --git a/l1-contracts/test/harnesses/NewInboxHarness.sol b/l1-contracts/test/harnesses/NewInboxHarness.sol
index 3640ebc1b40..3bd1f0d689b 100644
--- a/l1-contracts/test/harnesses/NewInboxHarness.sol
+++ b/l1-contracts/test/harnesses/NewInboxHarness.sol
@@ -42,6 +42,6 @@ contract NewInboxHarness is NewInbox {
     while (address(trees[blockNumber]) != address(0)) {
       blockNumber++;
     }
-    return blockNumber - 2; // -2 because first real tree is included in block 2
+    return blockNumber - 2; // - 2 because first real tree is included in block 2
   }
 }

From 474692c1150cf90a47f5a9ce2018f3d4c5fb1784 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 10:35:32 +0000
Subject: [PATCH 29/48] slither

---
 l1-contracts/slither_output.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/l1-contracts/slither_output.md b/l1-contracts/slither_output.md
index 771ad9681ed..c448cc2d09e 100644
--- a/l1-contracts/slither_output.md
+++ b/l1-contracts/slither_output.md
@@ -358,13 +358,13 @@ src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
 
 
  - [ ] ID-40
-In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L107-L126) variable [NewInbox.toInclude](src/core/messagebridge/NewInbox.sol#L33) is read multiple times
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L107-L126) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
 
 src/core/messagebridge/NewInbox.sol#L107-L126
 
 
  - [ ] ID-41
-In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L107-L126) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L107-L126) variable [NewInbox.toConsume](src/core/messagebridge/NewInbox.sol#L33) is read multiple times
 
 src/core/messagebridge/NewInbox.sol#L107-L126
 

From 6875f7e88156b37b4bcda601da82ddf482890a26 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 10:45:34 +0000
Subject: [PATCH 30/48] linter fix

---
 .../interfaces/messagebridge/INewInbox.sol    | 42 +++++++++++++++++++
 .../src/core/messagebridge/NewInbox.sol       |  9 ++--
 2 files changed, 46 insertions(+), 5 deletions(-)
 create mode 100644 l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol

diff --git a/l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol b/l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol
new file mode 100644
index 00000000000..35b90ed6c3a
--- /dev/null
+++ b/l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2023 Aztec Labs.
+pragma solidity >=0.8.18;
+
+import {DataStructures} from "../../libraries/DataStructures.sol";
+
+/**
+ * @title Inbox
+ * @author Aztec Labs
+ * @notice Lives on L1 and is used to pass messages into the rollup, e.g., L1 -> L2 messages.
+ */
+// TODO: rename to IInbox once all the pieces of the new message model are in place.
+interface INewInbox {
+  event LeafInserted(uint256 indexed blockNumber, uint256 index, bytes32 value);
+
+  // docs:start:send_l1_to_l2_message
+  /**
+   * @notice Inserts a new message into the Inbox
+   * @dev Emits `LeafInserted` with data for easy access by the sequencer
+   * @param _recipient - The recipient of the message
+   * @param _content - The content of the message (application specific)
+   * @param _secretHash - The secret hash of the message (make it possible to hide when a specific message is consumed on L2)
+   * @return The key of the message in the set
+   */
+  function sendL2Message(
+    DataStructures.L2Actor memory _recipient,
+    bytes32 _content,
+    bytes32 _secretHash
+  ) external returns (bytes32);
+  // docs:end:send_l1_to_l2_message
+
+  // docs:start:inbox_batch_consume
+  /**
+   * @notice Consumes the current tree, and starts a new one if needed
+   * @dev Only callable by the rollup contract
+   * @dev In the first iteration we return empty tree root because first block's messages tree is always
+   * empty because there has to be a 1 block lag to prevent sequencer DOS attacks
+   * @return The root of the consumed tree
+   */
+  function consume() external returns (bytes32);
+  // docs:end:inbox_batch_consume
+}
diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index f0f9d1db897..e43343b108e 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -5,6 +5,7 @@ pragma solidity >=0.8.18;
 // Interfaces
 import {IFrontier} from "../interfaces/messagebridge/IFrontier.sol";
 import {IRegistry} from "../interfaces/messagebridge/IRegistry.sol";
+import {INewInbox} from "../interfaces/messagebridge/INewInbox.sol";
 
 // Libraries
 import {Constants} from "../libraries/ConstantsGen.sol";
@@ -21,7 +22,7 @@ import {FrontierMerkle} from "./frontier_tree/Frontier.sol";
  * @notice Lives on L1 and is used to pass messages into the rollup, e.g., L1 -> L2 messages.
  */
 // TODO: rename to Inbox once all the pieces of the new message model are in place.
-contract NewInbox {
+contract NewInbox is INewInbox {
   using Hash for DataStructures.L1ToL2Msg;
 
   address public immutable ROLLUP;
@@ -35,8 +36,6 @@ contract NewInbox {
 
   mapping(uint256 blockNumber => IFrontier tree) internal trees;
 
-  event LeafInserted(uint256 indexed blockNumber, uint256 index, bytes32 value);
-
   constructor(address _rollup, uint256 _height) {
     ROLLUP = _rollup;
 
@@ -62,7 +61,7 @@ contract NewInbox {
     DataStructures.L2Actor memory _recipient,
     bytes32 _content,
     bytes32 _secretHash
-  ) external returns (bytes32) {
+  ) external override(INewInbox) returns (bytes32) {
     if (uint256(_recipient.actor) > Constants.MAX_FIELD_VALUE) {
       revert Errors.Inbox__ActorTooLarge(_recipient.actor);
     }
@@ -104,7 +103,7 @@ contract NewInbox {
    * empty because there has to be a 1 block lag to prevent sequencer DOS attacks
    * @return The root of the consumed tree
    */
-  function consume() external returns (bytes32) {
+  function consume() external override(INewInbox) returns (bytes32) {
     if (msg.sender != ROLLUP) {
       revert Errors.Inbox__Unauthorized();
     }

From 093753743c3a2734780d8eba1864094581a872a6 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 10:52:27 +0000
Subject: [PATCH 31/48] slither

---
 l1-contracts/slither_output.md | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/l1-contracts/slither_output.md b/l1-contracts/slither_output.md
index c448cc2d09e..1368756b90c 100644
--- a/l1-contracts/slither_output.md
+++ b/l1-contracts/slither_output.md
@@ -132,23 +132,23 @@ src/core/messagebridge/Inbox.sol#L122-L143
 Impact: Low
 Confidence: Medium
  - [ ] ID-12
-[NewInbox.constructor(address,uint256)._rollup](src/core/messagebridge/NewInbox.sol#L40) lacks a zero-check on :
-		- [ROLLUP = _rollup](src/core/messagebridge/NewInbox.sol#L41)
+[NewInbox.constructor(address,uint256)._rollup](src/core/messagebridge/NewInbox.sol#L39) lacks a zero-check on :
+		- [ROLLUP = _rollup](src/core/messagebridge/NewInbox.sol#L40)
 
-src/core/messagebridge/NewInbox.sol#L40
+src/core/messagebridge/NewInbox.sol#L39
 
 
 ## reentrancy-events
 Impact: Low
 Confidence: Medium
  - [ ] ID-13
-Reentrancy in [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L61-L98):
+Reentrancy in [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L60-L97):
 	External calls:
-	- [nextIndex = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L94)
+	- [nextIndex = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L93)
 	Event emitted after the call(s):
-	- [LeafInserted(inProgress,nextIndex - 1,leaf)](src/core/messagebridge/NewInbox.sol#L95)
+	- [LeafInserted(inProgress,nextIndex - 1,leaf)](src/core/messagebridge/NewInbox.sol#L94)
 
-src/core/messagebridge/NewInbox.sol#L61-L98
+src/core/messagebridge/NewInbox.sol#L60-L97
 
 
  - [ ] ID-14
@@ -239,10 +239,10 @@ src/core/messagebridge/Inbox.sol#L21-L231
 
 
  - [ ] ID-24
-The following public functions could be turned into external in [NewInbox](src/core/messagebridge/NewInbox.sol#L24-L127) contract:
-	[NewInbox.constructor(address,uint256)](src/core/messagebridge/NewInbox.sol#L40-L51)
+The following public functions could be turned into external in [NewInbox](src/core/messagebridge/NewInbox.sol#L25-L126) contract:
+	[NewInbox.constructor(address,uint256)](src/core/messagebridge/NewInbox.sol#L39-L50)
 
-src/core/messagebridge/NewInbox.sol#L24-L127
+src/core/messagebridge/NewInbox.sol#L25-L126
 
 
 ## assembly
@@ -346,9 +346,9 @@ src/core/Rollup.sol#L37
 Impact: Optimization
 Confidence: High
  - [ ] ID-38
-In a function [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L61-L98) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
+In a function [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L60-L97) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L35) is read multiple times
 
-src/core/messagebridge/NewInbox.sol#L61-L98
+src/core/messagebridge/NewInbox.sol#L60-L97
 
 
  - [ ] ID-39
@@ -358,15 +358,15 @@ src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
 
 
  - [ ] ID-40
-In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L107-L126) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L106-L125) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L35) is read multiple times
 
-src/core/messagebridge/NewInbox.sol#L107-L126
+src/core/messagebridge/NewInbox.sol#L106-L125
 
 
  - [ ] ID-41
-In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L107-L126) variable [NewInbox.toConsume](src/core/messagebridge/NewInbox.sol#L33) is read multiple times
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L106-L125) variable [NewInbox.toConsume](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
 
-src/core/messagebridge/NewInbox.sol#L107-L126
+src/core/messagebridge/NewInbox.sol#L106-L125
 
 
  - [ ] ID-42

From 3a556ef6e75b9d2a5c36ce3b6fbc479a8592c80d Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 10:54:51 +0000
Subject: [PATCH 32/48] cleanup

---
 l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol b/l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol
index 35b90ed6c3a..851d76a85c6 100644
--- a/l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol
+++ b/l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol
@@ -13,7 +13,6 @@ import {DataStructures} from "../../libraries/DataStructures.sol";
 interface INewInbox {
   event LeafInserted(uint256 indexed blockNumber, uint256 index, bytes32 value);
 
-  // docs:start:send_l1_to_l2_message
   /**
    * @notice Inserts a new message into the Inbox
    * @dev Emits `LeafInserted` with data for easy access by the sequencer
@@ -27,9 +26,7 @@ interface INewInbox {
     bytes32 _content,
     bytes32 _secretHash
   ) external returns (bytes32);
-  // docs:end:send_l1_to_l2_message
 
-  // docs:start:inbox_batch_consume
   /**
    * @notice Consumes the current tree, and starts a new one if needed
    * @dev Only callable by the rollup contract
@@ -38,5 +35,4 @@ interface INewInbox {
    * @return The root of the consumed tree
    */
   function consume() external returns (bytes32);
-  // docs:end:inbox_batch_consume
 }

From 83038d728fe42e85b073e264a1411d4a4679cc65 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 14:51:35 +0000
Subject: [PATCH 33/48] using constant

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index e43343b108e..8e036ea4f48 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -31,7 +31,7 @@ contract NewInbox is INewInbox {
   uint256 internal immutable SIZE;
   bytes32 internal immutable EMPTY_ROOT; // The root of an empty frontier tree
 
-  uint256 internal toConsume = 1;
+  uint256 internal toConsume = Constants.INITIAL_L2_BLOCK_NUM;
   uint256 internal inProgress = 2;
 
   mapping(uint256 blockNumber => IFrontier tree) internal trees;

From 4703b65199d6703e90f4657f85400af53483961a Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 14:52:29 +0000
Subject: [PATCH 34/48] linking TODO

---
 l1-contracts/src/core/messagebridge/NewInbox.sol | 2 +-
 l1-contracts/test/NewInbox.t.sol                 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 8e036ea4f48..f076ba3c180 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -84,7 +84,7 @@ contract NewInbox is INewInbox {
       recipient: _recipient,
       content: _content,
       secretHash: _secretHash,
-      // TODO: nuke the following 2 values from the struct once the new message model is in place
+      // TODO(#4833): nuke the following 2 values from the struct once the new message model is in place
       deadline: type(uint32).max,
       fee: 0
     });
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 94b09e80c31..ec1a6b298f6 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -62,7 +62,7 @@ contract NewInboxTest is Test {
     // update version
     _message.recipient.version = version;
 
-    // TODO: nuke the following 2 values from the struct once the new message model is in place
+    // TODO(#4833): nuke the following 2 values from the struct once the new message model is in place
     _message.deadline = type(uint32).max;
     _message.fee = 0;
 

From 3ef3474233f737b02cc8ef99bd194da417ffda87 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 14:57:56 +0000
Subject: [PATCH 35/48] returning index instead of next index

---
 l1-contracts/src/core/messagebridge/NewInbox.sol              | 4 ++--
 .../src/core/messagebridge/frontier_tree/Frontier.sol         | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index f076ba3c180..3b17498db57 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -90,8 +90,8 @@ contract NewInbox is INewInbox {
     });
 
     bytes32 leaf = message.sha256ToField();
-    uint256 nextIndex = currentTree.insertLeaf(leaf);
-    emit LeafInserted(inProgress, nextIndex - 1, leaf);
+    uint256 index = currentTree.insertLeaf(leaf);
+    emit LeafInserted(inProgress, index, leaf);
 
     return leaf;
   }
diff --git a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
index c81b38afc07..1a7947765fd 100644
--- a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
+++ b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
@@ -33,9 +33,8 @@ contract FrontierMerkle is IFrontier {
       right = sha256(bytes.concat(frontier[i], right));
     }
     frontier[level] = right;
-    nextIndex++;
 
-    return nextIndex;
+    return nextIndex++;
   }
 
   function root() external view override(IFrontier) returns (bytes32) {

From 6b6a7bb19f7f8812963e17a0f5a99ff21ba9b569 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 14:58:04 +0000
Subject: [PATCH 36/48] IFrontier natspec

---
 .../src/core/interfaces/messagebridge/IFrontier.sol | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol b/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
index 17354e1db73..c403836ad97 100644
--- a/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
+++ b/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
@@ -3,9 +3,22 @@
 pragma solidity >=0.8.18;
 
 interface IFrontier {
+  /**
+   * @notice Inserts a new leaf into the frontier tree and returns its index
+   * @param _leaf - The leaf to insert
+   * @return The index of the leaf in the tree
+   */
   function insertLeaf(bytes32 _leaf) external returns (uint256);
 
+  /**
+   * @notice Returns the root of the frontier tree
+   * @return The root of the tree
+   */
   function root() external view returns (bytes32);
 
+  /**
+   * @notice Returns whether the tree is full
+   * @return Whether the tree is full
+   */
   function isFull() external view returns (bool);
 }

From 2b1730385e98c43c31f3b5b0a4132a8e881b7207 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 15:04:52 +0000
Subject: [PATCH 37/48] cleanup of getNumTrees

---
 l1-contracts/test/harnesses/NewInboxHarness.sol | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/l1-contracts/test/harnesses/NewInboxHarness.sol b/l1-contracts/test/harnesses/NewInboxHarness.sol
index 3bd1f0d689b..1277ef350e2 100644
--- a/l1-contracts/test/harnesses/NewInboxHarness.sol
+++ b/l1-contracts/test/harnesses/NewInboxHarness.sol
@@ -38,10 +38,6 @@ contract NewInboxHarness is NewInbox {
   }
 
   function getNumTrees() external view returns (uint256) {
-    uint256 blockNumber = FIRST_REAL_TREE_NUM;
-    while (address(trees[blockNumber]) != address(0)) {
-      blockNumber++;
-    }
-    return blockNumber - 2; // - 2 because first real tree is included in block 2
+    return inProgress - 1; // -1 because tree number 1 is not real
   }
 }

From a4e96d90c897ba29f8a3cc0f0934a3ca29f785e2 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 15:06:02 +0000
Subject: [PATCH 38/48] slither

---
 l1-contracts/slither_output.md | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/l1-contracts/slither_output.md b/l1-contracts/slither_output.md
index 1368756b90c..47ce1768a30 100644
--- a/l1-contracts/slither_output.md
+++ b/l1-contracts/slither_output.md
@@ -13,7 +13,7 @@ Summary
  - [low-level-calls](#low-level-calls) (1 results) (Informational)
  - [similar-names](#similar-names) (3 results) (Informational)
  - [constable-states](#constable-states) (1 results) (Optimization)
- - [pess-multiple-storage-read](#pess-multiple-storage-read) (6 results) (Optimization)
+ - [pess-multiple-storage-read](#pess-multiple-storage-read) (5 results) (Optimization)
 ## pess-unprotected-setter
 Impact: High
 Confidence: Medium
@@ -128,7 +128,7 @@ Dubious typecast in [Inbox.batchConsume(bytes32[],address)](src/core/messagebrid
 src/core/messagebridge/Inbox.sol#L122-L143
 
 
-## reentrancy-events
+## missing-zero-check
 Impact: Low
 Confidence: Medium
  - [ ] ID-12
@@ -144,9 +144,9 @@ Confidence: Medium
  - [ ] ID-13
 Reentrancy in [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L60-L97):
 	External calls:
-	- [nextIndex = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L93)
+	- [index = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L93)
 	Event emitted after the call(s):
-	- [LeafInserted(inProgress,nextIndex - 1,leaf)](src/core/messagebridge/NewInbox.sol#L94)
+	- [LeafInserted(inProgress,index,leaf)](src/core/messagebridge/NewInbox.sol#L94)
 
 src/core/messagebridge/NewInbox.sol#L60-L97
 
@@ -201,10 +201,10 @@ src/core/messagebridge/Inbox.sol#L102-L113
 Impact: Low
 Confidence: Medium
  - [ ] ID-19
-The following public functions could be turned into external in [FrontierMerkle](src/core/messagebridge/frontier_tree/Frontier.sol#L7-L91) contract:
+The following public functions could be turned into external in [FrontierMerkle](src/core/messagebridge/frontier_tree/Frontier.sol#L7-L90) contract:
 	[FrontierMerkle.constructor(uint256)](src/core/messagebridge/frontier_tree/Frontier.sol#L19-L27)
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L7-L91
+src/core/messagebridge/frontier_tree/Frontier.sol#L7-L90
 
 
  - [ ] ID-20
@@ -248,7 +248,7 @@ src/core/messagebridge/NewInbox.sol#L25-L126
 ## assembly
 Impact: Informational
 Confidence: High
- - [ ] ID-22
+ - [ ] ID-25
 [MessagesDecoder.decode(bytes)](src/core/libraries/decoders/MessagesDecoder.sol#L60-L150) uses assembly
 	- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L79-L81)
 	- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L112-L118)
@@ -256,7 +256,7 @@ Confidence: High
 src/core/libraries/decoders/MessagesDecoder.sol#L60-L150
 
 
- - [ ] ID-23
+ - [ ] ID-26
 [TxsDecoder.computeRoot(bytes32[])](src/core/libraries/decoders/TxsDecoder.sol#L291-L310) uses assembly
 	- [INLINE ASM](src/core/libraries/decoders/TxsDecoder.sol#L298-L300)
 
@@ -352,9 +352,9 @@ src/core/messagebridge/NewInbox.sol#L60-L97
 
 
  - [ ] ID-39
-In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74) variable [FrontierMerkle.HEIGHT](src/core/messagebridge/frontier_tree/Frontier.sol#L8) is read multiple times
+In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L40-L73) variable [FrontierMerkle.HEIGHT](src/core/messagebridge/frontier_tree/Frontier.sol#L8) is read multiple times
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
+src/core/messagebridge/frontier_tree/Frontier.sol#L40-L73
 
 
  - [ ] ID-40
@@ -370,14 +370,8 @@ src/core/messagebridge/NewInbox.sol#L106-L125
 
 
  - [ ] ID-42
-In a function [FrontierMerkle.insertLeaf(bytes32)](src/core/messagebridge/frontier_tree/Frontier.sol#L29-L39) variable [FrontierMerkle.nextIndex](src/core/messagebridge/frontier_tree/Frontier.sol#L11) is read multiple times
-
-src/core/messagebridge/frontier_tree/Frontier.sol#L29-L39
-
-
- - [ ] ID-43
-In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74) variable [FrontierMerkle.frontier](src/core/messagebridge/frontier_tree/Frontier.sol#L13) is read multiple times
+In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L40-L73) variable [FrontierMerkle.frontier](src/core/messagebridge/frontier_tree/Frontier.sol#L13) is read multiple times
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L41-L74
+src/core/messagebridge/frontier_tree/Frontier.sol#L40-L73
 
 

From 5a2b9cb6f04dcdbb7eee9bf9351ddb7be6d2ea5a Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 15:10:35 +0000
Subject: [PATCH 39/48] making checkInvariant a modifier

---
 l1-contracts/test/NewInbox.t.sol | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index ec1a6b298f6..0bc7b00c968 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -71,7 +71,8 @@ contract NewInboxTest is Test {
 
   // Since there is a 1 block lag between tree to be consumed and tree in progress the following invariant should never
   // be violated
-  function _checkInvariant() internal {
+  modifier checkInvariant() {
+    _;
     assertLt(inbox.getToConsume(), inbox.getInProgress());
   }
 
@@ -81,7 +82,7 @@ contract NewInboxTest is Test {
     inbox.consume();
   }
 
-  function testFuzzInsert(DataStructures.L1ToL2Msg memory _message) public {
+  function testFuzzInsert(DataStructures.L1ToL2Msg memory _message) public checkInvariant {
     DataStructures.L1ToL2Msg memory message = _boundMessage(_message);
 
     bytes32 leaf = message.sha256ToField();
@@ -95,7 +96,7 @@ contract NewInboxTest is Test {
     assertEq(insertedLeaf, leaf);
   }
 
-  function testSendMultipleSameL2Messages() public {
+  function testSendMultipleSameL2Messages() public checkInvariant {
     DataStructures.L1ToL2Msg memory message = _fakeMessage();
     bytes32 leaf1 = inbox.sendL2Message(message.recipient, message.content, message.secretHash);
     bytes32 leaf2 = inbox.sendL2Message(message.recipient, message.content, message.secretHash);
@@ -153,7 +154,7 @@ contract NewInboxTest is Test {
     _consume(_numTreesToConsumeSecondBatch, _messagesSecondBatch.length);
   }
 
-  function _send(DataStructures.L1ToL2Msg[] memory _messages) internal {
+  function _send(DataStructures.L1ToL2Msg[] memory _messages) internal checkInvariant {
     bool isFirstRun = inbox.getInProgress() == inbox.FIRST_REAL_TREE_NUM();
     bytes32 toConsumeRoot = inbox.getToConsumeRoot();
 
@@ -162,8 +163,6 @@ contract NewInboxTest is Test {
 
       // We send the message and check that toConsume root did not change.
       inbox.sendL2Message(message.recipient, message.content, message.secretHash);
-      // We check that the "toConsume < inProgress" invariant is maintained
-      _checkInvariant();
     }
 
     // Root of a tree waiting to be consumed should not change because we introduced a 1 block lag to prevent sequencer
@@ -187,7 +186,7 @@ contract NewInboxTest is Test {
     }
   }
 
-  function _consume(uint256 _numTreesToConsume, uint256 _numMessages) internal {
+  function _consume(uint256 _numTreesToConsume, uint256 _numMessages) internal checkInvariant {
     bool isFirstRun = inbox.getToConsume() == Constants.INITIAL_L2_BLOCK_NUM;
     uint256 numTrees = inbox.getNumTrees();
 
@@ -198,8 +197,6 @@ contract NewInboxTest is Test {
     // Now we consume the trees
     for (uint256 i = 0; i < numTreesToConsume; i++) {
       bytes32 root = inbox.consume();
-      // We check that the "toConsume < inProgress" invariant is maintained
-      _checkInvariant();
 
       // We perform empty roots check only after first batch because after second one the following simple accounting
       // does not work

From d22b3691e8290b422366035afba021f54a57bb76 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 15:19:08 +0000
Subject: [PATCH 40/48] comment cleanup

---
 l1-contracts/test/NewInbox.t.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 0bc7b00c968..b1f015658f6 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -158,10 +158,10 @@ contract NewInboxTest is Test {
     bool isFirstRun = inbox.getInProgress() == inbox.FIRST_REAL_TREE_NUM();
     bytes32 toConsumeRoot = inbox.getToConsumeRoot();
 
+    // We send the messages and then check that toConsume root did not change.
     for (uint256 i = 0; i < _messages.length; i++) {
       DataStructures.L1ToL2Msg memory message = _boundMessage(_messages[i]);
 
-      // We send the message and check that toConsume root did not change.
       inbox.sendL2Message(message.recipient, message.content, message.secretHash);
     }
 

From 74a0abe91c5dacffdf08285f2b11f66354f7db95 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Tue, 5 Mar 2024 15:35:12 +0000
Subject: [PATCH 41/48] WIP

---
 l1-contracts/test/NewInbox.t.sol | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index b1f015658f6..23fea271384 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -196,8 +196,13 @@ contract NewInboxTest is Test {
 
     // Now we consume the trees
     for (uint256 i = 0; i < numTreesToConsume; i++) {
+      uint256 expectedNumTrees =
+        (inbox.getToConsume() + 1 == inbox.getInProgress()) ? numTrees + 1 : numTrees;
       bytes32 root = inbox.consume();
 
+      // We perform this check to verify that new tree initialization works as expected
+      assertEq(inbox.getNumTrees(), expectedNumTrees, "Unexptected number of trees");
+
       // We perform empty roots check only after first batch because after second one the following simple accounting
       // does not work
       if (isFirstRun) {

From a29b0967109455d748c6327c6f98dbdc0b0a3dc8 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Wed, 6 Mar 2024 08:00:27 +0000
Subject: [PATCH 42/48] checking tree initialization

---
 l1-contracts/test/NewInbox.t.sol | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 23fea271384..26d207d526e 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -188,14 +188,15 @@ contract NewInboxTest is Test {
 
   function _consume(uint256 _numTreesToConsume, uint256 _numMessages) internal checkInvariant {
     bool isFirstRun = inbox.getToConsume() == Constants.INITIAL_L2_BLOCK_NUM;
-    uint256 numTrees = inbox.getNumTrees();
 
-    // We use (numTrees * 2) as upper bound here because we want to test the case where we go beyond the currently
-    // initalized number of trees. When consuming the newly initialized trees we should get zero roots.
-    uint256 numTreesToConsume = bound(_numTreesToConsume, 1, numTrees * 2);
+    uint256 initialNumTrees = inbox.getNumTrees();
+    // We use (initialNumTrees * 2) as upper bound here because we want to test the case where we go beyond
+    // the currently initalized number of trees. When consuming the newly initialized trees we should get zero roots.
+    uint256 numTreesToConsume = bound(_numTreesToConsume, 1, initialNumTrees * 2);
 
     // Now we consume the trees
     for (uint256 i = 0; i < numTreesToConsume; i++) {
+      uint256 numTrees = inbox.getNumTrees();
       uint256 expectedNumTrees =
         (inbox.getToConsume() + 1 == inbox.getInProgress()) ? numTrees + 1 : numTrees;
       bytes32 root = inbox.consume();
@@ -210,7 +211,7 @@ contract NewInboxTest is Test {
         // 1) For i = 0 we should always get empty tree root because first block's messages tree is always
         // empty because we introduced a 1 block lag to prevent sequencer DOS attacks.
         // 2) For _numMessages = 0 and i = 1 we get empty root as well
-        if (i != 0 && i <= numTrees && _numMessages > 0) {
+        if (i != 0 && i <= initialNumTrees && _numMessages > 0) {
           assertNotEq(root, emptyTreeRoot, "Root should not be zero");
         } else {
           assertEq(root, emptyTreeRoot, "Root should be zero");

From 881627ceafaa5eec7cc6c5d67717a06a7af9f923 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Wed, 6 Mar 2024 08:29:12 +0000
Subject: [PATCH 43/48] checking tree initialization when inserting

---
 l1-contracts/test/NewInbox.t.sol                | 10 ++++++++--
 l1-contracts/test/harnesses/NewInboxHarness.sol |  4 ++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index 26d207d526e..b49891ba146 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -162,7 +162,13 @@ contract NewInboxTest is Test {
     for (uint256 i = 0; i < _messages.length; i++) {
       DataStructures.L1ToL2Msg memory message = _boundMessage(_messages[i]);
 
+      // We check whether a new tree is correctly initialized when the one in progress is full
+      uint256 numTrees = inbox.getNumTrees();
+      uint256 expectedNumTrees = inbox.treeInProgressFull() ? numTrees + 1 : numTrees;
+
       inbox.sendL2Message(message.recipient, message.content, message.secretHash);
+
+      assertEq(inbox.getNumTrees(), expectedNumTrees, "Unexpected number of trees");
     }
 
     // Root of a tree waiting to be consumed should not change because we introduced a 1 block lag to prevent sequencer
@@ -201,8 +207,8 @@ contract NewInboxTest is Test {
         (inbox.getToConsume() + 1 == inbox.getInProgress()) ? numTrees + 1 : numTrees;
       bytes32 root = inbox.consume();
 
-      // We perform this check to verify that new tree initialization works as expected
-      assertEq(inbox.getNumTrees(), expectedNumTrees, "Unexptected number of trees");
+      // We check whether a new tree is correctly initialized when the one which was in progress was set as to consume
+      assertEq(inbox.getNumTrees(), expectedNumTrees, "Unexpected number of trees");
 
       // We perform empty roots check only after first batch because after second one the following simple accounting
       // does not work
diff --git a/l1-contracts/test/harnesses/NewInboxHarness.sol b/l1-contracts/test/harnesses/NewInboxHarness.sol
index 1277ef350e2..959e44c1210 100644
--- a/l1-contracts/test/harnesses/NewInboxHarness.sol
+++ b/l1-contracts/test/harnesses/NewInboxHarness.sol
@@ -29,6 +29,10 @@ contract NewInboxHarness is NewInbox {
     return inProgress;
   }
 
+  function treeInProgressFull() external view returns (bool) {
+    return trees[inProgress].isFull();
+  }
+
   function getToConsumeRoot() external view returns (bytes32) {
     bytes32 root = EMPTY_ROOT;
     if (toConsume > Constants.INITIAL_L2_BLOCK_NUM) {

From 3e974f93289829c46425d59c9cedfad92f2526bd Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Wed, 6 Mar 2024 08:59:47 +0000
Subject: [PATCH 44/48] deshitification

---
 l1-contracts/test/NewInbox.t.sol | 36 ++++++--------------------------
 1 file changed, 6 insertions(+), 30 deletions(-)

diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index b49891ba146..aae3379a462 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -145,17 +145,16 @@ contract NewInboxTest is Test {
     _send(_messagesFirstBatch);
 
     // Consume first few trees
-    _consume(_numTreesToConsumeFirstBatch, _messagesFirstBatch.length);
+    _consume(_numTreesToConsumeFirstBatch);
 
     // Send second batch of messages
     _send(_messagesSecondBatch);
 
     // Consume second batch of trees
-    _consume(_numTreesToConsumeSecondBatch, _messagesSecondBatch.length);
+    _consume(_numTreesToConsumeSecondBatch);
   }
 
   function _send(DataStructures.L1ToL2Msg[] memory _messages) internal checkInvariant {
-    bool isFirstRun = inbox.getInProgress() == inbox.FIRST_REAL_TREE_NUM();
     bytes32 toConsumeRoot = inbox.getToConsumeRoot();
 
     // We send the messages and then check that toConsume root did not change.
@@ -178,23 +177,9 @@ contract NewInboxTest is Test {
       toConsumeRoot,
       "Root of a tree waiting to be consumed should not change"
     );
-
-    // We perform expected num trees check only after first batch because after second one the following accounting
-    // does not work because a tree might have been consumed when not full or we might have consumed an empty tree
-    if (isFirstRun) {
-      uint256 expectedNumTrees = _divideAndRoundUp(_messages.length, inbox.getSize());
-      if (expectedNumTrees == 0) {
-        // This occurs when there are no messages but we initialize the first tree in the constructor so there are
-        // never zero trees
-        expectedNumTrees = 1;
-      }
-      assertEq(inbox.getNumTrees(), expectedNumTrees);
-    }
   }
 
-  function _consume(uint256 _numTreesToConsume, uint256 _numMessages) internal checkInvariant {
-    bool isFirstRun = inbox.getToConsume() == Constants.INITIAL_L2_BLOCK_NUM;
-
+  function _consume(uint256 _numTreesToConsume) internal checkInvariant {
     uint256 initialNumTrees = inbox.getNumTrees();
     // We use (initialNumTrees * 2) as upper bound here because we want to test the case where we go beyond
     // the currently initalized number of trees. When consuming the newly initialized trees we should get zero roots.
@@ -210,18 +195,9 @@ contract NewInboxTest is Test {
       // We check whether a new tree is correctly initialized when the one which was in progress was set as to consume
       assertEq(inbox.getNumTrees(), expectedNumTrees, "Unexpected number of trees");
 
-      // We perform empty roots check only after first batch because after second one the following simple accounting
-      // does not work
-      if (isFirstRun) {
-        // Notes:
-        // 1) For i = 0 we should always get empty tree root because first block's messages tree is always
-        // empty because we introduced a 1 block lag to prevent sequencer DOS attacks.
-        // 2) For _numMessages = 0 and i = 1 we get empty root as well
-        if (i != 0 && i <= initialNumTrees && _numMessages > 0) {
-          assertNotEq(root, emptyTreeRoot, "Root should not be zero");
-        } else {
-          assertEq(root, emptyTreeRoot, "Root should be zero");
-        }
+      // If we go beyong the number of trees initialized before consuming we should get empty root
+      if (i > initialNumTrees) {
+        assertEq(root, emptyTreeRoot, "Root of a newly initialized tree not empty");
       }
     }
   }

From 66e219c98d709c975bd2925faea99f9e49e7643f Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Wed, 6 Mar 2024 11:59:59 +0000
Subject: [PATCH 45/48] final touches

---
 .../src/core/messagebridge/frontier_tree/Frontier.sol      | 7 +++++--
 l1-contracts/test/harnesses/NewInboxHarness.sol            | 5 ++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
index 1a7947765fd..c8a274816a5 100644
--- a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
+++ b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
@@ -27,14 +27,17 @@ contract FrontierMerkle is IFrontier {
   }
 
   function insertLeaf(bytes32 _leaf) external override(IFrontier) returns (uint256) {
-    uint256 level = _computeLevel(nextIndex);
+    uint256 index = nextIndex;
+    uint256 level = _computeLevel(index);
     bytes32 right = _leaf;
     for (uint256 i = 0; i < level; i++) {
       right = sha256(bytes.concat(frontier[i], right));
     }
     frontier[level] = right;
 
-    return nextIndex++;
+    nextIndex++;
+
+    return index;
   }
 
   function root() external view override(IFrontier) returns (bytes32) {
diff --git a/l1-contracts/test/harnesses/NewInboxHarness.sol b/l1-contracts/test/harnesses/NewInboxHarness.sol
index 959e44c1210..4a85acc507f 100644
--- a/l1-contracts/test/harnesses/NewInboxHarness.sol
+++ b/l1-contracts/test/harnesses/NewInboxHarness.sol
@@ -9,8 +9,6 @@ import {Constants} from "../../src/core/libraries/ConstantsGen.sol";
 
 // TODO: rename to InboxHarness once all the pieces of the new message model are in place.
 contract NewInboxHarness is NewInbox {
-  uint256 public constant FIRST_REAL_TREE_NUM = Constants.INITIAL_L2_BLOCK_NUM + 1;
-
   constructor(address _rollup, uint256 _height) NewInbox(_rollup, _height) {}
 
   function getSize() external view returns (uint256) {
@@ -42,6 +40,7 @@ contract NewInboxHarness is NewInbox {
   }
 
   function getNumTrees() external view returns (uint256) {
-    return inProgress - 1; // -1 because tree number 1 is not real
+    // -INITIAL_L2_BLOCK_NUM because tree number INITIAL_L2_BLOCK_NUM is not real
+    return inProgress - Constants.INITIAL_L2_BLOCK_NUM;
   }
 }

From d3e6e80af17c00ba9442f5581325b66766c29bee Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Wed, 6 Mar 2024 12:03:07 +0000
Subject: [PATCH 46/48] slither output

---
 l1-contracts/slither_output.md | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/l1-contracts/slither_output.md b/l1-contracts/slither_output.md
index 47ce1768a30..14e887155af 100644
--- a/l1-contracts/slither_output.md
+++ b/l1-contracts/slither_output.md
@@ -201,10 +201,10 @@ src/core/messagebridge/Inbox.sol#L102-L113
 Impact: Low
 Confidence: Medium
  - [ ] ID-19
-The following public functions could be turned into external in [FrontierMerkle](src/core/messagebridge/frontier_tree/Frontier.sol#L7-L90) contract:
+The following public functions could be turned into external in [FrontierMerkle](src/core/messagebridge/frontier_tree/Frontier.sol#L7-L93) contract:
 	[FrontierMerkle.constructor(uint256)](src/core/messagebridge/frontier_tree/Frontier.sol#L19-L27)
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L7-L90
+src/core/messagebridge/frontier_tree/Frontier.sol#L7-L93
 
 
  - [ ] ID-20
@@ -352,9 +352,9 @@ src/core/messagebridge/NewInbox.sol#L60-L97
 
 
  - [ ] ID-39
-In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L40-L73) variable [FrontierMerkle.HEIGHT](src/core/messagebridge/frontier_tree/Frontier.sol#L8) is read multiple times
+In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L43-L76) variable [FrontierMerkle.HEIGHT](src/core/messagebridge/frontier_tree/Frontier.sol#L8) is read multiple times
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L40-L73
+src/core/messagebridge/frontier_tree/Frontier.sol#L43-L76
 
 
  - [ ] ID-40
@@ -370,8 +370,8 @@ src/core/messagebridge/NewInbox.sol#L106-L125
 
 
  - [ ] ID-42
-In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L40-L73) variable [FrontierMerkle.frontier](src/core/messagebridge/frontier_tree/Frontier.sol#L13) is read multiple times
+In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L43-L76) variable [FrontierMerkle.frontier](src/core/messagebridge/frontier_tree/Frontier.sol#L13) is read multiple times
 
-src/core/messagebridge/frontier_tree/Frontier.sol#L40-L73
+src/core/messagebridge/frontier_tree/Frontier.sol#L43-L76
 
 

From 1900f6e9968ed61489f709bf8255534d921c3a87 Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Wed, 6 Mar 2024 12:04:42 +0000
Subject: [PATCH 47/48] better natspec

---
 l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol b/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
index c403836ad97..9f158cb0258 100644
--- a/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
+++ b/l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
@@ -18,7 +18,7 @@ interface IFrontier {
 
   /**
    * @notice Returns whether the tree is full
-   * @return Whether the tree is full
+   * @return True if full, false otherwise
    */
   function isFull() external view returns (bool);
 }

From e3ac55d5edb4f7e2562fc0f3b0e731fb4c33b20b Mon Sep 17 00:00:00 2001
From: benesjan <janbenes1234@gmail.com>
Date: Wed, 6 Mar 2024 12:09:13 +0000
Subject: [PATCH 48/48] exposing toConsume and inProgress

---
 l1-contracts/slither_output.md                | 42 +++++++++----------
 .../src/core/messagebridge/NewInbox.sol       |  6 ++-
 l1-contracts/test/NewInbox.t.sol              |  4 +-
 .../test/harnesses/NewInboxHarness.sol        |  8 ----
 4 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/l1-contracts/slither_output.md b/l1-contracts/slither_output.md
index 14e887155af..dff0feae6a1 100644
--- a/l1-contracts/slither_output.md
+++ b/l1-contracts/slither_output.md
@@ -132,34 +132,34 @@ src/core/messagebridge/Inbox.sol#L122-L143
 Impact: Low
 Confidence: Medium
  - [ ] ID-12
-[NewInbox.constructor(address,uint256)._rollup](src/core/messagebridge/NewInbox.sol#L39) lacks a zero-check on :
-		- [ROLLUP = _rollup](src/core/messagebridge/NewInbox.sol#L40)
+[NewInbox.constructor(address,uint256)._rollup](src/core/messagebridge/NewInbox.sol#L41) lacks a zero-check on :
+		- [ROLLUP = _rollup](src/core/messagebridge/NewInbox.sol#L42)
 
-src/core/messagebridge/NewInbox.sol#L39
+src/core/messagebridge/NewInbox.sol#L41
 
 
 ## reentrancy-events
 Impact: Low
 Confidence: Medium
  - [ ] ID-13
-Reentrancy in [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L60-L97):
+Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L53-L91):
 	External calls:
-	- [index = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L93)
+	- [inbox.batchConsume(l1ToL2Msgs,msg.sender)](src/core/Rollup.sol#L85)
+	- [outbox.sendL1Messages(l2ToL1Msgs)](src/core/Rollup.sol#L88)
 	Event emitted after the call(s):
-	- [LeafInserted(inProgress,index,leaf)](src/core/messagebridge/NewInbox.sol#L94)
+	- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L90)
 
-src/core/messagebridge/NewInbox.sol#L60-L97
+src/core/Rollup.sol#L53-L91
 
 
  - [ ] ID-14
-Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L53-L91):
+Reentrancy in [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L62-L99):
 	External calls:
-	- [inbox.batchConsume(l1ToL2Msgs,msg.sender)](src/core/Rollup.sol#L85)
-	- [outbox.sendL1Messages(l2ToL1Msgs)](src/core/Rollup.sol#L88)
+	- [index = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L95)
 	Event emitted after the call(s):
-	- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L90)
+	- [LeafInserted(inProgress,index,leaf)](src/core/messagebridge/NewInbox.sol#L96)
 
-src/core/Rollup.sol#L53-L91
+src/core/messagebridge/NewInbox.sol#L62-L99
 
 
 ## timestamp
@@ -239,10 +239,10 @@ src/core/messagebridge/Inbox.sol#L21-L231
 
 
  - [ ] ID-24
-The following public functions could be turned into external in [NewInbox](src/core/messagebridge/NewInbox.sol#L25-L126) contract:
-	[NewInbox.constructor(address,uint256)](src/core/messagebridge/NewInbox.sol#L39-L50)
+The following public functions could be turned into external in [NewInbox](src/core/messagebridge/NewInbox.sol#L25-L128) contract:
+	[NewInbox.constructor(address,uint256)](src/core/messagebridge/NewInbox.sol#L41-L52)
 
-src/core/messagebridge/NewInbox.sol#L25-L126
+src/core/messagebridge/NewInbox.sol#L25-L128
 
 
 ## assembly
@@ -346,9 +346,9 @@ src/core/Rollup.sol#L37
 Impact: Optimization
 Confidence: High
  - [ ] ID-38
-In a function [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L60-L97) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L35) is read multiple times
+In a function [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L62-L99) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L37) is read multiple times
 
-src/core/messagebridge/NewInbox.sol#L60-L97
+src/core/messagebridge/NewInbox.sol#L62-L99
 
 
  - [ ] ID-39
@@ -358,15 +358,15 @@ src/core/messagebridge/frontier_tree/Frontier.sol#L43-L76
 
 
  - [ ] ID-40
-In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L106-L125) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L35) is read multiple times
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L108-L127) variable [NewInbox.inProgress](src/core/messagebridge/NewInbox.sol#L37) is read multiple times
 
-src/core/messagebridge/NewInbox.sol#L106-L125
+src/core/messagebridge/NewInbox.sol#L108-L127
 
 
  - [ ] ID-41
-In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L106-L125) variable [NewInbox.toConsume](src/core/messagebridge/NewInbox.sol#L34) is read multiple times
+In a function [NewInbox.consume()](src/core/messagebridge/NewInbox.sol#L108-L127) variable [NewInbox.toConsume](src/core/messagebridge/NewInbox.sol#L35) is read multiple times
 
-src/core/messagebridge/NewInbox.sol#L106-L125
+src/core/messagebridge/NewInbox.sol#L108-L127
 
 
  - [ ] ID-42
diff --git a/l1-contracts/src/core/messagebridge/NewInbox.sol b/l1-contracts/src/core/messagebridge/NewInbox.sol
index 3b17498db57..1a0185cb30b 100644
--- a/l1-contracts/src/core/messagebridge/NewInbox.sol
+++ b/l1-contracts/src/core/messagebridge/NewInbox.sol
@@ -31,8 +31,10 @@ contract NewInbox is INewInbox {
   uint256 internal immutable SIZE;
   bytes32 internal immutable EMPTY_ROOT; // The root of an empty frontier tree
 
-  uint256 internal toConsume = Constants.INITIAL_L2_BLOCK_NUM;
-  uint256 internal inProgress = 2;
+  // Number of a tree which is ready to be consumed
+  uint256 public toConsume = Constants.INITIAL_L2_BLOCK_NUM;
+  // Number of a tree which is currently being filled
+  uint256 public inProgress = 2;
 
   mapping(uint256 blockNumber => IFrontier tree) internal trees;
 
diff --git a/l1-contracts/test/NewInbox.t.sol b/l1-contracts/test/NewInbox.t.sol
index aae3379a462..869070d692f 100644
--- a/l1-contracts/test/NewInbox.t.sol
+++ b/l1-contracts/test/NewInbox.t.sol
@@ -73,7 +73,7 @@ contract NewInboxTest is Test {
   // be violated
   modifier checkInvariant() {
     _;
-    assertLt(inbox.getToConsume(), inbox.getInProgress());
+    assertLt(inbox.toConsume(), inbox.inProgress());
   }
 
   function testRevertIfNotConsumingFromRollup() public {
@@ -189,7 +189,7 @@ contract NewInboxTest is Test {
     for (uint256 i = 0; i < numTreesToConsume; i++) {
       uint256 numTrees = inbox.getNumTrees();
       uint256 expectedNumTrees =
-        (inbox.getToConsume() + 1 == inbox.getInProgress()) ? numTrees + 1 : numTrees;
+        (inbox.toConsume() + 1 == inbox.inProgress()) ? numTrees + 1 : numTrees;
       bytes32 root = inbox.consume();
 
       // We check whether a new tree is correctly initialized when the one which was in progress was set as to consume
diff --git a/l1-contracts/test/harnesses/NewInboxHarness.sol b/l1-contracts/test/harnesses/NewInboxHarness.sol
index 4a85acc507f..93f3b1b4f06 100644
--- a/l1-contracts/test/harnesses/NewInboxHarness.sol
+++ b/l1-contracts/test/harnesses/NewInboxHarness.sol
@@ -19,14 +19,6 @@ contract NewInboxHarness is NewInbox {
     return EMPTY_ROOT;
   }
 
-  function getToConsume() external view returns (uint256) {
-    return toConsume;
-  }
-
-  function getInProgress() external view returns (uint256) {
-    return inProgress;
-  }
-
   function treeInProgressFull() external view returns (bool) {
     return trees[inProgress].isFull();
   }