feat: returning non-nullified messages only (#5390)

Fixes
[#3976](#3976)

docs/docs/developers/debugging/sandbox-errors.md

@@ -177,7 +177,7 @@ Users may create a proof against a historical state in Aztec. The rollup circuit
 
 ## Archiver Errors
 
-- "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.
+- "No non-nullified 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 - the 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 transactions 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 need 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.
 

noir-projects/aztec-nr/aztec/src/context/private_context.nr

@@ -4,8 +4,7 @@ use crate::{
     oracle::{
     arguments, call_private_function::call_private_function_internal,
     enqueue_public_function_call::enqueue_public_function_call_internal, context::get_portal_address,
-    header::get_header_at, nullifier_key::{get_nullifier_key_pair, NullifierKeyPair},
-    debug_log::debug_log
+    header::get_header_at, nullifier_key::{get_nullifier_key_pair, NullifierKeyPair}
 }
 };
 use dep::protocol_types::{

noir-projects/aztec-nr/aztec/src/context/public_context.nr

@@ -233,6 +233,8 @@ impl PublicContextInterface for PublicContext {
         self.new_l2_to_l1_msgs.push(message);
     }
 
+    // We can consume message with a secret in public context because the message cannot be modified and therefore
+    // there is no front-running risk (e.g. somebody could front run you to claim your tokens to your address).
     fn consume_l1_to_l2_message(&mut self, content: Field, secret: Field, sender: EthAddress) {
         let this = (*self).this_address();
         let nullifier = process_l1_to_l2_message(

noir-projects/aztec-nr/aztec/src/messaging.nr

@@ -15,7 +15,7 @@ pub fn process_l1_to_l2_message(
     content: Field,
     secret: Field
 ) -> Field {
-    let msg = L1ToL2Message::new(
+    let mut msg = L1ToL2Message::new(
         portal_contract_address,
         chain_id,
         storage_contract_address,
@@ -25,7 +25,7 @@ pub fn process_l1_to_l2_message(
     );
     let message_hash = msg.hash();
 
-    let returned_message = get_l1_to_l2_membership_witness(message_hash);
+    let returned_message = get_l1_to_l2_membership_witness(storage_contract_address, message_hash, secret);
     let leaf_index = returned_message[0];
     let sibling_path = arr_copy_slice(returned_message, [0; L1_TO_L2_MSG_TREE_HEIGHT], 1);
 
@@ -34,5 +34,12 @@ pub fn process_l1_to_l2_message(
     let root = compute_merkle_root(message_hash, leaf_index, sibling_path);
     assert(root == l1_to_l2_root, "Message not in state");
 
+    // Note: Had to add this line to make it work (wasted an hour debugging this). L1ToL2Message noir struct is
+    // an abomination and it would not have ever got to that state if people followed the logical rule of 
+    // "deserialize(serialize(object)) == object" always being true (there are a few params which are not
+    // serialzied). Will refactor that in a separate PR.
+    // TODO(#5420)
+    msg.tree_index = leaf_index;
+
     msg.compute_nullifier()
 }

noir-projects/aztec-nr/aztec/src/messaging/l1_to_l2_message.nr

@@ -39,8 +39,8 @@ impl L1ToL2Message {
 
     pub fn deserialize(
         fields: [Field; L1_TO_L2_MESSAGE_LENGTH],
-        secret: Field,
-        tree_index: Field
+        secret: Field, // TODO(#5420): refactor this so that this param does not have to be passed separately
+        tree_index: Field // TODO(#5420): refactor this so that this param does not have to be passed separately
     ) -> L1ToL2Message {
         L1ToL2Message {
             sender: EthAddress::from_field(fields[0]),

noir-projects/aztec-nr/aztec/src/oracle/get_l1_to_l2_membership_witness.nr

@@ -1,9 +1,12 @@
-use dep::protocol_types::constants::L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH;
+use dep::protocol_types::{
+    address::AztecAddress,
+    constants::L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH,
+};
 
 // Obtains membership witness (index and sibling path) for a message in the L1 to L2 message tree.
 #[oracle(getL1ToL2MembershipWitness)]
-fn get_l1_to_l2_membership_witness_oracle(_message_hash: Field) -> [Field; L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH] {}
+fn get_l1_to_l2_membership_witness_oracle(_contract_address: AztecAddress, _message_hash: Field, _secret: Field) -> [Field; L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH] {}
 
-unconstrained pub fn get_l1_to_l2_membership_witness(message_hash: Field) -> [Field; L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH] {
-    get_l1_to_l2_membership_witness_oracle(message_hash)
+unconstrained pub fn get_l1_to_l2_membership_witness(contract_address: AztecAddress, message_hash: Field, secret: Field) -> [Field; L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH] {
+    get_l1_to_l2_membership_witness_oracle(contract_address, message_hash, secret)
 }

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

@@ -476,12 +476,13 @@ export class Archiver implements ArchiveSource {
   }
 
   /**
-   * Gets the L1 to L2 message index in the L1 to L2 message tree.
+   * Gets the first L1 to L2 message index in the L1 to L2 message tree which is greater than or equal to `startIndex`.
    * @param l1ToL2Message - The L1 to L2 message.
+   * @param startIndex - The index to start searching from.
    * @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 | undefined> {
-    return this.store.getL1ToL2MessageIndex(l1ToL2Message);
+  getL1ToL2MessageIndex(l1ToL2Message: Fr, startIndex: bigint): Promise<bigint | undefined> {
+    return this.store.getL1ToL2MessageIndex(l1ToL2Message, startIndex);
   }
 
   getContractClassIds(): Promise<Fr[]> {

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

@@ -108,11 +108,12 @@ export interface ArchiverDataStore {
   getL1ToL2Messages(blockNumber: bigint): Promise<Fr[]>;
 
   /**
-   * Gets the L1 to L2 message index in the L1 to L2 message tree.
+   * Gets the first L1 to L2 message index in the L1 to L2 message tree which is greater than or equal to `startIndex`.
    * @param l1ToL2Message - The L1 to L2 message.
+   * @param startIndex - The index to start searching from.
    * @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 | undefined>;
+  getL1ToL2MessageIndex(l1ToL2Message: Fr, startIndex: bigint): Promise<bigint | undefined>;
 
   /**
    * Gets up to `limit` amount of logs starting from `from`.

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

@@ -211,6 +211,20 @@ export function describeArchiverDataStore(testName: string, getStore: () => Arch
           await store.addL1ToL2Messages({ lastProcessedL1BlockNumber: 100n, retrievedData: msgs });
         }).rejects.toThrow(`Message index ${l1ToL2MessageSubtreeSize} out of subtree range`);
       });
+
+      it('correctly handles duplicate messages', async () => {
+        const messageHash = Fr.random();
+
+        const msgs = [new InboxLeaf(1n, 0n, messageHash), new InboxLeaf(2n, 0n, messageHash)];
+
+        await store.addL1ToL2Messages({ lastProcessedL1BlockNumber: 100n, retrievedData: msgs });
+
+        const index1 = (await store.getL1ToL2MessageIndex(messageHash, 0n))!;
+        const index2 = await store.getL1ToL2MessageIndex(messageHash, index1 + 1n);
+
+        expect(index2).toBeDefined();
+        expect(index2).toBeGreaterThan(index1);
+      });
     });
 
     describe('contractInstances', () => {

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

@@ -167,12 +167,13 @@ export class KVArchiverDataStore implements ArchiverDataStore {
   }
 
   /**
-   * Gets the L1 to L2 message index in the L1 to L2 message tree.
+   * Gets the first L1 to L2 message index in the L1 to L2 message tree which is greater than or equal to `startIndex`.
    * @param l1ToL2Message - The L1 to L2 message.
+   * @param startIndex - The index to start searching from.
    * @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 | undefined> {
-    return Promise.resolve(this.#messageStore.getL1ToL2MessageIndex(l1ToL2Message));
+  getL1ToL2MessageIndex(l1ToL2Message: Fr, startIndex: bigint): Promise<bigint | undefined> {
+    return Promise.resolve(this.#messageStore.getL1ToL2MessageIndex(l1ToL2Message, startIndex));
   }
 
   /**

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

@@ -15,7 +15,7 @@ import { DataRetrieval } from '../data_retrieval.js';
  */
 export class MessageStore {
   #l1ToL2Messages: AztecMap<string, Buffer>;
-  #l1ToL2MessageIndices: AztecMap<string, bigint>;
+  #l1ToL2MessageIndices: AztecMap<string, bigint[]>; // We store array of bigints here because there can be duplicate messages
   #lastL1BlockMessages: AztecSingleton<bigint>;
 
   #log = createDebugLogger('aztec:archiver:message_store');
@@ -60,20 +60,25 @@ export class MessageStore {
         const indexInTheWholeTree =
           (message.blockNumber - BigInt(INITIAL_L2_BLOCK_NUM)) * BigInt(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP) +
           message.index;
-        void this.#l1ToL2MessageIndices.setIfNotExists(message.leaf.toString(), indexInTheWholeTree);
+
+        const indices = this.#l1ToL2MessageIndices.get(message.leaf.toString()) ?? [];
+        indices.push(indexInTheWholeTree);
+        void this.#l1ToL2MessageIndices.set(message.leaf.toString(), indices);
       }
 
       return true;
     });
   }
 
   /**
-   * Gets the L1 to L2 message index in the L1 to L2 message tree.
+   * Gets the first L1 to L2 message index in the L1 to L2 message tree which is greater than or equal to `startIndex`.
    * @param l1ToL2Message - The L1 to L2 message.
+   * @param startIndex - The index to start searching from.
    * @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 | undefined> {
-    const index = this.#l1ToL2MessageIndices.get(l1ToL2Message.toString());
+  getL1ToL2MessageIndex(l1ToL2Message: Fr, startIndex: bigint): Promise<bigint | undefined> {
+    const indices = this.#l1ToL2MessageIndices.get(l1ToL2Message.toString()) ?? [];
+    const index = indices.find(i => i >= startIndex);
     return Promise.resolve(index);
   }
 

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

@@ -25,9 +25,22 @@ describe('l1_to_l2_message_store', () => {
     expect(retrievedMsgs.length).toEqual(10);
 
     const msg = msgs[4];
-    const index = store.getMessageIndex(msg.leaf);
+    const index = store.getMessageIndex(msg.leaf, 0n);
     expect(index).toEqual(
       (blockNumber - BigInt(INITIAL_L2_BLOCK_NUM)) * BigInt(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP) + msg.index,
     );
   });
+
+  it('correctly handles duplicate messages', () => {
+    const messageHash = Fr.random();
+
+    store.addMessage(new InboxLeaf(1n, 0n, messageHash));
+    store.addMessage(new InboxLeaf(2n, 0n, messageHash));
+
+    const index1 = store.getMessageIndex(messageHash, 0n)!;
+    const index2 = store.getMessageIndex(messageHash, index1 + 1n);
+
+    expect(index2).toBeDefined();
+    expect(index2).toBeGreaterThan(index1);
+  });
 });

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

@@ -49,17 +49,21 @@ export class L1ToL2MessageStore {
   }
 
   /**
-   * Gets the L1 to L2 message index in the L1 to L2 message tree.
+   * Gets the first L1 to L2 message index in the L1 to L2 message tree which is greater than or equal to `startIndex`.
    * @param l1ToL2Message - The L1 to L2 message.
+   * @param startIndex - The index to start searching from.
    * @returns The index of the L1 to L2 message in the L1 to L2 message tree (undefined if not found).
    */
-  getMessageIndex(l1ToL2Message: Fr): bigint | undefined {
+  getMessageIndex(l1ToL2Message: Fr, startIndex: bigint): bigint | undefined {
     for (const [key, message] of this.store.entries()) {
       if (message.equals(l1ToL2Message)) {
-        const [blockNumber, messageIndex] = key.split('-');
+        const keyParts = key.split('-');
+        const [blockNumber, messageIndex] = [BigInt(keyParts[0]), BigInt(keyParts[1])];
         const indexInTheWholeTree =
-          (BigInt(blockNumber) - BigInt(INITIAL_L2_BLOCK_NUM)) * BigInt(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP) +
-          BigInt(messageIndex);
+          (blockNumber - BigInt(INITIAL_L2_BLOCK_NUM)) * BigInt(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP) + messageIndex;
+        if (indexInTheWholeTree < startIndex) {
+          continue;
+        }
         return indexInTheWholeTree;
       }
     }

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

@@ -213,12 +213,13 @@ export class MemoryArchiverStore implements ArchiverDataStore {
   }
 
   /**
-   * Gets the L1 to L2 message index in the L1 to L2 message tree.
+   * Gets the first L1 to L2 message index in the L1 to L2 message tree which is greater than or equal to `startIndex`.
    * @param l1ToL2Message - The L1 to L2 message.
+   * @param startIndex - The index to start searching from.
    * @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 | undefined> {
-    return Promise.resolve(this.l1ToL2Messages.getMessageIndex(l1ToL2Message));
+  getL1ToL2MessageIndex(l1ToL2Message: Fr, startIndex: bigint): Promise<bigint | undefined> {
+    return Promise.resolve(this.l1ToL2Messages.getMessageIndex(l1ToL2Message, startIndex));
   }
 
   /**

yarn-project/aztec-node/src/aztec-node/server.ts

@@ -33,6 +33,7 @@ import {
   L2_TO_L1_MESSAGE_LENGTH,
   NOTE_HASH_TREE_HEIGHT,
   NULLIFIER_TREE_HEIGHT,
+  NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP,
   NullifierLeafPreimage,
   PUBLIC_DATA_TREE_HEIGHT,
   PublicDataTreeLeafPreimage,
@@ -388,13 +389,15 @@ export class AztecNodeService implements AztecNode {
    * Returns the index and a sibling path for a leaf in the committed l1 to l2 data tree.
    * @param blockNumber - The block number at which to get the data.
    * @param l1ToL2Message - The l1ToL2Message to get the index / sibling path for.
+   * @param startIndex - The index to start searching from (used when skipping nullified messages)
    * @returns A tuple of the index and the sibling path of the L1ToL2Message (undefined if not found).
    */
   public async getL1ToL2MessageMembershipWitness(
     blockNumber: L2BlockNumber,
     l1ToL2Message: Fr,
+    startIndex = 0n,
   ): Promise<[bigint, SiblingPath<typeof L1_TO_L2_MSG_TREE_HEIGHT>] | undefined> {
-    const index = await this.l1ToL2MessageSource.getL1ToL2MessageIndex(l1ToL2Message);
+    const index = await this.l1ToL2MessageSource.getL1ToL2MessageIndex(l1ToL2Message, startIndex);
     if (index === undefined) {
       return undefined;
     }
@@ -409,10 +412,15 @@ export class AztecNodeService implements AztecNode {
   /**
    * Returns whether an L1 to L2 message is synced by archiver and if it's ready to be included in a block.
    * @param l1ToL2Message - The L1 to L2 message to check.
+   * @param startL2BlockNumber - The block number after which we are interested in checking if the message was
+   * included.
+   * @remarks We pass in the minL2BlockNumber because there can be duplicate messages and the block number allow us
+   * to skip the duplicates (we know after which block a given message is to be included).
    * @returns Whether the message is synced and ready to be included in a block.
    */
-  public async isL1ToL2MessageSynced(l1ToL2Message: Fr): Promise<boolean> {
-    return (await this.l1ToL2MessageSource.getL1ToL2MessageIndex(l1ToL2Message)) !== undefined;
+  public async isL1ToL2MessageSynced(l1ToL2Message: Fr, startL2BlockNumber = INITIAL_L2_BLOCK_NUM): Promise<boolean> {
+    const startIndex = BigInt(startL2BlockNumber - INITIAL_L2_BLOCK_NUM) * BigInt(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP);
+    return (await this.l1ToL2MessageSource.getL1ToL2MessageIndex(l1ToL2Message, startIndex)) !== undefined;
   }
 
   /**

yarn-project/circuit-types/src/interfaces/aztec-node.ts

@@ -62,19 +62,25 @@ export interface AztecNode {
    * Returns the index and a sibling path for a leaf in the committed l1 to l2 data tree.
    * @param blockNumber - The block number at which to get the data.
    * @param l1ToL2Message - The l1ToL2Message to get the index / sibling path for.
-   * @returns A tuple of the index and the sibling path of the message  (undefined if not found).
+   * @param startIndex - The index to start searching from.
+   * @returns A tuple of the index and the sibling path of the L1ToL2Message (undefined if not found).
    */
   getL1ToL2MessageMembershipWitness(
     blockNumber: L2BlockNumber,
     l1ToL2Message: Fr,
+    startIndex: bigint,
   ): Promise<[bigint, SiblingPath<typeof L1_TO_L2_MSG_TREE_HEIGHT>] | undefined>;
 
   /**
    * Returns whether an L1 to L2 message is synced by archiver and if it's ready to be included in a block.
    * @param l1ToL2Message - The L1 to L2 message to check.
+   * @param startL2BlockNumber - The block number after which we are interested in checking if the message was
+   * included.
+   * @remarks We pass in the minL2BlockNumber because there can be duplicate messages and the block number allow us
+   * to skip the duplicates (we know after which block a given message is to be included).
    * @returns Whether the message is synced and ready to be included in a block.
    */
-  isL1ToL2MessageSynced(l1ToL2Message: Fr): Promise<boolean>;
+  isL1ToL2MessageSynced(l1ToL2Message: Fr, startL2BlockNumber: number): Promise<boolean>;
 
   /**
    * Returns a membership witness of an l2ToL1Message in an ephemeral l2 to l1 message tree.

yarn-project/circuit-types/src/messaging/l1_to_l2_message_source.ts

@@ -12,11 +12,12 @@ export interface L1ToL2MessageSource {
   getL1ToL2Messages(blockNumber: bigint): Promise<Fr[]>;
 
   /**
-   * Gets the L1 to L2 message index in the L1 to L2 message tree.
+   * Gets the first L1 to L2 message index in the L1 to L2 message tree which is greater than or equal to `startIndex`.
    * @param l1ToL2Message - The L1 to L2 message.
+   * @param startIndex - The index to start searching from.
    * @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 | undefined>;
+  getL1ToL2MessageIndex(l1ToL2Message: Fr, startIndex: bigint): Promise<bigint | undefined>;
 
   /**
    * Gets the number of the latest L2 block processed by the implementation.

yarn-project/circuits.js/src/hash/hash.ts

@@ -173,3 +173,13 @@ export function computeNullifierHash(input: SideEffectLinkedToNoteHash) {
 export function computeMessageSecretHash(secretMessage: Fr) {
   return pedersenHash([secretMessage.toBuffer()], GeneratorIndex.L1_TO_L2_MESSAGE_SECRET);
 }
+
+export function computeL1ToL2MessageNullifier(
+  contract: AztecAddress,
+  messageHash: Fr,
+  secret: Fr,
+  messageIndex: bigint,
+) {
+  const innerMessageNullifier = pedersenHash([messageHash, secret, messageIndex], GeneratorIndex.NULLIFIER);
+  return siloNullifier(contract, innerMessageNullifier);
+}