feat!: Updates singleton usage

docs/docs/dev_docs/contracts/syntax/storage/main.md

@@ -288,7 +288,11 @@ As part of the initialization of the `Storage` struct, the `Singleton` is create
 
 As mentioned, the Singleton is initialized to create the first note and value.
 
-When this function is called, a nullifier of the storage slot is created, preventing this Singleton from being initialized again. If an `owner` is specified, the nullifier will be hashed with the owner's secret key. It's crucial to provide an owner if the Singleton is associated with an account. Initializing it without an owner may inadvertently reveal important information about the owner's intention.
+When this function is called, a nullifier of the storage slot is created, preventing this Singleton from being initialized again.
+
+:::danger Privacy-Leak
+Beware that because this nullifier is created only from the storage slot without randomness it is "leaky". This means that if the storage slot depends on the an address then it is possible to link the nullifier to the address. For example, if the singleton is part of a `map` with an `AztecAddress` as the key then the nullifier will be linked to the address.
+:::
 
 Unlike public states, which have a default initial value of `0` (or many zeros, in the case of a struct, array or map), a private state (of type `Singleton`, `ImmutableSingleton` or `Set`) does not have a default initial value. The `initialize` method (or `insert`, in the case of a `Set`) must be called.
 
@@ -338,7 +342,11 @@ As part of the initialization of the `Storage` struct, the `Singleton` is create
 
 ### `initialize`
 
-When this function is invoked, it creates a nullifier for the storage slot, ensuring that the ImmutableSingleton cannot be initialized again. If an owner is specified, the nullifier will be hashed with the owner's secret key. It is crucial to provide an owner if the ImmutableSingleton is linked to an account; initializing it without one may inadvertently disclose sensitive information about the owner's intent.
+When this function is invoked, it creates a nullifier for the storage slot, ensuring that the ImmutableSingleton cannot be initialized again. 
+
+:::danger Privacy-Leak
+Beware that because this nullifier is created only from the storage slot without randomness it is "leaky". This means that if the storage slot depends on the an address then it is possible to link the nullifier to the address. For example, if the singleton is part of a `map` with an `AztecAddress` as the key then the nullifier will be linked to the address.
+:::
 
 Set the value of an ImmutableSingleton by calling the `initialize` method:
 

yarn-project/aztec-nr/aztec/src/state_vars/immutable_singleton.nr

@@ -1,6 +1,10 @@
 use dep::std::option::Option;
 use dep::protocol_types::{
     address::AztecAddress,
+    constants::{
+        GENERATOR_INDEX__INITIALIZATION_NULLIFIER,
+    },
+    hash::pedersen_hash,
 };
 
 use crate::context::{PrivateContext, Context};
@@ -11,14 +15,12 @@ use crate::note::{
     note_viewer_options::NoteViewerOptions,
 };
 use crate::oracle::notes::check_nullifier_exists;
-use crate::state_vars::singleton::compute_singleton_initialization_nullifier;
 
 // docs:start:struct
 struct ImmutableSingleton<Note, N> {
     context: Option<&mut PrivateContext>,
     storage_slot: Field,
     note_interface: NoteInterface<Note, N>,
-    compute_initialization_nullifier: fn (Field, Option<AztecAddress>, Option<&mut PrivateContext>) -> Field,
 }
 // docs:end:struct
 
@@ -30,19 +32,25 @@ impl<Note, N> ImmutableSingleton<Note, N> {
         note_interface: NoteInterface<Note, N>,
     ) -> Self {
         assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
-        ImmutableSingleton {
+        Self {
             context: context.private,
             storage_slot,
             note_interface,
-            compute_initialization_nullifier: compute_singleton_initialization_nullifier,
         }
     }
     // docs:end:new
 
+    // The following computation is leaky - the storage slot can easily be "guessed" by an adversary 
+    // by looking at the nullifier in the transaction data. 
+    // This is especially dangerous for `maps` because the storage slot is often also identifies the
+    // actor that is executing the transaction. e.g, `map.at(msg.sender)` will leak `msg.sender`.
+    pub fn compute_initialization_nullifier(self) -> Field {
+        pedersen_hash([self.storage_slot], GENERATOR_INDEX__INITIALIZATION_NULLIFIER)
+    }
+
     // docs:start:is_initialized
-    unconstrained pub fn is_initialized(self, owner: Option<AztecAddress>) -> bool {
-        let compute_initialization_nullifier = self.compute_initialization_nullifier;
-        let nullifier = compute_initialization_nullifier(self.storage_slot, owner, Option::none());
+    unconstrained pub fn is_initialized(self) -> bool {
+        let nullifier = self.compute_initialization_nullifier();
         check_nullifier_exists(nullifier)
     }
     // docs:end:is_initialized
@@ -51,14 +59,12 @@ impl<Note, N> ImmutableSingleton<Note, N> {
     pub fn initialize(
         self,
         note: &mut Note, 
-        owner: Option<AztecAddress>,
         broadcast: bool,
     ) {
         let context = self.context.unwrap();
 
         // Nullify the storage slot.
-        let compute_initialization_nullifier = self.compute_initialization_nullifier;
-        let nullifier = compute_initialization_nullifier(self.storage_slot, owner, self.context);
+        let nullifier = self.compute_initialization_nullifier();
         context.push_new_nullifier(nullifier, 0);
 
         create_note(

yarn-project/aztec-nr/aztec/src/state_vars/singleton.nr

@@ -20,32 +20,11 @@ use crate::oracle::{
     notes::check_nullifier_exists,
 };
 
-pub fn compute_singleton_initialization_nullifier(
-    storage_slot: Field,
-    owner: Option<AztecAddress>,
-    context: Option<&mut PrivateContext>
-) -> Field {
-    if owner.is_some() {
-        let secret = if context.is_some() {
-            context.unwrap_unchecked().request_nullifier_secret_key(owner.unwrap_unchecked())
-        } else {
-            get_nullifier_secret_key(owner.unwrap_unchecked())
-        };
-        pedersen_hash(
-            [storage_slot, secret.low, secret.high],
-            GENERATOR_INDEX__INITIALIZATION_NULLIFIER
-        )
-    } else {
-        pedersen_hash([storage_slot], GENERATOR_INDEX__INITIALIZATION_NULLIFIER)
-    }
-}
-
 // docs:start:struct
 struct Singleton<Note, N> {
     context: Option<&mut PrivateContext>,
     storage_slot: Field,
     note_interface: NoteInterface<Note, N>,
-    compute_initialization_nullifier: fn (Field, Option<AztecAddress>, Option<&mut PrivateContext>) -> Field,
 }
 // docs:end:struct
 
@@ -57,19 +36,25 @@ impl<Note, N> Singleton<Note, N> {
         note_interface: NoteInterface<Note, N>,
     ) -> Self {
         assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
-        Singleton {
+        Self {
             context: context.private,
             storage_slot,
             note_interface,
-            compute_initialization_nullifier: compute_singleton_initialization_nullifier,
         }
     }
     // docs:end:new
 
+    // The following computation is leaky - the storage slot can easily be "guessed" by an adversary 
+    // by looking at the nullifier in the transaction data. 
+    // This is especially dangerous for `maps` because the storage slot is often also identifies the
+    // actor that is executing the transaction. e.g, `map.at(msg.sender)` will leak `msg.sender`.
+    pub fn compute_initialization_nullifier(self) -> Field {
+        pedersen_hash([self.storage_slot], GENERATOR_INDEX__INITIALIZATION_NULLIFIER)
+    }
+
     // docs:start:is_initialized
-    unconstrained pub fn is_initialized(self, owner: Option<AztecAddress>) -> bool {
-        let compute_initialization_nullifier = self.compute_initialization_nullifier;
-        let nullifier = compute_initialization_nullifier(self.storage_slot, owner, Option::none());
+    unconstrained pub fn is_initialized(self) -> bool {
+        let nullifier = self.compute_initialization_nullifier();
         check_nullifier_exists(nullifier)
     }
     // docs:end:is_initialized
@@ -78,14 +63,12 @@ impl<Note, N> Singleton<Note, N> {
     pub fn initialize(
         self,
         note: &mut Note,
-        owner: Option<AztecAddress>,
         broadcast: bool,
     ) {
         let context = self.context.unwrap();
 
         // Nullify the storage slot.
-        let compute_initialization_nullifier = self.compute_initialization_nullifier;
-        let nullifier = compute_initialization_nullifier(self.storage_slot, owner, self.context);
+        let nullifier = self.compute_initialization_nullifier();
         context.push_new_nullifier(nullifier, 0);
 
         create_note(context, self.storage_slot, note, self.note_interface, broadcast);

yarn-project/end-to-end/src/e2e_singleton.test.ts

@@ -1,4 +1,4 @@
-import { Fr, Wallet } from '@aztec/aztec.js';
+import { TxStatus, Wallet } from '@aztec/aztec.js';
 import { DocsExampleContract } from '@aztec/noir-contracts';
 
 import { setup } from './fixtures/utils.js';
@@ -9,23 +9,134 @@ describe('e2e_singleton', () => {
   let teardown: () => Promise<void>;
   let contract: DocsExampleContract;
 
+  const POINTS = 1n;
+  const RANDOMNESS = 2n;
+
   beforeAll(async () => {
     ({ teardown, wallet } = await setup());
     contract = await DocsExampleContract.deploy(wallet).send().deployed();
-    // sets card value to 1 and leader to sender.
-    await contract.methods.initialize_private(Fr.random(), 1).send().wait();
   }, 25_000);
 
   afterAll(() => teardown());
 
-  // Singleton tests:
-  it('can read singleton and replace/update it in the same call', async () => {
-    await expect(contract.methods.update_legendary_card(Fr.random(), 0).simulate()).rejects.toThrowError(
-      'Assertion failed: can only update to higher value',
-    );
+  describe('Singleton', () => {
+    it('fail to read uninitialized singleton', async () => {
+      expect(await contract.methods.is_legendary_initialized().view()).toEqual(false);
+      await expect(contract.methods.get_legendary_card().view()).rejects.toThrowError();
+    });
+
+    it('initialize singleton', async () => {
+      expect(await contract.methods.is_legendary_initialized().view()).toEqual(false);
+      const receipt = await contract.methods.initialize_private(RANDOMNESS, POINTS).send().wait();
+      expect(receipt.status).toEqual(TxStatus.MINED);
+
+      const tx = await wallet.getTx(receipt.txHash);
+      expect(tx?.newCommitments.length).toEqual(1);
+      // 1 for the tx, another for the initializer
+      expect(tx?.newNullifiers.length).toEqual(2);
+      expect(await contract.methods.is_legendary_initialized().view()).toEqual(true);
+    });
+
+    it('fail to reinitialize', async () => {
+      expect(await contract.methods.is_legendary_initialized().view()).toEqual(true);
+      await expect(contract.methods.initialize_private(RANDOMNESS, POINTS).send().wait()).rejects.toThrowError();
+      expect(await contract.methods.is_legendary_initialized().view()).toEqual(true);
+    });
+
+    it('read initialized singleton', async () => {
+      expect(await contract.methods.is_legendary_initialized().view()).toEqual(true);
+      const { points, randomness } = await contract.methods.get_legendary_card().view();
+      expect(points).toEqual(POINTS);
+      expect(randomness).toEqual(RANDOMNESS);
+    });
+
+    it('replace with same value', async () => {
+      expect(await contract.methods.is_legendary_initialized().view()).toEqual(true);
+      const noteBefore = await contract.methods.get_legendary_card().view();
+      const receipt = await contract.methods.update_legendary_card(RANDOMNESS, POINTS).send().wait();
+      expect(receipt.status).toEqual(TxStatus.MINED);
+
+      const tx = await wallet.getTx(receipt.txHash);
+      expect(tx?.newCommitments.length).toEqual(1);
+      // 1 for the tx, another for the nullifier of the previous note
+      expect(tx?.newNullifiers.length).toEqual(2);
+
+      const noteAfter = await contract.methods.get_legendary_card().view();
+
+      expect(noteBefore.owner).toEqual(noteAfter.owner);
+      expect(noteBefore.points).toEqual(noteAfter.points);
+      expect(noteBefore.randomness).toEqual(noteAfter.randomness);
+      expect(noteBefore.header.contract_address).toEqual(noteAfter.header.contract_address);
+      expect(noteBefore.header.storage_slot).toEqual(noteAfter.header.storage_slot);
+      expect(noteBefore.header.is_transient).toEqual(noteAfter.header.is_transient);
+      // !!! Nonce must be different
+      expect(noteBefore.header.nonce).not.toEqual(noteAfter.header.nonce);
+    });
+
+    it('replace singleton with other values', async () => {
+      expect(await contract.methods.is_legendary_initialized().view()).toEqual(true);
+      const receipt = await contract.methods
+        .update_legendary_card(RANDOMNESS + 2n, POINTS + 1n)
+        .send()
+        .wait();
+      expect(receipt.status).toEqual(TxStatus.MINED);
+      const tx = await wallet.getTx(receipt.txHash);
+      expect(tx?.newCommitments.length).toEqual(1);
+      // 1 for the tx, another for the nullifier of the previous note
+      expect(tx?.newNullifiers.length).toEqual(2);
+
+      const { points, randomness } = await contract.methods.get_legendary_card().view();
+      expect(points).toEqual(POINTS + 1n);
+      expect(randomness).toEqual(RANDOMNESS + 2n);
+    });
+
+    it('replace singleton dependent on prior value', async () => {
+      expect(await contract.methods.is_legendary_initialized().view()).toEqual(true);
+      const noteBefore = await contract.methods.get_legendary_card().view();
+      const receipt = await contract.methods.increase_legendary_points().send().wait();
+      expect(receipt.status).toEqual(TxStatus.MINED);
+      const tx = await wallet.getTx(receipt.txHash);
+      expect(tx?.newCommitments.length).toEqual(1);
+      // 1 for the tx, another for the nullifier of the previous note
+      expect(tx?.newNullifiers.length).toEqual(2);
+
+      const { points, randomness } = await contract.methods.get_legendary_card().view();
+      expect(points).toEqual(noteBefore.points + 1n);
+      expect(randomness).toEqual(noteBefore.randomness);
+    });
+  });
+
+  describe('Immutable Singleton', () => {
+    it('fail to read uninitialized singleton', async () => {
+      expect(await contract.methods.is_imm_initialized().view()).toEqual(false);
+      await expect(contract.methods.get_imm_card().view()).rejects.toThrowError();
+    });
+
+    it('initialize singleton', async () => {
+      expect(await contract.methods.is_imm_initialized().view()).toEqual(false);
+      const receipt = await contract.methods.initialize_immutable_singleton(RANDOMNESS, POINTS).send().wait();
+      expect(receipt.status).toEqual(TxStatus.MINED);
+
+      const tx = await wallet.getTx(receipt.txHash);
+      expect(tx?.newCommitments.length).toEqual(1);
+      // 1 for the tx, another for the initializer
+      expect(tx?.newNullifiers.length).toEqual(2);
+      expect(await contract.methods.is_imm_initialized().view()).toEqual(true);
+    });
+
+    it('fail to reinitialize', async () => {
+      expect(await contract.methods.is_imm_initialized().view()).toEqual(true);
+      await expect(
+        contract.methods.initialize_immutable_singleton(RANDOMNESS, POINTS).send().wait(),
+      ).rejects.toThrowError();
+      expect(await contract.methods.is_imm_initialized().view()).toEqual(true);
+    });
 
-    const newPoints = 3n;
-    await contract.methods.update_legendary_card(Fr.random(), newPoints).send().wait();
-    expect((await contract.methods.get_leader().view()).points).toEqual(newPoints);
+    it('read initialized singleton', async () => {
+      expect(await contract.methods.is_imm_initialized().view()).toEqual(true);
+      const { points, randomness } = await contract.methods.get_imm_card().view();
+      expect(points).toEqual(POINTS);
+      expect(randomness).toEqual(RANDOMNESS);
+    });
   });
 });