Skip to content

Commit

Permalink
feat: parity circuits (#5082)
Browse files Browse the repository at this point in the history
Fixes #4633

Note: The new model currently can't be used to consume the message on L2
because old model root is being inserted to the messages tree in root
rollup circuit. We can be sure the root computed by new model is correct
because it is already being set in the header and checked in the Rollup
contract. I will start inserting the new root in my next PR in which
I'll purge the old inbox.

---------

Co-authored-by: esau <152162806+sklppy88@users.noreply.github.com>
  • Loading branch information
benesjan and sklppy88 committed Mar 13, 2024
1 parent 2ee13b3 commit 335c46e
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 335c46e

Please sign in to comment.