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

refactor: nuking l1 to l2 messages from block body #5272

Merged
merged 18 commits into from
Mar 21, 2024
+339 −518
Merged

refactor: nuking l1 to l2 messages from block body #5272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ccb4b55
WIP
benesjan Mar 19, 2024
722c7a0
slither
benesjan Mar 19, 2024
fc80953
WIP
benesjan Mar 19, 2024
7285af8
WIP
benesjan Mar 21, 2024
f6d98e5
updated data
benesjan Mar 21, 2024
1f9e099
linking issue
benesjan Mar 21, 2024
60cea24
deleted old comment
benesjan Mar 21, 2024
5128ffd
adding comment
benesjan Mar 21, 2024
18b2055
fixed naming
benesjan Mar 21, 2024
f2382ea
forgotten receipts check
benesjan Mar 21, 2024
899027f
fix
benesjan Mar 21, 2024
de1f112
e2e_persistence test cleanup
benesjan Mar 21, 2024
e1730e9
slither
benesjan Mar 21, 2024
f9c180d
nuked messages decoder
benesjan Mar 21, 2024
4702e2d
WIP
benesjan Mar 21, 2024
7c34501
fix
benesjan Mar 21, 2024
62589ad
fix
benesjan Mar 21, 2024
b94bce1
non zero messages
benesjan Mar 21, 2024
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
4 changes: 1 addition & 3 deletions boxes/boxes/react/tests/node.test.ts
View file
Open in desktop
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to nuke this one in this PR so I sneaked that in here.

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
View file
Open in desktop
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
View file
Open in desktop
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

🤩

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
Loading