From e60e698c91c68458fae6f41f093fbd7354625578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 25 Sep 2024 20:27:23 +0000 Subject: [PATCH 1/8] Make pack_arguments safe --- .../aztec/src/context/call_interfaces.nr | 80 ++++--------------- .../aztec/src/context/private_context.nr | 14 ++-- .../aztec-nr/aztec/src/oracle/arguments.nr | 44 +++++----- 3 files changed, 49 insertions(+), 89 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr b/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr index 49d5a424ac0..21bcb6fb7ef 100644 --- a/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr +++ b/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr @@ -53,10 +53,7 @@ pub struct PrivateCallInterface { impl PrivateCallInterface { pub fn call(self, context: &mut PrivateContext) -> T where T: Deserialize { - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(self.args_hash == packed_args_hash); + pack_arguments(self.args); let returns = context.call_private_function_with_packed_args( self.target_contract, self.selector, @@ -69,19 +66,13 @@ impl PrivateCallInterface { } pub fn view(self, context: &mut PrivateContext) -> T where T: Deserialize { - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(self.args_hash == packed_args_hash); + pack_arguments(self.args); let returns = context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false); returns.unpack_into() } pub fn delegate_call(self, context: &mut PrivateContext) -> T where T: Deserialize { - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(self.args_hash == packed_args_hash); + pack_arguments(self.args); let returns = context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, false, true); returns.unpack_into() } @@ -105,10 +96,7 @@ pub struct PrivateVoidCallInterface { impl PrivateVoidCallInterface { pub fn call(self, context: &mut PrivateContext) { - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(self.args_hash == packed_args_hash); + pack_arguments(self.args); context.call_private_function_with_packed_args( self.target_contract, self.selector, @@ -119,18 +107,12 @@ impl PrivateVoidCallInterface { } pub fn view(self, context: &mut PrivateContext) { - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(self.args_hash == packed_args_hash); + pack_arguments(self.args); context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false).assert_empty(); } pub fn delegate_call(self, context: &mut PrivateContext) { - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(self.args_hash == packed_args_hash); + pack_arguments(self.args); context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, false, true).assert_empty(); } } @@ -153,10 +135,7 @@ pub struct PrivateStaticCallInterface { impl PrivateStaticCallInterface { pub fn view(self, context: &mut PrivateContext) -> T where T: Deserialize { - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(self.args_hash == packed_args_hash); + pack_arguments(self.args); let returns = context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false); returns.unpack_into() } @@ -180,10 +159,7 @@ pub struct PrivateStaticVoidCallInterface { impl PrivateStaticVoidCallInterface { pub fn view(self, context: &mut PrivateContext) { - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(self.args_hash == packed_args_hash); + pack_arguments(self.args); context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false).assert_empty(); } } @@ -227,10 +203,7 @@ impl PublicCallInterface { pub fn enqueue(self, context: &mut PrivateContext) { let args_hash = hash_args(self.args); - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(args_hash == packed_args_hash); + pack_arguments(self.args); context.call_public_function_with_packed_args( self.target_contract, self.selector, @@ -242,10 +215,7 @@ impl PublicCallInterface { pub fn enqueue_view(self, context: &mut PrivateContext) { let args_hash = hash_args(self.args); - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(args_hash == packed_args_hash); + pack_arguments(self.args); context.call_public_function_with_packed_args( self.target_contract, self.selector, @@ -257,10 +227,7 @@ impl PublicCallInterface { pub fn delegate_enqueue(self, context: &mut PrivateContext) { let args_hash = hash_args(self.args); - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(args_hash == packed_args_hash); + pack_arguments(self.args); context.call_public_function_with_packed_args( self.target_contract, self.selector, @@ -310,10 +277,7 @@ impl PublicVoidCallInterface { pub fn enqueue(self, context: &mut PrivateContext) { let args_hash = hash_args(self.args); - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(args_hash == packed_args_hash); + pack_arguments(self.args); context.call_public_function_with_packed_args( self.target_contract, self.selector, @@ -325,10 +289,7 @@ impl PublicVoidCallInterface { pub fn enqueue_view(self, context: &mut PrivateContext) { let args_hash = hash_args(self.args); - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(args_hash == packed_args_hash); + pack_arguments(self.args); context.call_public_function_with_packed_args( self.target_contract, self.selector, @@ -340,10 +301,7 @@ impl PublicVoidCallInterface { pub fn delegate_enqueue(self, context: &mut PrivateContext) { let args_hash = hash_args(self.args); - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(args_hash == packed_args_hash); + pack_arguments(self.args); context.call_public_function_with_packed_args( self.target_contract, self.selector, @@ -384,10 +342,7 @@ impl PublicStaticCallInterface { pub fn enqueue_view(self, context: &mut PrivateContext) { let args_hash = hash_args(self.args); - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(args_hash == packed_args_hash); + pack_arguments(self.args); context.call_public_function_with_packed_args( self.target_contract, self.selector, @@ -427,10 +382,7 @@ impl PublicStaticVoidCallInterface { pub fn enqueue_view(self, context: &mut PrivateContext) { let args_hash = hash_args(self.args); - let packed_args_hash = unsafe { - pack_arguments(self.args) - }; - assert(args_hash == packed_args_hash); + pack_arguments(self.args); context.call_public_function_with_packed_args( self.target_contract, self.selector, diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr index c5b79df6937..0b002a3cc67 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -295,7 +295,7 @@ impl PrivateContext { args: [Field; ARGS_COUNT] ) -> PackedReturns { let args_hash = hash_args_array(args); - assert(args_hash == arguments::pack_arguments_array(args)); + arguments::pack_arguments_array(args); self.call_private_function_with_packed_args(contract_address, function_selector, args_hash, false, false) } @@ -306,7 +306,7 @@ impl PrivateContext { args: [Field; ARGS_COUNT] ) -> PackedReturns { let args_hash = hash_args_array(args); - assert(args_hash == arguments::pack_arguments_array(args)); + arguments::pack_arguments_array(args); self.call_private_function_with_packed_args(contract_address, function_selector, args_hash, true, false) } @@ -317,7 +317,7 @@ impl PrivateContext { args: [Field; ARGS_COUNT] ) -> PackedReturns { let args_hash = hash_args_array(args); - assert(args_hash == arguments::pack_arguments_array(args)); + arguments::pack_arguments_array(args); self.call_private_function_with_packed_args(contract_address, function_selector, args_hash, false, true) } @@ -407,7 +407,7 @@ impl PrivateContext { args: [Field; ARGS_COUNT] ) { let args_hash = hash_args_array(args); - assert(args_hash == arguments::pack_arguments_array(args)); + arguments::pack_arguments_array(args); self.call_public_function_with_packed_args(contract_address, function_selector, args_hash, false, false) } @@ -418,7 +418,7 @@ impl PrivateContext { args: [Field; ARGS_COUNT] ) { let args_hash = hash_args_array(args); - assert(args_hash == arguments::pack_arguments_array(args)); + arguments::pack_arguments_array(args); self.call_public_function_with_packed_args(contract_address, function_selector, args_hash, true, false) } @@ -429,7 +429,7 @@ impl PrivateContext { args: [Field; ARGS_COUNT] ) { let args_hash = hash_args_array(args); - assert(args_hash == arguments::pack_arguments_array(args)); + arguments::pack_arguments_array(args); self.call_public_function_with_packed_args(contract_address, function_selector, args_hash, false, true) } @@ -495,7 +495,7 @@ impl PrivateContext { args: [Field; ARGS_COUNT] ) { let args_hash = hash_args_array(args); - assert(args_hash == arguments::pack_arguments_array(args)); + arguments::pack_arguments_array(args); self.set_public_teardown_function_with_packed_args(contract_address, function_selector, args_hash, false, false) } diff --git a/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr b/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr index 50316d50893..8cd5bfa657e 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr @@ -1,24 +1,32 @@ -#[oracle(packArgumentsArray)] -unconstrained fn pack_arguments_array_oracle(_args: [Field; N]) -> Field {} +/// Notifies the simulator that `args` will later be used at some point during execution, referenced by their hash. This +/// allows the simulator to know how to respond to this future request. +/// +/// This is only used during private execution, since in public it is the VM itself that keeps track of arguments. +pub fn pack_arguments(args: [Field]) { + // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. + unsafe { + pack_arguments_oracle_wrapper(args) + }; +} -#[oracle(packArguments)] -unconstrained fn pack_arguments_oracle(_args: [Field]) -> Field {} +/// Same as `pack_arguments`, but using arrays instead of slices. +pub fn pack_arguments_array(args: [Field; N]) { + // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. + unsafe { + pack_arguments_array_oracle_wrapper(args) + }; +} -/// - Pack arguments (array version) will notify the simulator that these arguments will be used later at -/// some point in the call. -/// - When the external call is made later, the simulator will know what the values unpack to. -/// - This oracle will not be required in public vm functions, as the vm will keep track of arguments -/// itself. -unconstrained pub fn pack_arguments_array(args: [Field; N]) -> Field { - pack_arguments_array_oracle(args) +unconstrained fn pack_arguments_oracle_wrapper(args: [Field]) { + let _ = pack_arguments_oracle(args); } -/// - Pack arguments (slice version) will notify the simulator that these arguments will be used later at -/// some point in the call. -/// - When the external call is made later, the simulator will know what the values unpack to. -/// - This oracle will not be required in public vm functions, as the vm will keep track of arguments -/// itself. -unconstrained pub fn pack_arguments(args: [Field]) -> Field { - pack_arguments_oracle(args) +unconstrained fn pack_arguments_array_oracle_wrapper(args: [Field; N]) { + let _ = pack_arguments_array_oracle(args); } +#[oracle(packArguments)] +unconstrained fn pack_arguments_oracle(_args: [Field]) -> Field {} + +#[oracle(packArgumentsArray)] +unconstrained fn pack_arguments_array_oracle(_args: [Field; N]) -> Field {} From 14fb115b1e8e1070a3a85a9c404e283e3d82a461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 25 Sep 2024 20:35:02 +0000 Subject: [PATCH 2/8] Make pack_returns safe --- .../aztec/src/context/packed_returns.nr | 6 ++++- .../aztec-nr/aztec/src/oracle/arguments.nr | 6 +++-- .../aztec-nr/aztec/src/oracle/returns.nr | 26 ++++++++++++++----- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/context/packed_returns.nr b/noir-projects/aztec-nr/aztec/src/context/packed_returns.nr index abf2ee7ea39..015324f0ede 100644 --- a/noir-projects/aztec-nr/aztec/src/context/packed_returns.nr +++ b/noir-projects/aztec-nr/aztec/src/context/packed_returns.nr @@ -19,7 +19,11 @@ impl PackedReturns { } pub fn unpack(self) -> [Field; N] { - let unpacked: [Field; N] = unpack_returns(self.packed_returns); + // We verify that the value returned by `unpack_returns` is the preimage of `packed_returns`, fully constraining + // it. + let unpacked: [Field; N] = unsafe { + unpack_returns(self.packed_returns) + }; assert_eq(self.packed_returns, hash_args_array(unpacked)); unpacked } diff --git a/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr b/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr index 8cd5bfa657e..a6ea2b95de2 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr @@ -3,7 +3,8 @@ /// /// This is only used during private execution, since in public it is the VM itself that keeps track of arguments. pub fn pack_arguments(args: [Field]) { - // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. + // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. When + // unpacking however the caller must check that the returned value is indeed the preimage. unsafe { pack_arguments_oracle_wrapper(args) }; @@ -11,7 +12,8 @@ pub fn pack_arguments(args: [Field]) { /// Same as `pack_arguments`, but using arrays instead of slices. pub fn pack_arguments_array(args: [Field; N]) { - // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. + // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. When + // unpacking however the caller must check that the returned value is indeed the preimage. unsafe { pack_arguments_array_oracle_wrapper(args) }; diff --git a/noir-projects/aztec-nr/aztec/src/oracle/returns.nr b/noir-projects/aztec-nr/aztec/src/oracle/returns.nr index b7372f40a0d..ec06b98a8d4 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/returns.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/returns.nr @@ -1,13 +1,25 @@ -#[oracle(packReturns)] -unconstrained fn pack_returns_oracle(_returns: [Field]) -> Field {} - -unconstrained pub fn pack_returns(returns: [Field]) { - let _unused = pack_returns_oracle(returns); +/// Notifies the simulator that `returns` will be later fetched once the function return is processed, referenced by +/// their hash. This allows the simulator to know how to respond to this future request. +/// +/// This is only used during private execution, since in public it is the VM itself that keeps track of return values. +pub fn pack_returns(returns: [Field]) { + // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. When + // unpacking however the caller must check that the returned value is indeed the preimage. + unsafe { + pack_returns_wrapper(returns) + }; } -#[oracle(unpackReturns)] -unconstrained fn unpack_returns_oracle(_return_hash: Field) -> [Field; N] {} +unconstrained pub fn pack_returns_wrapper(returns: [Field]) { + let _ = pack_returns_oracle(returns); +} unconstrained pub fn unpack_returns(return_hash: Field) -> [Field; N] { unpack_returns_oracle(return_hash) } + +#[oracle(packReturns)] +unconstrained fn pack_returns_oracle(_returns: [Field]) -> Field {} + +#[oracle(unpackReturns)] +unconstrained fn unpack_returns_oracle(_return_hash: Field) -> [Field; N] {} From 59fc54f6592f520a5b848f6bdaff11de8733fddf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 25 Sep 2024 20:45:04 +0000 Subject: [PATCH 3/8] Make the note lifecycle oracle fns safe --- .../aztec-nr/aztec/src/note/lifecycle.nr | 8 +-- .../aztec-nr/aztec/src/oracle/notes.nr | 49 +++++++++++++++---- .../src/test/helpers/test_environment.nr | 5 +- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/note/lifecycle.nr b/noir-projects/aztec-nr/aztec/src/note/lifecycle.nr index 58321fff061..8d07d8c40ba 100644 --- a/noir-projects/aztec-nr/aztec/src/note/lifecycle.nr +++ b/noir-projects/aztec-nr/aztec/src/note/lifecycle.nr @@ -19,15 +19,12 @@ pub fn create_note( let note_hash = note.compute_note_hash(); let serialized_note = Note::serialize_content(*note); - assert( - notify_created_note( + notify_created_note( storage_slot, Note::get_note_type_id(), serialized_note, note_hash, note_hash_counter - ) - == 0 ); context.push_note_hash(note_hash); @@ -81,8 +78,7 @@ pub fn destroy_note_unsafe( }; let nullifier_counter = context.side_effect_counter; - assert(notify_nullified_note(nullifier, notification_note_hash, nullifier_counter) == 0); + notify_nullified_note(nullifier, notification_note_hash, nullifier_counter); context.push_nullifier_for_note_hash(nullifier, notification_note_hash) } - diff --git a/noir-projects/aztec-nr/aztec/src/oracle/notes.nr b/noir-projects/aztec-nr/aztec/src/oracle/notes.nr index b3f95b57a4a..37dbe2319c5 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/notes.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/notes.nr @@ -2,6 +2,41 @@ use crate::note::{note_header::NoteHeader, note_interface::NoteInterface}; use dep::protocol_types::{address::AztecAddress, utils::arr_copy_slice}; +/// Notifies the simulator that a note has been created, so that it can be returned in future read requests in the same +/// transaction. This note should only be added to the non-volatile database if found in an actual block. +pub fn notify_created_note( + storage_slot: Field, + note_type_id: Field, + serialized_note: [Field; N], + note_hash: Field, + counter: u32 +) { + // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. + unsafe { + notify_created_note_oracle_wrapper(storage_slot, note_type_id, serialized_note, note_hash, counter) + }; +} + +/// Notifies the simulator that a note has been nullified, so that it is no longer returned in future read requests in +/// the same transaction. This note should only be removed to the non-volatile database if its nullifier is found in an +/// actual block. +pub fn notify_nullified_note(nullifier: Field, note_hash: Field, counter: u32) { + // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. + unsafe { + notify_nullified_note_oracle_wrapper(nullifier, note_hash, counter) + }; +} + +unconstrained fn notify_created_note_oracle_wrapper( + storage_slot: Field, + note_type_id: Field, + serialized_note: [Field; N], + note_hash: Field, + counter: u32 +) { + let _ = notify_created_note_oracle(storage_slot, note_type_id, serialized_note, note_hash, counter); +} + #[oracle(notifyCreatedNote)] unconstrained fn notify_created_note_oracle( _storage_slot: Field, @@ -11,23 +46,17 @@ unconstrained fn notify_created_note_oracle( _counter: u32 ) -> Field {} -unconstrained pub fn notify_created_note( - storage_slot: Field, - note_type_id: Field, - serialized_note: [Field; N], +unconstrained fn notify_nullified_note_oracle_wrapper( + nullifier: Field, note_hash: Field, counter: u32 -) -> Field { - notify_created_note_oracle(storage_slot, note_type_id, serialized_note, note_hash, counter) +) { + let _ = notify_nullified_note_oracle(nullifier, note_hash, counter); } #[oracle(notifyNullifiedNote)] unconstrained fn notify_nullified_note_oracle(_nullifier: Field, _note_hash: Field, _counter: u32) -> Field {} -unconstrained pub fn notify_nullified_note(nullifier: Field, note_hash: Field, counter: u32) -> Field { - notify_nullified_note_oracle(nullifier, note_hash, counter) -} - #[oracle(getNotes)] unconstrained fn get_notes_oracle( _storage_slot: Field, diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr index d176db97cb2..72533505bf0 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr @@ -202,15 +202,12 @@ impl TestEnvironment { note.set_header(header); let note_hash = note.compute_note_hash(); let serialized_note = Note::serialize_content(*note); - assert( - notify_created_note( + notify_created_note( storage_slot, Note::get_note_type_id(), serialized_note, note_hash, note_hash_counter - ) - == 0 ); cheatcodes::set_contract_address(original_contract_address); } From 3419baaf2ba252ba288c6b6a1b6bc436d1b65d13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 25 Sep 2024 20:48:35 +0000 Subject: [PATCH 4/8] Imporve docs for check_nullifier_exists --- noir-projects/aztec-nr/aztec/src/oracle/notes.nr | 12 +++++++----- .../aztec-nr/aztec/src/state_vars/private_mutable.nr | 10 +++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/oracle/notes.nr b/noir-projects/aztec-nr/aztec/src/oracle/notes.nr index 37dbe2319c5..f1f08a15f17 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/notes.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/notes.nr @@ -171,11 +171,13 @@ unconstrained pub fn get_notes Field {} - -// Only ever use this in private! +/// Returns true if the nullifier exists. Note that a `true` value can be constrained by proving existence of the +/// nullifier, but a `false` value should not be relied upon since other transactions may emit this nullifier before the +/// current transaction is included in a block. While this might seem of little use at first, certain design patterns +/// benefit from this abstraction (see e.g. `PrivateMutable`). unconstrained pub fn check_nullifier_exists(inner_nullifier: Field) -> bool { check_nullifier_exists_oracle(inner_nullifier) == 1 } + +#[oracle(checkNullifierExists)] +unconstrained fn check_nullifier_exists_oracle(_inner_nullifier: Field) -> Field {} 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 453f46b5515..66412d6b1d4 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 @@ -68,11 +68,7 @@ impl PrivateMutable where Note: Not // docs:end:replace pub fn initialize_or_replace(self, note: &mut Note) -> NoteEmission { - let is_initialized = unsafe { - check_nullifier_exists(self.compute_initialization_nullifier()) - }; - - // check_nullifier_exists() is an unconstrained function - we can constrain a true value by providing an + // `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: @@ -82,6 +78,10 @@ impl PrivateMutable where Note: Not // 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. + let is_initialized = unsafe { + check_nullifier_exists(self.compute_initialization_nullifier()) + }; + if (!is_initialized) { self.initialize(note) } else { From 44cf2f650e261652ca6bfa45a0a00e57f8d10655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 25 Sep 2024 21:11:00 +0000 Subject: [PATCH 5/8] Improve get_l1_to_l2_membership_witness --- noir-projects/aztec-nr/aztec/src/messaging.nr | 13 +++++----- .../oracle/get_l1_to_l2_membership_witness.nr | 26 +++++++++++-------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/messaging.nr b/noir-projects/aztec-nr/aztec/src/messaging.nr index f0555bbf449..e54c44c45c0 100644 --- a/noir-projects/aztec-nr/aztec/src/messaging.nr +++ b/noir-projects/aztec-nr/aztec/src/messaging.nr @@ -3,8 +3,7 @@ use crate::{ oracle::get_l1_to_l2_membership_witness::get_l1_to_l2_membership_witness }; -use dep::protocol_types::merkle_tree::root::root_from_sibling_path; -use dep::protocol_types::{constants::L1_TO_L2_MSG_TREE_HEIGHT, address::{AztecAddress, EthAddress}, utils::arr_copy_slice}; +use dep::protocol_types::{address::{AztecAddress, EthAddress}, merkle_tree::root::root_from_sibling_path}; pub fn process_l1_to_l2_message( l1_to_l2_root: Field, @@ -25,12 +24,12 @@ pub fn process_l1_to_l2_message( secret_hash ); - let returned_message = get_l1_to_l2_membership_witness(storage_contract_address, message_hash, secret); - let leaf_index = returned_message[0]; - let sibling_path = arr_copy_slice(returned_message, [0; L1_TO_L2_MSG_TREE_HEIGHT], 1); + // We prove that `message_hash` is in the tree by showing the derivation of the tree root, using a merkle path we + // get from an oracle. + let (leaf_index, sibling_path) = unsafe { + get_l1_to_l2_membership_witness(storage_contract_address, message_hash, secret) + }; - // Check that the message is in the tree - // This is implicitly checking that the values of the message are correct let root = root_from_sibling_path(message_hash, leaf_index, sibling_path); assert(root == l1_to_l2_root, "Message not in state"); diff --git a/noir-projects/aztec-nr/aztec/src/oracle/get_l1_to_l2_membership_witness.nr b/noir-projects/aztec-nr/aztec/src/oracle/get_l1_to_l2_membership_witness.nr index 0a93fa3f8f2..b1adf7818c7 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/get_l1_to_l2_membership_witness.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/get_l1_to_l2_membership_witness.nr @@ -1,6 +1,18 @@ -use dep::protocol_types::address::AztecAddress; +use dep::protocol_types::{address::AztecAddress, constants::L1_TO_L2_MSG_TREE_HEIGHT, utils::arr_copy_slice}; -global L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH: u32 = 17; +/// Returns the leaf index and sibling path of an entry in the L1 to L2 messaging tree, which can then be used to prove +/// its existence. +unconstrained pub fn get_l1_to_l2_membership_witness( + contract_address: AztecAddress, + message_hash: Field, + secret: Field +) -> (Field, [Field; L1_TO_L2_MSG_TREE_HEIGHT]) { + let returned_message = get_l1_to_l2_membership_witness_oracle(contract_address, message_hash, secret); + let leaf_index = returned_message[0]; + let sibling_path = arr_copy_slice(returned_message, [0; L1_TO_L2_MSG_TREE_HEIGHT], 1); + + (leaf_index, sibling_path) +} // Obtains membership witness (index and sibling path) for a message in the L1 to L2 message tree. #[oracle(getL1ToL2MembershipWitness)] @@ -8,12 +20,4 @@ unconstrained fn get_l1_to_l2_membership_witness_oracle( _contract_address: AztecAddress, _message_hash: Field, _secret: Field -) -> [Field; L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH] {} - -unconstrained pub fn get_l1_to_l2_membership_witness( - contract_address: AztecAddress, - message_hash: Field, - secret: Field -) -> [Field; L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH] { - get_l1_to_l2_membership_witness_oracle(contract_address, message_hash, secret) -} +) -> [Field; L1_TO_L2_MSG_TREE_HEIGHT + 1] {} From e277b4aa72478ca60b47aa38f1359b09738cfbb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 25 Sep 2024 21:13:17 +0000 Subject: [PATCH 6/8] Make compute_note_hash_and_optionally_a_nullifier unconstrained --- noir-projects/aztec-nr/aztec/src/note/utils.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/note/utils.nr b/noir-projects/aztec-nr/aztec/src/note/utils.nr index 7b2f671f6ed..47f58177c04 100644 --- a/noir-projects/aztec-nr/aztec/src/note/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/note/utils.nr @@ -105,7 +105,7 @@ pub fn compute_note_hash_for_nullify(note: Note) -> Field wher compute_note_hash_for_nullify_internal(note, note_hash_for_read_request) } -pub fn compute_note_hash_and_optionally_a_nullifier( +unconstrained pub fn compute_note_hash_and_optionally_a_nullifier( deserialize_content: fn([Field; N]) -> T, note_header: NoteHeader, compute_nullifier: bool, From 8a50ae79abe38eb1ca47f3a10341825f392e77f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 25 Sep 2024 21:27:33 +0000 Subject: [PATCH 7/8] Make debug logs safe --- .../crates/types/src/debug_log.nr | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/debug_log.nr b/noir-projects/noir-protocol-circuits/crates/types/src/debug_log.nr index bccbc9a2adf..e5045456b65 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/debug_log.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/debug_log.nr @@ -1,23 +1,26 @@ -// Utility function to console.log data in the acir simulator -// WARNING: sometimes when using debug logs the ACVM errors with: `thrown: "solver opcode resolution error: cannot solve opcode: expression has too many unknowns x155"` +/// Utility function to console.log data in the acir simulator. +/// Example: +/// debug_log("blah blah this is a debug string"); +pub fn debug_log(msg: str) { + debug_log_format(msg, []); +} -#[oracle(debugLog)] -unconstrained fn debug_log_oracle(_msg: str, args: [Field]) {} +/// Utility function to console.log data in the acir simulator. This variant receives a format string in which the +/// `${k}` tokens will be replaced with the k-eth value in the `args` array. +/// Examples: +/// debug_log_format("get_2(slot:{0}) =>\n\t0:{1}\n\t1:{2}", [storage_slot, note0_hash, note1_hash]); +/// debug_log_format("whole array: {}", [e1, e2, e3, e4]); +pub fn debug_log_format(msg: str, args: [Field; N]) { + // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. + unsafe { + debug_log_oracle_wrapper(msg, args) + }; +} -/// NOTE: call this with a str msg of form -/// "some string with {0} and {1} ... {N}" -/// and an array of N field which will be formatted -/// into the string in the simulator. -/// Example: -/// debug_log_format("get_2(slot:{0}) =>\n\t0:{1}\n\t1:{2}", [storage_slot, note0_hash, note1_hash]); -/// debug_log_format("whole array: {}", [e1, e2, e3, e4]); -unconstrained pub fn debug_log_format(msg: str, args: [Field; N]) { +unconstrained pub fn debug_log_oracle_wrapper(msg: str, args: [Field; N]) { debug_log_oracle(msg, args.as_slice()); } -/// NOTE: call this with a str msg of length > 1 -/// Example: -/// `debug_log("blah blah this is a debug string");` -unconstrained pub fn debug_log(msg: str) { - debug_log_format(msg, []); -} +// WARNING: sometimes when using debug logs the ACVM errors with: `thrown: "solver opcode resolution error: cannot solve opcode: expression has too many unknowns x155"` +#[oracle(debugLog)] +unconstrained fn debug_log_oracle(_msg: str, args: [Field]) {} From c13beb0c92069369c592fc31382bc66a2e968f66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 25 Sep 2024 21:27:48 +0000 Subject: [PATCH 8/8] Improve fn names --- .../aztec/src/oracle/enqueue_public_function_call.nr | 12 +++++++++--- noir-projects/aztec-nr/aztec/src/oracle/returns.nr | 4 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr b/noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr index 484c4e59030..fabfb4d8e2b 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr @@ -56,9 +56,15 @@ unconstrained pub fn set_public_teardown_function_call_internal( ); } -#[oracle(notifySetMinRevertibleSideEffectCounter)] -unconstrained fn notify_set_min_revertible_side_effect_counter_oracle(_counter: u32) {} +pub fn notify_set_min_revertible_side_effect_counter(counter: u32) { + unsafe { + notify_set_min_revertible_side_effect_counter_oracle_wrapper(counter) + }; +} -unconstrained pub fn notify_set_min_revertible_side_effect_counter(counter: u32) { +unconstrained pub fn notify_set_min_revertible_side_effect_counter_oracle_wrapper(counter: u32) { notify_set_min_revertible_side_effect_counter_oracle(counter); } + +#[oracle(notifySetMinRevertibleSideEffectCounter)] +unconstrained fn notify_set_min_revertible_side_effect_counter_oracle(_counter: u32) {} diff --git a/noir-projects/aztec-nr/aztec/src/oracle/returns.nr b/noir-projects/aztec-nr/aztec/src/oracle/returns.nr index ec06b98a8d4..4b1dec74105 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/returns.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/returns.nr @@ -6,11 +6,11 @@ pub fn pack_returns(returns: [Field]) { // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. When // unpacking however the caller must check that the returned value is indeed the preimage. unsafe { - pack_returns_wrapper(returns) + pack_returns_oracle_wrapper(returns) }; } -unconstrained pub fn pack_returns_wrapper(returns: [Field]) { +unconstrained pub fn pack_returns_oracle_wrapper(returns: [Field]) { let _ = pack_returns_oracle(returns); }