refactor: remove addNullifiedNote from pxe (#11822)

This PR removes `addNullifiedNote` from the PXE as it is unused. It was also suggested to rework `getNoteHashAndOptionallyANullifier` into `getNoteHashAndNullifier`, in TS as now the second case where we don't get a nullifier is almost trivially used. I have kept `get_note_hash_and_optionally_a_nullifier` in nr intact as I wasn't sure this desired changing. If so, I will do that in another PR to keep things relatively isolated. Resolves #11821 Co-authored-by: Nicolás Venturo <[email protected]>
AztecProtocol · Feb 18, 2025 · 022f2d6 · 022f2d6
1 parent f897f03
commit 022f2d6
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 190 deletions.
diff --git a/yarn-project/aztec.js/src/wallet/base_wallet.ts b/yarn-project/aztec.js/src/wallet/base_wallet.ts
@@ -139,9 +139,6 @@ export abstract class BaseWallet implements Wallet {
   addNote(note: ExtendedNote): Promise<void> {
     return this.pxe.addNote(note, this.getAddress());
   }
-  addNullifiedNote(note: ExtendedNote): Promise<void> {
-    return this.pxe.addNullifiedNote(note);
-  }
   getBlock(number: number): Promise<L2Block | undefined> {
     return this.pxe.getBlock(number);
   }

diff --git a/yarn-project/circuit-types/src/interfaces/pxe.test.ts b/yarn-project/circuit-types/src/interfaces/pxe.test.ts
@@ -236,10 +236,6 @@ describe('PXESchema', () => {
     await context.client.addNote(await ExtendedNote.random(), address);
   });
 
-  it('addNullifiedNote', async () => {
-    await context.client.addNullifiedNote(await ExtendedNote.random());
-  });
-
   it('getBlock', async () => {
     const result = await context.client.getBlock(1);
     expect(result).toBeInstanceOf(L2Block);
@@ -463,10 +459,6 @@ class MockPXE implements PXE {
     expect(scope).toEqual(this.address);
     return Promise.resolve();
   }
-  addNullifiedNote(note: ExtendedNote): Promise<void> {
-    expect(note).toBeInstanceOf(ExtendedNote);
-    return Promise.resolve();
-  }
   getBlock(number: number): Promise<L2Block | undefined> {
     return Promise.resolve(L2Block.random(number));
   }

diff --git a/yarn-project/circuit-types/src/interfaces/pxe.ts b/yarn-project/circuit-types/src/interfaces/pxe.ts
@@ -283,16 +283,6 @@ export interface PXE {
    */
   addNote(note: ExtendedNote, scope?: AztecAddress): Promise<void>;
 
-  /**
-   * Adds a nullified note to the database.
-   * @throws If the note hash of the note doesn't exist in the tree.
-   * @param note - The note to add.
-   * @dev We are not deriving a nullifier in this function since that would require having the nullifier secret key
-   * which is undesirable. Instead, we are just adding the note to the database as nullified and the nullifier is set
-   * to 0 in the db.
-   */
-  addNullifiedNote(note: ExtendedNote): Promise<void>;
-
   /**
    * Get the given block.
    * @param number - The block number being requested.
@@ -520,7 +510,6 @@ export const PXESchema: ApiSchemaFor<PXE> = {
     .args(z.number(), schemas.Fr)
     .returns(z.tuple([schemas.BigInt, SiblingPath.schema])),
   addNote: z.function().args(ExtendedNote.schema, optional(schemas.AztecAddress)).returns(z.void()),
-  addNullifiedNote: z.function().args(ExtendedNote.schema).returns(z.void()),
   getBlock: z
     .function()
     .args(z.number())

diff --git a/yarn-project/end-to-end/src/e2e_2_pxes.test.ts b/yarn-project/end-to-end/src/e2e_2_pxes.test.ts
@@ -1,17 +1,7 @@
 import { getSchnorrAccount } from '@aztec/accounts/schnorr';
 import { type InitialAccountData, deployFundedSchnorrAccount } from '@aztec/accounts/testing';
-import {
-  type AztecAddress,
-  type AztecNode,
-  type ExtendedNote,
-  Fr,
-  type Logger,
-  type PXE,
-  type Wallet,
-  sleep,
-} from '@aztec/aztec.js';
+import { type AztecAddress, type AztecNode, Fr, type Logger, type PXE, type Wallet, sleep } from '@aztec/aztec.js';
 import { ChildContract } from '@aztec/noir-contracts.js/Child';
-import { TestContract } from '@aztec/noir-contracts.js/Test';
 import { TokenContract } from '@aztec/noir-contracts.js/Token';
 
 import { expect, jest } from '@jest/globals';
@@ -226,52 +216,4 @@ describe('e2e_2_pxes', () => {
     await expectTokenBalance(walletB, token, walletB.getAddress(), transferAmount2, logger);
     await expectTokenBalance(sharedWalletOnB, token, sharedAccountAddress, transferAmount1 - transferAmount2, logger);
   });
-
-  it('adds and fetches a nullified note', async () => {
-    // 1. Deploys test contract through PXE A
-    const testContract = await TestContract.deploy(walletA).send().deployed();
-
-    // 2. Create a note
-    const noteStorageSlot = 10;
-    const noteValue = 5;
-    let note: ExtendedNote;
-    {
-      const owner = walletA.getAddress();
-      const sender = owner;
-
-      const receipt = await testContract.methods
-        .call_create_note(noteValue, owner, sender, noteStorageSlot)
-        .send()
-        .wait();
-      await testContract.methods.sync_notes().simulate();
-      const notes = await walletA.getNotes({ txHash: receipt.txHash });
-      expect(notes).toHaveLength(1);
-      note = notes[0];
-    }
-
-    // TODO(#12013): We need to do this hack because NoteDao no longer populates noteTypeId
-    note.noteTypeId = TestContract.notes.ValueNote.id;
-
-    // 3. Nullify the note
-    {
-      const receipt = await testContract.methods.call_destroy_note(noteStorageSlot).send().wait({ debug: true });
-      // Check that we got 2 nullifiers - 1 for tx hash, 1 for the note
-      expect(receipt.debugInfo?.nullifiers).toHaveLength(2);
-    }
-
-    // 4. Adds the nullified public key note to PXE B
-    {
-      // We need to register the contract to be able to compute the note hash by calling compute_note_hash_and_optionally_a_nullifier(...)
-      await pxeB.registerContract(testContract);
-      await pxeB.addNullifiedNote(note);
-    }
-
-    // 5. Try fetching the nullified note
-    {
-      const testContractWithWalletB = await TestContract.at(testContract.address, walletB);
-      const noteValue = await testContractWithWalletB.methods.call_get_notes(noteStorageSlot, true).simulate();
-      expect(noteValue).toBe(noteValue);
-      // --> We have successfully obtained the nullified note from PXE B verifying that pxe.addNullifiedNote(...) works
-    }
-  });
 });
diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts
@@ -382,12 +382,11 @@ export class PXEService implements PXE {
     }
 
     for (const nonce of nonces) {
-      const { noteHash, uniqueNoteHash, innerNullifier } = await this.simulator.computeNoteHashAndOptionallyANullifier(
+      const { noteHash, uniqueNoteHash, innerNullifier } = await this.simulator.computeNoteHashAndNullifier(
         note.contractAddress,
         nonce,
         note.storageSlot,
         note.noteTypeId,
-        true,
         note.note,
       );
 
@@ -424,50 +423,6 @@ export class PXEService implements PXE {
     }
   }
 
-  public async addNullifiedNote(note: ExtendedNote) {
-    const { data: nonces, l2BlockHash, l2BlockNumber } = await this.#getNoteNonces(note);
-    if (nonces.length === 0) {
-      throw new Error(`Cannot find the note in tx: ${note.txHash}.`);
-    }
-
-    for (const nonce of nonces) {
-      const { noteHash, uniqueNoteHash, innerNullifier } = await this.simulator.computeNoteHashAndOptionallyANullifier(
-        note.contractAddress,
-        nonce,
-        note.storageSlot,
-        note.noteTypeId,
-        false,
-        note.note,
-      );
-
-      if (!innerNullifier.equals(Fr.ZERO)) {
-        throw new Error('Unexpectedly received non-zero nullifier.');
-      }
-
-      const [index] = await this.node.findLeavesIndexes('latest', MerkleTreeId.NOTE_HASH_TREE, [uniqueNoteHash]);
-      if (index === undefined) {
-        throw new Error('Note does not exist.');
-      }
-
-      await this.db.addNullifiedNote(
-        new NoteDao(
-          note.note,
-          note.contractAddress,
-          note.storageSlot,
-          nonce,
-          noteHash,
-          Fr.ZERO, // We are not able to derive
-          note.txHash,
-          l2BlockNumber,
-          l2BlockHash,
-          index,
-          await note.owner.toAddressPoint(),
-          note.noteTypeId,
-        ),
-      );
-    }
-  }
-
   /**
    * Finds the nonce(s) for a given note.
    * @param note - The note to find the nonces for.
@@ -490,12 +445,11 @@ export class PXEService implements PXE {
       }
 
       const nonce = await computeNoteHashNonce(firstNullifier, i);
-      const { uniqueNoteHash } = await this.simulator.computeNoteHashAndOptionallyANullifier(
+      const { uniqueNoteHash } = await this.simulator.computeNoteHashAndNullifier(
         note.contractAddress,
         nonce,
         note.storageSlot,
         note.noteTypeId,
-        false,
         note.note,
       );
       if (hash.equals(uniqueNoteHash)) {

diff --git a/yarn-project/pxe/src/simulator_oracle/simulator_oracle.test.ts b/yarn-project/pxe/src/simulator_oracle/simulator_oracle.test.ts
@@ -571,7 +571,7 @@ describe('Simulator oracle', () => {
       addNotesSpy.mockReset();
       getNotesSpy.mockReset();
       removeNullifiedNotesSpy.mockReset();
-      simulator.computeNoteHashAndOptionallyANullifier.mockReset();
+      simulator.computeNoteHashAndNullifier.mockReset();
       aztecNode.getTxEffect.mockReset();
     });
 

diff --git a/yarn-project/simulator/src/client/private_execution.test.ts b/yarn-project/simulator/src/client/private_execution.test.ts
@@ -388,7 +388,15 @@ describe('Private Execution test suite', () => {
       const noteHashes = getNonEmptyItems(result.publicInputs.noteHashes);
       expect(noteHashes).toHaveLength(1);
       expect(noteHashes[0].value).toEqual(
-        await acirSimulator.computeNoteHash(contractAddress, newNote.storageSlot, newNote.noteTypeId, newNote.note),
+        (
+          await acirSimulator.computeNoteHashAndNullifier(
+            contractAddress,
+            Fr.ZERO,
+            newNote.storageSlot,
+            newNote.noteTypeId,
+            newNote.note,
+          )
+        ).noteHash,
       );
 
       const privateLogs = getNonEmptyItems(result.publicInputs.privateLogs);
@@ -410,7 +418,15 @@ describe('Private Execution test suite', () => {
       const noteHashes = getNonEmptyItems(result.publicInputs.noteHashes);
       expect(noteHashes).toHaveLength(1);
       expect(noteHashes[0].value).toEqual(
-        await acirSimulator.computeNoteHash(contractAddress, newNote.storageSlot, newNote.noteTypeId, newNote.note),
+        (
+          await acirSimulator.computeNoteHashAndNullifier(
+            contractAddress,
+            Fr.ZERO,
+            newNote.storageSlot,
+            newNote.noteTypeId,
+            newNote.note,
+          )
+        ).noteHash,
       );
 
       const privateLogs = getNonEmptyItems(result.publicInputs.privateLogs);
@@ -435,14 +451,7 @@ describe('Private Execution test suite', () => {
       oracle.getNotes.mockResolvedValue(notes);
 
       const consumedNotes = await asyncMap(notes, ({ nonce, note }) =>
-        acirSimulator.computeNoteHashAndOptionallyANullifier(
-          contractAddress,
-          nonce,
-          storageSlot,
-          valueNoteTypeId,
-          true,
-          note,
-        ),
+        acirSimulator.computeNoteHashAndNullifier(contractAddress, nonce, storageSlot, valueNoteTypeId, note),
       );
       await insertLeaves(consumedNotes.map(n => n.uniqueNoteHash));
 
@@ -474,8 +483,24 @@ describe('Private Execution test suite', () => {
       expect(noteHashes).toHaveLength(2);
       const [changeNoteHash, recipientNoteHash] = noteHashes;
       const [siloedChangeNoteHash, siloedRecipientNoteHash] = [
-        await acirSimulator.computeNoteHash(contractAddress, storageSlot, valueNoteTypeId, changeNote.note),
-        await acirSimulator.computeNoteHash(contractAddress, recipientStorageSlot, valueNoteTypeId, recipientNote.note),
+        (
+          await acirSimulator.computeNoteHashAndNullifier(
+            contractAddress,
+            Fr.ZERO,
+            storageSlot,
+            valueNoteTypeId,
+            changeNote.note,
+          )
+        ).noteHash,
+        (
+          await acirSimulator.computeNoteHashAndNullifier(
+            contractAddress,
+            Fr.ZERO,
+            recipientStorageSlot,
+            valueNoteTypeId,
+            recipientNote.note,
+          )
+        ).noteHash,
       ];
       expect(changeNoteHash.value).toEqual(siloedChangeNoteHash);
       expect(recipientNoteHash.value).toEqual(siloedRecipientNoteHash);
@@ -503,14 +528,7 @@ describe('Private Execution test suite', () => {
       oracle.getNotes.mockResolvedValue(notes);
 
       const consumedNotes = await asyncMap(notes, ({ nonce, note }) =>
-        acirSimulator.computeNoteHashAndOptionallyANullifier(
-          contractAddress,
-          nonce,
-          storageSlot,
-          valueNoteTypeId,
-          true,
-          note,
-        ),
+        acirSimulator.computeNoteHashAndNullifier(contractAddress, nonce, storageSlot, valueNoteTypeId, note),
       );
       await insertLeaves(consumedNotes.map(n => n.uniqueNoteHash));
 
@@ -1002,8 +1020,9 @@ describe('Private Execution test suite', () => {
         owner,
       );
 
-      const derivedNoteHash = await acirSimulator.computeNoteHash(
+      const { noteHash: derivedNoteHash } = await acirSimulator.computeNoteHashAndNullifier(
         contractAddress,
+        Fr.ZERO,
         storageSlot,
         valueNoteTypeId,
         noteAndSlot.note,
@@ -1079,8 +1098,9 @@ describe('Private Execution test suite', () => {
       const noteHashes = getNonEmptyItems(execInsert.publicInputs.noteHashes);
       expect(noteHashes).toHaveLength(1);
 
-      const derivedNoteHash = await acirSimulator.computeNoteHash(
+      const { noteHash: derivedNoteHash } = await acirSimulator.computeNoteHashAndNullifier(
         contractAddress,
+        Fr.ZERO,
         noteAndSlot.storageSlot,
         noteAndSlot.noteTypeId,
         noteAndSlot.note,

diff --git a/yarn-project/simulator/src/client/simulator.test.ts b/yarn-project/simulator/src/client/simulator.test.ts
@@ -46,7 +46,7 @@ describe('Simulator', () => {
     simulator = new AcirSimulator(oracle, node, simulationProvider);
   });
 
-  describe('computeNoteHashAndOptionallyANullifier', () => {
+  describe('compute_note_hash_and_optionally_a_nullifier', () => {
     const artifact = getFunctionArtifactByName(
       TokenBlacklistContractArtifact,
       'compute_note_hash_and_optionally_a_nullifier',
@@ -64,7 +64,7 @@ describe('Simulator', () => {
 
       const note = await createNote();
       await expect(
-        simulator.computeNoteHashAndOptionallyANullifier(contractAddress, nonce, storageSlot, noteTypeId, true, note),
+        simulator.computeNoteHashAndNullifier(contractAddress, nonce, storageSlot, noteTypeId, note),
       ).rejects.toThrow(/Mandatory implementation of "compute_note_hash_and_optionally_a_nullifier" missing/);
     });
 
@@ -78,7 +78,7 @@ describe('Simulator', () => {
       oracle.getFunctionArtifactByName.mockResolvedValue(modifiedArtifact);
 
       await expect(
-        simulator.computeNoteHashAndOptionallyANullifier(contractAddress, nonce, storageSlot, noteTypeId, true, note),
+        simulator.computeNoteHashAndNullifier(contractAddress, nonce, storageSlot, noteTypeId, note),
       ).rejects.toThrow(
         new RegExp(
           `Expected 6 parameters in mandatory implementation of "compute_note_hash_and_optionally_a_nullifier", but found 5 in noir contract ${contractAddress}.`,
@@ -110,7 +110,7 @@ describe('Simulator', () => {
       oracle.getFunctionArtifactByName.mockResolvedValue(modifiedArtifact);
 
       await expect(
-        simulator.computeNoteHashAndOptionallyANullifier(contractAddress, nonce, storageSlot, noteTypeId, true, note),
+        simulator.computeNoteHashAndNullifier(contractAddress, nonce, storageSlot, noteTypeId, note),
       ).rejects.toThrow(
         new RegExp(
           `"compute_note_hash_and_optionally_a_nullifier" can only handle a maximum of ${wrongPreimageLength} fields`,