diff --git a/l1-contracts/src/core/interfaces/messagebridge/IInbox.sol b/l1-contracts/src/core/interfaces/messagebridge/IInbox.sol index 5a9be37abae..547fc773693 100644 --- a/l1-contracts/src/core/interfaces/messagebridge/IInbox.sol +++ b/l1-contracts/src/core/interfaces/messagebridge/IInbox.sol @@ -47,4 +47,6 @@ interface IInbox { */ function consume(uint256 _toConsume) external returns (bytes32); // docs:end:consume + + function getRoot(uint256 _blockNumber) external view returns (bytes32); } diff --git a/l1-contracts/src/core/libraries/Errors.sol b/l1-contracts/src/core/libraries/Errors.sol index 19be2872e3f..469b1bc7c02 100644 --- a/l1-contracts/src/core/libraries/Errors.sol +++ b/l1-contracts/src/core/libraries/Errors.sol @@ -15,6 +15,7 @@ library Errors { error Inbox__ActorTooLarge(bytes32 actor); // 0xa776a06e error Inbox__ContentTooLarge(bytes32 content); // 0x47452014 error Inbox__SecretHashTooLarge(bytes32 secretHash); // 0xecde7e2c + error Inbox__MustBuildBeforeConsume(); // 0xc4901999 // Outbox error Outbox__Unauthorized(); // 0x2c9490c2 diff --git a/l1-contracts/src/core/messagebridge/Inbox.sol b/l1-contracts/src/core/messagebridge/Inbox.sol index c5513a44f9e..babf2f5ec07 100644 --- a/l1-contracts/src/core/messagebridge/Inbox.sol +++ b/l1-contracts/src/core/messagebridge/Inbox.sol @@ -3,8 +3,6 @@ pragma solidity >=0.8.18; // Interfaces -import {IFrontier} from "../interfaces/messagebridge/IFrontier.sol"; -import {IRegistry} from "../interfaces/messagebridge/IRegistry.sol"; import {IInbox} from "../interfaces/messagebridge/IInbox.sol"; // Libraries @@ -13,18 +11,17 @@ import {DataStructures} from "../libraries/DataStructures.sol"; import {Errors} from "../libraries/Errors.sol"; import {Hash} from "../libraries/Hash.sol"; -// Contracts -import {FrontierMerkle} from "./frontier_tree/Frontier.sol"; +import {FrontierLib} from "./frontier_tree/FrontierLib.sol"; /** * @title Inbox * @author Aztec Labs * @notice Lives on L1 and is used to pass messages into the rollup, e.g., L1 -> L2 messages. - * - * @dev The current code is horribly gas inefficient, and should only be used as a reference implementation. */ contract Inbox is IInbox { using Hash for DataStructures.L1ToL2Msg; + using FrontierLib for FrontierLib.Forest; + using FrontierLib for FrontierLib.Tree; address public immutable ROLLUP; @@ -35,7 +32,10 @@ contract Inbox is IInbox { // Number of a tree which is currently being filled uint256 public inProgress = Constants.INITIAL_L2_BLOCK_NUM + 1; - mapping(uint256 blockNumber => IFrontier tree) public trees; + // Practically immutable value as we only set it in the constructor. + FrontierLib.Forest internal forest; + + mapping(uint256 blockNumber => FrontierLib.Tree tree) public trees; constructor(address _rollup, uint256 _height) { ROLLUP = _rollup; @@ -43,11 +43,8 @@ contract Inbox is IInbox { HEIGHT = _height; SIZE = 2 ** _height; - // We deploy the first tree - IFrontier firstTree = IFrontier(new FrontierMerkle(_height)); - trees[inProgress] = firstTree; - - EMPTY_ROOT = firstTree.root(); + forest.initialize(_height); + EMPTY_ROOT = trees[inProgress].root(forest, HEIGHT, SIZE); } /** @@ -76,11 +73,11 @@ contract Inbox is IInbox { revert Errors.Inbox__SecretHashTooLarge(_secretHash); } - IFrontier currentTree = trees[inProgress]; - if (currentTree.isFull()) { + FrontierLib.Tree storage currentTree = trees[inProgress]; + + if (currentTree.isFull(SIZE)) { inProgress += 1; - currentTree = IFrontier(new FrontierMerkle(HEIGHT)); - trees[inProgress] = currentTree; + currentTree = trees[inProgress]; } DataStructures.L1ToL2Msg memory message = DataStructures.L1ToL2Msg({ @@ -113,17 +110,24 @@ contract Inbox is IInbox { revert Errors.Inbox__Unauthorized(); } + if (_toConsume >= inProgress) { + revert Errors.Inbox__MustBuildBeforeConsume(); + } + bytes32 root = EMPTY_ROOT; if (_toConsume > Constants.INITIAL_L2_BLOCK_NUM) { - root = trees[_toConsume].root(); + root = trees[_toConsume].root(forest, HEIGHT, SIZE); } // If we are "catching up" we skip the tree creation as it is already there if (_toConsume + 1 == inProgress) { inProgress += 1; - trees[inProgress] = IFrontier(new FrontierMerkle(HEIGHT)); } return root; } + + function getRoot(uint256 _blockNumber) external view override(IInbox) returns (bytes32) { + return trees[_blockNumber].root(forest, HEIGHT, SIZE); + } } diff --git a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol index e80099fe0d9..48582bc47a8 100644 --- a/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol +++ b/l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol @@ -2,7 +2,8 @@ // Copyright 2023 Aztec Labs. pragma solidity >=0.8.18; -import {Hash} from "../../libraries/Hash.sol"; +import {FrontierLib} from "./FrontierLib.sol"; + import {IFrontier} from "../../interfaces/messagebridge/IFrontier.sol"; import {Ownable} from "@oz/access/Ownable.sol"; @@ -11,89 +12,32 @@ import {Ownable} from "@oz/access/Ownable.sol"; // TODO(Miranda): Possibly nuke this contract, and use a generic version which can either use // regular sha256 or sha256ToField when emulating circuits contract FrontierMerkle is IFrontier, Ownable { + using FrontierLib for FrontierLib.Tree; + using FrontierLib for FrontierLib.Forest; + uint256 public immutable HEIGHT; uint256 public immutable SIZE; - uint256 internal nextIndex = 0; - - mapping(uint256 level => bytes32 node) public frontier; + // Practically immutable value as we only set it in the constructor. + FrontierLib.Forest internal forest; - // Below can be pre-computed so it would be possible to have constants - // for the zeros at each level. This would save gas on computations - mapping(uint256 level => bytes32 zero) public zeros; + FrontierLib.Tree internal tree; constructor(uint256 _height) Ownable(msg.sender) { HEIGHT = _height; SIZE = 2 ** _height; - - zeros[0] = bytes32(0); - for (uint256 i = 1; i <= HEIGHT; i++) { - zeros[i] = Hash.sha256ToField(bytes.concat(zeros[i - 1], zeros[i - 1])); - } + forest.initialize(_height); } function insertLeaf(bytes32 _leaf) external override(IFrontier) onlyOwner returns (uint256) { - uint256 index = nextIndex; - uint256 level = _computeLevel(index); - bytes32 right = _leaf; - for (uint256 i = 0; i < level; i++) { - right = Hash.sha256ToField(bytes.concat(frontier[i], right)); - } - frontier[level] = right; - - nextIndex++; - - return index; + return tree.insertLeaf(_leaf); } function root() external view override(IFrontier) returns (bytes32) { - uint256 next = nextIndex; - if (next == 0) { - return zeros[HEIGHT]; - } - if (next == SIZE) { - return frontier[HEIGHT]; - } - - uint256 index = next - 1; - uint256 level = _computeLevel(index); - - // We should start at the highest frontier level with a left leaf - bytes32 temp = frontier[level]; - - uint256 bits = index >> level; - for (uint256 i = level; i < HEIGHT; i++) { - bool isRight = bits & 1 == 1; - if (isRight) { - if (frontier[i] == temp) { - // We will never hit the case that frontier[i] == temp - // because this require that frontier[i] is the right child - // and in that case we started higher up the tree - revert("Mistakes were made"); - } - temp = Hash.sha256ToField(bytes.concat(frontier[i], temp)); - } else { - temp = Hash.sha256ToField(bytes.concat(temp, zeros[i])); - } - bits >>= 1; - } - - return temp; + return tree.root(forest, HEIGHT, SIZE); } 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. - uint256 count = 0; - uint256 index = _leafIndex; - while (index & 1 == 1) { - count++; - index >>= 1; - } - return count; + return tree.isFull(SIZE); } } diff --git a/l1-contracts/src/core/messagebridge/frontier_tree/FrontierLib.sol b/l1-contracts/src/core/messagebridge/frontier_tree/FrontierLib.sol new file mode 100644 index 00000000000..74bfb78e215 --- /dev/null +++ b/l1-contracts/src/core/messagebridge/frontier_tree/FrontierLib.sol @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2023 Aztec Labs. +pragma solidity >=0.8.18; + +import {Hash} from "../../libraries/Hash.sol"; + +/** + * @title FrontierLib + * @author Aztec Labs + * @notice Library for managing frontier trees. + */ +library FrontierLib { + struct Forest { + mapping(uint256 index => bytes32 zero) zeros; + } + + struct Tree { + uint256 nextIndex; + mapping(uint256 => bytes32) frontier; + } + + function initialize(Forest storage self, uint256 _height) internal { + self.zeros[0] = bytes32(0); + for (uint256 i = 1; i <= _height; i++) { + self.zeros[i] = Hash.sha256ToField(bytes.concat(self.zeros[i - 1], self.zeros[i - 1])); + } + } + + function insertLeaf(Tree storage self, bytes32 _leaf) internal returns (uint256) { + uint256 index = self.nextIndex; + uint256 level = _computeLevel(index); + bytes32 right = _leaf; + for (uint256 i = 0; i < level; i++) { + right = Hash.sha256ToField(bytes.concat(self.frontier[i], right)); + } + self.frontier[level] = right; + + self.nextIndex++; + + return index; + } + + function root(Tree storage self, Forest storage _forest, uint256 _height, uint256 _size) + internal + view + returns (bytes32) + { + uint256 next = self.nextIndex; + if (next == 0) { + return _forest.zeros[_height]; + } + if (next == _size) { + return self.frontier[_height]; + } + + uint256 index = next - 1; + uint256 level = _computeLevel(index); + + // We should start at the highest frontier level with a left leaf + bytes32 temp = self.frontier[level]; + + uint256 bits = index >> level; + for (uint256 i = level; i < _height; i++) { + bool isRight = bits & 1 == 1; + if (isRight) { + if (self.frontier[i] == temp) { + // We will never hit the case that frontier[i] == temp + // because this require that frontier[i] is the right child + // and in that case we started higher up the tree + revert("Mistakes were made"); + } + temp = Hash.sha256ToField(bytes.concat(self.frontier[i], temp)); + } else { + temp = Hash.sha256ToField(bytes.concat(temp, _forest.zeros[i])); + } + bits >>= 1; + } + + return temp; + } + + function isFull(Tree storage self, uint256 _size) internal view returns (bool) { + return self.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. + uint256 count = 0; + uint256 index = _leafIndex; + while (index & 1 == 1) { + count++; + index >>= 1; + } + return count; + } +} diff --git a/l1-contracts/test/Inbox.t.sol b/l1-contracts/test/Inbox.t.sol index 94e460d709b..a879cadf67e 100644 --- a/l1-contracts/test/Inbox.t.sol +++ b/l1-contracts/test/Inbox.t.sol @@ -76,6 +76,11 @@ contract InboxTest is Test { inbox.consume(blockNumber); } + function testRevertIFConsumingInFuture() public { + vm.expectRevert(Errors.Inbox__MustBuildBeforeConsume.selector); + inbox.consume(blockNumber + 1000); + } + function testFuzzInsert(DataStructures.L1ToL2Msg memory _message) public checkInvariant { DataStructures.L1ToL2Msg memory message = _boundMessage(_message); diff --git a/l1-contracts/test/Rollup.t.sol b/l1-contracts/test/Rollup.t.sol index 7023439b845..a9205542fbc 100644 --- a/l1-contracts/test/Rollup.t.sol +++ b/l1-contracts/test/Rollup.t.sol @@ -16,7 +16,6 @@ import {IFeeJuicePortal} from "../src/core/interfaces/IFeeJuicePortal.sol"; import {FeeJuicePortal} from "../src/core/FeeJuicePortal.sol"; import {Leonidas} from "../src/core/sequencer_selection/Leonidas.sol"; import {AvailabilityOracle} from "../src/core/availability_oracle/AvailabilityOracle.sol"; -import {FrontierMerkle} from "../src/core/messagebridge/frontier_tree/Frontier.sol"; import {NaiveMerkle} from "./merkle/Naive.sol"; import {MerkleTestUtil} from "./merkle/TestUtil.sol"; import {PortalERC20} from "./portals/PortalERC20.sol"; @@ -101,7 +100,7 @@ contract RollupTest is DecoderBase { // @note Fetch the inbox root of block 2. This should be frozen when block 1 is proposed. // Even if we end up reverting block 1, we should still see the same root in the inbox. - bytes32 inboxRoot2 = inbox.trees(2).root(); + bytes32 inboxRoot2 = inbox.getRoot(2); (,, uint128 slot,) = rollup.blocks(1); uint256 prunableAt = uint256(slot) + rollup.TIMELINESS_PROVING_IN_SLOTS(); @@ -136,7 +135,7 @@ contract RollupTest is DecoderBase { _testBlock("empty_block_1", false, prunableAt); assertEq(inbox.inProgress(), 3, "Invalid in progress"); - assertEq(inbox.trees(2).root(), inboxRoot2, "Invalid inbox root"); + assertEq(inbox.getRoot(2), inboxRoot2, "Invalid inbox root"); assertEq(rollup.pendingBlockCount(), 2, "Invalid pending block count"); assertEq(rollup.provenBlockCount(), 1, "Invalid proven block count"); diff --git a/l1-contracts/test/harnesses/InboxHarness.sol b/l1-contracts/test/harnesses/InboxHarness.sol index 8d51447906d..d0152d137af 100644 --- a/l1-contracts/test/harnesses/InboxHarness.sol +++ b/l1-contracts/test/harnesses/InboxHarness.sol @@ -3,11 +3,14 @@ pragma solidity >=0.8.18; import {Inbox} from "../../src/core/messagebridge/Inbox.sol"; +import {FrontierLib} from "../../src/core/messagebridge/frontier_tree/FrontierLib.sol"; // Libraries import {Constants} from "../../src/core/libraries/ConstantsGen.sol"; contract InboxHarness is Inbox { + using FrontierLib for FrontierLib.Tree; + constructor(address _rollup, uint256 _height) Inbox(_rollup, _height) {} function getSize() external view returns (uint256) { @@ -19,13 +22,13 @@ contract InboxHarness is Inbox { } function treeInProgressFull() external view returns (bool) { - return trees[inProgress].isFull(); + return trees[inProgress].isFull(SIZE); } function getToConsumeRoot(uint256 _toConsume) external view returns (bytes32) { bytes32 root = EMPTY_ROOT; if (_toConsume > Constants.INITIAL_L2_BLOCK_NUM) { - root = trees[_toConsume].root(); + root = trees[_toConsume].root(forest, HEIGHT, SIZE); } return root; }