feat!: Updates singleton usage (AztecProtocol#4186)

Fixes AztecProtocol#4174 by making the singletons leaky. 

- Updates the docs
- Adds more tests to the singleton e2e.

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,27 @@ 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, in that it doesn't hide the storage slot that has been initialized, nor does it hide the contract address of this contract.
+    // When this initialization nullifier is emitted, an observer could do a dictionary or rainbow attack to learn the preimage of this nullifier to deduce the storage slot and contract address.
+    // For some applications, leaking the details that a particular state variable of a particular contract has been initialized will be unacceptable.
+    // Under such circumstances, such application developers might wish to _not_ use this state variable type.
+    // This is especially dangerous for initial assignment to elements of a `Map<AztecAddress, ImmutableSingleton>` type (for example), because the storage slot often also identifies an actor. 
+    // e.g. the initial assignment to `my_map.at(msg.sender)` will leak: `msg.sender`, the fact that an element of `my_map` was assigned-to for the first time, and the contract_address.
+    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 +61,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,29 @@ 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, in that it doesn't hide the storage slot that has been initialized, nor does it hide the contract address of this contract.
+    // When this initialization nullifier is emitted, an observer could do a dictionary or rainbow attack to learn the preimage of this nullifier to deduce the storage slot and contract address.
+    // For some applications, leaking the details that a particular state variable of a particular contract has been initialized will be unacceptable.
+    // Under such circumstances, such application developers might wish to _not_ use this state variable type.
+    // This is especially dangerous for initial assignment to elements of a `Map<AztecAddress, Singleton>` type (for example), because the storage slot often also identifies an actor. e.g. 
+    // the initial assignment to `my_map.at(msg.sender)` will leak: `msg.sender`, the fact that an element of `my_map` was assigned-to for the first time, and the contract_address.
+    // Note: subsequent nullification of this state variable, via the `replace` method will not be leaky, if the `compute_nullifier()` method of the underlying note is designed to ensure privacy. 
+    // For example, if the `compute_nullifier()` method injects the secret key of a note owner into the computed nullifier's preimage.
+    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 +67,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);
+    });
   });
 });