Skip to content

Commit

Permalink
feat: update header to match message extension (#4627)
Browse files Browse the repository at this point in the history 
Fixes #4560.

Also removes the unnecessary `txsHash` input on `process` as we can just
use the `txsHash` from the `ContentCommitment` introduced here.

The `inHash` and `outHash` values are currently just set to be `zero`.

Deleted a stale and unused snapshot which made the diff look larger than
it is.
  • Loading branch information
@LHerskind
LHerskind authored Feb 16, 2024
1 parent 379ded4 commit dc01e1d
Show file tree
Hide file tree
Showing 47 changed files with 1,629 additions and 34,257 deletions.
95 changes: 49 additions & 46 deletions l1-contracts/slither_output.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,27 @@ Summary
Impact: High
Confidence: Medium
- [ ] ID-0
Function [Rollup.process(bytes,bytes32,bytes32,bytes,bytes)](src/core/Rollup.sol#L54-L94) is a non-protected setter archive is written
Function [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L53-L91) is a non-protected setter archive is written

src/core/Rollup.sol#L54-L94
src/core/Rollup.sol#L53-L91


## uninitialized-local
Impact: Medium
Confidence: Medium
- [ ] ID-1
[HeaderLib.decode(bytes).header](src/core/libraries/HeaderLib.sol#L139) is a local variable never initialized
[HeaderLib.decode(bytes).header](src/core/libraries/HeaderLib.sol#L150) is a local variable never initialized

src/core/libraries/HeaderLib.sol#L139
src/core/libraries/HeaderLib.sol#L150


## unused-return
Impact: Medium
Confidence: Medium
- [ ] ID-2
[Rollup.process(bytes,bytes32,bytes32,bytes,bytes)](src/core/Rollup.sol#L54-L94) ignores return value by [(inHash,l1ToL2Msgs,l2ToL1Msgs) = MessagesDecoder.decode(_body)](src/core/Rollup.sol#L71-L72)
[Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L53-L91) ignores return value by [(l1ToL2Msgs,l2ToL1Msgs) = MessagesDecoder.decode(_body)](src/core/Rollup.sol#L69)

src/core/Rollup.sol#L54-L94
src/core/Rollup.sol#L53-L91


## pess-dubious-typecast
Expand All @@ -59,52 +59,55 @@ src/core/libraries/decoders/Decoder.sol#L415-L417


- [ ] ID-5
Dubious typecast in [HeaderLib.decode(bytes)](src/core/libraries/HeaderLib.sol#L134-L175):
bytes => bytes32 casting occurs in [header.lastArchive = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L142-L144)
bytes => bytes4 casting occurs in [header.lastArchive = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L142-L144)
bytes => bytes32 casting occurs in [header.bodyHash = bytes32(_header)](src/core/libraries/HeaderLib.sol#L147)
bytes => bytes32 casting occurs in [header.stateReference.l1ToL2MessageTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L150-L152)
bytes => bytes4 casting occurs in [header.stateReference.l1ToL2MessageTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L150-L152)
bytes => bytes32 casting occurs in [header.stateReference.partialStateReference.noteHashTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L153-L155)
bytes => bytes4 casting occurs in [header.stateReference.partialStateReference.noteHashTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L153-L155)
bytes => bytes32 casting occurs in [header.stateReference.partialStateReference.nullifierTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L156-L158)
bytes => bytes4 casting occurs in [header.stateReference.partialStateReference.nullifierTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L156-L158)
bytes => bytes32 casting occurs in [header.stateReference.partialStateReference.contractTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L159-L161)
bytes => bytes4 casting occurs in [header.stateReference.partialStateReference.contractTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L159-L161)
bytes => bytes32 casting occurs in [header.stateReference.partialStateReference.publicDataTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L162-L164)
bytes => bytes4 casting occurs in [header.stateReference.partialStateReference.publicDataTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L162-L164)
bytes => bytes32 casting occurs in [header.globalVariables.chainId = uint256(bytes32(_header))](src/core/libraries/HeaderLib.sol#L167)
bytes => bytes32 casting occurs in [header.globalVariables.version = uint256(bytes32(_header))](src/core/libraries/HeaderLib.sol#L168)
bytes => bytes32 casting occurs in [header.globalVariables.blockNumber = uint256(bytes32(_header))](src/core/libraries/HeaderLib.sol#L169)
bytes => bytes32 casting occurs in [header.globalVariables.timestamp = uint256(bytes32(_header))](src/core/libraries/HeaderLib.sol#L170)
bytes => bytes20 casting occurs in [header.globalVariables.coinbase = address(bytes20(_header))](src/core/libraries/HeaderLib.sol#L171)
bytes => bytes32 casting occurs in [header.globalVariables.feeRecipient = bytes32(_header)](src/core/libraries/HeaderLib.sol#L172)

src/core/libraries/HeaderLib.sol#L134-L175


- [ ] ID-6
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 [MessagesDecoder.read4(bytes,uint256)](src/core/libraries/decoders/MessagesDecoder.sol#L110-L112):
bytes => bytes4 casting occurs in [uint256(uint32(bytes4(_data)))](src/core/libraries/decoders/MessagesDecoder.sol#L111)

src/core/libraries/decoders/MessagesDecoder.sol#L110-L112


- [ ] ID-8
Dubious typecast in [HeaderLib.decode(bytes)](src/core/libraries/HeaderLib.sol#L145-L189):
bytes => bytes32 casting occurs in [header.lastArchive = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L153-L155)
bytes => bytes4 casting occurs in [header.lastArchive = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L153-L155)
bytes => bytes32 casting occurs in [header.contentCommitment.txTreeHeight = uint256(bytes32(_header))](src/core/libraries/HeaderLib.sol#L158)
bytes => bytes32 casting occurs in [header.contentCommitment.txsHash = bytes32(_header)](src/core/libraries/HeaderLib.sol#L159)
bytes => bytes32 casting occurs in [header.contentCommitment.inHash = bytes32(_header)](src/core/libraries/HeaderLib.sol#L160)
bytes => bytes32 casting occurs in [header.contentCommitment.outHash = bytes32(_header)](src/core/libraries/HeaderLib.sol#L161)
bytes => bytes32 casting occurs in [header.stateReference.l1ToL2MessageTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L164-L166)
bytes => bytes4 casting occurs in [header.stateReference.l1ToL2MessageTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L164-L166)
bytes => bytes32 casting occurs in [header.stateReference.partialStateReference.noteHashTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L167-L169)
bytes => bytes4 casting occurs in [header.stateReference.partialStateReference.noteHashTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L167-L169)
bytes => bytes32 casting occurs in [header.stateReference.partialStateReference.nullifierTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L170-L172)
bytes => bytes4 casting occurs in [header.stateReference.partialStateReference.nullifierTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L170-L172)
bytes => bytes32 casting occurs in [header.stateReference.partialStateReference.contractTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L173-L175)
bytes => bytes4 casting occurs in [header.stateReference.partialStateReference.contractTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L173-L175)
bytes => bytes32 casting occurs in [header.stateReference.partialStateReference.publicDataTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L176-L178)
bytes => bytes4 casting occurs in [header.stateReference.partialStateReference.publicDataTree = AppendOnlyTreeSnapshot(bytes32(_header),uint32(bytes4(_header)))](src/core/libraries/HeaderLib.sol#L176-L178)
bytes => bytes32 casting occurs in [header.globalVariables.chainId = uint256(bytes32(_header))](src/core/libraries/HeaderLib.sol#L181)
bytes => bytes32 casting occurs in [header.globalVariables.version = uint256(bytes32(_header))](src/core/libraries/HeaderLib.sol#L182)
bytes => bytes32 casting occurs in [header.globalVariables.blockNumber = uint256(bytes32(_header))](src/core/libraries/HeaderLib.sol#L183)
bytes => bytes32 casting occurs in [header.globalVariables.timestamp = uint256(bytes32(_header))](src/core/libraries/HeaderLib.sol#L184)
bytes => bytes20 casting occurs in [header.globalVariables.coinbase = address(bytes20(_header))](src/core/libraries/HeaderLib.sol#L185)
bytes => bytes32 casting occurs in [header.globalVariables.feeRecipient = bytes32(_header)](src/core/libraries/HeaderLib.sol#L186)

src/core/libraries/HeaderLib.sol#L145-L189


- [ ] 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)
Expand All @@ -123,14 +126,14 @@ src/core/libraries/decoders/Decoder.sol#L132-L134
Impact: Low
Confidence: Medium
- [ ] ID-11
Reentrancy in [Rollup.process(bytes,bytes32,bytes32,bytes,bytes)](src/core/Rollup.sol#L54-L94):
Reentrancy in [Rollup.process(bytes,bytes32,bytes,bytes)](src/core/Rollup.sol#L53-L91):
External calls:
- [inbox.batchConsume(l1ToL2Msgs,msg.sender)](src/core/Rollup.sol#L88)
- [outbox.sendL1Messages(l2ToL1Msgs)](src/core/Rollup.sol#L91)
- [inbox.batchConsume(l1ToL2Msgs,msg.sender)](src/core/Rollup.sol#L85)
- [outbox.sendL1Messages(l2ToL1Msgs)](src/core/Rollup.sol#L88)
Event emitted after the call(s):
- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L93)
- [L2BlockProcessed(header.globalVariables.blockNumber)](src/core/Rollup.sol#L90)

src/core/Rollup.sol#L54-L94
src/core/Rollup.sol#L53-L91


## timestamp
Expand All @@ -145,19 +148,19 @@ src/core/messagebridge/Inbox.sol#L122-L143


- [ ] ID-13
[Inbox.sendL2Message(DataStructures.L2Actor,uint32,bytes32,bytes32)](src/core/messagebridge/Inbox.sol#L45-L91) uses timestamp for comparisons
[HeaderLib.validate(HeaderLib.Header,uint256,uint256,bytes32)](src/core/libraries/HeaderLib.sol#L108-L138) uses timestamp for comparisons
Dangerous comparisons:
- [_deadline <= block.timestamp](src/core/messagebridge/Inbox.sol#L54)
- [_header.globalVariables.timestamp > block.timestamp](src/core/libraries/HeaderLib.sol#L122)

src/core/messagebridge/Inbox.sol#L45-L91
src/core/libraries/HeaderLib.sol#L108-L138


- [ ] ID-14
[HeaderLib.validate(HeaderLib.Header,uint256,uint256,bytes32)](src/core/libraries/HeaderLib.sol#L97-L127) uses timestamp for comparisons
[Inbox.sendL2Message(DataStructures.L2Actor,uint32,bytes32,bytes32)](src/core/messagebridge/Inbox.sol#L45-L91) uses timestamp for comparisons
Dangerous comparisons:
- [_header.globalVariables.timestamp > block.timestamp](src/core/libraries/HeaderLib.sol#L111)
- [_deadline <= block.timestamp](src/core/messagebridge/Inbox.sol#L54)

src/core/libraries/HeaderLib.sol#L97-L127
src/core/messagebridge/Inbox.sol#L45-L91


- [ ] ID-15
Expand Down Expand Up @@ -186,10 +189,10 @@ src/core/messagebridge/Registry.sol#L22-L129


- [ ] ID-18
The following public functions could be turned into external in [Rollup](src/core/Rollup.sol#L27-L103) contract:
The following public functions could be turned into external in [Rollup](src/core/Rollup.sol#L27-L100) contract:
[Rollup.constructor(IRegistry,IAvailabilityOracle)](src/core/Rollup.sol#L39-L44)

src/core/Rollup.sol#L27-L103
src/core/Rollup.sol#L27-L100


- [ ] ID-19
Expand Down
17 changes: 7 additions & 10 deletions l1-contracts/src/core/Rollup.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,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 _txsHash - Transactions hash.
* @param _body - The L2 block body
* @param _proof - The proof of correct execution
*/
function process(
bytes calldata _header,
bytes32 _archive,
bytes32 _txsHash, // TODO(#3938) Update this to be actual txs hash and not the old block calldata hash.
bytes calldata _body, // TODO(#3938) Update this to pass in only th messages and not the whole body.
bytes memory _proof
) external override(IRollup) {
Expand All @@ -63,16 +61,15 @@ contract Rollup is IRollup {
HeaderLib.validate(header, VERSION, lastBlockTs, archive);

// Check if the data is available using availability oracle (change availability oracle if you want a different DA layer)
if (!AVAILABILITY_ORACLE.isAvailable(_txsHash)) {
revert Errors.Rollup__UnavailableTxs(_txsHash);
if (!AVAILABILITY_ORACLE.isAvailable(header.contentCommitment.txsHash)) {
revert Errors.Rollup__UnavailableTxs(header.contentCommitment.txsHash);
}

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

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

// @todo @benesjan We will need `nextAvailableLeafIndex` of archive to verify the proof. This value is equal to
// current block number which is stored in the header (header.globalVariables.blockNumber).
Expand All @@ -93,11 +90,11 @@ contract Rollup is IRollup {
emit L2BlockProcessed(header.globalVariables.blockNumber);
}

function _computePublicInputHash(bytes calldata _header, bytes32 _txsHash, bytes32 _inHash)
function _computePublicInputHash(bytes calldata _header, bytes32 _archive)
internal
pure
returns (bytes32)
{
return Hash.sha256ToField(bytes.concat(_header, _txsHash, _inHash));
return Hash.sha256ToField(bytes.concat(_header, _archive));
}
}
1 change: 0 additions & 1 deletion l1-contracts/src/core/interfaces/IRollup.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ interface IRollup {
function process(
bytes calldata _header,
bytes32 _archive,
bytes32 _txsHash,
bytes calldata _body,
bytes memory _proof
) external;
Expand Down
9 changes: 5 additions & 4 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,15 @@ library Constants {
uint256 internal constant GLOBAL_VARIABLES_LENGTH = 6;
uint256 internal constant PARTIAL_STATE_REFERENCE_LENGTH = 8;
uint256 internal constant STATE_REFERENCE_LENGTH = 10;
uint256 internal constant HEADER_LENGTH = 20;
uint256 internal constant CONTENT_COMMITMENT_LENGTH = 7;
uint256 internal constant HEADER_LENGTH = 25;
uint256 internal constant FUNCTION_DATA_LENGTH = 4;
uint256 internal constant CONTRACT_DEPLOYMENT_DATA_LENGTH = 6;
uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 209;
uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 214;
uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 214;
uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 219;
uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 194;
uint256 internal constant CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH = 2;
uint256 internal constant CONTRACT_STORAGE_READ_LENGTH = 2;
uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 189;
uint256 internal constant GET_NOTES_ORACLE_RETURN_LENGTH = 674;
uint256 internal constant COMMITMENTS_NUM_BYTES_PER_BASE_ROLLUP = 2048;
uint256 internal constant NULLIFIERS_NUM_BYTES_PER_BASE_ROLLUP = 2048;
Expand Down
Loading

0 comments on commit dc01e1d

Please sign in to comment.