Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: new Inbox #4880

Merged
merged 48 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
e2c9d1e
refactor: mocing types consts to constants.nr
benesjan Mar 4, 2024
92a39ed
feat: new Inbox
benesjan Mar 1, 2024
418e1c4
WIP
benesjan Mar 1, 2024
1ba2bd6
unifying naming with inbox and yellow paper
benesjan Mar 1, 2024
8019330
initial impl
benesjan Mar 1, 2024
b79e97f
test setup
benesjan Mar 1, 2024
23828d4
WIP
benesjan Mar 1, 2024
5b4cfb5
fix
benesjan Mar 4, 2024
0eedc35
testSendMultipleSameL2Messages
benesjan Mar 4, 2024
5de257d
WIP
benesjan Mar 4, 2024
00f4165
WIP
benesjan Mar 4, 2024
e769196
fix
benesjan Mar 4, 2024
2adfe77
slither
benesjan Mar 4, 2024
8bac58c
Re-introducing lag
benesjan Mar 4, 2024
3c16058
updated docs
benesjan Mar 4, 2024
164a7ea
setting deadline to max
benesjan Mar 4, 2024
a54fee9
indexing by block num
benesjan Mar 4, 2024
3d79858
using type(uint32).max instead of 2**32 - 1
benesjan Mar 4, 2024
0d970f2
event index fix
benesjan Mar 4, 2024
3228fb4
fix
benesjan Mar 4, 2024
83c9e38
sendL2Message instead of insert
benesjan Mar 4, 2024
334b74f
slither
benesjan Mar 5, 2024
9a6308d
_boundMessage
benesjan Mar 5, 2024
b4f1f3e
NewInboxHarness
benesjan Mar 5, 2024
8864c2b
slither
benesjan Mar 5, 2024
6ae47b2
WIP
benesjan Mar 5, 2024
88acc55
more consistent naming
benesjan Mar 5, 2024
abc3279
checking invariant
benesjan Mar 5, 2024
474692c
slither
benesjan Mar 5, 2024
6875f7e
linter fix
benesjan Mar 5, 2024
0937537
slither
benesjan Mar 5, 2024
3a556ef
cleanup
benesjan Mar 5, 2024
83038d7
using constant
benesjan Mar 5, 2024
4703b65
linking TODO
benesjan Mar 5, 2024
3ef3474
returning index instead of next index
benesjan Mar 5, 2024
6b6a7bb
IFrontier natspec
benesjan Mar 5, 2024
2b17303
cleanup of getNumTrees
benesjan Mar 5, 2024
a4e96d9
slither
benesjan Mar 5, 2024
5a2b9cb
making checkInvariant a modifier
benesjan Mar 5, 2024
d22b369
comment cleanup
benesjan Mar 5, 2024
74a0abe
WIP
benesjan Mar 5, 2024
a29b096
checking tree initialization
benesjan Mar 6, 2024
881627c
checking tree initialization when inserting
benesjan Mar 6, 2024
3e974f9
deshitification
benesjan Mar 6, 2024
66e219c
final touches
benesjan Mar 6, 2024
d3e6e80
slither output
benesjan Mar 6, 2024
1900f6e
better natspec
benesjan Mar 6, 2024
e3ac55d
exposing toConsume and inProgress
benesjan Mar 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 80 additions & 34 deletions l1-contracts/slither_output.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) (5 results) (Optimization)
## pess-unprotected-setter
Impact: High
Confidence: Medium
Expand Down Expand Up @@ -127,10 +128,20 @@ 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
[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#L41


## 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)
Expand All @@ -141,34 +152,44 @@ Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L5
src/core/Rollup.sol#L53-L91


- [ ] 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)
Event emitted after the call(s):
- [LeafInserted(inProgress,index,leaf)](src/core/messagebridge/NewInbox.sol#L96)

src/core/messagebridge/NewInbox.sol#L62-L99


## 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)

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)

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)

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)
Expand All @@ -179,28 +200,28 @@ 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 [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-L85
src/core/messagebridge/frontier_tree/Frontier.sol#L7-L93


- [ ] ID-18
- [ ] 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-19
- [ ] 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-20
- [ ] 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)
Expand All @@ -209,26 +230,33 @@ The following public functions could be turned into external in [Outbox](src/cor
src/core/messagebridge/Outbox.sol#L21-L148


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

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-L128) contract:
[NewInbox.constructor(address,uint256)](src/core/messagebridge/NewInbox.sol#L41-L52)

src/core/messagebridge/NewInbox.sol#L25-L128


## 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)

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)

Expand All @@ -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
Expand All @@ -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)

Expand All @@ -287,19 +315,19 @@ 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


- [ ] 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
Expand All @@ -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
Expand All @@ -317,15 +345,33 @@ 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 [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/frontier_tree/Frontier.sol#L39-L72
src/core/messagebridge/NewInbox.sol#L62-L99


- [ ] 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 [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
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
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
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#L39-L72
src/core/messagebridge/frontier_tree/Frontier.sol#L43-L76


17 changes: 16 additions & 1 deletion l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,22 @@
pragma solidity >=0.8.18;

interface IFrontier {
function insertLeaf(bytes32 _leaf) external;
/**
* @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 True if full, false otherwise
*/
function isFull() external view returns (bool);
}
38 changes: 38 additions & 0 deletions l1-contracts/src/core/interfaces/messagebridge/INewInbox.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// 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);

/**
* @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);

/**
* @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);
}
Loading
Loading