diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr index e566bc5a28fb..a67f0d2efab9 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable.nr @@ -18,6 +18,8 @@ struct PrivateMutable { } // docs:end:struct +mod test; + impl Storage for PrivateMutable {} impl PrivateMutable { @@ -28,10 +30,6 @@ impl PrivateMutable { } // docs:end:new - pub fn to_unconstrained(self) -> PrivateMutable { - PrivateMutable { context: (), storage_slot: self.storage_slot } - } - // 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. @@ -81,6 +79,31 @@ impl PrivateMutable { } // docs:end:replace + pub fn initialize_or_replace( + self, + note: &mut Note, + broadcast: bool, + ivpk_m: GrumpkinPoint + ) where Note: NoteInterface { + let is_initialized = check_nullifier_exists(self.compute_initialization_nullifier()); + + // check_nullifier_exists() is an unconstrained function - we can constrain a true value by providing an + // inclusion proof of the nullifier, but cannot constrain a false value since a non-inclusion proof would only + // be valid if done in public. + // Ultimately, this is not an issue ginen that we'll either: + // - initialize the state variable, which would fail if it was already initialized due to the duplicate + // nullifier, or + // - replace the current value, which would fail if it was not initialized since we wouldn't be able to produce + // an inclusion proof for the current note + // This means that an honest oracle will assist the prover to produce a valid proof, while a malicious oracle + // (i.e. one that returns an incorrect value for is_initialized) will simply fail to produce a proof. + if (!is_initialized) { + self.initialize(note, broadcast, ivpk_m); + } else { + self.replace(note, broadcast, ivpk_m) + } + } + // docs:start:get_note pub fn get_note( self, diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable/test.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable/test.nr new file mode 100644 index 000000000000..9d12f3001dbb --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable/test.nr @@ -0,0 +1,53 @@ +use dep::protocol_types::{address::AztecAddress, grumpkin_point::GrumpkinPoint}; +use crate::{context::PrivateContext, state_vars::private_mutable::PrivateMutable}; +use crate::test::{mocks::mock_note::MockNote, helpers::context_builder::ContextBuilder}; +use dep::std::{unsafe::zeroed, test::OracleMock}; + +global contract_address = AztecAddress::from_field(13); +global storage_slot = 17; + +fn setup() -> PrivateMutable { + let mut context = ContextBuilder::new().contract_address(contract_address).private(); + let state_var = PrivateMutable::new(&mut context, storage_slot); + + // This oracle is called for its side effects alone - it's always expected to return 0. + OracleMock::mock("notifyCreatedNote").returns(0); + + state_var +} + +#[test] +fn test_initialize_or_replace_without_nullifier() { + let state_var = setup(); + + let ivpk_m: GrumpkinPoint = zeroed(); + let broadcast = false; + + let value = 42; + let mut note = MockNote::new(value).contract_address(contract_address).storage_slot(storage_slot).build(); + + OracleMock::mock("checkNullifierExists").returns(0); + state_var.initialize_or_replace(&mut note, broadcast, ivpk_m); + + // Since we reported there was no nullifier, we should initialize and see the following side-effects: + // - a new note being created + // - no notes being read + // - the initialization nullifier being emitted + assert_eq(state_var.context.new_note_hashes.len(), 1); + assert_eq(state_var.context.note_hash_read_requests.len(), 0); + assert_eq(state_var.context.new_nullifiers.len(), 1); + + // Note that if the oracle was wrong and the initialization nullifier did exist, this attempt to write it again + // would cause the sequencer to revert this transaction - we are therefore safe from bad oracles. + let nullifier = state_var.context.new_nullifiers.get(0); + assert_eq(nullifier.value, state_var.compute_initialization_nullifier()); + assert_eq(nullifier.note_hash, 0); +} + +#[test] +fn test_initialize_or_replace_with_nullifier() { + // Here we'd want to test a scenario like the one above with the oracle indicating that the initialization + // nullifier does exist. Unfortunately that requires us to produce a valid oracle return value for getNotes, + // which is fairly involved as it deals with serialization of notes, and is relatively complicated to replicate + // in Noir. +} diff --git a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr index 279edf5e021f..fbd4a2da9311 100644 --- a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr @@ -115,17 +115,7 @@ contract AppSubscription { let subscriber_ivpk_m = get_ivpk_m(&mut context, subscriber_address); let mut subscription_note = SubscriptionNote::new(subscriber_npk_m_hash, expiry_block_number, tx_count); - - // is_initialized is an unconstrained function, so we must first convert the state variable to an unconstrained - // one to avoid having a mutable reference (to the private context) cross the constrained <> unconstrained - // boundary, which is not allowed. - let is_initialized = storage.subscriptions.at(subscriber_address).to_unconstrained().is_initialized(); - - if (!is_initialized) { - storage.subscriptions.at(subscriber_address).initialize(&mut subscription_note, true, subscriber_ivpk_m); - } else { - storage.subscriptions.at(subscriber_address).replace(&mut subscription_note, true, subscriber_ivpk_m) - } + storage.subscriptions.at(subscriber_address).initialize_or_replace(&mut subscription_note, true, subscriber_ivpk_m); } unconstrained fn is_initialized(subscriber_address: AztecAddress) -> pub bool {