feat: enforce note hash read requests to read within own contract (#6310)

Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.

Author: LeilaWang

noir-projects/aztec-nr/aztec/src/note/note_getter.nr

@@ -4,7 +4,7 @@ use crate::note::{
     constants::{GET_NOTE_ORACLE_RETURN_LENGTH, MAX_NOTES_PER_PAGE, VIEW_NOTE_ORACLE_RETURN_LENGTH},
     note_getter_options::{NoteGetterOptions, Select, Sort, SortOrder, Comparator, NoteStatus, PropertySelector},
     note_interface::NoteInterface, note_viewer_options::NoteViewerOptions,
-    utils::compute_note_hash_for_consumption
+    utils::compute_note_hash_for_read_request
 };
 use crate::oracle;
 
@@ -92,7 +92,7 @@ pub fn get_note<Note, N>(
 
     check_note_header(*context, storage_slot, note);
 
-    let note_hash_for_read_request = compute_note_hash_for_consumption(note);
+    let note_hash_for_read_request = compute_note_hash_for_read_request(note);
 
     context.push_note_hash_read_request(note_hash_for_read_request);
     note
@@ -130,7 +130,7 @@ pub fn _get_notes_constrain_get_notes_internal<Note, N, FILTER_ARGS>(
             }
             prev_fields = fields;
 
-            let note_hash_for_read_request = compute_note_hash_for_consumption(note);
+            let note_hash_for_read_request = compute_note_hash_for_read_request(note);
             // TODO(https://github.com/AztecProtocol/aztec-packages/issues/1410): test to ensure
             // failure if malicious oracle injects 0 nonce here for a "pre-existing" note.
             context.push_note_hash_read_request(note_hash_for_read_request);

noir-projects/aztec-nr/aztec/src/note/utils.nr

@@ -67,6 +67,16 @@ pub fn compute_note_hash_for_insertion<Note, N>(note: Note) -> Field where Note:
     compute_inner_note_hash(note)
 }
 
+pub fn compute_note_hash_for_read_request<Note, N>(note: Note) -> Field where Note: NoteInterface<N> {
+    let header = note.get_header();
+
+    if (header.nonce != 0) {
+        compute_unique_note_hash(note)
+    } else {
+        compute_inner_note_hash(note)
+    }
+}
+
 pub fn compute_note_hash_for_consumption<Note, N>(note: Note) -> Field where Note: NoteInterface<N> {
     let header = note.get_header();
     // There are 3 cases for reading a note intended for consumption:

noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr

@@ -4,7 +4,7 @@ use crate::note::{
     constants::MAX_NOTES_PER_PAGE, lifecycle::{create_note, create_note_hash_from_public, destroy_note},
     note_getter::{get_notes, view_notes}, note_getter_options::NoteGetterOptions,
     note_header::NoteHeader, note_interface::NoteInterface, note_viewer_options::NoteViewerOptions,
-    utils::compute_note_hash_for_consumption
+    utils::compute_note_hash_for_read_request
 };
 use crate::state_vars::storage::Storage;
 
@@ -58,7 +58,7 @@ impl<Note> PrivateSet<Note> {
     // docs:start:remove
     pub fn remove<N>(self, note: Note) where Note: NoteInterface<N> {
         let context = self.context.private.unwrap();
-        let note_hash = compute_note_hash_for_consumption(note);
+        let note_hash = compute_note_hash_for_read_request(note);
         let has_been_read = context.note_hash_read_requests.any(|r: ReadRequest| r.value == note_hash);
         assert(has_been_read, "Can only remove a note that has been read from the set.");
 

noir-projects/noir-protocol-circuits/crates/reset-kernel-lib/src/note_hash_read_request_reset.nr

@@ -59,12 +59,11 @@ mod tests {
     global note_hashes = inner_note_hashes.map(|n| silo_note_hash(contract_address, n));
 
     // Create 5 read requests. 0 and 3 are reading settled note hashes. 1, 2 and 4 are reading pending note hashes.
-    // TODO(#2847): Read request values for settled note hashes shouldn't have been siloed by apps.
     global read_requests = [
-        ReadRequest { value: note_hashes[1], counter: 11 }.scope(contract_address), // settled
+        ReadRequest { value: inner_note_hashes[1], counter: 11 }.scope(contract_address), // settled
         ReadRequest { value: inner_note_hashes[3], counter: 13 }.scope(contract_address), // pending
         ReadRequest { value: inner_note_hashes[2], counter: 39 }.scope(contract_address), // pending
-        ReadRequest { value: note_hashes[0], counter: 46 }.scope(contract_address), // settled
+        ReadRequest { value: inner_note_hashes[0], counter: 46 }.scope(contract_address), // settled
         ReadRequest { value: inner_note_hashes[3], counter: 78 }.scope(contract_address), // pending
     ];
 

noir-projects/noir-protocol-circuits/crates/types/src/abis/note_hash_leaf_preimage.nr

@@ -29,10 +29,8 @@ impl LeafPreimage for NoteHashLeafPreimage {
 
 impl Readable for NoteHashLeafPreimage {
     fn assert_match_read_request(self, read_request: ScopedReadRequest) {
-        // TODO(#2847): Read request value shouldn't have been siloed by apps.
-        // let siloed_value = silo_note_hash(read_request.contract_address, read_request.value);
-        // assert_eq(self.value, siloed_value, "Value of the note hash leaf does not match read request");
-        assert_eq(self.value, read_request.value(), "Value of the note hash leaf does not match read request");
+        let siloed_value = silo_note_hash(read_request.contract_address, read_request.value());
+        assert_eq(self.value, siloed_value, "Value of the note hash leaf does not match read request");
     }
 }
 

yarn-project/circuits.js/src/hints/build_note_hash_read_request_hints.test.ts

@@ -42,32 +42,22 @@ describe('buildNoteHashReadRequestHints', () => {
 
   const makeNoteHash = (value: number, counter = 1) => new NoteHash(new Fr(value), counter).scope(0, contractAddress);
 
-  const readPendingNoteHash = ({
-    noteHashIndex,
-    readRequestIndex = numReadRequests,
-    hintIndex = numPendingReads,
-  }: {
-    noteHashIndex: number;
-    readRequestIndex?: number;
-    hintIndex?: number;
-  }) => {
+  const readPendingNoteHash = (noteHashIndex: number) => {
+    const readRequestIndex = numReadRequests;
+    const hintIndex = numPendingReads;
     noteHashReadRequests[readRequestIndex] = makeReadRequest(innerNoteHash(noteHashIndex));
     expectedHints.readRequestStatuses[readRequestIndex] = ReadRequestStatus.pending(hintIndex);
     expectedHints.pendingReadHints[hintIndex] = new PendingReadHint(readRequestIndex, noteHashIndex);
     numReadRequests++;
     numPendingReads++;
   };
 
-  const readSettledNoteHash = ({
-    readRequestIndex = numReadRequests,
-    hintIndex = numSettledReads,
-  }: {
-    readRequestIndex?: number;
-    hintIndex?: number;
-  } = {}) => {
-    const value = settledNoteHashes[hintIndex];
-    noteHashLeafIndexMap.set(value.toBigInt(), settledLeafIndexes[hintIndex]);
-    noteHashReadRequests[readRequestIndex] = new ReadRequest(value, 1).scope(contractAddress);
+  const readSettledNoteHash = (noteHashIndex: number) => {
+    const readRequestIndex = numReadRequests;
+    const hintIndex = numSettledReads;
+    const value = settledNoteHashes[noteHashIndex];
+    noteHashLeafIndexMap.set(value.toBigInt(), settledLeafIndexes[noteHashIndex]);
+    noteHashReadRequests[readRequestIndex] = makeReadRequest(settledNoteHashInnerValues[noteHashIndex]);
     expectedHints.readRequestStatuses[readRequestIndex] = ReadRequestStatus.settled(hintIndex);
     expectedHints.settledReadHints[hintIndex] = new SettledReadHint(readRequestIndex, {} as any, value);
     numReadRequests++;
@@ -93,32 +83,32 @@ describe('buildNoteHashReadRequestHints', () => {
   });
 
   it('builds hints for pending note hash read requests', async () => {
-    readPendingNoteHash({ noteHashIndex: 2 });
-    readPendingNoteHash({ noteHashIndex: 1 });
+    readPendingNoteHash(2);
+    readPendingNoteHash(1);
     const hints = await buildHints();
     expect(hints).toEqual(expectedHints);
   });
 
   it('builds hints for settled note hash read requests', async () => {
-    readSettledNoteHash();
-    readSettledNoteHash();
+    readSettledNoteHash(0);
+    readSettledNoteHash(1);
     const hints = await buildHints();
     expect(hints).toEqual(expectedHints);
   });
 
   it('builds hints for mixed pending and settled note hash read requests', async () => {
-    readPendingNoteHash({ noteHashIndex: 2 });
-    readSettledNoteHash();
-    readSettledNoteHash();
-    readPendingNoteHash({ noteHashIndex: 1 });
-    readPendingNoteHash({ noteHashIndex: 1 });
-    readSettledNoteHash();
+    readPendingNoteHash(2);
+    readSettledNoteHash(2);
+    readSettledNoteHash(0);
+    readPendingNoteHash(1);
+    readPendingNoteHash(1);
+    readSettledNoteHash(2);
     const hints = await buildHints();
     expect(hints).toEqual(expectedHints);
   });
 
   it('throws if cannot find a match in pending set and in the tree', async () => {
-    readPendingNoteHash({ noteHashIndex: 2 });
+    readPendingNoteHash(2);
     // Tweak the value of the read request.
     noteHashReadRequests[0].readRequest.value = new Fr(123);
     await expect(() => buildHints()).rejects.toThrow('Read request is reading an unknown note hash.');

yarn-project/circuits.js/src/hints/build_note_hash_read_request_hints.ts

@@ -5,6 +5,7 @@ import {
   type MAX_NOTE_HASH_READ_REQUESTS_PER_TX,
   type NOTE_HASH_TREE_HEIGHT,
 } from '../constants.gen.js';
+import { siloNoteHash } from '../hash/index.js';
 import {
   type MembershipWitness,
   NoteHashReadRequestHintsBuilder,
@@ -51,13 +52,13 @@ export async function buildNoteHashReadRequestHints(
     if (pendingNoteHash !== undefined) {
       builder.addPendingReadRequest(i, pendingNoteHash.index);
     } else {
-      // TODO(#2847): Read request value for settled note hash shouldn't have been siloed by apps.
-      const leafIndex = noteHashLeafIndexMap.get(value.toBigInt());
+      const siloedValue = siloNoteHash(readRequest.contractAddress, value);
+      const leafIndex = noteHashLeafIndexMap.get(siloedValue.toBigInt());
       if (leafIndex === undefined) {
         throw new Error('Read request is reading an unknown note hash.');
       }
       const membershipWitness = await oracle.getNoteHashMembershipWitness(leafIndex);
-      builder.addSettledReadRequest(i, membershipWitness, value);
+      builder.addSettledReadRequest(i, membershipWitness, siloedValue);
     }
   }
   return builder.toHints();

yarn-project/simulator/src/client/private_execution.test.ts

@@ -438,7 +438,7 @@ describe('Private Execution test suite', () => {
 
       const readRequests = getNonEmptyItems(result.callStackItem.publicInputs.noteHashReadRequests).map(r => r.value);
       expect(readRequests).toHaveLength(consumedNotes.length);
-      expect(readRequests).toEqual(expect.arrayContaining(consumedNotes.map(n => n.siloedNoteHash)));
+      expect(readRequests).toEqual(expect.arrayContaining(consumedNotes.map(n => n.uniqueNoteHash)));
     });
 
     it('should be able to destroy_and_create with dummy notes', async () => {