Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
79 changes: 51 additions & 28 deletions l1-contracts/slither_output.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ Summary
- [pess-dubious-typecast](#pess-dubious-typecast) (8 results) (Medium)
- [reentrancy-events](#reentrancy-events) (1 results) (Low)
- [timestamp](#timestamp) (4 results) (Low)
- [pess-public-vs-external](#pess-public-vs-external) (4 results) (Low)
- [pess-public-vs-external](#pess-public-vs-external) (5 results) (Low)
- [assembly](#assembly) (5 results) (Informational)
- [dead-code](#dead-code) (13 results) (Informational)
- [solc-version](#solc-version) (1 results) (Informational)
- [low-level-calls](#low-level-calls) (1 results) (Informational)
- [similar-names](#similar-names) (1 results) (Informational)
- [unused-state](#unused-state) (2 results) (Informational)
- [constable-states](#constable-states) (1 results) (Optimization)
- [pess-multiple-storage-read](#pess-multiple-storage-read) (2 results) (Optimization)
## pess-unprotected-setter
Impact: High
Confidence: Medium
Expand Down Expand Up @@ -171,20 +172,27 @@ src/core/messagebridge/Inbox.sol#L102-L113
Impact: Low
Confidence: Medium
- [ ] ID-16
The following public functions could be turned into external in [FrontierMerkle](src/core/messagebridge/frontier_tree/Frontier.sol#L7-L85) contract:
[FrontierMerkle.constructor(uint256)](src/core/messagebridge/frontier_tree/Frontier.sol#L19-L27)

src/core/messagebridge/frontier_tree/Frontier.sol#L7-L85


- [ ] ID-17
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-17
- [ ] ID-18
The following public functions could be turned into external in [Rollup](src/core/Rollup.sol#L27-L103) contract:
[Rollup.constructor(IRegistry,IAvailabilityOracle)](src/core/Rollup.sol#L39-L44)

src/core/Rollup.sol#L27-L103


- [ ] ID-18
- [ ] ID-19
The following public functions could be turned into external in [Outbox](src/core/messagebridge/Outbox.sol#L21-L149) contract:
[Outbox.constructor(address)](src/core/messagebridge/Outbox.sol#L29-L31)
[Outbox.get(bytes32)](src/core/messagebridge/Outbox.sol#L78-L85)
Expand All @@ -193,7 +201,7 @@ The following public functions could be turned into external in [Outbox](src/cor
src/core/messagebridge/Outbox.sol#L21-L149


- [ ] ID-19
- [ ] ID-20
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)
Expand All @@ -204,36 +212,36 @@ src/core/messagebridge/Inbox.sol#L21-L231
## assembly
Impact: Informational
Confidence: High
- [ ] ID-20
- [ ] ID-21
[Decoder.computeRoot(bytes32[])](src/core/libraries/decoders/Decoder.sol#L373-L392) uses assembly
- [INLINE ASM](src/core/libraries/decoders/Decoder.sol#L380-L382)

src/core/libraries/decoders/Decoder.sol#L373-L392


- [ ] ID-21
- [ ] ID-22
[TxsDecoder.decode(bytes)](src/core/libraries/decoders/TxsDecoder.sol#L71-L184) uses assembly
- [INLINE ASM](src/core/libraries/decoders/TxsDecoder.sol#L98-L104)

src/core/libraries/decoders/TxsDecoder.sol#L71-L184


- [ ] ID-22
- [ ] ID-23
[Decoder.computeConsumables(bytes)](src/core/libraries/decoders/Decoder.sol#L164-L301) uses assembly
- [INLINE ASM](src/core/libraries/decoders/Decoder.sol#L196-L202)
- [INLINE ASM](src/core/libraries/decoders/Decoder.sol#L289-L295)

src/core/libraries/decoders/Decoder.sol#L164-L301


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

src/core/libraries/decoders/TxsDecoder.sol#L256-L275


- [ ] ID-24
- [ ] ID-25
[MessagesDecoder.decode(bytes)](src/core/libraries/decoders/MessagesDecoder.sol#L52-L102) uses assembly
- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L81-L83)
- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L94-L96)
Expand All @@ -244,79 +252,79 @@ src/core/libraries/decoders/MessagesDecoder.sol#L52-L102
## dead-code
Impact: Informational
Confidence: Medium
- [ ] ID-25
- [ ] ID-26
[Decoder.computeConsumables(bytes)](src/core/libraries/decoders/Decoder.sol#L164-L301) is never used and should be removed

src/core/libraries/decoders/Decoder.sol#L164-L301


- [ ] ID-26
- [ ] 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-27
- [ ] ID-28
[Decoder.slice(bytes,uint256,uint256)](src/core/libraries/decoders/Decoder.sol#L401-L407) is never used and should be removed

src/core/libraries/decoders/Decoder.sol#L401-L407


- [ ] ID-28
- [ ] ID-29
[Outbox._errNothingToConsume(bytes32)](src/core/messagebridge/Outbox.sol#L115-L117) is never used and should be removed

src/core/messagebridge/Outbox.sol#L115-L117


- [ ] ID-29
- [ ] ID-30
[Decoder.computeRoot(bytes32[])](src/core/libraries/decoders/Decoder.sol#L373-L392) is never used and should be removed

src/core/libraries/decoders/Decoder.sol#L373-L392


- [ ] ID-30
- [ ] 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-31
- [ ] ID-32
[Decoder.computeKernelLogsHash(uint256,bytes)](src/core/libraries/decoders/Decoder.sol#L335-L365) is never used and should be removed

src/core/libraries/decoders/Decoder.sol#L335-L365


- [ ] ID-32
- [ ] ID-33
[Decoder.read4(bytes,uint256)](src/core/libraries/decoders/Decoder.sol#L415-L417) is never used and should be removed

src/core/libraries/decoders/Decoder.sol#L415-L417


- [ ] ID-33
- [ ] ID-34
[Decoder.computeStateHash(uint256,uint256,bytes)](src/core/libraries/decoders/Decoder.sol#L146-L154) is never used and should be removed

src/core/libraries/decoders/Decoder.sol#L146-L154


- [ ] ID-34
- [ ] ID-35
[Decoder.computePublicInputHash(bytes,bytes32,bytes32)](src/core/libraries/decoders/Decoder.sol#L118-L125) is never used and should be removed

src/core/libraries/decoders/Decoder.sol#L118-L125


- [ ] ID-35
- [ ] ID-36
[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-36
- [ ] ID-37
[Decoder.getL2BlockNumber(bytes)](src/core/libraries/decoders/Decoder.sol#L132-L134) is never used and should be removed

src/core/libraries/decoders/Decoder.sol#L132-L134


- [ ] ID-37
- [ ] ID-38
[Outbox._errIncompatibleEntryArguments(bytes32,uint64,uint64,uint32,uint32,uint32,uint32)](src/core/messagebridge/Outbox.sol#L130-L148) is never used and should be removed

src/core/messagebridge/Outbox.sol#L130-L148
Expand All @@ -325,13 +333,13 @@ src/core/messagebridge/Outbox.sol#L130-L148
## solc-version
Impact: Informational
Confidence: High
- [ ] ID-38
- [ ] ID-39
solc-0.8.21 is not recommended for deployment

## low-level-calls
Impact: Informational
Confidence: High
- [ ] ID-39
- [ ] ID-40
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 @@ -341,7 +349,7 @@ src/core/messagebridge/Inbox.sol#L148-L153
## similar-names
Impact: Informational
Confidence: Medium
- [ ] ID-40
- [ ] ID-41
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 @@ -350,13 +358,13 @@ src/core/Rollup.sol#L30
## unused-state
Impact: Informational
Confidence: High
- [ ] ID-41
- [ ] ID-42
[Decoder.END_TREES_BLOCK_HEADER_OFFSET](src/core/libraries/decoders/Decoder.sol#L103-L104) is never used in [Decoder](src/core/libraries/decoders/Decoder.sol#L72-L418)

src/core/libraries/decoders/Decoder.sol#L103-L104


- [ ] ID-42
- [ ] ID-43
[Decoder.BLOCK_HEADER_OFFSET](src/core/libraries/decoders/Decoder.sol#L107-L108) is never used in [Decoder](src/core/libraries/decoders/Decoder.sol#L72-L418)

src/core/libraries/decoders/Decoder.sol#L107-L108
Expand All @@ -365,9 +373,24 @@ src/core/libraries/decoders/Decoder.sol#L107-L108
## constable-states
Impact: Optimization
Confidence: High
- [ ] ID-43
- [ ] ID-44
[Rollup.lastWarpedBlockTs](src/core/Rollup.sol#L37) should be constant

src/core/Rollup.sol#L37


## pess-multiple-storage-read
Impact: Optimization
Confidence: High
- [ ] ID-45
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

src/core/messagebridge/frontier_tree/Frontier.sol#L39-L72


- [ ] ID-46
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

src/core/messagebridge/frontier_tree/Frontier.sol#L39-L72


9 changes: 9 additions & 0 deletions l1-contracts/src/core/interfaces/messagebridge/IFrontier.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2023 Aztec Labs.
pragma solidity >=0.8.18;

interface IFrontier {
function insertLeaf(bytes32 _leaf) external;

function root() external view returns (bytes32);
}
85 changes: 85 additions & 0 deletions l1-contracts/src/core/messagebridge/frontier_tree/Frontier.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2023 Aztec Labs.
pragma solidity >=0.8.18;

import {IFrontier} from "../../interfaces/messagebridge/IFrontier.sol";

contract FrontierMerkle is IFrontier {
uint256 public immutable DEPTH;
uint256 public immutable SIZE;

uint256 internal nextIndex = 0;

mapping(uint256 level => bytes32 node) public frontier;

// 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;

constructor(uint256 _depth) {
DEPTH = _depth;
SIZE = 2 ** _depth;

zeros[0] = bytes32(0);
for (uint256 i = 1; i <= DEPTH; i++) {
zeros[i] = sha256(bytes.concat(zeros[i - 1], zeros[i - 1]));
}
}

function insertLeaf(bytes32 _leaf) external override(IFrontier) {
uint256 level = _computeLevel(nextIndex);
bytes32 right = _leaf;
for (uint256 i = 0; i < level; i++) {
right = sha256(bytes.concat(frontier[i], right));
}
frontier[level] = right;
nextIndex++;
}

function root() external view override(IFrontier) returns (bytes32) {
uint256 next = nextIndex;
if (next == 0) {
return zeros[DEPTH];
}
if (next == SIZE) {
return frontier[DEPTH];
}

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 < DEPTH; 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 = sha256(bytes.concat(frontier[i], temp));
} else {
temp = sha256(bytes.concat(temp, zeros[i]));
}
bits >>= 1;
}

return temp;
}

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;
}
}
30 changes: 30 additions & 0 deletions l1-contracts/test/merkle/Merkle.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2023 Aztec Labs.
pragma solidity >=0.8.18;

import {Test} from "forge-std/Test.sol";

import {NaiveMerkle} from "./Naive.sol";
import {FrontierMerkle} from "./../../src/core/messagebridge/frontier_tree/Frontier.sol";

contract MerkleTest is Test {
NaiveMerkle internal merkle;
FrontierMerkle internal frontier;

uint256 public constant DEPTH = 10;

function setUp() public {
merkle = new NaiveMerkle(DEPTH);
frontier = new FrontierMerkle(DEPTH);
}

function testFrontier() public {
uint256 upper = frontier.SIZE();
for (uint256 i = 0; i < upper; i++) {
bytes32 leaf = sha256(abi.encode(i + 1));
merkle.insertLeaf(leaf);
frontier.insertLeaf(leaf);
assertEq(merkle.computeRoot(), frontier.root(), "Frontier Roots should be equal");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool way to test this 👍

}
}
}
Loading