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.

Sign up for GitHub

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

Jump to bottom

chore: reduce size of revert code from Field to u8 #5309

Merged
merged 1 commit into from
Mar 20, 2024
Merged

chore: reduce size of revert code from Field to u8 #5309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
93 changes: 43 additions & 50 deletions l1-contracts/slither_output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Summary
- [pess-unprotected-setter](#pess-unprotected-setter) (1 results) (High)
- [uninitialized-local](#uninitialized-local) (2 results) (Medium)
- [unused-return](#unused-return) (1 results) (Medium)
- [pess-dubious-typecast](#pess-dubious-typecast) (7 results) (Medium)
- [pess-dubious-typecast](#pess-dubious-typecast) (6 results) (Medium)
- [missing-zero-check](#missing-zero-check) (2 results) (Low)
- [reentrancy-events](#reentrancy-events) (2 results) (Low)
- [timestamp](#timestamp) (1 results) (Low)
Expand Down Expand Up @@ -50,31 +50,31 @@ src/core/Rollup.sol#L57-L96
Impact: Medium
Confidence: High
- [ ] ID-4
Dubious typecast in [MessagesDecoder.read4(bytes,uint256)](src/core/libraries/decoders/MessagesDecoder.sol#L164-L166):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(_data)))](src/core/libraries/decoders/MessagesDecoder.sol#L165)
Dubious typecast in [TxsDecoder.read1(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L341-L343):
bytes => bytes1 casting occurs in [uint256(uint8(bytes1(slice(_data,_offset,1))))](src/core/libraries/decoders/TxsDecoder.sol#L342)

src/core/libraries/decoders/MessagesDecoder.sol#L164-L166
src/core/libraries/decoders/TxsDecoder.sol#L341-L343


- [ ] 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)
Dubious typecast in [TxsDecoder.read4(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L351-L353):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(slice(_data,_offset,4))))](src/core/libraries/decoders/TxsDecoder.sol#L352)

src/core/messagebridge/Outbox.sol#L38-L46
src/core/libraries/decoders/TxsDecoder.sol#L351-L353


- [ ] ID-6
Dubious typecast in [TxsDecoder.read1(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L322-L324):
bytes => bytes1 casting occurs in [uint256(uint8(bytes1(slice(_data,_offset,1))))](src/core/libraries/decoders/TxsDecoder.sol#L323)
Dubious typecast in [MessagesDecoder.read4(bytes,uint256)](src/core/libraries/decoders/MessagesDecoder.sol#L164-L166):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(_data)))](src/core/libraries/decoders/MessagesDecoder.sol#L165)

src/core/libraries/decoders/TxsDecoder.sol#L322-L324
src/core/libraries/decoders/MessagesDecoder.sol#L164-L166


- [ ] ID-7
Dubious typecast in [TxsDecoder.read4(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L332-L334):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(slice(_data,_offset,4))))](src/core/libraries/decoders/TxsDecoder.sol#L333)
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/libraries/decoders/TxsDecoder.sol#L332-L334
src/core/messagebridge/Outbox.sol#L38-L46


- [ ] ID-8
Expand Down Expand Up @@ -104,13 +104,6 @@ src/core/libraries/HeaderLib.sol#L143-L184


- [ ] ID-9
Dubious typecast in [TxsDecoder.decode(bytes)](src/core/libraries/decoders/TxsDecoder.sol#L78-L192):
bytes => bytes32 casting occurs in [vars.baseLeaf = bytes.concat(bytes32(slice(_body,offsets.revertCode,0x20)),bytes.concat(sliceAndPad(_body,offsets.noteHash,counts.noteHash * 0x20,Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP),sliceAndPad(_body,offsets.nullifier,counts.nullifier * 0x20,Constants.NULLIFIERS_NUM_BYTES_PER_BASE_ROLLUP),sliceAndPad(_body,offsets.l2ToL1Msgs,counts.l2ToL1Msgs * 0x20,Constants.L2_TO_L1_MSGS_NUM_BYTES_PER_BASE_ROLLUP),sliceAndPad(_body,offsets.publicData,counts.publicData * 0x40,Constants.PUBLIC_DATA_WRITES_NUM_BYTES_PER_BASE_ROLLUP)),bytes.concat(vars.encryptedLogsHash,vars.unencryptedLogsHash))](src/core/libraries/decoders/TxsDecoder.sol#L156-L185)

src/core/libraries/decoders/TxsDecoder.sol#L78-L192


- [ ] ID-10
Dubious typecast in [MessagesDecoder.read1(bytes,uint256)](src/core/libraries/decoders/MessagesDecoder.sol#L154-L156):
bytes => bytes1 casting occurs in [uint256(uint8(bytes1(_data)))](src/core/libraries/decoders/MessagesDecoder.sol#L155)

Expand All @@ -120,14 +113,14 @@ src/core/libraries/decoders/MessagesDecoder.sol#L154-L156
## missing-zero-check
Impact: Low
Confidence: Medium
- [ ] ID-11
- [ ] ID-10
[Inbox.constructor(address,uint256)._rollup](src/core/messagebridge/Inbox.sol#L40) lacks a zero-check on :
- [ROLLUP = _rollup](src/core/messagebridge/Inbox.sol#L41)

src/core/messagebridge/Inbox.sol#L40


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

Expand All @@ -137,7 +130,7 @@ src/core/messagebridge/NewOutbox.sol#L31
## reentrancy-events
Impact: Low
Confidence: Medium
- [ ] ID-13
- [ ] ID-12
Reentrancy in [Inbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/Inbox.sol#L61-L95):
External calls:
- [index = currentTree.insertLeaf(leaf)](src/core/messagebridge/Inbox.sol#L91)
Expand All @@ -147,7 +140,7 @@ Reentrancy in [Inbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/
src/core/messagebridge/Inbox.sol#L61-L95


- [ ] ID-14
- [ ] ID-13
Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L57-L96):
External calls:
- [inHash = INBOX.consume()](src/core/Rollup.sol#L87)
Expand All @@ -161,7 +154,7 @@ src/core/Rollup.sol#L57-L96
## timestamp
Impact: Low
Confidence: Medium
- [ ] ID-15
- [ ] ID-14
[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)
Expand All @@ -172,35 +165,35 @@ src/core/libraries/HeaderLib.sol#L106-L136
## pess-public-vs-external
Impact: Low
Confidence: Medium
- [ ] ID-16
- [ ] ID-15
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-17
- [ ] ID-16
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-18
- [ ] ID-17
The following public functions could be turned into external in [Inbox](src/core/messagebridge/Inbox.sol#L24-L124) contract:
[Inbox.constructor(address,uint256)](src/core/messagebridge/Inbox.sol#L40-L51)

src/core/messagebridge/Inbox.sol#L24-L124


- [ ] ID-19
- [ ] ID-18
The following public functions could be turned into external in [Rollup](src/core/Rollup.sol#L29-L105) contract:
[Rollup.constructor(IRegistry,IAvailabilityOracle)](src/core/Rollup.sol#L42-L48)

src/core/Rollup.sol#L29-L105


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


- [ ] ID-21
- [ ] ID-20
The following public functions could be turned into external in [NewOutbox](src/core/messagebridge/NewOutbox.sol#L18-L132) contract:
[NewOutbox.constructor(address)](src/core/messagebridge/NewOutbox.sol#L31-L33)

Expand All @@ -219,37 +212,37 @@ src/core/messagebridge/NewOutbox.sol#L18-L132
## assembly
Impact: Informational
Confidence: High
- [ ] ID-22
- [ ] ID-21
[MessagesDecoder.decode(bytes)](src/core/libraries/decoders/MessagesDecoder.sol#L61-L146) uses assembly
- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L80-L82)
- [INLINE ASM](src/core/libraries/decoders/MessagesDecoder.sol#L116-L122)

src/core/libraries/decoders/MessagesDecoder.sol#L61-L146


- [ ] ID-23
[TxsDecoder.computeRoot(bytes32[])](src/core/libraries/decoders/TxsDecoder.sol#L264-L283) uses assembly
- [INLINE ASM](src/core/libraries/decoders/TxsDecoder.sol#L271-L273)
- [ ] ID-22
[TxsDecoder.computeRoot(bytes32[])](src/core/libraries/decoders/TxsDecoder.sol#L265-L284) uses assembly
- [INLINE ASM](src/core/libraries/decoders/TxsDecoder.sol#L272-L274)

src/core/libraries/decoders/TxsDecoder.sol#L264-L283
src/core/libraries/decoders/TxsDecoder.sol#L265-L284


## dead-code
Impact: Informational
Confidence: Medium
- [ ] ID-24
- [ ] ID-23
[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-25
- [ ] ID-24
[Hash.sha256ToField(bytes32)](src/core/libraries/Hash.sol#L52-L54) is never used and should be removed

src/core/libraries/Hash.sol#L52-L54


- [ ] ID-26
- [ ] ID-25
[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 @@ -258,25 +251,25 @@ src/core/messagebridge/Outbox.sol#L129-L147
## solc-version
Impact: Informational
Confidence: High
- [ ] ID-27
- [ ] ID-26
solc-0.8.23 is not recommended for deployment

## similar-names
Impact: Informational
Confidence: Medium
- [ ] ID-28
- [ ] ID-27
Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L130) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L123)

src/core/libraries/ConstantsGen.sol#L130


- [ ] ID-29
- [ ] ID-28
Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L110) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L111)

src/core/libraries/ConstantsGen.sol#L110


- [ ] ID-30
- [ ] ID-29
Variable [Rollup.AVAILABILITY_ORACLE](src/core/Rollup.sol#L32) is too similar to [Rollup.constructor(IRegistry,IAvailabilityOracle)._availabilityOracle](src/core/Rollup.sol#L42)

src/core/Rollup.sol#L32
Expand All @@ -285,7 +278,7 @@ src/core/Rollup.sol#L32
## constable-states
Impact: Optimization
Confidence: High
- [ ] ID-31
- [ ] ID-30
[Rollup.lastWarpedBlockTs](src/core/Rollup.sol#L40) should be constant

src/core/Rollup.sol#L40
Expand All @@ -294,37 +287,37 @@ src/core/Rollup.sol#L40
## pess-multiple-storage-read
Impact: Optimization
Confidence: High
- [ ] ID-32
- [ ] ID-31
In a function [NewOutbox.insert(uint256,bytes32,uint256)](src/core/messagebridge/NewOutbox.sol#L44-L64) variable [NewOutbox.roots](src/core/messagebridge/NewOutbox.sol#L29) is read multiple times

src/core/messagebridge/NewOutbox.sol#L44-L64


- [ ] ID-33
- [ ] ID-32
In a function [Inbox.consume()](src/core/messagebridge/Inbox.sol#L104-L123) variable [Inbox.toConsume](src/core/messagebridge/Inbox.sol#L34) is read multiple times

src/core/messagebridge/Inbox.sol#L104-L123


- [ ] ID-34
- [ ] ID-33
In a function [Inbox.consume()](src/core/messagebridge/Inbox.sol#L104-L123) variable [Inbox.inProgress](src/core/messagebridge/Inbox.sol#L36) is read multiple times

src/core/messagebridge/Inbox.sol#L104-L123


- [ ] ID-35
- [ ] ID-34
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-36
- [ ] ID-35
In a function [Inbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/Inbox.sol#L61-L95) variable [Inbox.inProgress](src/core/messagebridge/Inbox.sol#L36) is read multiple times

src/core/messagebridge/Inbox.sol#L61-L95


- [ ] ID-37
- [ ] ID-36
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
28 changes: 14 additions & 14 deletions l1-contracts/src/core/libraries/decoders/MessagesDecoder.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,19 @@ import {Hash} from "../Hash.sol";
* | 0x4 | a * 0x20 | newL1ToL2Msgs
* | 0x4 + a * 0x20 = tx0Start | 0x4 | len(numTxs) (denoted t)
* | | | TxEffect 0 {
* | tx0Start | 0x20 | revertCode
* | tx0Start + 0x20 | 0x1 | len(newNoteHashes) (denoted b)
* | tx0Start + 0x20 + 0x1 | b * 0x20 | newNoteHashes
* | tx0Start + 0x20 + 0x1 + b * 0x20 | 0x1 | len(newNullifiers) (denoted c)
* | tx0Start + 0x20 + 0x1 + b * 0x20 + 0x1 | c * 0x20 | newNullifiers
* | tx0Start + 0x20 + 0x1 + b * 0x20 + 0x1 + c * 0x20 | 0x1 | len(newL2ToL1Msgs) (denoted d)
* | tx0Start + 0x20 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 | d * 0x20 | newL2ToL1Msgs
* | tx0Start + 0x20 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 | 0x1 | len(newPublicDataWrites) (denoted e)
* | tx0Start + 0x20 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 | e * 0x40 | newPublicDataWrites
* | tx0Start + 0x20 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 + e * 0x40 | 0x04 | byteLen(newEncryptedLogs) (denoted f)
* | tx0Start + 0x20 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 + e * 0x40 + 0x4 | f | newEncryptedLogs
* | tx0Start + 0x20 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 + e * 0x40 + 0x4 + f | 0x04 | byteLen(newUnencryptedLogs) (denoted g)
* | tx0Start + 0x20 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 + e * 0x40 + 0x4 + f + 0x4 | g | newUnencryptedLogs
* | tx0Start | 0x1 | revertCode
* | tx0Start + 0x1 | 0x1 | len(newNoteHashes) (denoted b)
* | tx0Start + 0x1 + 0x1 | b * 0x20 | newNoteHashes
* | tx0Start + 0x1 + 0x1 + b * 0x20 | 0x1 | len(newNullifiers) (denoted c)
* | tx0Start + 0x1 + 0x1 + b * 0x20 + 0x1 | c * 0x20 | newNullifiers
* | tx0Start + 0x1 + 0x1 + b * 0x20 + 0x1 + c * 0x20 | 0x1 | len(newL2ToL1Msgs) (denoted d)
* | tx0Start + 0x1 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 | d * 0x20 | newL2ToL1Msgs
* | tx0Start + 0x1 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 | 0x1 | len(newPublicDataWrites) (denoted e)
* | tx0Start + 0x1 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 | e * 0x40 | newPublicDataWrites
* | tx0Start + 0x1 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 + e * 0x40 | 0x04 | byteLen(newEncryptedLogs) (denoted f)
* | tx0Start + 0x1 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 + e * 0x40 + 0x4 | f | newEncryptedLogs
* | tx0Start + 0x1 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 + e * 0x40 + 0x4 + f | 0x04 | byteLen(newUnencryptedLogs) (denoted g)
* | tx0Start + 0x1 + 0x1 + b * 0x20 + 0x1 + c * 0x20 + 0x1 + d * 0x20 + 0x01 + e * 0x40 + 0x4 + f + 0x4 | g | newUnencryptedLogs
* | | | },
* | | | TxEffect 1 {
* | | | ...
Expand Down Expand Up @@ -91,7 +91,7 @@ library MessagesDecoder {
// Now we iterate over the tx effects
for (uint256 i = 0; i < numTxs; i++) {
// revertCode
offset += 0x20;
offset += 0x1;

// Note hashes
count = read1(_body, offset);
Expand Down
Loading
Loading