diff --git a/docs/docs/dev_docs/contracts/syntax/storage/main.md b/docs/docs/dev_docs/contracts/syntax/storage/main.md index b774ee0c682..263363832f1 100644 --- a/docs/docs/dev_docs/contracts/syntax/storage/main.md +++ b/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: diff --git a/yarn-project/aztec-nr/aztec/src/state_vars/immutable_singleton.nr b/yarn-project/aztec-nr/aztec/src/state_vars/immutable_singleton.nr index 699cecae250..5bdc4d072d3 100644 --- a/yarn-project/aztec-nr/aztec/src/state_vars/immutable_singleton.nr +++ b/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 { context: Option<&mut PrivateContext>, storage_slot: Field, note_interface: NoteInterface, - compute_initialization_nullifier: fn (Field, Option, Option<&mut PrivateContext>) -> Field, } // docs:end:struct @@ -30,19 +32,27 @@ impl ImmutableSingleton { note_interface: NoteInterface, ) -> 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` 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) -> 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 ImmutableSingleton { pub fn initialize( self, note: &mut Note, - owner: Option, 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( diff --git a/yarn-project/aztec-nr/aztec/src/state_vars/singleton.nr b/yarn-project/aztec-nr/aztec/src/state_vars/singleton.nr index d564512a1ab..b5a2d3fe280 100644 --- a/yarn-project/aztec-nr/aztec/src/state_vars/singleton.nr +++ b/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, - 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 { context: Option<&mut PrivateContext>, storage_slot: Field, note_interface: NoteInterface, - compute_initialization_nullifier: fn (Field, Option, Option<&mut PrivateContext>) -> Field, } // docs:end:struct @@ -57,19 +36,29 @@ impl Singleton { note_interface: NoteInterface, ) -> 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` 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) -> 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 Singleton { pub fn initialize( self, note: &mut Note, - owner: Option, 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); diff --git a/yarn-project/end-to-end/src/e2e_singleton.test.ts b/yarn-project/end-to-end/src/e2e_singleton.test.ts index 251b4e7a44f..6c1407ef053 100644 --- a/yarn-project/end-to-end/src/e2e_singleton.test.ts +++ b/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; 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); + }); }); }); diff --git a/yarn-project/noir-contracts/contracts/docs_example_contract/src/main.nr b/yarn-project/noir-contracts/contracts/docs_example_contract/src/main.nr index 6662f6de2f0..e7c9befa319 100644 --- a/yarn-project/noir-contracts/contracts/docs_example_contract/src/main.nr +++ b/yarn-project/noir-contracts/contracts/docs_example_contract/src/main.nr @@ -23,7 +23,7 @@ contract DocsExample { utils as note_utils, }, context::{PrivateContext, PublicContext, Context}, - state_vars::{map::Map, public_state::PublicState,singleton::Singleton}, + state_vars::{map::Map, public_state::PublicState,singleton::Singleton, immutable_singleton::ImmutableSingleton}, }; // how to import methods from other files/folders within your workspace use crate::options::create_account_card_getter_options; @@ -42,6 +42,7 @@ contract DocsExample { // docs:start:storage-map-singleton-declaration profiles: Map>, // docs:end:storage-map-singleton-declaration + imm_singleton: ImmutableSingleton, } impl Storage { @@ -65,6 +66,7 @@ contract DocsExample { }, ), // docs:end:state_vars-MapSingleton + imm_singleton: ImmutableSingleton::new(context, 4, CardNoteMethods), } } } @@ -72,25 +74,44 @@ contract DocsExample { #[aztec(private)] fn constructor() {} + #[aztec(private)] + fn initialize_immutable_singleton(randomness: Field, points: u8) { + let mut new_card = CardNote::new(points, randomness, context.msg_sender()); + storage.imm_singleton.initialize(&mut new_card, true); + } + #[aztec(private)] // msg_sender() is 0 at deploy time. So created another function fn initialize_private(randomness: Field, points: u8) { let mut legendary_card = CardNote::new(points, randomness, context.msg_sender()); // create and broadcast note - storage.legendary_card.initialize(&mut legendary_card, Option::none(), true); + storage.legendary_card.initialize(&mut legendary_card, true); } #[aztec(private)] fn update_legendary_card(randomness: Field, points: u8) { + let mut new_card = CardNote::new(points, randomness, context.msg_sender()); + storage.legendary_card.replace(&mut new_card, true); + + context.call_public_function( + context.this_address(), + FunctionSelector::from_signature("update_leader((Field),u8)"), + [context.msg_sender().to_field(), points as Field] + ); + } + + #[aztec(private)] + fn increase_legendary_points() { // Ensure `points` > current value // Also serves as a e2e test that you can `get_note()` and then `replace()` // docs:start:state_vars-SingletonGet - let card = storage.legendary_card.get_note(true); + let card = storage.legendary_card.get_note(false); // docs:end:state_vars-SingletonGet - assert(points > card.points, "can only update to higher value"); - let mut new_card = CardNote::new(points, randomness, context.msg_sender()); + let points = card.points + 1; + + let mut new_card = CardNote::new(points, card.randomness, context.msg_sender()); // docs:start:state_vars-SingletonReplace storage.legendary_card.replace(&mut new_card, true); // docs:end:state_vars-SingletonReplace @@ -112,6 +133,22 @@ contract DocsExample { storage.leader.read() } + unconstrained fn get_legendary_card() -> pub CardNote { + storage.legendary_card.view_note() + } + + unconstrained fn is_legendary_initialized() -> pub bool { + storage.legendary_card.is_initialized() + } + + unconstrained fn get_imm_card() -> pub CardNote { + storage.imm_singleton.view_note() + } + + unconstrained fn is_imm_initialized() -> pub bool { + storage.imm_singleton.is_initialized() + } + // TODO: remove this placeholder once https://github.com/AztecProtocol/aztec-packages/issues/2918 is implemented unconstrained fn compute_note_hash_and_nullifier( contract_address: AztecAddress, diff --git a/yarn-project/noir-contracts/contracts/ecdsa_account_contract/src/main.nr b/yarn-project/noir-contracts/contracts/ecdsa_account_contract/src/main.nr index a12e0c02600..da730b23c32 100644 --- a/yarn-project/noir-contracts/contracts/ecdsa_account_contract/src/main.nr +++ b/yarn-project/noir-contracts/contracts/ecdsa_account_contract/src/main.nr @@ -45,7 +45,7 @@ contract EcdsaAccount { fn constructor(signing_pub_key_x: pub [u8; 32], signing_pub_key_y: pub [u8; 32]) { let this = context.this_address(); let mut pub_key_note = EcdsaPublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this); - storage.public_key.initialize(&mut pub_key_note, Option::none(), true); + storage.public_key.initialize(&mut pub_key_note, true); } // Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts diff --git a/yarn-project/noir-contracts/contracts/schnorr_account_contract/src/main.nr b/yarn-project/noir-contracts/contracts/schnorr_account_contract/src/main.nr index 77203a70388..001437e9c9d 100644 --- a/yarn-project/noir-contracts/contracts/schnorr_account_contract/src/main.nr +++ b/yarn-project/noir-contracts/contracts/schnorr_account_contract/src/main.nr @@ -46,7 +46,7 @@ contract SchnorrAccount { let this = context.this_address(); // docs:start:initialize let mut pub_key_note = PublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this); - storage.signing_public_key.initialize(&mut pub_key_note, Option::none(), true); + storage.signing_public_key.initialize(&mut pub_key_note, true); // docs:end:initialize } diff --git a/yarn-project/noir-contracts/contracts/test_contract/src/main.nr b/yarn-project/noir-contracts/contracts/test_contract/src/main.nr index aac608c1bd8..6402ec444e8 100644 --- a/yarn-project/noir-contracts/contracts/test_contract/src/main.nr +++ b/yarn-project/noir-contracts/contracts/test_contract/src/main.nr @@ -193,7 +193,7 @@ contract Test { #[aztec(private)] fn set_constant(value: Field) { let mut note = FieldNote::new(value); - storage.example_constant.initialize(&mut note, Option::none(), false); + storage.example_constant.initialize(&mut note, false); } unconstrained fn get_constant() -> pub Field { diff --git a/yarn-project/noir-contracts/contracts/token_blacklist_contract/src/main.nr b/yarn-project/noir-contracts/contracts/token_blacklist_contract/src/main.nr index 0845c0ec0da..46ff827b00c 100644 --- a/yarn-project/noir-contracts/contracts/token_blacklist_contract/src/main.nr +++ b/yarn-project/noir-contracts/contracts/token_blacklist_contract/src/main.nr @@ -110,7 +110,7 @@ contract TokenBlacklist { #[aztec(private)] fn constructor(admin: AztecAddress, slow_updates_contract: AztecAddress) { let mut slow_note = FieldNote::new(slow_updates_contract.to_field()); - storage.slow_update.initialize(&mut slow_note, Option::none(), false); + storage.slow_update.initialize(&mut slow_note, false); // docs:end:constructor let selector = FunctionSelector::from_signature("_initialize((Field),(Field))"); context.call_public_function(