Skip to content

Commit

Permalink
Merge 50a6a48 into d666f6f
Browse files Browse the repository at this point in the history
  • Loading branch information
benesjan authored Mar 13, 2024
2 parents d666f6f + 50a6a48 commit 34394c0
Show file tree
Hide file tree
Showing 65 changed files with 1,135 additions and 199 deletions.
108 changes: 51 additions & 57 deletions l1-contracts/slither_output.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Summary
- [pess-unprotected-setter](#pess-unprotected-setter) (1 results) (High)
- [uninitialized-local](#uninitialized-local) (2 results) (Medium)
- [unused-return](#unused-return) (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)
- [reentrancy-events](#reentrancy-events) (2 results) (Low)
Expand All @@ -18,9 +18,9 @@ Summary
Impact: High
Confidence: Medium
- [ ] ID-0
Function [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L58-L103) is a non-protected setter archive is written
Function [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L58-L101) is a non-protected setter archive is written

src/core/Rollup.sol#L58-L103
src/core/Rollup.sol#L58-L101


## uninitialized-local
Expand All @@ -42,64 +42,58 @@ src/core/libraries/decoders/TxsDecoder.sol#L79
Impact: Medium
Confidence: Medium
- [ ] ID-3
[Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L58-L103) ignores return value by [NEW_INBOX.consume()](src/core/Rollup.sol#L93)
[Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L58-L101) ignores return value by [(l1ToL2Msgs,l2ToL1Msgs) = MessagesDecoder.decode(_body)](src/core/Rollup.sol#L74)

src/core/Rollup.sol#L58-L103


- [ ] ID-4
[Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L58-L103) ignores return value by [(l1ToL2Msgs,l2ToL1Msgs) = MessagesDecoder.decode(_body)](src/core/Rollup.sol#L74)

src/core/Rollup.sol#L58-L103
src/core/Rollup.sol#L58-L101


## pess-dubious-typecast
Impact: Medium
Confidence: High
- [ ] ID-5
- [ ] ID-4
Dubious typecast in [TxsDecoder.read1(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L314-L316):
bytes => bytes1 casting occurs in [uint256(uint8(bytes1(slice(_data,_offset,1))))](src/core/libraries/decoders/TxsDecoder.sol#L315)

src/core/libraries/decoders/TxsDecoder.sol#L314-L316


- [ ] ID-6
- [ ] ID-5
Dubious typecast in [Outbox.sendL1Messages(bytes32[])](src/core/messagebridge/Outbox.sol#L38-L46):
uint256 => uint32 casting occurs in [version = uint32(REGISTRY.getVersionFor(msg.sender))](src/core/messagebridge/Outbox.sol#L40)

src/core/messagebridge/Outbox.sol#L38-L46


- [ ] ID-7
- [ ] ID-6
Dubious typecast in [Inbox.sendL2Message(DataStructures.L2Actor,uint32,bytes32,bytes32)](src/core/messagebridge/Inbox.sol#L45-L91):
uint256 => uint64 casting occurs in [fee = uint64(msg.value)](src/core/messagebridge/Inbox.sol#L64)
uint256 => uint32 casting occurs in [entries.insert(key,fee,uint32(_recipient.version),_deadline,_errIncompatibleEntryArguments)](src/core/messagebridge/Inbox.sol#L76)

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


- [ ] ID-8
- [ ] ID-7
Dubious typecast in [TxsDecoder.read4(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L324-L326):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(slice(_data,_offset,4))))](src/core/libraries/decoders/TxsDecoder.sol#L325)

src/core/libraries/decoders/TxsDecoder.sol#L324-L326


- [ ] ID-9
- [ ] ID-8
Dubious typecast in [MessagesDecoder.read4(bytes,uint256)](src/core/libraries/decoders/MessagesDecoder.sol#L160-L162):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(_data)))](src/core/libraries/decoders/MessagesDecoder.sol#L161)

src/core/libraries/decoders/MessagesDecoder.sol#L160-L162


- [ ] ID-10
- [ ] ID-9
Dubious typecast in [Inbox.batchConsume(bytes32[],address)](src/core/messagebridge/Inbox.sol#L122-L143):
uint256 => uint32 casting occurs in [expectedVersion = uint32(REGISTRY.getVersionFor(msg.sender))](src/core/messagebridge/Inbox.sol#L128)

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


- [ ] ID-11
- [ ] ID-10
Dubious typecast in [HeaderLib.decode(bytes)](src/core/libraries/HeaderLib.sol#L143-L184):
bytes => bytes32 casting occurs in [header.lastArchive = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L151-L153)
bytes => bytes4 casting occurs in [header.lastArchive = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L151-L153)
Expand All @@ -125,7 +119,7 @@ Dubious typecast in [HeaderLib.decode(bytes)](src/core/libraries/HeaderLib.sol#L
src/core/libraries/HeaderLib.sol#L143-L184


- [ ] ID-12
- [ ] ID-11
Dubious typecast in [MessagesDecoder.read1(bytes,uint256)](src/core/libraries/decoders/MessagesDecoder.sol#L150-L152):
bytes => bytes1 casting occurs in [uint256(uint8(bytes1(_data)))](src/core/libraries/decoders/MessagesDecoder.sol#L151)

Expand All @@ -135,7 +129,7 @@ src/core/libraries/decoders/MessagesDecoder.sol#L150-L152
## missing-zero-check
Impact: Low
Confidence: Medium
- [ ] ID-13
- [ ] 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)

Expand All @@ -145,7 +139,7 @@ src/core/messagebridge/NewInbox.sol#L41
## reentrancy-events
Impact: Low
Confidence: Medium
- [ ] ID-14
- [ ] ID-13
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 @@ -155,46 +149,46 @@ Reentrancy in [NewInbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](s
src/core/messagebridge/NewInbox.sol#L62-L99


- [ ] ID-15
Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L58-L103):
- [ ] ID-14
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)
- [NEW_INBOX.consume()](src/core/Rollup.sol#L93)
- [outbox.sendL1Messages(l2ToL1Msgs)](src/core/Rollup.sol#L100)
- [inHash = NEW_INBOX.consume()](src/core/Rollup.sol#L92)
- [outbox.sendL1Messages(l2ToL1Msgs)](src/core/Rollup.sol#L98)
Event emitted after the call(s):
- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L102)
- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L100)

src/core/Rollup.sol#L58-L103
src/core/Rollup.sol#L58-L101


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


- [ ] 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
The following public functions could be turned into external in [Rollup](src/core/Rollup.sol#L30-L112) contract:
- [ ] ID-21
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-L112
src/core/Rollup.sol#L30-L110


- [ ] 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)
Expand All @@ -235,15 +229,15 @@ 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)

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


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

Expand All @@ -253,15 +247,15 @@ src/core/messagebridge/NewInbox.sol#L25-L128
## assembly
Impact: Informational
Confidence: High
- [ ] ID-26
- [ ] ID-25
[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-27
- [ ] ID-26
[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 @@ -271,31 +265,31 @@ src/core/libraries/decoders/TxsDecoder.sol#L256-L275
## dead-code
Impact: Informational
Confidence: Medium
- [ ] ID-28
- [ ] 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-29
- [ ] 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-30
- [ ] 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-31
- [ ] 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-32
- [ ] 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 @@ -304,13 +298,13 @@ src/core/messagebridge/Outbox.sol#L129-L147
## solc-version
Impact: Informational
Confidence: High
- [ ] ID-33
- [ ] ID-32
solc-0.8.21 is not recommended for deployment

## low-level-calls
Impact: Informational
Confidence: High
- [ ] ID-34
- [ ] 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 @@ -320,19 +314,19 @@ src/core/messagebridge/Inbox.sol#L148-L153
## similar-names
Impact: Informational
Confidence: Medium
- [ ] ID-35
- [ ] ID-34
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-36
- [ ] ID-35
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-37
- [ ] ID-36
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 @@ -341,7 +335,7 @@ src/core/Rollup.sol#L33
## constable-states
Impact: Optimization
Confidence: High
- [ ] ID-38
- [ ] ID-37
[Rollup.lastWarpedBlockTs](src/core/Rollup.sol#L41) should be constant

src/core/Rollup.sol#L41
Expand All @@ -350,31 +344,31 @@ src/core/Rollup.sol#L41
## pess-multiple-storage-read
Impact: Optimization
Confidence: High
- [ ] ID-39
- [ ] 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/NewInbox.sol#L62-L99


- [ ] ID-40
- [ ] 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-41
- [ ] 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-42
- [ ] 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-43
- [ ] 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#L43-L76
Expand Down
10 changes: 4 additions & 6 deletions l1-contracts/src/core/Rollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,10 @@ contract Rollup is IRollup {
IInbox inbox = REGISTRY.getInbox();
inbox.batchConsume(l1ToL2Msgs, msg.sender);

// TODO(#4633): enable the inHash check
NEW_INBOX.consume();
// bytes32 inHash = NEW_INBOX.consume();
// if (header.contentCommitment.inHash != inHash) {
// revert Errors.Rollup__InvalidInHash(inHash, header.contentCommitment.inHash);
// }
bytes32 inHash = NEW_INBOX.consume();
if (header.contentCommitment.inHash != inHash) {
revert Errors.Rollup__InvalidInHash(inHash, header.contentCommitment.inHash);
}

IOutbox outbox = REGISTRY.getOutbox();
outbox.sendL1Messages(l2ToL1Msgs);
Expand Down
2 changes: 2 additions & 0 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,6 @@ library Constants {
uint256 internal constant CONTRACT_DATA_NUM_BYTES_PER_BASE_ROLLUP_UNPADDED = 52;
uint256 internal constant L2_TO_L1_MSGS_NUM_BYTES_PER_BASE_ROLLUP = 64;
uint256 internal constant LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP = 64;
uint256 internal constant NUM_MSGS_PER_BASE_PARITY = 4;
uint256 internal constant NUM_BASE_PARITY_PER_ROOT_PARITY = 4;
}
Loading

0 comments on commit 34394c0

Please sign in to comment.