Skip to content

Commit

Permalink
feat: New Outbox Contract #4768 (#5090)
Browse files Browse the repository at this point in the history
Resolves #4768

---------

Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
Co-authored-by: Jan Beneš <janbenes1234@gmail.com>
Co-authored-by: Santiago Palladino <santiago@aztecprotocol.com>
Co-authored-by: josh crites <critesjosh@gmail.com>
Co-authored-by: Aztec Bot <49558828+AztecBot@users.noreply.github.com>
  • Loading branch information
6 people authored Mar 15, 2024
1 parent 438d16f commit 6421a3d
Show file tree
Hide file tree
Showing 14 changed files with 856 additions and 56 deletions.
2 changes: 1 addition & 1 deletion l1-contracts/foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
src = 'src'
out = 'out'
libs = ['lib']
solc = "0.8.21"
solc = "0.8.23"

remappings = [
"@oz/=lib/openzeppelin-contracts/contracts/"
Expand Down
88 changes: 54 additions & 34 deletions l1-contracts/slither_output.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +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)
- [missing-zero-check](#missing-zero-check) (1 results) (Low)
- [missing-zero-check](#missing-zero-check) (2 results) (Low)
- [reentrancy-events](#reentrancy-events) (2 results) (Low)
- [timestamp](#timestamp) (4 results) (Low)
- [pess-public-vs-external](#pess-public-vs-external) (6 results) (Low)
- [pess-public-vs-external](#pess-public-vs-external) (7 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) (5 results) (Optimization)
- [pess-multiple-storage-read](#pess-multiple-storage-read) (6 results) (Optimization)
## pess-unprotected-setter
Impact: High
Confidence: Medium
Expand Down Expand Up @@ -136,10 +136,17 @@ Confidence: Medium
src/core/messagebridge/NewInbox.sol#L41


- [ ] ID-13
[NewOutbox.constructor(address)._rollup](src/core/messagebridge/NewOutbox.sol#L30) lacks a zero-check on :
- [ROLLUP_CONTRACT = _rollup](src/core/messagebridge/NewOutbox.sol#L31)

src/core/messagebridge/NewOutbox.sol#L30


## reentrancy-events
Impact: Low
Confidence: Medium
- [ ] ID-13
- [ ] ID-14
Reentrancy in [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/NewInbox.sol#L62-L99):
External calls:
- [index = currentTree.insertLeaf(leaf)](src/core/messagebridge/NewInbox.sol#L95)
Expand All @@ -149,7 +156,7 @@ Reentrancy in [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](s
src/core/messagebridge/NewInbox.sol#L62-L99


- [ ] ID-14
- [ ] ID-15
Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L58-L101):
External calls:
- [inbox.batchConsume(l1ToL2Msgs,msg.sender)](src/core/Rollup.sol#L90)
Expand All @@ -164,31 +171,31 @@ src/core/Rollup.sol#L58-L101
## timestamp
Impact: Low
Confidence: Medium
- [ ] ID-15
- [ ] ID-16
[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)

src/core/messagebridge/Inbox.sol#L122-L143


- [ ] ID-16
- [ ] ID-17
[HeaderLib.validate(HeaderLib.Header,uint256,uint256,bytes32)](src/core/libraries/HeaderLib.sol#L106-L136) uses timestamp for comparisons
Dangerous comparisons:
- [_header.globalVariables.timestamp > block.timestamp](src/core/libraries/HeaderLib.sol#L120)

src/core/libraries/HeaderLib.sol#L106-L136


- [ ] ID-17
- [ ] ID-18
[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)

src/core/messagebridge/Inbox.sol#L45-L91


- [ ] ID-18
- [ ] ID-19
[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)
Expand All @@ -199,28 +206,28 @@ src/core/messagebridge/Inbox.sol#L102-L113
## pess-public-vs-external
Impact: Low
Confidence: Medium
- [ ] ID-19
- [ ] ID-20
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-L93


- [ ] ID-20
- [ ] 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-21
- [ ] ID-22
The following public functions could be turned into external in [Rollup](src/core/Rollup.sol#L30-L110) contract:
[Rollup.constructor(IRegistry,IAvailabilityOracle)](src/core/Rollup.sol#L43-L49)

src/core/Rollup.sol#L30-L110


- [ ] ID-22
- [ ] 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)
Expand All @@ -229,15 +236,22 @@ The following public functions could be turned into external in [Outbox](src/cor
src/core/messagebridge/Outbox.sol#L21-L148


- [ ] ID-23
- [ ] 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)

src/core/messagebridge/Inbox.sol#L21-L231


- [ ] ID-24
- [ ] ID-25
The following public functions could be turned into external in [NewOutbox](src/core/messagebridge/NewOutbox.sol#L18-L131) contract:
[NewOutbox.constructor(address)](src/core/messagebridge/NewOutbox.sol#L30-L32)

src/core/messagebridge/NewOutbox.sol#L18-L131


- [ ] ID-26
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)

Expand All @@ -247,15 +261,15 @@ src/core/messagebridge/NewInbox.sol#L25-L128
## assembly
Impact: Informational
Confidence: High
- [ ] ID-25
- [ ] ID-27
[MessagesDecoder.decode(bytes)](src/core/libraries/decoders/MessagesDecoder.sol#L60-L142) uses assembly
- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L79-L81)
- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L112-L118)

src/core/libraries/decoders/MessagesDecoder.sol#L60-L142


- [ ] ID-26
- [ ] ID-28
[TxsDecoder.computeRoot(bytes32[])](src/core/libraries/decoders/TxsDecoder.sol#L256-L275) uses assembly
- [INLINE ASM](src/core/libraries/decoders/TxsDecoder.sol#L263-L265)

Expand All @@ -265,31 +279,31 @@ src/core/libraries/decoders/TxsDecoder.sol#L256-L275
## dead-code
Impact: Informational
Confidence: Medium
- [ ] ID-27
- [ ] ID-29
[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-28
- [ ] ID-30
[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-29
- [ ] ID-31
[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-30
- [ ] ID-32
[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-31
- [ ] ID-33
[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
Expand All @@ -298,13 +312,13 @@ src/core/messagebridge/Outbox.sol#L129-L147
## solc-version
Impact: Informational
Confidence: High
- [ ] ID-32
solc-0.8.21 is not recommended for deployment
- [ ] ID-34
solc-0.8.23 is not recommended for deployment

## low-level-calls
Impact: Informational
Confidence: High
- [ ] ID-33
- [ ] ID-35
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)

Expand All @@ -314,19 +328,19 @@ src/core/messagebridge/Inbox.sol#L148-L153
## similar-names
Impact: Informational
Confidence: Medium
- [ ] ID-34
- [ ] ID-36
Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L132) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L125)

src/core/libraries/ConstantsGen.sol#L132


- [ ] ID-35
- [ ] ID-37
Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L112) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L113)

src/core/libraries/ConstantsGen.sol#L112


- [ ] ID-36
- [ ] ID-38
Variable [Rollup.AVAILABILITY_ORACLE](src/core/Rollup.sol#L33) is too similar to [Rollup.constructor(IRegistry,IAvailabilityOracle)._availabilityOracle](src/core/Rollup.sol#L43)

src/core/Rollup.sol#L33
Expand All @@ -335,7 +349,7 @@ src/core/Rollup.sol#L33
## constable-states
Impact: Optimization
Confidence: High
- [ ] ID-37
- [ ] ID-39
[Rollup.lastWarpedBlockTs](src/core/Rollup.sol#L41) should be constant

src/core/Rollup.sol#L41
Expand All @@ -344,31 +358,37 @@ src/core/Rollup.sol#L41
## pess-multiple-storage-read
Impact: Optimization
Confidence: High
- [ ] ID-38
- [ ] ID-40
In a function [NewOutbox.insert(uint256,bytes32,uint256)](src/core/messagebridge/NewOutbox.sol#L43-L63) variable [NewOutbox.roots](src/core/messagebridge/NewOutbox.sol#L28) is read multiple times

src/core/messagebridge/NewOutbox.sol#L43-L63


- [ ] ID-41
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#L62-L99


- [ ] ID-39
- [ ] ID-42
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#L43-L76


- [ ] ID-40
- [ ] ID-43
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#L108-L127


- [ ] ID-41
- [ ] ID-44
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#L108-L127


- [ ] ID-42
- [ ] ID-45
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#L43-L76
Expand Down
62 changes: 62 additions & 0 deletions l1-contracts/src/core/interfaces/messagebridge/INewOutbox.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2024 Aztec Labs.
pragma solidity >=0.8.18;

import {DataStructures} from "../../libraries/DataStructures.sol";

/**
* @title INewOutbox
* @author Aztec Labs
* @notice Lives on L1 and is used to consume L2 -> L1 messages. Messages are inserted by the Rollup
* and will be consumed by the portal contracts.
*/
// TODO: rename to IOutbox once all the pieces of the new message model are in place.
interface INewOutbox {
event RootAdded(uint256 indexed l2BlockNumber, bytes32 indexed root, uint256 height);
event MessageConsumed(
uint256 indexed l2BlockNumber,
bytes32 indexed root,
bytes32 indexed messageHash,
uint256 leafIndex
);

/**
* @notice Inserts the root of a merkle tree containing all of the L2 to L1 messages in
* a block specified by _l2BlockNumber.
* @dev Only callable by the rollup contract
* @dev Emits `RootAdded` upon inserting the root successfully
* @param _l2BlockNumber - The L2 Block Number in which the L2 to L1 messages reside
* @param _root - The merkle root of the tree where all the L2 to L1 messages are leaves
* @param _height - The height of the merkle tree that the root corresponds to
*/
function insert(uint256 _l2BlockNumber, bytes32 _root, uint256 _height) external;

/**
* @notice Consumes an entry from the Outbox
* @dev Only useable by portals / recipients of messages
* @dev Emits `MessageConsumed` when consuming messages
* @param _l2BlockNumber - The block number specifying the block that contains the message we want to consume
* @param _leafIndex - The index inside the merkle tree where the message is located
* @param _message - The L2 to L1 message
* @param _path - The sibling path used to prove inclusion of the message, the _path length directly depends
* on the total amount of L2 to L1 messages in the block. i.e. the length of _path is equal to the depth of the
* L1 to L2 message tree.
*/
function consume(
uint256 _l2BlockNumber,
uint256 _leafIndex,
DataStructures.L2ToL1Msg calldata _message,
bytes32[] calldata _path
) external;

/**
* @notice Checks to see if an index of the L2 to L1 message tree for a specific block has been consumed
* @dev - This function does not throw. Out-of-bounds access is considered valid, but will always return false
* @param _l2BlockNumber - The block number specifying the block that contains the index of the message we want to check
* @param _leafIndex - The index of the message inside the merkle tree
*/
function hasMessageBeenConsumedAtBlockAndIndex(uint256 _l2BlockNumber, uint256 _leafIndex)
external
view
returns (bool);
}
9 changes: 9 additions & 0 deletions l1-contracts/src/core/libraries/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ library Errors {
uint32 storedDeadline,
uint32 deadlinePassed
); // 0x5e789f34
error Outbox__InvalidPathLength(uint256 expected, uint256 actual); // 0x481bcd9c
error Outbox__InsertingInvalidRoot(); // 0x73c2daca
error Outbox__RootAlreadySetAtBlock(uint256 l2BlockNumber); // 0x3eccfd3e
error Outbox__InvalidRecipient(address expected, address actual); // 0x57aad581
error Outbox__AlreadyNullified(uint256 l2BlockNumber, uint256 leafIndex); // 0xfd71c2d4
error Outbox__NothingToConsumeAtBlock(uint256 l2BlockNumber); // 0xa4508f22

// Rollup
error Rollup__InvalidArchive(bytes32 expected, bytes32 actual); // 0xb682a40e
Expand All @@ -63,4 +69,7 @@ library Errors {

// HeaderLib
error HeaderLib__InvalidHeaderSize(uint256 expected, uint256 actual); // 0xf3ccb247

// MerkleLib
error MerkleLib__InvalidRoot(bytes32 expected, bytes32 actual); // 0xb77e99
}
Loading

0 comments on commit 6421a3d

Please sign in to comment.