Skip to content

Commit

Permalink
refactor: messaging naming fixes (#5383)
Browse files Browse the repository at this point in the history
Some naming fixes we discussed in our team chat and which Lasse
[disliked](#5206 (comment))
in my purge PR a few days ago.
\+ nuked some stale stuff from contracts.
  • Loading branch information
benesjan authored Mar 22, 2024
1 parent 6a7ccca commit 0226102
Show file tree
Hide file tree
Showing 44 changed files with 258 additions and 422 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,6 @@ The `DataStructures` are structs that we are using throughout the message infras

**Links**: [Implementation](https://github.com/AztecProtocol/aztec-packages/blob/master/l1-contracts/src/core/libraries/DataStructures.sol).

## `Entry`

An entry for the messageboxes multi-sets.

#include_code data_structure_entry l1-contracts/src/core/libraries/DataStructures.sol solidity

| Name | Type | Description |
| -------------- | ------- | ----------- |
| `count` | `uint32` | The occurrence of the entry in the dataset |
| `version` | `uint32` | The version of the entry |


## `L1Actor`

An entity on L1, specifying the address and the chainId for the entity. Used when specifying sender/recipient with an entity that is on L1.
Expand Down
4 changes: 1 addition & 3 deletions docs/docs/developers/debugging/sandbox-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,7 @@ Users may create a proof against a historical state in Aztec. The rollup circuit

## Archiver Errors

- "L1 to L2 Message with key ${entryKey.toString()} not found in the confirmed messages store" - happens when the L1 to L2 message doesn't exist or is "pending", when the user has sent a message on L1 via the Inbox contract but it has yet to be included in an L2 block by the sequencer - user has to wait for sequencer to pick it up and the archiver to sync the respective L2 block. You can get the sequencer to pick it up by doing an arbitrary transaction on L2 (eg send DAI to yourself). This would give the sequencer a transaction to process and as a side effect it would look for any pending messages it should include.

- "Unable to remove message: L1 to L2 Message with key ${entryKeyBigInt} not found in store" - happens when trying to confirm a non-existent pending message or cancelling such a message. Perhaps the sequencer has already confirmed the message?
- "No L1 to L2 message found for message hash ${messageHash.toString()}" - happens when the L1 to L2 message doesn't exist or is "pending", when the user has sent a message on L1 via the Inbox contract but it has yet to be included in an L2 block by the sequencer - user has to wait for enough blocks to progress and for the archiver to sync the respective L2 block. You can get the sequencer to pick it up by doing 2 arbitrary transaction on L2 (eg. send DAI to yourself 2 times). This would give the sequencer a transaction to process and as a side effect it would consume 2 subtrees of new messages from the Inbox contract. 2 subtrees needs to be consumed and not just 1 because there is a 1 block lag to prevent the subtree from changing when the sequencer is proving.

- "Block number mismatch: expected ${l2BlockNum} but got ${block.number}" - The archiver keeps track of the next expected L2 block number. It throws this error if it got a different one when trying to sync with the rollup contract's events on L1.

Expand Down
17 changes: 11 additions & 6 deletions l1-contracts/GUIDE_LINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,18 @@ Natspec should be written for all functions (`internal` mainly for clarity). Use

```solidity
/**
* @notice Consumes an entry from the Outbox
* @dev Only meaningfully callable by portals, otherwise should never hit an entry
* @dev Emits the `MessageConsumed` event when consuming messages
* @param _message - The L2 to L1 message
* @return entryKey - The key of the entry removed
* @notice Inserts a new message into the Inbox
* @dev Emits `MessageSent` with data for easy access by the sequencer
* @param _recipient - The recipient of the message
* @param _content - The content of the message (application specific)
* @param _secretHash - The secret hash of the message (make it possible to hide when a specific message is consumed on L2)
* @return Hash of the sent message.
*/
function consume(DataStructures.L2ToL1Msg memory _message)
function sendL2Message(
DataStructures.L2Actor memory _recipient,
bytes32 _content,
bytes32 _secretHash
) external override(IInbox) returns (bytes32) {
```

### Solhint configuration
Expand Down
50 changes: 13 additions & 37 deletions l1-contracts/slither_output.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Summary
- [timestamp](#timestamp) (1 results) (Low)
- [pess-public-vs-external](#pess-public-vs-external) (5 results) (Low)
- [assembly](#assembly) (1 results) (Informational)
- [dead-code](#dead-code) (5 results) (Informational)
- [dead-code](#dead-code) (1 results) (Informational)
- [solc-version](#solc-version) (1 results) (Informational)
- [similar-names](#similar-names) (3 results) (Informational)
- [constable-states](#constable-states) (1 results) (Optimization)
Expand Down Expand Up @@ -123,7 +123,7 @@ Reentrancy in [Inbox.sendL2Message(DataStructures.L2Actor,bytes32,bytes32)](src/
External calls:
- [index = currentTree.insertLeaf(leaf)](src/core/messagebridge/Inbox.sol#L91)
Event emitted after the call(s):
- [LeafInserted(inProgress,index,leaf)](src/core/messagebridge/Inbox.sol#L92)
- [MessageSent(inProgress,index,leaf)](src/core/messagebridge/Inbox.sol#L92)

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

Expand Down Expand Up @@ -191,30 +191,6 @@ src/core/libraries/decoders/TxsDecoder.sol#L258-L277
Impact: Informational
Confidence: Medium
- [ ] ID-18
[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-19
[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-20
[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-21
[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-22
[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 @@ -223,25 +199,25 @@ src/core/libraries/Hash.sol#L52-L54
## solc-version
Impact: Informational
Confidence: High
- [ ] ID-23
- [ ] ID-19
solc-0.8.23 is not recommended for deployment

## similar-names
Impact: Informational
Confidence: Medium
- [ ] ID-24
- [ ] ID-20
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-25
- [ ] ID-21
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-26
- [ ] ID-22
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#L32
Expand All @@ -250,7 +226,7 @@ src/core/Rollup.sol#L32
## constable-states
Impact: Optimization
Confidence: High
- [ ] ID-27
- [ ] ID-23
[Rollup.lastWarpedBlockTs](src/core/Rollup.sol#L41) should be constant

src/core/Rollup.sol#L41
Expand All @@ -259,37 +235,37 @@ src/core/Rollup.sol#L41
## pess-multiple-storage-read
Impact: Optimization
Confidence: High
- [ ] ID-28
- [ ] ID-24
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-29
- [ ] ID-25
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-30
- [ ] ID-26
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-31
- [ ] ID-27
In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L48-L81) variable [FrontierMerkle.HEIGHT](src/core/messagebridge/frontier_tree/Frontier.sol#L13) is read multiple times

src/core/messagebridge/frontier_tree/Frontier.sol#L48-L81


- [ ] ID-32
- [ ] ID-28
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-33
- [ ] ID-29
In a function [FrontierMerkle.root()](src/core/messagebridge/frontier_tree/Frontier.sol#L48-L81) variable [FrontierMerkle.frontier](src/core/messagebridge/frontier_tree/Frontier.sol#L18) is read multiple times

src/core/messagebridge/frontier_tree/Frontier.sol#L48-L81
Expand Down
10 changes: 8 additions & 2 deletions l1-contracts/src/core/interfaces/messagebridge/IInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@ import {DataStructures} from "../../libraries/DataStructures.sol";
* @notice Lives on L1 and is used to pass messages into the rollup from L1.
*/
interface IInbox {
event LeafInserted(uint256 indexed blockNumber, uint256 index, bytes32 value);
/**
* @notice Emitted when a message is sent
* @param l2BlockNumber - The L2 block number in which the message is included
* @param index - The index of the message in the block
* @param hash - The hash of the message
*/
event MessageSent(uint256 indexed l2BlockNumber, uint256 index, bytes32 hash);

// docs:start:send_l1_to_l2_message
/**
* @notice Inserts a new message into the Inbox
* @dev Emits `LeafInserted` with data for easy access by the sequencer
* @dev Emits `MessageSent` with data for easy access by the sequencer
* @param _recipient - The recipient of the message
* @param _content - The content of the message (application specific)
* @param _secretHash - The secret hash of the message (make it possible to hide when a specific message is consumed on L2)
Expand Down
16 changes: 0 additions & 16 deletions l1-contracts/src/core/libraries/DataStructures.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,6 @@ pragma solidity >=0.8.18;
* @notice Library that contains data structures used throughout the Aztec protocol
*/
library DataStructures {
// docs:start:data_structure_entry
/**
* @notice Entry struct - Done as struct to easily support extensions if needed
* @param fee - The fee provided to sequencer for including in the inbox. 0 if Outbox (as not applicable).
* @param count - The occurrence of the entry in the dataset
* @param version - The version of the entry
* @param deadline - The deadline to consume a message. Only after it, can a message be cancelled.
*/
struct Entry {
uint64 fee;
uint32 count;
uint32 version;
uint32 deadline;
}
// docs:end:data_structure_entry

// docs:start:l1_actor
/**
* @notice Actor on L1.
Expand Down
4 changes: 2 additions & 2 deletions l1-contracts/src/core/libraries/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ library Errors {
error Outbox__Unauthorized(); // 0x2c9490c2
error Outbox__InvalidChainId(); // 0x577ec7c4
error Outbox__InvalidVersion(uint256 entry, uint256 message); // 0x7915cac3
error Outbox__NothingToConsume(bytes32 entryKey); // 0xfb4fb506
error Outbox__NothingToConsume(bytes32 messageHash); // 0xfb4fb506
error Outbox__IncompatibleEntryArguments(
bytes32 entryKey,
bytes32 messageHash,
uint64 storedFee,
uint64 feePassed,
uint32 storedVersion,
Expand Down
113 changes: 0 additions & 113 deletions l1-contracts/src/core/libraries/MessageBox.sol

This file was deleted.

6 changes: 3 additions & 3 deletions l1-contracts/src/core/messagebridge/Inbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ contract Inbox is IInbox {

/**
* @notice Inserts a new message into the Inbox
* @dev Emits `LeafInserted` with data for easy access by the sequencer
* @dev Emits `MessageSent` with data for easy access by the sequencer
* @param _recipient - The recipient of the message
* @param _content - The content of the message (application specific)
* @param _secretHash - The secret hash of the message (make it possible to hide when a specific message is consumed on L2)
* @return The key of the message in the set
* @return Hash of the sent message.
*/
function sendL2Message(
DataStructures.L2Actor memory _recipient,
Expand Down Expand Up @@ -89,7 +89,7 @@ contract Inbox is IInbox {

bytes32 leaf = message.sha256ToField();
uint256 index = currentTree.insertLeaf(leaf);
emit LeafInserted(inProgress, index, leaf);
emit MessageSent(inProgress, index, leaf);

return leaf;
}
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/test/Inbox.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ contract InboxTest is Test {
bytes32 leaf = message.sha256ToField();
vm.expectEmit(true, true, true, true);
// event we expect
emit IInbox.LeafInserted(FIRST_REAL_TREE_NUM, 0, leaf);
emit IInbox.MessageSent(FIRST_REAL_TREE_NUM, 0, leaf);
// event we will get
bytes32 insertedLeaf =
inbox.sendL2Message(message.recipient, message.content, message.secretHash);
Expand Down
1 change: 0 additions & 1 deletion l1-contracts/test/Registry.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {Registry} from "../src/core/messagebridge/Registry.sol";
import {Errors} from "../src/core/libraries/Errors.sol";

import {DataStructures} from "../src/core/libraries/DataStructures.sol";
import {MessageBox} from "../src/core/libraries/MessageBox.sol";

contract RegistryTest is Test {
address internal constant DEAD = address(0xdead);
Expand Down
Loading

0 comments on commit 0226102

Please sign in to comment.