refactor: l1 l2 messages cleanup (#5270)

Fixes #5264

l1-contracts/src/core/interfaces/messagebridge/IInbox.sol

@@ -7,7 +7,7 @@ import {DataStructures} from "../../libraries/DataStructures.sol";
 /**
  * @title Inbox
  * @author Aztec Labs
- * @notice Lives on L1 and is used to pass messages into the rollup, e.g., L1 -> L2 messages.
+ * @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);

l1-contracts/test/Inbox.t.sol

@@ -4,6 +4,7 @@ pragma solidity >=0.8.18;
 
 import {Test} from "forge-std/Test.sol";
 
+import {IInbox} from "../src/core/interfaces/messagebridge/IInbox.sol";
 import {InboxHarness} from "./harnesses/InboxHarness.sol";
 import {Constants} from "../src/core/libraries/ConstantsGen.sol";
 import {Errors} from "../src/core/libraries/Errors.sol";
@@ -19,8 +20,6 @@ contract InboxTest is Test {
   uint256 internal version = 0;
   bytes32 internal emptyTreeRoot;
 
-  event LeafInserted(uint256 indexed blockNumber, uint256 index, bytes32 value);
-
   function setUp() public {
     address rollup = address(this);
     // We set low depth (5) to ensure we sufficiently test the tree transitions
@@ -82,7 +81,7 @@ contract InboxTest is Test {
     bytes32 leaf = message.sha256ToField();
     vm.expectEmit(true, true, true, true);
     // event we expect
-    emit LeafInserted(FIRST_REAL_TREE_NUM, 0, leaf);
+    emit IInbox.LeafInserted(FIRST_REAL_TREE_NUM, 0, leaf);
     // event we will get
     bytes32 insertedLeaf =
       inbox.sendL2Message(message.recipient, message.content, message.secretHash);

l1-contracts/test/Rollup.t.sol

@@ -137,7 +137,6 @@ contract RollupTest is DecoderBase {
 
     assertEq(inbox.toConsume(), toConsume + 1, "Message subtree not consumed");
 
-    (, bytes32[] memory inboxWrites) = vm.accesses(address(inbox));
     (, bytes32[] memory outboxWrites) = vm.accesses(address(outbox));
 
     {

l1-contracts/test/portals/TokenPortal.t.sol

@@ -25,7 +25,6 @@ contract TokenPortalTest is Test {
 
   uint256 internal constant FIRST_REAL_TREE_NUM = Constants.INITIAL_L2_BLOCK_NUM + 1;
 
-  event LeafInserted(uint256 indexed blockNumber, uint256 index, bytes32 value);
   event MessageConsumed(bytes32 indexed entryKey, address indexed recipient);
 
   Registry internal registry;
@@ -69,7 +68,7 @@ contract TokenPortalTest is Test {
     vm.deal(address(this), 100 ether);
   }
 
-  function _createExpectedMintPrivateL1ToL2Message(address _canceller)
+  function _createExpectedMintPrivateL1ToL2Message()
     internal
     view
     returns (DataStructures.L1ToL2Msg memory)
@@ -105,15 +104,14 @@ contract TokenPortalTest is Test {
     portalERC20.approve(address(tokenPortal), mintAmount);
 
     // Check for the expected message
-    DataStructures.L1ToL2Msg memory expectedMessage =
-      _createExpectedMintPrivateL1ToL2Message(address(this));
+    DataStructures.L1ToL2Msg memory expectedMessage = _createExpectedMintPrivateL1ToL2Message();
 
     bytes32 expectedLeaf = expectedMessage.sha256ToField();
 
     // Check the event was emitted
     vm.expectEmit(true, true, true, true);
     // event we expect
-    emit LeafInserted(FIRST_REAL_TREE_NUM, 0, expectedLeaf);
+    emit IInbox.LeafInserted(FIRST_REAL_TREE_NUM, 0, expectedLeaf);
     // event we will get
 
     // Perform op
@@ -138,7 +136,7 @@ contract TokenPortalTest is Test {
     // Check the event was emitted
     vm.expectEmit(true, true, true, true);
     // event we expect
-    emit LeafInserted(FIRST_REAL_TREE_NUM, 0, expectedLeaf);
+    emit IInbox.LeafInserted(FIRST_REAL_TREE_NUM, 0, expectedLeaf);
 
     // Perform op
     bytes32 leaf = tokenPortal.depositToAztecPublic(to, amount, secretHashForL2MessageConsumption);

l1-contracts/test/portals/UniswapPortal.t.sol

@@ -231,7 +231,7 @@ contract UniswapPortalTest is Test {
     bytes32 swapEntryKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
     _addMessagesToOutbox(daiWithdrawEntryKey, swapEntryKey);
 
-    bytes32 l1ToL2EntryKey = uniswapPortal.swapPublic(
+    uniswapPortal.swapPublic(
       address(daiTokenPortal),
       amount,
       uniswapFeePool,
@@ -260,7 +260,7 @@ contract UniswapPortalTest is Test {
     _addMessagesToOutbox(daiWithdrawEntryKey, swapEntryKey);
 
     vm.prank(_caller);
-    bytes32 l1ToL2EntryKey = uniswapPortal.swapPublic(
+    uniswapPortal.swapPublic(
       address(daiTokenPortal),
       amount,
       uniswapFeePool,

noir-projects/noir-protocol-circuits/crates/rollup-lib/src/root.nr

@@ -64,8 +64,6 @@ mod tests {
 
     #[test]
     fn check_block_hashes_empty_blocks() {
-        let expected_messages_hash = U256::from_bytes32(dep::std::hash::sha256([0; NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP_NUM_BYTES])).to_u128_limbs();
-
         let expected_txs_effects_hash = accumulate_sha256(
             [
             U128::from_integer(0),
@@ -80,8 +78,6 @@ mod tests {
 
         // check txs effects hash
         assert_eq(outputs.header.content_commitment.txs_effects_hash, expected_txs_effects_hash);
-        // Check messages hash
-        assert_eq(outputs.l1_to_l2_messages_hash, expected_messages_hash);
     }
 
     #[test]

noir-projects/noir-protocol-circuits/crates/rollup-lib/src/root/root_rollup_inputs.nr

@@ -87,9 +87,7 @@ impl RootRollupInputs {
         RootRollupPublicInputs {
             aggregation_object,
             archive,
-            header,
-            // TODO(#5264): update this when implementing the new message model
-            l1_to_l2_messages_hash: compute_messages_hash(self.new_l1_to_l2_messages)
+            header
         }
     }
 }

noir-projects/noir-protocol-circuits/crates/rollup-lib/src/root/root_rollup_public_inputs.nr

@@ -1,6 +1,6 @@
 use dep::types::{
-    abis::{append_only_tree_snapshot::AppendOnlyTreeSnapshot, global_variables::GlobalVariables},
-    constants::{NUM_FIELDS_PER_SHA256}, header::Header
+    abis::append_only_tree_snapshot::AppendOnlyTreeSnapshot,
+    header::Header
 };
 use dep::types::mocked::AggregationObject;
 
@@ -13,7 +13,4 @@ struct RootRollupPublicInputs {
 
     // New block header
     header: Header,
-
-    // TODO(#5264): Nuke this once message hashing is moved out
-    l1_to_l2_messages_hash : [Field; NUM_FIELDS_PER_SHA256],
 }

yarn-project/archiver/src/archiver/archiver.ts

@@ -195,12 +195,7 @@ export class Archiver implements ArchiveSource {
       );
     }
 
-    await this.store.addL1ToL2Messages(
-      retrievedL1ToL2Messages.retrievedData,
-      // -1n because the function expects the last block in which the message was emitted and not the one after next
-      // TODO(#5264): Check whether this could be cleaned up - `nextEthBlockNumber` value doesn't seem to be used much
-      retrievedL1ToL2Messages.nextEthBlockNumber - 1n,
-    );
+    await this.store.addL1ToL2Messages(retrievedL1ToL2Messages);
 
     // Read all data from chain and then write to our stores at the end
     const nextExpectedL2BlockNum = BigInt((await this.store.getSynchedL2BlockNumber()) + 1);
@@ -430,9 +425,9 @@ export class Archiver implements ArchiveSource {
   /**
    * Gets the L1 to L2 message index in the L1 to L2 message tree.
    * @param l1ToL2Message - The L1 to L2 message.
-   * @returns The index of the L1 to L2 message in the L1 to L2 message tree.
+   * @returns The index of the L1 to L2 message in the L1 to L2 message tree (undefined if not found).
    */
-  getL1ToL2MessageIndex(l1ToL2Message: Fr): Promise<bigint> {
+  getL1ToL2MessageIndex(l1ToL2Message: Fr): Promise<bigint | undefined> {
     return this.store.getL1ToL2MessageIndex(l1ToL2Message);
   }
 

yarn-project/archiver/src/archiver/archiver_store.ts

@@ -14,6 +14,8 @@ import { Fr } from '@aztec/circuits.js';
 import { AztecAddress } from '@aztec/foundation/aztec-address';
 import { ContractClassPublic, ContractInstanceWithAddress } from '@aztec/types/contracts';
 
+import { DataRetrieval } from './data_retrieval.js';
+
 /**
  * Represents the latest L1 block processed by the archiver for various objects in L2.
  */
@@ -88,11 +90,10 @@ export interface ArchiverDataStore {
 
   /**
    * Append L1 to L2 messages to the store.
-   * @param messages - The L1 to L2 messages to be added to the store.
-   * @param lastMessageL1BlockNumber - The L1 block number in which the last message was emitted.
+   * @param messages - The L1 to L2 messages to be added to the store and the last processed L1 block.
    * @returns True if the operation is successful.
    */
-  addL1ToL2Messages(messages: InboxLeaf[], lastMessageL1BlockNumber: bigint): Promise<boolean>;
+  addL1ToL2Messages(messages: DataRetrieval<InboxLeaf>): Promise<boolean>;
 
   /**
    * Gets L1 to L2 message (to be) included in a given block.
@@ -104,9 +105,9 @@ export interface ArchiverDataStore {
   /**
    * Gets the L1 to L2 message index in the L1 to L2 message tree.
    * @param l1ToL2Message - The L1 to L2 message.
-   * @returns The index of the L1 to L2 message in the L1 to L2 message tree.
+   * @returns The index of the L1 to L2 message in the L1 to L2 message tree (undefined if not found).
    */
-  getL1ToL2MessageIndex(l1ToL2Message: Fr): Promise<bigint>;
+  getL1ToL2MessageIndex(l1ToL2Message: Fr): Promise<bigint | undefined>;
 
   /**
    * Gets up to `limit` amount of logs starting from `from`.

yarn-project/archiver/src/archiver/archiver_store_test_suite.ts

@@ -98,7 +98,10 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch
       });
 
       it('returns the L1 block number that most recently added messages from inbox', async () => {
-        await store.addL1ToL2Messages([new InboxLeaf(0n, 0n, Fr.ZERO)], 1n);
+        await store.addL1ToL2Messages({
+          lastProcessedL1BlockNumber: 1n,
+          retrievedData: [new InboxLeaf(0n, 0n, Fr.ZERO)],
+        });
         await expect(store.getSynchedL1BlockNumbers()).resolves.toEqual({
           blocks: 0n,
           messages: 1n,
@@ -169,7 +172,7 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch
       it('returns messages in correct order', async () => {
         const msgs = generateBlockMessages(l2BlockNumber, l1ToL2MessageSubtreeSize);
         const shuffledMessages = msgs.slice().sort(() => randomInt(1) - 0.5);
-        await store.addL1ToL2Messages(shuffledMessages, 100n);
+        await store.addL1ToL2Messages({ lastProcessedL1BlockNumber: 100n, retrievedData: shuffledMessages });
         const retrievedMessages = await store.getL1ToL2Messages(l2BlockNumber);
 
         const expectedLeavesOrder = msgs.map(msg => msg.leaf);
@@ -182,7 +185,7 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch
         // --> with that there will be a gap and it will be impossible to sequence the messages
         msgs[4] = new InboxLeaf(l2BlockNumber, BigInt(l1ToL2MessageSubtreeSize - 1), Fr.random());
 
-        await store.addL1ToL2Messages(msgs, 100n);
+        await store.addL1ToL2Messages({ lastProcessedL1BlockNumber: 100n, retrievedData: msgs });
         await expect(async () => {
           await store.getL1ToL2Messages(l2BlockNumber);
         }).rejects.toThrow(`L1 to L2 message gap found in block ${l2BlockNumber}`);
@@ -192,7 +195,7 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch
         const msgs = generateBlockMessages(l2BlockNumber, l1ToL2MessageSubtreeSize + 1);
 
         await expect(async () => {
-          await store.addL1ToL2Messages(msgs, 100n);
+          await store.addL1ToL2Messages({ lastProcessedL1BlockNumber: 100n, retrievedData: msgs });
         }).rejects.toThrow(`Message index ${l1ToL2MessageSubtreeSize} out of subtree range`);
       });
     });

yarn-project/archiver/src/archiver/data_retrieval.ts

@@ -16,11 +16,11 @@ import {
 /**
  * Data retrieved from logs
  */
-type DataRetrieval<T> = {
+export type DataRetrieval<T> = {
   /**
-   * The next block number.
+   * Blocknumber of the last L1 block from which we obtained data.
    */
-  nextEthBlockNumber: bigint;
+  lastProcessedL1BlockNumber: bigint;
   /**
    * The data returned.
    */
@@ -69,7 +69,7 @@ export async function retrieveBlockMetadataFromRollup(
     searchStartBlock = l2BlockProcessedLogs[l2BlockProcessedLogs.length - 1].blockNumber! + 1n;
     expectedNextL2BlockNum += BigInt(newBlockMetadata.length);
   } while (blockUntilSynced && searchStartBlock <= searchEndBlock);
-  return { nextEthBlockNumber: searchStartBlock, retrievedData: retrievedBlockMetadata };
+  return { lastProcessedL1BlockNumber: searchStartBlock - 1n, retrievedData: retrievedBlockMetadata };
 }
 
 /**
@@ -108,7 +108,7 @@ export async function retrieveBlockBodiesFromAvailabilityOracle(
     retrievedBlockBodies.push(...newBlockBodies);
     searchStartBlock = l2TxsPublishedLogs[l2TxsPublishedLogs.length - 1].blockNumber! + 1n;
   } while (blockUntilSynced && searchStartBlock <= searchEndBlock);
-  return { nextEthBlockNumber: searchStartBlock, retrievedData: retrievedBlockBodies };
+  return { lastProcessedL1BlockNumber: searchStartBlock - 1n, retrievedData: retrievedBlockBodies };
 }
 
 /**
@@ -141,5 +141,5 @@ export async function retrieveL1ToL2Messages(
     // handles the case when there are no new messages:
     searchStartBlock = (leafInsertedLogs.findLast(msgLog => !!msgLog)?.blockNumber || searchStartBlock) + 1n;
   } while (blockUntilSynced && searchStartBlock <= searchEndBlock);
-  return { nextEthBlockNumber: searchStartBlock, retrievedData: retrievedL1ToL2Messages };
+  return { lastProcessedL1BlockNumber: searchStartBlock - 1n, retrievedData: retrievedL1ToL2Messages };
 }

yarn-project/archiver/src/archiver/kv_archiver_store/kv_archiver_store.ts

@@ -17,6 +17,7 @@ import { AztecKVStore } from '@aztec/kv-store';
 import { ContractClassPublic, ContractInstanceWithAddress } from '@aztec/types/contracts';
 
 import { ArchiverDataStore, ArchiverL1SynchPoint } from '../archiver_store.js';
+import { DataRetrieval } from '../data_retrieval.js';
 import { BlockBodyStore } from './block_body_store.js';
 import { BlockStore } from './block_store.js';
 import { ContractClassStore } from './contract_class_store.js';
@@ -145,20 +146,19 @@ export class KVArchiverDataStore implements ArchiverDataStore {
 
   /**
    * Append L1 to L2 messages to the store.
-   * @param messages - The L1 to L2 messages to be added to the store.
-   * @param lastMessageL1BlockNumber - The L1 block number in which the last message was emitted.
+   * @param messages - The L1 to L2 messages to be added to the store and the last processed L1 block.
    * @returns True if the operation is successful.
    */
-  addL1ToL2Messages(messages: InboxLeaf[], lastMessageL1BlockNumber: bigint): Promise<boolean> {
-    return Promise.resolve(this.#messageStore.addL1ToL2Messages(messages, lastMessageL1BlockNumber));
+  addL1ToL2Messages(messages: DataRetrieval<InboxLeaf>): Promise<boolean> {
+    return Promise.resolve(this.#messageStore.addL1ToL2Messages(messages));
   }
 
   /**
    * Gets the L1 to L2 message index in the L1 to L2 message tree.
    * @param l1ToL2Message - The L1 to L2 message.
-   * @returns The index of the L1 to L2 message in the L1 to L2 message tree.
+   * @returns The index of the L1 to L2 message in the L1 to L2 message tree (undefined if not found).
    */
-  public getL1ToL2MessageIndex(l1ToL2Message: Fr): Promise<bigint> {
+  public getL1ToL2MessageIndex(l1ToL2Message: Fr): Promise<bigint | undefined> {
     return Promise.resolve(this.#messageStore.getL1ToL2MessageIndex(l1ToL2Message));
   }
 

yarn-project/archiver/src/archiver/kv_archiver_store/message_store.ts

@@ -8,6 +8,8 @@ import {
 import { createDebugLogger } from '@aztec/foundation/log';
 import { AztecKVStore, AztecMap, AztecSingleton } from '@aztec/kv-store';
 
+import { DataRetrieval } from '../data_retrieval.js';
+
 /**
  * LMDB implementation of the ArchiverDataStore interface.
  */
@@ -36,20 +38,19 @@ export class MessageStore {
 
   /**
    * Append L1 to L2 messages to the store.
-   * @param messages - The L1 to L2 messages to be added to the store.
-   * @param lastMessageL1BlockNumber - The L1 block number in which the last message was emitted.
+   * @param messages - The L1 to L2 messages to be added to the store and the last processed L1 block.
    * @returns True if the operation is successful.
    */
-  addL1ToL2Messages(messages: InboxLeaf[], lastMessageL1BlockNumber: bigint): Promise<boolean> {
+  addL1ToL2Messages(messages: DataRetrieval<InboxLeaf>): Promise<boolean> {
     return this.db.transaction(() => {
       const lastL1BlockNumber = this.#lastL1BlockMessages.get() ?? 0n;
-      if (lastL1BlockNumber >= lastMessageL1BlockNumber) {
+      if (lastL1BlockNumber >= messages.lastProcessedL1BlockNumber) {
         return false;
       }
 
-      void this.#lastL1BlockMessages.set(lastMessageL1BlockNumber);
+      void this.#lastL1BlockMessages.set(messages.lastProcessedL1BlockNumber);
 
-      for (const message of messages) {
+      for (const message of messages.retrievedData) {
         if (message.index >= this.#l1ToL2MessagesSubtreeSize) {
           throw new Error(`Message index ${message.index} out of subtree range`);
         }
@@ -69,13 +70,10 @@ export class MessageStore {
   /**
    * Gets the L1 to L2 message index in the L1 to L2 message tree.
    * @param l1ToL2Message - The L1 to L2 message.
-   * @returns The index of the L1 to L2 message in the L1 to L2 message tree.
+   * @returns The index of the L1 to L2 message in the L1 to L2 message tree (undefined if not found).
    */
-  public getL1ToL2MessageIndex(l1ToL2Message: Fr): Promise<bigint> {
+  public getL1ToL2MessageIndex(l1ToL2Message: Fr): Promise<bigint | undefined> {
     const index = this.#l1ToL2MessageIndices.get(l1ToL2Message.toString());
-    if (index === undefined) {
-      throw new Error(`L1 to L2 message index not found in the store for message ${l1ToL2Message.toString()}`);
-    }
     return Promise.resolve(index);
   }
 

yarn-project/archiver/src/archiver/memory_archiver_store/l1_to_l2_message_store.ts

@@ -52,9 +52,9 @@ export class L1ToL2MessageStore {
   /**
    * Gets the L1 to L2 message index in the L1 to L2 message tree.
    * @param l1ToL2Message - The L1 to L2 message.
-   * @returns The index of the L1 to L2 message in the L1 to L2 message tree.
+   * @returns The index of the L1 to L2 message in the L1 to L2 message tree (undefined if not found).
    */
-  getMessageIndex(l1ToL2Message: Fr): bigint {
+  getMessageIndex(l1ToL2Message: Fr): bigint | undefined {
     for (const [key, message] of this.store.entries()) {
       if (message.equals(l1ToL2Message)) {
         const [blockNumber, messageIndex] = key.split('-');
@@ -64,6 +64,6 @@ export class L1ToL2MessageStore {
         return indexInTheWholeTree;
       }
     }
-    throw new Error(`L1 to L2 message index not found in the store for message ${l1ToL2Message.toString()}`);
+    return undefined;
   }
 }