Skip to content

Commit

Permalink
refactor: nuking l1 to l2 messages from block body (#5272)
Browse files Browse the repository at this point in the history
Fixes #5072
  • Loading branch information
benesjan authored Mar 21, 2024
1 parent 5c0e15d commit ee176d2
Show file tree
Hide file tree
Showing 30 changed files with 339 additions and 518 deletions.
4 changes: 1 addition & 3 deletions boxes/boxes/react/tests/node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ describe('BoxReact Contract Tests', () => {

test('Can set a number', async () => {
logger(`${await wallet.getRegisteredAccounts()}`);
const callTxReceipt = await contract.methods.setNumber(numberToSet, wallet.getCompleteAddress()).send().wait();

expect(callTxReceipt.status).toBe(TxStatus.MINED);
await contract.methods.setNumber(numberToSet, wallet.getCompleteAddress()).send().wait();
}, 40000);

test('Can read a number', async () => {
Expand Down
152 changes: 60 additions & 92 deletions l1-contracts/slither_output.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
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) (5 results) (Medium)
- [pess-dubious-typecast](#pess-dubious-typecast) (3 results) (Medium)
- [missing-zero-check](#missing-zero-check) (2 results) (Low)
- [reentrancy-events](#reentrancy-events) (2 results) (Low)
- [timestamp](#timestamp) (1 results) (Low)
- [pess-public-vs-external](#pess-public-vs-external) (5 results) (Low)
- [assembly](#assembly) (2 results) (Informational)
- [assembly](#assembly) (1 results) (Informational)
- [dead-code](#dead-code) (5 results) (Informational)
- [solc-version](#solc-version) (1 results) (Informational)
- [similar-names](#similar-names) (3 results) (Informational)
Expand All @@ -17,9 +16,9 @@ Summary
Impact: High
Confidence: Medium
- [ ] ID-0
Function [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L60-L104) is a non-protected setter archive is written
Function [Rollup.process(bytes,bytes32,bytes)](src/core/Rollup.sol#L58-L96) is a non-protected setter archive is written

src/core/Rollup.sol#L60-L104
src/core/Rollup.sol#L58-L96


## uninitialized-local
Expand All @@ -32,45 +31,29 @@ src/core/libraries/HeaderLib.sol#L148


- [ ] ID-2
[TxsDecoder.decode(bytes).vars](src/core/libraries/decoders/TxsDecoder.sol#L81) is a local variable never initialized
[TxsDecoder.decode(bytes).vars](src/core/libraries/decoders/TxsDecoder.sol#L78) is a local variable never initialized

src/core/libraries/decoders/TxsDecoder.sol#L81
src/core/libraries/decoders/TxsDecoder.sol#L78


## unused-return
## pess-dubious-typecast
Impact: Medium
Confidence: Medium
Confidence: High
- [ ] ID-3
[Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L60-L104) ignores return value by [(l2ToL1Msgs) = MessagesDecoder.decode(_body)](src/core/Rollup.sol#L77)
Dubious typecast in [TxsDecoder.read1(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L333-L335):
bytes => bytes1 casting occurs in [uint256(uint8(bytes1(slice(_data,_offset,1))))](src/core/libraries/decoders/TxsDecoder.sol#L334)

src/core/Rollup.sol#L60-L104
src/core/libraries/decoders/TxsDecoder.sol#L333-L335


## pess-dubious-typecast
Impact: Medium
Confidence: High
- [ ] ID-4
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)
Dubious typecast in [TxsDecoder.read4(bytes,uint256)](src/core/libraries/decoders/TxsDecoder.sol#L343-L345):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(slice(_data,_offset,4))))](src/core/libraries/decoders/TxsDecoder.sol#L344)

src/core/libraries/decoders/TxsDecoder.sol#L341-L343
src/core/libraries/decoders/TxsDecoder.sol#L343-L345


- [ ] ID-5
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/libraries/decoders/TxsDecoder.sol#L351-L353


- [ ] ID-6
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/MessagesDecoder.sol#L164-L166


- [ ] ID-7
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 @@ -96,24 +79,17 @@ Dubious typecast in [HeaderLib.decode(bytes)](src/core/libraries/HeaderLib.sol#L
src/core/libraries/HeaderLib.sol#L143-L184


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

src/core/libraries/decoders/MessagesDecoder.sol#L154-L156


## missing-zero-check
Impact: Low
Confidence: Medium
- [ ] ID-9
- [ ] ID-6
[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-10
- [ ] ID-7
[Outbox.constructor(address)._rollup](src/core/messagebridge/Outbox.sol#L31) lacks a zero-check on :
- [ROLLUP_CONTRACT = _rollup](src/core/messagebridge/Outbox.sol#L32)

Expand All @@ -123,31 +99,31 @@ src/core/messagebridge/Outbox.sol#L31
## reentrancy-events
Impact: Low
Confidence: Medium
- [ ] ID-11
Reentrancy in [Inbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/Inbox.sol#L61-L95):
- [ ] ID-8
Reentrancy in [Rollup.process(bytes,bytes32,bytes)](src/core/Rollup.sol#L58-L96):
External calls:
- [index = currentTree.insertLeaf(leaf)](src/core/messagebridge/Inbox.sol#L91)
- [inHash = INBOX.consume()](src/core/Rollup.sol#L83)
- [OUTBOX.insert(header.globalVariables.blockNumber,header.contentCommitment.outHash,l2ToL1TreeHeight)](src/core/Rollup.sol#L91-L93)
Event emitted after the call(s):
- [LeafInserted(inProgress,index,leaf)](src/core/messagebridge/Inbox.sol#L92)
- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L95)

src/core/messagebridge/Inbox.sol#L61-L95
src/core/Rollup.sol#L58-L96


- [ ] ID-12
Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L60-L104):
- [ ] ID-9
Reentrancy in [Inbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/core/messagebridge/Inbox.sol#L61-L95):
External calls:
- [inHash = INBOX.consume()](src/core/Rollup.sol#L91)
- [OUTBOX.insert(header.globalVariables.blockNumber,header.contentCommitment.outHash,l2ToL1TreeHeight)](src/core/Rollup.sol#L99-L101)
- [index = currentTree.insertLeaf(leaf)](src/core/messagebridge/Inbox.sol#L91)
Event emitted after the call(s):
- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L103)
- [LeafInserted(inProgress,index,leaf)](src/core/messagebridge/Inbox.sol#L92)

src/core/Rollup.sol#L60-L104
src/core/messagebridge/Inbox.sol#L61-L95


## timestamp
Impact: Low
Confidence: Medium
- [ ] ID-13
- [ ] ID-10
[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 @@ -158,35 +134,35 @@ src/core/libraries/HeaderLib.sol#L106-L136
## pess-public-vs-external
Impact: Low
Confidence: Medium
- [ ] ID-14
- [ ] ID-11
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-15
- [ ] ID-12
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-16
- [ ] ID-13
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-17
The following public functions could be turned into external in [Rollup](src/core/Rollup.sol#L30-L113) contract:
[Rollup.constructor(IRegistry,IAvailabilityOracle)](src/core/Rollup.sol#L44-L51)
- [ ] ID-14
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#L43-L50)

src/core/Rollup.sol#L30-L113
src/core/Rollup.sol#L29-L105


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

Expand All @@ -196,49 +172,41 @@ src/core/messagebridge/Outbox.sol#L18-L132
## assembly
Impact: Informational
Confidence: High
- [ ] ID-19
[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-20
[TxsDecoder.computeRoot(bytes32[])](src/core/libraries/decoders/TxsDecoder.sol#L265-L284) uses assembly
- [INLINE ASM](src/core/libraries/decoders/TxsDecoder.sol#L272-L274)
- [ ] ID-16
[TxsDecoder.computeRoot(bytes32[])](src/core/libraries/decoders/TxsDecoder.sol#L257-L276) uses assembly
- [INLINE ASM](src/core/libraries/decoders/TxsDecoder.sol#L264-L266)

src/core/libraries/decoders/TxsDecoder.sol#L265-L284
src/core/libraries/decoders/TxsDecoder.sol#L257-L276


## dead-code
Impact: Informational
Confidence: Medium
- [ ] ID-21
- [ ] ID-17
[MessageBox.consume(mapping(bytes32 => DataStructures.Entry),bytes32,function(bytes32))](src/core/libraries/MessageBox.sol#L71-L79) is never used and should be removed

src/core/libraries/MessageBox.sol#L71-L79


- [ ] ID-22
- [ ] ID-18
[MessageBox.contains(mapping(bytes32 => DataStructures.Entry),bytes32)](src/core/libraries/MessageBox.sol#L87-L92) is never used and should be removed

src/core/libraries/MessageBox.sol#L87-L92


- [ ] ID-23
- [ ] ID-19
[MessageBox.get(mapping(bytes32 => DataStructures.Entry),bytes32,function(bytes32))](src/core/libraries/MessageBox.sol#L104-L112) is never used and should be removed

src/core/libraries/MessageBox.sol#L104-L112


- [ ] ID-24
- [ ] ID-20
[MessageBox.insert(mapping(bytes32 => DataStructures.Entry),bytes32,uint64,uint32,uint32,function(bytes32,uint64,uint64,uint32,uint32,uint32,uint32))](src/core/libraries/MessageBox.sol#L30-L60) is never used and should be removed

src/core/libraries/MessageBox.sol#L30-L60


- [ ] ID-25
- [ ] ID-21
[Hash.sha256ToField(bytes32)](src/core/libraries/Hash.sol#L52-L54) is never used and should be removed

src/core/libraries/Hash.sol#L52-L54
Expand All @@ -247,73 +215,73 @@ src/core/libraries/Hash.sol#L52-L54
## solc-version
Impact: Informational
Confidence: High
- [ ] ID-26
- [ ] ID-22
solc-0.8.23 is not recommended for deployment

## similar-names
Impact: Informational
Confidence: Medium
- [ ] ID-27
- [ ] ID-23
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-28
- [ ] ID-24
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-29
Variable [Rollup.AVAILABILITY_ORACLE](src/core/Rollup.sol#L33) is too similar to [Rollup.constructor(IRegistry,IAvailabilityOracle)._availabilityOracle](src/core/Rollup.sol#L44)
- [ ] ID-25
Variable [Rollup.AVAILABILITY_ORACLE](src/core/Rollup.sol#L32) is too similar to [Rollup.constructor(IRegistry,IAvailabilityOracle)._availabilityOracle](src/core/Rollup.sol#L43)

src/core/Rollup.sol#L33
src/core/Rollup.sol#L32


## constable-states
Impact: Optimization
Confidence: High
- [ ] ID-30
[Rollup.lastWarpedBlockTs](src/core/Rollup.sol#L42) should be constant
- [ ] ID-26
[Rollup.lastWarpedBlockTs](src/core/Rollup.sol#L41) should be constant

src/core/Rollup.sol#L42
src/core/Rollup.sol#L41


## pess-multiple-storage-read
Impact: Optimization
Confidence: High
- [ ] ID-31
- [ ] ID-27
In a function [Outbox.insert(uint256,bytes32,uint256)](src/core/messagebridge/Outbox.sol#L44-L64) variable [Outbox.roots](src/core/messagebridge/Outbox.sol#L29) is read multiple times

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


- [ ] ID-32
- [ ] ID-28
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-33
- [ ] ID-29
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-34
- [ ] ID-30
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-35
- [ ] ID-31
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-36
- [ ] ID-32
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
16 changes: 4 additions & 12 deletions l1-contracts/src/core/Rollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {IRegistry} from "./interfaces/messagebridge/IRegistry.sol";

// Libraries
import {HeaderLib} from "./libraries/HeaderLib.sol";
import {MessagesDecoder} from "./libraries/decoders/MessagesDecoder.sol";
import {Hash} from "./libraries/Hash.sol";
import {Errors} from "./libraries/Errors.sol";
import {Constants} from "./libraries/ConstantsGen.sol";
Expand Down Expand Up @@ -54,15 +53,12 @@ contract Rollup is IRollup {
* @notice Process an incoming L2 block and progress the state
* @param _header - The L2 block header
* @param _archive - A root of the archive tree after the L2 block is applied
* @param _body - The L2 block body
* @param _proof - The proof of correct execution
*/
function process(
bytes calldata _header,
bytes32 _archive,
bytes calldata _body, // TODO(#5073) Nuke this when updating to the new message model
bytes memory _proof
) external override(IRollup) {
function process(bytes calldata _header, bytes32 _archive, bytes memory _proof)
external
override(IRollup)
{
// Decode and validate header
HeaderLib.Header memory header = HeaderLib.decode(_header);
HeaderLib.validate(header, VERSION, lastBlockTs, archive);
Expand All @@ -72,10 +68,6 @@ contract Rollup is IRollup {
revert Errors.Rollup__UnavailableTxs(header.contentCommitment.txsEffectsHash);
}

// Decode the cross-chain messages (Will be removed as part of message model change)
// TODO(#5339)
(,,, bytes32[] memory l2ToL1Msgs) = MessagesDecoder.decode(_body);

bytes32[] memory publicInputs = new bytes32[](1);
publicInputs[0] = _computePublicInputHash(_header, _archive);

Expand Down
Loading

0 comments on commit ee176d2

Please sign in to comment.