From f10d88ab571945a883aa96071cc8bff1d73fc917 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 9 Jan 2025 11:32:59 -0300 Subject: [PATCH] fix: Sequencer times out L1 tx at end of L2 slot (#11112) Sets the L1 tx utils timeout to the end of the L2 slot. --- .../aztec/src/context/packed_returns.nr | 7 +- .../aztec/src/context/private_context.nr | 41 +++++---- .../aztec/src/context/public_context.nr | 92 ++++++++++++------- .../encrypted_event_emission.nr | 9 +- .../encrypted_logs/encrypted_note_emission.nr | 24 ++--- .../aztec/src/encrypted_logs/payload.nr | 38 +++++--- .../aztec/src/history/note_inclusion.nr | 1 + .../aztec/src/history/nullifier_inclusion.nr | 1 + .../src/history/nullifier_non_inclusion.nr | 1 + .../aztec/src/history/public_storage.nr | 1 + .../aztec-nr/aztec/src/keys/getters/mod.nr | 6 +- noir-projects/aztec-nr/aztec/src/messaging.nr | 6 +- .../aztec/src/note/note_getter/mod.nr | 14 ++- .../aztec-nr/aztec/src/oracle/arguments.nr | 16 ++-- .../aztec-nr/aztec/src/oracle/block_header.nr | 10 +- .../oracle/enqueue_public_function_call.nr | 6 +- .../aztec/src/oracle/get_contract_instance.nr | 25 +++-- .../aztec-nr/aztec/src/oracle/notes.nr | 16 +++- .../aztec-nr/aztec/src/oracle/returns.nr | 8 +- .../aztec/src/state_vars/private_mutable.nr | 26 +++--- .../aztec/src/state_vars/shared_mutable.nr | 1 + .../aztec/src/utils/array/collapse.nr | 5 +- .../aztec-nr/uint-note/src/uint_note.nr | 12 ++- .../aztec-nr/value-note/src/value_note.nr | 13 ++- .../src/subscription_note.nr | 12 ++- .../schnorr_account_contract/src/main.nr | 5 +- .../src/main.nr | 5 +- .../src/main.nr | 6 +- .../spam_contract/src/types/token_note.nr | 12 ++- .../src/types/token_note.nr | 12 ++- .../contracts/token_contract/src/main.nr | 8 +- .../types/src/abis/private_kernel_data.nr | 2 + .../crates/types/src/debug_log.nr | 7 +- .../crates/types/src/proof/vk_data.nr | 7 +- .../crates/types/src/utils/arrays.nr | 25 +++-- .../src/utils/arrays/assert_combined_array.nr | 9 +- .../assert_combined_transformed_array.nr | 24 +++-- .../get_order_hints.nr | 16 ++-- .../get_split_order_hints.nr | 32 +++---- .../assert_split_transformed_value_arrays.nr | 6 +- .../src/utils/arrays/get_sorted_result.nr | 18 ++-- .../src/utils/arrays/get_sorted_tuple.nr | 4 +- .../crates/types/src/utils/arrays/sort_by.nr | 4 +- .../types/src/utils/arrays/sort_by_counter.nr | 32 +++---- 44 files changed, 378 insertions(+), 247 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 ebb86a9621f..aa77358b93d 100644 --- a/noir-projects/aztec-nr/aztec/src/context/packed_returns.nr +++ b/noir-projects/aztec-nr/aztec/src/context/packed_returns.nr @@ -19,9 +19,10 @@ impl PackedReturns { } pub fn unpack(self) -> [Field; N] { - // 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) }; + let unpacked: [Field; N] = unsafe { + //@safety We check `unpacked` against the hash below. + unpack_returns(self.packed_returns) + }; assert_eq(self.packed_returns, hash_args_array(unpacked)); unpacked } 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 e0ea249df30..d2903d8a33c 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -264,8 +264,11 @@ impl PrivateContext { // about secrets from other contracts. We therefore silo secret keys, and rely on the private kernel to // validate that we siloed secret key corresponds to correct siloing of the master secret key that hashes // to `pk_m_hash`. - let request = unsafe { get_key_validation_request(pk_m_hash, key_index) }; - assert(request.pk_m.hash() == pk_m_hash); + let request = unsafe { + //@safety Kernels verify that the key validation request is valid and below we verify that a request + // for the correct public key has been received. + get_key_validation_request(pk_m_hash, key_index) + }; self.key_validation_requests_and_generators.push( KeyValidationRequestAndGenerator { @@ -385,12 +388,12 @@ impl PrivateContext { let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call; let start_side_effect_counter = self.side_effect_counter; - // The oracle simulates the private call and returns the value of the side effects counter after execution of - // the call (which means that end_side_effect_counter - start_side_effect_counter is the number of side effects - // that took place), along with the hash of the return values. We validate these by requesting a private kernel - // iteration in which the return values are constrained to hash to `returns_hash` and the side effects counter - // to increment from start to end. let (end_side_effect_counter, returns_hash) = unsafe { + //@safety The oracle simulates the private call and returns the value of the side effects counter after + // execution of the call (which means that end_side_effect_counter - start_side_effect_counter is + // the number of side effects that took place), along with the hash of the return values. We validate these + // by requesting a private kernel iteration in which the return values are constrained to hash + // to `returns_hash` and the side effects counter to increment from start to end. call_private_function_internal( contract_address, function_selector, @@ -488,13 +491,13 @@ impl PrivateContext { let counter = self.next_counter(); let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call; - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this. - // WARNING: This is insecure and should be temporary! - // The oracle repacks the arguments and returns a new args_hash. - // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. - // We don't validate or compute it in the circuit because a) it's harder to do with slices, and - // b) this is only temporary. let args_hash = unsafe { + //@safety TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this. + // WARNING: This is insecure and should be temporary! + // The oracle repacks the arguments and returns a new args_hash. + // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. + // We don't validate or compute it in the circuit because a) it's harder to do with slices, and + // b) this is only temporary. enqueue_public_function_call_internal( contract_address, function_selector, @@ -544,13 +547,13 @@ impl PrivateContext { let counter = self.next_counter(); let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call; - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this. - // WARNING: This is insecure and should be temporary! - // The oracle repacks the arguments and returns a new args_hash. - // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. - // We don't validate or compute it in the circuit because a) it's harder to do with slices, and - // b) this is only temporary. let args_hash = unsafe { + //@safety TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this. + // WARNING: This is insecure and should be temporary! + // The oracle repacks the arguments and returns a new args_hash. + // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function. + // We don't validate or compute it in the circuit because a) it's harder to do with slices, and + // b) this is only temporary. set_public_teardown_function_call_internal( contract_address, function_selector, diff --git a/noir-projects/aztec-nr/aztec/src/context/public_context.nr b/noir-projects/aztec-nr/aztec/src/context/public_context.nr index d8095be42dd..d03335010e6 100644 --- a/noir-projects/aztec-nr/aztec/src/context/public_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/public_context.nr @@ -21,23 +21,34 @@ impl PublicContext { where T: Serialize, { - // AVM opcodes are constrained by the AVM itself - unsafe { emit_unencrypted_log(Serialize::serialize(log).as_slice()) }; + unsafe { + //@safety AVM opcodes are constrained by the AVM itself + emit_unencrypted_log(Serialize::serialize(log).as_slice()) + }; } pub fn note_hash_exists(_self: Self, note_hash: Field, leaf_index: Field) -> bool { - // AVM opcodes are constrained by the AVM itself - unsafe { note_hash_exists(note_hash, leaf_index) } == 1 + unsafe { + //@safety AVM opcodes are constrained by the AVM itself + note_hash_exists(note_hash, leaf_index) + } + == 1 } pub fn l1_to_l2_msg_exists(_self: Self, msg_hash: Field, msg_leaf_index: Field) -> bool { - // AVM opcodes are constrained by the AVM itself - unsafe { l1_to_l2_msg_exists(msg_hash, msg_leaf_index) } == 1 + unsafe { + //@safety AVM opcodes are constrained by the AVM itself + l1_to_l2_msg_exists(msg_hash, msg_leaf_index) + } + == 1 } pub fn nullifier_exists(_self: Self, unsiloed_nullifier: Field, address: AztecAddress) -> bool { - // AVM opcodes are constrained by the AVM itself - unsafe { nullifier_exists(unsiloed_nullifier, address.to_field()) } == 1 + unsafe { + //@safety AVM opcodes are constrained by the AVM itself + nullifier_exists(unsiloed_nullifier, address.to_field()) + } + == 1 } pub fn consume_l1_to_l2_message( @@ -73,8 +84,10 @@ impl PublicContext { } pub fn message_portal(_self: &mut Self, recipient: EthAddress, content: Field) { - // AVM opcodes are constrained by the AVM itself - unsafe { send_l2_to_l1_msg(recipient, content) }; + unsafe { + //@safety AVM opcodes are constrained by the AVM itself + send_l2_to_l1_msg(recipient, content) + }; } pub unconstrained fn call_public_function( @@ -114,30 +127,36 @@ impl PublicContext { } pub fn push_note_hash(_self: &mut Self, note_hash: Field) { - // AVM opcodes are constrained by the AVM itself - unsafe { emit_note_hash(note_hash) }; + unsafe { + //@safety AVM opcodes are constrained by the AVM itself + emit_note_hash(note_hash) + }; } pub fn push_nullifier(_self: &mut Self, nullifier: Field) { - // AVM opcodes are constrained by the AVM itself - unsafe { emit_nullifier(nullifier) }; + unsafe { + //@safety AVM opcodes are constrained by the AVM itself + emit_nullifier(nullifier) + }; } pub fn this_address(_self: Self) -> AztecAddress { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself address() } } pub fn msg_sender(_self: Self) -> AztecAddress { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself sender() } } pub fn selector(_self: Self) -> FunctionSelector { // The selector is the first element of the calldata when calling a public function through dispatch. - // AVM opcodes are constrained by the AVM itself. - let raw_selector: [Field; 1] = unsafe { calldata_copy(0, 1) }; + let raw_selector: [Field; 1] = unsafe { + //@safety AVM opcodes are constrained by the AVM itself + calldata_copy(0, 1) + }; FunctionSelector::from_field(raw_selector[0]) } pub fn get_args_hash(mut self) -> Field { @@ -148,71 +167,76 @@ impl PublicContext { self.args_hash.unwrap_unchecked() } pub fn transaction_fee(_self: Self) -> Field { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself transaction_fee() } } pub fn chain_id(_self: Self) -> Field { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself chain_id() } } pub fn version(_self: Self) -> Field { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself version() } } pub fn block_number(_self: Self) -> Field { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself block_number() } } pub fn timestamp(_self: Self) -> u64 { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself timestamp() } } pub fn fee_per_l2_gas(_self: Self) -> Field { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself fee_per_l2_gas() } } pub fn fee_per_da_gas(_self: Self) -> Field { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself fee_per_da_gas() } } pub fn l2_gas_left(_self: Self) -> Field { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself l2_gas_left() } } pub fn da_gas_left(_self: Self) -> Field { - // AVM opcodes are constrained by the AVM itself unsafe { + //@safety AVM opcodes are constrained by the AVM itself da_gas_left() } } pub fn is_static_call(_self: Self) -> bool { - // AVM opcodes are constrained by the AVM itself - unsafe { is_static_call() } == 1 + unsafe { + //@safety AVM opcodes are constrained by the AVM itself + is_static_call() + } + == 1 } pub fn raw_storage_read(_self: Self, storage_slot: Field) -> [Field; N] { let mut out = [0; N]; for i in 0..N { - // AVM opcodes are constrained by the AVM itself - out[i] = unsafe { storage_read(storage_slot + i as Field) }; + out[i] = unsafe { + //@safety AVM opcodes are constrained by the AVM itself + storage_read(storage_slot + i as Field) + }; } out } @@ -226,8 +250,10 @@ impl PublicContext { pub fn raw_storage_write(_self: Self, storage_slot: Field, values: [Field; N]) { for i in 0..N { - // AVM opcodes are constrained by the AVM itself - unsafe { storage_write(storage_slot + i as Field, values[i]) }; + unsafe { + //@safety AVM opcodes are constrained by the AVM itself + storage_write(storage_slot + i as Field, values[i]) + }; } } diff --git a/noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_event_emission.nr b/noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_event_emission.nr index a0fd3619a3c..410a7350fe1 100644 --- a/noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_event_emission.nr +++ b/noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_event_emission.nr @@ -55,10 +55,11 @@ where Event: EventInterface, { |e: Event| { - // Unconstrained logs have both their content and encryption unconstrained - it could occur that the - // recipient is unable to decrypt the payload. - let encrypted_log = - unsafe { compute_payload_unconstrained(*context, e, recipient, sender) }; + let encrypted_log = unsafe { + //@safety Unconstrained logs have both their content and encryption unconstrained - it could occur that the + // recipient is unable to decrypt the payload. + compute_payload_unconstrained(*context, e, recipient, sender) + }; context.emit_private_log(encrypted_log); } } diff --git a/noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_note_emission.nr b/noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_note_emission.nr index f7f6f7cf52f..d33c1243159 100644 --- a/noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_note_emission.nr +++ b/noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_note_emission.nr @@ -73,18 +73,18 @@ where Note: NoteInterface, { |e: NoteEmission| { - // Unconstrained logs have both their content and encryption unconstrained - it could occur that the - // recipient is unable to decrypt the payload. - // Regarding the note hash counter, this is used for squashing. The kernel assumes that a given note can have - // more than one log and removes all of the matching ones, so all a malicious sender could do is either: cause - // for the log to be deleted when it shouldn't have (which is fine - they can already make the content be - // whatever), or cause for the log to not be deleted when it should have (which is also fine - it'll be a log - // for a note that doesn't exist). - // It's important here that we do not - // return the log from this function to the app, otherwise it could try to do stuff with it and then that might - // be wrong. - let (encrypted_log, note_hash_counter) = - unsafe { compute_payload_unconstrained(*context, e.note, recipient, sender) }; + let (encrypted_log, note_hash_counter) = unsafe { + //@safety Unconstrained logs have both their content and encryption unconstrained - it could occur that + // the recipient is unable to decrypt the payload. + // Regarding the note hash counter, this is used for squashing. The kernel assumes that a given note can + // have more than one log and removes all of the matching ones, so all a malicious sender could do is + // either: cause for the log to be deleted when it shouldn't have (which is fine - they can already make + // the content be whatever), or cause for the log to not be deleted when it should have (which is also fine + // - it'll be a log for a note that doesn't exist). + // It's important here that we do not return the log from this function to the app, otherwise it could + // try to do stuff with it and then that might be wrong. + compute_payload_unconstrained(*context, e.note, recipient, sender) + }; context.emit_raw_note_log(encrypted_log, note_hash_counter); } } diff --git a/noir-projects/aztec-nr/aztec/src/encrypted_logs/payload.nr b/noir-projects/aztec-nr/aztec/src/encrypted_logs/payload.nr index 3ea33631215..f34c95b7696 100644 --- a/noir-projects/aztec-nr/aztec/src/encrypted_logs/payload.nr +++ b/noir-projects/aztec-nr/aztec/src/encrypted_logs/payload.nr @@ -62,10 +62,12 @@ pub fn compute_private_log_payload( let encrypted: [u8; ENCRYPTED_PAYLOAD_SIZE_IN_BYTES] = compute_encrypted_log(contract_address, recipient, extended_plaintext); - // We assume that the sender wants for the recipient to find the tagged note, and therefore that they will cooperate - // and use the correct tag. Usage of a bad tag will result in the recipient not being able to find the note - // automatically. - let tag = unsafe { get_app_tag_as_sender(sender, recipient) }; + let tag = unsafe { + //@safety We assume that the sender wants for the recipient to find the tagged note, and therefore that they + // will cooperate and use the correct tag. Usage of a bad tag will result in the recipient not being able to + // find the note automatically. + get_app_tag_as_sender(sender, recipient) + }; increment_app_tagging_secret_index_as_sender(sender, recipient); array_concat([tag], bytes_to_fields(encrypted)) @@ -82,10 +84,12 @@ pub fn compute_partial_public_log_payload( let encrypted: [u8; M - 32] = compute_encrypted_log(contract_address, recipient, extended_plaintext); - // We assume that the sender wants for the recipient to find the tagged note, and therefore that they will cooperate - // and use the correct tag. Usage of a bad tag will result in the recipient not being able to find the note - // automatically. - let tag = unsafe { get_app_tag_as_sender(sender, recipient) }; + let tag = unsafe { + //@safety We assume that the sender wants for the recipient to find the tagged note, and therefore that they + // will cooperate and use the correct tag. Usage of a bad tag will result in the recipient not being able to + // find the note automatically. + get_app_tag_as_sender(sender, recipient) + }; increment_app_tagging_secret_index_as_sender(sender, recipient); // Silo the tag with contract address. // This is done by the kernel circuit to the private logs, but since the partial log will be finalized and emitted @@ -158,7 +162,11 @@ fn compute_encrypted_log( // Prepend the plaintext length as the first byte, then copy the plaintext itself starting from the second byte. // Fill the remaining bytes with random values to reach a fixed length of N. fn extend_private_log_plaintext(plaintext: [u8; P]) -> [u8; N] { - let mut padded = unsafe { get_random_bytes() }; + let mut padded = unsafe { + //@safety A malicious sender could reveal the whole contents of the encrypted log so trusting it to set + // a random padding in plaintext is fine. + get_random_bytes() + }; padded[0] = (P >> 8) as u8; padded[1] = P as u8; for i in 0..P { @@ -192,11 +200,13 @@ fn fr_to_fq(r: Field) -> Scalar { fn generate_ephemeral_key_pair() -> (Scalar, Point) { // @todo Need to draw randomness from the full domain of Fq not only Fr - // We use the randomness to preserve the privacy of both the sender and recipient via encryption, so a malicious - // sender could use non-random values to reveal the plaintext. But they already know it themselves anyway, and so - // the recipient already trusts them to not disclose this information. We can therefore assume that the sender will - // cooperate in the random value generation. - let randomness = unsafe { random() }; + let randomness = unsafe { + //@safety We use the randomness to preserve the privacy of both the sender and recipient via encryption, so + // a malicious sender could use non-random values to reveal the plaintext. But they already know it themselves + // anyway, and so the recipient already trusts them to not disclose this information. We can therefore assume + // that the sender will cooperate in the random value generation. + random() + }; // We use the unsafe version of `fr_to_fq` because multi_scalar_mul (called by derive_public_key) will constrain // the scalars. diff --git a/noir-projects/aztec-nr/aztec/src/history/note_inclusion.nr b/noir-projects/aztec-nr/aztec/src/history/note_inclusion.nr index 886801dea6f..9ba41f60543 100644 --- a/noir-projects/aztec-nr/aztec/src/history/note_inclusion.nr +++ b/noir-projects/aztec-nr/aztec/src/history/note_inclusion.nr @@ -20,6 +20,7 @@ impl ProveNoteInclusion for BlockHeader { let note_hash = compute_note_hash_for_nullify(note); let witness = unsafe { + //@safety The witness is only used as a "magical value" that makes the merkle proof below pass. Hence it's safe. get_note_hash_membership_witness(self.global_variables.block_number as u32, note_hash) }; diff --git a/noir-projects/aztec-nr/aztec/src/history/nullifier_inclusion.nr b/noir-projects/aztec-nr/aztec/src/history/nullifier_inclusion.nr index f1278658b89..7b6202f540c 100644 --- a/noir-projects/aztec-nr/aztec/src/history/nullifier_inclusion.nr +++ b/noir-projects/aztec-nr/aztec/src/history/nullifier_inclusion.nr @@ -15,6 +15,7 @@ impl ProveNullifierInclusion for BlockHeader { fn prove_nullifier_inclusion(self, nullifier: Field) { // 1) Get the membership witness of the nullifier let witness = unsafe { + //@safety The witness is only used as a "magical value" that makes the proof below pass. Hence it's safe. get_nullifier_membership_witness(self.global_variables.block_number as u32, nullifier) }; diff --git a/noir-projects/aztec-nr/aztec/src/history/nullifier_non_inclusion.nr b/noir-projects/aztec-nr/aztec/src/history/nullifier_non_inclusion.nr index e40c199ea7d..d7822307bc2 100644 --- a/noir-projects/aztec-nr/aztec/src/history/nullifier_non_inclusion.nr +++ b/noir-projects/aztec-nr/aztec/src/history/nullifier_non_inclusion.nr @@ -17,6 +17,7 @@ impl ProveNullifierNonInclusion for BlockHeader { fn prove_nullifier_non_inclusion(self, nullifier: Field) { // 1) Get the membership witness of a low nullifier of the nullifier let witness = unsafe { + //@safety The witness is only used as a "magical value" that makes the proof below pass. Hence it's safe. get_low_nullifier_membership_witness( self.global_variables.block_number as u32, nullifier, diff --git a/noir-projects/aztec-nr/aztec/src/history/public_storage.nr b/noir-projects/aztec-nr/aztec/src/history/public_storage.nr index 37cf287cde4..9dfcfb82bb7 100644 --- a/noir-projects/aztec-nr/aztec/src/history/public_storage.nr +++ b/noir-projects/aztec-nr/aztec/src/history/public_storage.nr @@ -28,6 +28,7 @@ impl PublicStorageHistoricalRead for BlockHeader { // 2) Get the membership witness for the tree index. let witness = unsafe { + //@safety The witness is only used as a "magical value" that makes the proof below pass. Hence it's safe. get_public_data_witness( self.global_variables.block_number as u32, public_data_tree_index, diff --git a/noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr b/noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr index d04a08baee2..96834b42635 100644 --- a/noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr @@ -24,8 +24,10 @@ pub unconstrained fn get_ovsk_app(ovpk_m_hash: Field) -> Field { // keys at once since the constraints for reading them all are actually fewer than if we read them one at a time - any // read keys that are not required by the caller can simply be discarded. pub fn get_public_keys(account: AztecAddress) -> PublicKeys { - // Public keys are constrained by showing their inclusion in the address's preimage. - let (public_keys, partial_address) = unsafe { get_public_keys_and_partial_address(account) }; + let (public_keys, partial_address) = unsafe { + //@safety Public keys are constrained by showing their inclusion in the address's preimage. + get_public_keys_and_partial_address(account) + }; assert_eq( account, AztecAddress::compute(public_keys, partial_address), diff --git a/noir-projects/aztec-nr/aztec/src/messaging.nr b/noir-projects/aztec-nr/aztec/src/messaging.nr index 98a4110e628..7e364b576c0 100644 --- a/noir-projects/aztec-nr/aztec/src/messaging.nr +++ b/noir-projects/aztec-nr/aztec/src/messaging.nr @@ -31,8 +31,10 @@ pub fn process_l1_to_l2_message( // 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(contract_address, message_hash, secret) }; + let (_leaf_index, sibling_path) = unsafe { + //@safety The witness is only used as a "magical value" that makes the merkle proof below pass. Hence it's safe. + get_l1_to_l2_membership_witness(contract_address, message_hash, secret) + }; 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/note/note_getter/mod.nr b/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr index c79c2e18730..e663e85dd74 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_getter/mod.nr @@ -89,10 +89,11 @@ pub fn get_note( where Note: NoteInterface + NullifiableNote, { - let note = unsafe { get_note_internal(storage_slot) }; - - // Constraining that we got a valid note from the oracle is fairly straightforward: all we need to do is check that - // the metadata is correct, and that the note exists. + let note = unsafe { + //@safety Constraining that we got a valid note from the oracle is fairly straightforward: all we need to do + // is check that the metadata is correct, and that the note exists. + get_note_internal(storage_slot) + }; check_note_header(*context, storage_slot, note); let note_hash_for_read_request = compute_note_hash_for_read_request(note); @@ -109,7 +110,10 @@ pub fn get_notes( where Note: NoteInterface + NullifiableNote + Eq, { - let opt_notes = unsafe { get_notes_internal(storage_slot, options) }; + let opt_notes = unsafe { + //@safety The notes are constrained below. + get_notes_internal(storage_slot, options) + }; // We apply the constraints in a separate function instead of inlining them here to make it easier to test that // these checks correctly reject bad notes. diff --git a/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr b/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr index 8321bccc27a..20a22c3d8ba 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/arguments.nr @@ -3,16 +3,20 @@ /// /// 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. When - // unpacking however the caller must check that the returned value is indeed the preimage. - unsafe { pack_arguments_oracle_wrapper(args) }; + unsafe { + //@safety This oracle call returns nothing: we only call it for its side effects. It is therefore always safe + // to call. + pack_arguments_oracle_wrapper(args) + }; } /// 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. When - // unpacking however the caller must check that the returned value is indeed the preimage. - unsafe { pack_arguments_array_oracle_wrapper(args) }; + unsafe { + //@safety This oracle call returns nothing: we only call it for its side effects. It is therefore always safe + // to call. + pack_arguments_array_oracle_wrapper(args) + }; } unconstrained fn pack_arguments_oracle_wrapper(args: [Field]) { diff --git a/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr b/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr index 3139b6d85a6..727e0686033 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr @@ -35,7 +35,10 @@ pub fn get_block_header_at(block_number: u32, context: PrivateContext) -> BlockH ); // 3) Get the header hint of a given block from an oracle - let header = unsafe { get_block_header_at_internal(block_number) }; + let header = unsafe { + //@safety The header is verified against the archive tree root below. + get_block_header_at_internal(block_number) + }; // 4) We make sure that the header hint we received from the oracle exists in the state tree and is the actual header // at the desired block number @@ -61,7 +64,10 @@ fn constrain_get_block_header_at_internal( let block_hash = header_hint.hash(); // 2) Get the membership witness of the block in the archive tree - let witness = unsafe { get_archive_membership_witness(last_archive_block_number, block_hash) }; + let witness = unsafe { + //@safety The witness is only used as a "magical value" that makes the merkle proof below pass. Hence it's safe. + get_archive_membership_witness(last_archive_block_number, block_hash) + }; // 3) Check that the block is in the archive (i.e. the witness is valid) assert( 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 1bf5812d8b2..70dc338e193 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 @@ -51,7 +51,11 @@ pub unconstrained fn set_public_teardown_function_call_internal( } pub fn notify_set_min_revertible_side_effect_counter(counter: u32) { - unsafe { notify_set_min_revertible_side_effect_counter_oracle_wrapper(counter) }; + unsafe { + //@safety This oracle call returns nothing: we only call it for its side effects. It is therefore always safe + // to call. + notify_set_min_revertible_side_effect_counter_oracle_wrapper(counter) + }; } pub unconstrained fn notify_set_min_revertible_side_effect_counter_oracle_wrapper(counter: u32) { diff --git a/noir-projects/aztec-nr/aztec/src/oracle/get_contract_instance.nr b/noir-projects/aztec-nr/aztec/src/oracle/get_contract_instance.nr index 14066aba8ae..5fb89e3b89a 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/get_contract_instance.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/get_contract_instance.nr @@ -18,10 +18,11 @@ unconstrained fn get_contract_instance_internal( // NOTE: this is for use in private only pub fn get_contract_instance(address: AztecAddress) -> ContractInstance { - let instance = - unsafe { ContractInstance::deserialize(get_contract_instance_internal(address)) }; - // The to_address function combines all values in the instance object to produce an address, so by checking that we - // get the expected address we validate the entire struct. + let instance = unsafe { + //@safety The to_address function combines all values in the instance object to produce an address, + // so by checking that we get the expected address we validate the entire struct. + ContractInstance::deserialize(get_contract_instance_internal(address)) + }; assert_eq(instance.to_address(), address); instance @@ -59,7 +60,10 @@ pub unconstrained fn get_contract_instance_initialization_hash_internal_avm( } pub fn get_contract_instance_deployer_avm(address: AztecAddress) -> Option { - let (member, exists) = unsafe { get_contract_instance_deployer_internal_avm(address) }; + let (member, exists) = unsafe { + //@safety AVM opcodes are constrained by the AVM itself + get_contract_instance_deployer_internal_avm(address) + }; if exists { Option::some(AztecAddress::from_field(member)) } else { @@ -67,7 +71,10 @@ pub fn get_contract_instance_deployer_avm(address: AztecAddress) -> Option Option { - let (member, exists) = unsafe { get_contract_instance_class_id_internal_avm(address) }; + let (member, exists) = unsafe { + //@safety AVM opcodes are constrained by the AVM itself + get_contract_instance_class_id_internal_avm(address) + }; if exists { Option::some(ContractClassId::from_field(member)) } else { @@ -75,8 +82,10 @@ pub fn get_contract_instance_class_id_avm(address: AztecAddress) -> Option Option { - let (member, exists) = - unsafe { get_contract_instance_initialization_hash_internal_avm(address) }; + let (member, exists) = unsafe { + //@safety AVM opcodes are constrained by the AVM itself + get_contract_instance_initialization_hash_internal_avm(address) + }; if exists { Option::some(member) } else { diff --git a/noir-projects/aztec-nr/aztec/src/oracle/notes.nr b/noir-projects/aztec-nr/aztec/src/oracle/notes.nr index e7534b14fb9..c70b4ba5b37 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/notes.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/notes.nr @@ -14,8 +14,9 @@ pub fn notify_created_note( 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 { + //@safety This oracle call returns nothing: we only call it for its side effects. It is therefore always safe + // to call. notify_created_note_oracle_wrapper( storage_slot, note_type_id, @@ -30,8 +31,11 @@ pub fn notify_created_note( /// 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) }; + unsafe { + //@safety This oracle call returns nothing: we only call it for its side effects. It is therefore always safe + // to call. + notify_nullified_note_oracle_wrapper(nullifier, note_hash, counter) + }; } unconstrained fn notify_created_note_oracle_wrapper( @@ -234,8 +238,9 @@ unconstrained fn get_indexed_tagging_secret_as_sender_oracle( /// otherwise e.g. a reverting transaction can cause the sender to accidentally skip indices and later produce notes /// that are not found by the recipient. pub fn increment_app_tagging_secret_index_as_sender(sender: AztecAddress, recipient: AztecAddress) { - // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. unsafe { + //@safety This oracle call returns nothing: we only call it for its side effects. It is therefore always safe + // to call. increment_app_tagging_secret_index_as_sender_wrapper(sender, recipient); } } @@ -256,8 +261,9 @@ unconstrained fn increment_app_tagging_secret_index_as_sender_oracle( /// Finds new notes that may have been sent to all registered accounts in PXE in the current contract and makes them available /// for later querying via the `get_notes` oracle. pub fn sync_notes() { - // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call. unsafe { + //@safety This oracle call returns nothing: we only call it for its side effects. It is therefore always safe + // to call. sync_notes_oracle_wrapper(); } } diff --git a/noir-projects/aztec-nr/aztec/src/oracle/returns.nr b/noir-projects/aztec-nr/aztec/src/oracle/returns.nr index b12009e6313..90f8d35f147 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/returns.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/returns.nr @@ -3,9 +3,11 @@ /// /// 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_oracle_wrapper(returns) }; + unsafe { + //@safety 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. + pack_returns_oracle_wrapper(returns) + }; } pub unconstrained fn pack_returns_oracle_wrapper(returns: [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 4dd7d13dd2f..67aee3c1be5 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 @@ -85,18 +85,20 @@ where // docs:end:replace pub fn initialize_or_replace(self, note: &mut Note) -> NoteEmission { - // `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. - let is_initialized = - unsafe { check_nullifier_exists(self.compute_initialization_nullifier()) }; + let is_initialized = unsafe { + //@safety `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 given 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. + check_nullifier_exists(self.compute_initialization_nullifier()) + }; if (!is_initialized) { self.initialize(note) diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable.nr b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable.nr index 5e9a2f0a334..4797458dc41 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable.nr @@ -207,6 +207,7 @@ where // prove inclusion of their hash, which is both a much smaller proof (a single slot), and also independent of // the size of T. let (value_change_hint, delay_change_hint) = unsafe { + //@safety The hints are checked against the hash below. get_public_storage_hints(address, self.storage_slot, historical_block_number) }; diff --git a/noir-projects/aztec-nr/aztec/src/utils/array/collapse.nr b/noir-projects/aztec-nr/aztec/src/utils/array/collapse.nr index 22ac88aeb33..a0991eaf800 100644 --- a/noir-projects/aztec-nr/aztec/src/utils/array/collapse.nr +++ b/noir-projects/aztec-nr/aztec/src/utils/array/collapse.nr @@ -14,7 +14,10 @@ where // proving backend. // Instead, we use an unconstrained function to produce the final collapsed array, along with some hints, and then // verify that the input and collapsed arrays are equivalent. - let (collapsed, collapsed_to_input_index_mapping) = unsafe { get_collapse_hints(input) }; + let (collapsed, collapsed_to_input_index_mapping) = unsafe { + //@safety The values get verified by the `verify_collapse_hints` function. + get_collapse_hints(input) + }; verify_collapse_hints(input, collapsed, collapsed_to_input_index_mapping); collapsed } diff --git a/noir-projects/aztec-nr/uint-note/src/uint_note.nr b/noir-projects/aztec-nr/uint-note/src/uint_note.nr index 45b1001932f..e1305a8d1e6 100644 --- a/noir-projects/aztec-nr/uint-note/src/uint_note.nr +++ b/noir-projects/aztec-nr/uint-note/src/uint_note.nr @@ -58,11 +58,13 @@ impl Eq for UintNote { impl UintNote { pub fn new(value: U128, owner: AztecAddress) -> Self { - // We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, so a - // malicious sender could use non-random values to make the note less private. But they already know the full - // note pre-image anyway, and so the recipient already trusts them to not disclose this information. We can - // therefore assume that the sender will cooperate in the random value generation. - let randomness = unsafe { random() }; + let randomness = unsafe { + //@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, + // so a malicious sender could use non-random values to make the note less private. But they already know + // the full note pre-image anyway, and so the recipient already trusts them to not disclose this + // information. We can therefore assume that the sender will cooperate in the random value generation. + random() + }; Self { value, owner, randomness, header: NoteHeader::empty() } } diff --git a/noir-projects/aztec-nr/value-note/src/value_note.nr b/noir-projects/aztec-nr/value-note/src/value_note.nr index 250a299f233..fdea35b39ed 100644 --- a/noir-projects/aztec-nr/value-note/src/value_note.nr +++ b/noir-projects/aztec-nr/value-note/src/value_note.nr @@ -58,11 +58,14 @@ impl NullifiableNote for ValueNote { impl ValueNote { pub fn new(value: Field, owner: AztecAddress) -> Self { - // We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, so a - // malicious sender could use non-random values to make the note less private. But they already know the full - // note pre-image anyway, and so the recipient already trusts them to not disclose this information. We can - // therefore assume that the sender will cooperate in the random value generation. - let randomness = unsafe { random() }; + let randomness = unsafe { + //@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, + // so a malicious sender could use non-random values to make the note less private. But they already know + // the full note pre-image anyway, and so the recipient already trusts them to not disclose this + // information. We can therefore assume that the sender will cooperate in the random value generation. + random() + }; + let header = NoteHeader::empty(); ValueNote { value, owner, randomness, header } } diff --git a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/subscription_note.nr b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/subscription_note.nr index 65f1cdbbab5..eb2c08a0b29 100644 --- a/noir-projects/noir-contracts/contracts/app_subscription_contract/src/subscription_note.nr +++ b/noir-projects/noir-contracts/contracts/app_subscription_contract/src/subscription_note.nr @@ -44,11 +44,13 @@ impl NullifiableNote for SubscriptionNote { impl SubscriptionNote { pub fn new(owner: AztecAddress, expiry_block_number: Field, remaining_txs: Field) -> Self { - // We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, so a - // malicious sender could use non-random values to make the note less private. But they already know the full - // note pre-image anyway, and so the recipient already trusts them to not disclose this information. We can - // therefore assume that the sender will cooperate in the random value generation. - let randomness = unsafe { random() }; + let randomness = unsafe { + //@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, + // so a malicious sender could use non-random values to make the note less private. But they already know + // the full note pre-image anyway, and so the recipient already trusts them to not disclose this + // information. We can therefore assume that the sender will cooperate in the random value generation. + random() + }; Self { owner, expiry_block_number, remaining_txs, randomness, header: NoteHeader::empty() } } } diff --git a/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr index 09a935f95e3..53741d88fe9 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr @@ -64,7 +64,10 @@ contract SchnorrAccount { let storage = Storage::init(context); let public_key = storage.signing_public_key.get_note(); // Load auth witness - let witness: [Field; 64] = unsafe { get_auth_witness(outer_hash) }; + let witness: [Field; 64] = unsafe { + //@safety The witness is only used as a "magical value" that makes the signature verification below pass. Hence it's safe. + get_auth_witness(outer_hash) + }; let mut signature: [u8; 64] = [0; 64]; for i in 0..64 { signature[i] = witness[i] as u8; diff --git a/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr index 02582e3e097..2a0824f0a37 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr @@ -38,7 +38,10 @@ contract SchnorrHardcodedAccount { #[contract_library_method] fn is_valid_impl(_context: &mut PrivateContext, outer_hash: Field) -> bool { // Load auth witness and format as an u8 array - let witness: [Field; 64] = unsafe { get_auth_witness(outer_hash) }; + let witness: [Field; 64] = unsafe { + //@safety The witness is only used as a "magical value" that makes the signature verification below pass. Hence it's safe. + get_auth_witness(outer_hash) + }; let mut signature: [u8; 64] = [0; 64]; for i in 0..64 { signature[i] = witness[i] as u8; diff --git a/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr index 3de21d464a1..dd0719b4b5f 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr @@ -29,7 +29,11 @@ contract SchnorrSingleKeyAccount { #[contract_library_method] fn is_valid_impl(context: &mut PrivateContext, outer_hash: Field) -> bool { - let witness = unsafe { get_auth_witness(outer_hash) }; + let witness = unsafe { + //@safety The witness is only used as a "magical value" that makes the signature verification + // in `recover_address` and the address check below pass. Hence it's safe. + get_auth_witness(outer_hash) + }; recover_address(outer_hash, witness).eq(context.this_address()) } } diff --git a/noir-projects/noir-contracts/contracts/spam_contract/src/types/token_note.nr b/noir-projects/noir-contracts/contracts/spam_contract/src/types/token_note.nr index cf83b3eabf0..468c0add9b3 100644 --- a/noir-projects/noir-contracts/contracts/spam_contract/src/types/token_note.nr +++ b/noir-projects/noir-contracts/contracts/spam_contract/src/types/token_note.nr @@ -63,11 +63,13 @@ impl Eq for TokenNote { impl OwnedNote for TokenNote { fn new(amount: U128, owner: AztecAddress) -> Self { - // We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, so a - // malicious sender could use non-random values to make the note less private. But they already know the full - // note pre-image anyway, and so the recipient already trusts them to not disclose this information. We can - // therefore assume that the sender will cooperate in the random value generation. - let randomness = unsafe { random() }; + let randomness = unsafe { + //@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, + // so a malicious sender could use non-random values to make the note less private. But they already know + // the full note pre-image anyway, and so the recipient already trusts them to not disclose this + // information. We can therefore assume that the sender will cooperate in the random value generation. + random() + }; Self { amount, owner, randomness, header: NoteHeader::empty() } } diff --git a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/token_note.nr b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/token_note.nr index 298835848f8..492ab2532ff 100644 --- a/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/token_note.nr +++ b/noir-projects/noir-contracts/contracts/token_blacklist_contract/src/types/token_note.nr @@ -61,11 +61,13 @@ impl Eq for TokenNote { impl OwnedNote for TokenNote { fn new(amount: U128, owner: AztecAddress) -> Self { - // We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, so a - // malicious sender could use non-random values to make the note less private. But they already know the full - // note pre-image anyway, and so the recipient already trusts them to not disclose this information. We can - // therefore assume that the sender will cooperate in the random value generation. - let randomness = unsafe { random() }; + let randomness = unsafe { + //@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, + // so a malicious sender could use non-random values to make the note less private. But they already know + // the full note pre-image anyway, and so the recipient already trusts them to not disclose this + // information. We can therefore assume that the sender will cooperate in the random value generation. + random() + }; Self { amount, owner, randomness, header: NoteHeader::empty() } } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr index d0e0f6f758a..d9a33aec0a1 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr @@ -450,7 +450,13 @@ contract Token { // We create a setup payload with unpopulated/zero `amount` for 'to' // TODO(#7775): Manually fetching the randomness here is not great. If we decide to include randomness in all // notes we could just inject it in macros. - let note_randomness = unsafe { random() }; + let note_randomness = unsafe { + //@safety We use the randomness to preserve the privacy of the note recipient by preventing brute-forcing, + // so a malicious sender could use non-random values to make the note less private. But they already know + // the full note pre-image anyway, and so the recipient already trusts them to not disclose this + // information. We can therefore assume that the sender will cooperate in the random value generation. + random() + }; let note_setup_payload = UintNote::setup_payload().new(to, note_randomness, to_note_slot); // We get the keys and encrypt the log of the note diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_kernel_data.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_kernel_data.nr index 5737775889e..2674df3fe6f 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_kernel_data.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_kernel_data.nr @@ -24,6 +24,8 @@ impl PrivateKernelData { self.vk_index }; let index_hint = unsafe { + //@safety find_index_hint should return an index into allowed_indices where `index == index_in_allowed_list`. + // The assertion below then verifies that the condition is met. find_index_hint(allowed_indices, |index: u32| index == index_in_allowed_list) }; assert(index_hint < N, "Invalid vk index"); 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 706139c8409..666cb6363fb 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 @@ -11,8 +11,11 @@ pub fn debug_log(msg: str) { /// 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) }; + unsafe { + //@safety This oracle call returns nothing: we only call it for its side effects. It is therefore always safe + // to call. + debug_log_oracle_wrapper(msg, args) + }; } pub unconstrained fn debug_log_oracle_wrapper( diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/proof/vk_data.nr b/noir-projects/noir-protocol-circuits/crates/types/src/proof/vk_data.nr index cede48d01a5..f0465ea44d5 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/proof/vk_data.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/proof/vk_data.nr @@ -14,8 +14,11 @@ impl VkData { pub fn validate_in_vk_tree(self, vk_tree_root: Field, allowed_indices: [u32; N]) { self.vk.check_hash(); - let index_hint = - unsafe { find_index_hint(allowed_indices, |index: u32| index == self.vk_index) }; + let index_hint = unsafe { + //@safety find_index_hint should return an index into allowed_indices where + // `index == index_in_allowed_list`. The assertion below then verifies that the condition is met. + find_index_hint(allowed_indices, |index: u32| index == self.vk_index) + }; assert(index_hint < N, "Invalid vk index"); assert_eq(allowed_indices[index_hint], self.vk_index, "Invalid vk index"); diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr index 3f3c91c3885..38244b0f6fa 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr @@ -99,12 +99,21 @@ where } // Helper function to count the number of non-empty elements in a validated array. -// Important: Only use it for validated arrays: validate_array(array) should be true. +// Important: Only use it for validated arrays where validate_array(array) returns true, +// which ensures that: +// 1. All elements before the first empty element are non-empty +// 2. All elements after and including the first empty element are empty +// 3. The array forms a contiguous sequence of non-empty elements followed by empty elements pub fn array_length(array: [T; N]) -> u32 where T: Empty + Eq, { - let length = unsafe { find_index_hint(array, |elem: T| is_empty(elem)) }; + // We get the length by checking the index of the first empty element. + let length = unsafe { + //@safety This is safe because we have validated the array (see function doc above) and the emptiness + // of the element and non-emptiness of the previous element is checked below. + find_index_hint(array, |elem: T| is_empty(elem)) + }; if length != 0 { assert(!is_empty(array[length - 1])); } @@ -240,18 +249,18 @@ fn test_array_length_invalid_arrays() { } #[test] -fn find_index_greater_than_min() { +unconstrained fn find_index_greater_than_min() { let values = [10, 20, 30, 40]; let min = 22; - let index = unsafe { find_index_hint(values, |v: Field| min.lt(v)) }; + let index = find_index_hint(values, |v: Field| min.lt(v)); assert_eq(index, 2); } #[test] -fn find_index_not_found() { +unconstrained fn find_index_not_found() { let values = [10, 20, 30, 40]; let min = 100; - let index = unsafe { find_index_hint(values, |v: Field| min.lt(v)) }; + let index = find_index_hint(values, |v: Field| min.lt(v)); assert_eq(index, 4); } @@ -259,8 +268,8 @@ fn find_index_not_found() { fn test_array_concat() { let array0 = [1, 2, 3]; let array1 = [4, 5]; - let concated = array_concat(array0, array1); - assert_eq(concated, [1, 2, 3, 4, 5]); + let concatenated = array_concat(array0, array1); + assert_eq(concatenated, [1, 2, 3, 4, 5]); } #[test] diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_combined_array.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_combined_array.nr index f2ad12fd38e..15b5326c2f1 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_combined_array.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_combined_array.nr @@ -90,9 +90,8 @@ mod tests { ); } - pub fn check_and_execute(self) { - let combined = - unsafe { combine_arrays(self.original_array_lt, self.original_array_gte) }; + pub unconstrained fn check_and_execute(self) { + let combined = combine_arrays(self.original_array_lt, self.original_array_gte); assert_eq(combined, self.combined_array); self.execute(); @@ -100,13 +99,13 @@ mod tests { } #[test] - fn assert_combined_array_empty_succeeds() { + unconstrained fn assert_combined_array_empty_succeeds() { let builder = TestBuilder::new_empty(); builder.check_and_execute(); } #[test] - fn assert_combined_array_succeeds() { + unconstrained fn assert_combined_array_succeeds() { let builder = TestBuilder::new(); builder.check_and_execute(); } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_combined_transformed_array.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_combined_transformed_array.nr index 082f33ebdd6..e96cc0ef5d7 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_combined_transformed_array.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_combined_transformed_array.nr @@ -115,14 +115,12 @@ mod tests { ); } - pub fn check_and_execute(self) { - let combined = unsafe { - combine_and_transform_arrays( - self.original_array_lt, - self.original_array_gte, - sum_two_values, - ) - }; + pub unconstrained fn check_and_execute(self) { + let combined = combine_and_transform_arrays( + self.original_array_lt, + self.original_array_gte, + sum_two_values, + ); assert_eq(combined, self.combined_array); self.execute(); @@ -130,19 +128,19 @@ mod tests { } #[test] - fn assert_combined_transformed_array_empty_succeeds() { + unconstrained fn assert_combined_transformed_array_empty_succeeds() { let builder = TestBuilder::new_empty(); builder.check_and_execute(); } #[test] - fn assert_combined_transformed_array_succeeds() { + unconstrained fn assert_combined_transformed_array_succeeds() { let builder = TestBuilder::new(); builder.check_and_execute(); } #[test(should_fail_with = "hinted item in the commbined array does not match")] - fn assert_combined_transformed_array_extra_item_fails() { + unconstrained fn assert_combined_transformed_array_extra_item_fails() { let mut builder = TestBuilder::new(); // Add random value to an empty item. @@ -152,7 +150,7 @@ mod tests { } #[test(should_fail_with = "hinted item in the commbined array does not match")] - fn assert_combined_transformed_array_missing_item_fails() { + unconstrained fn assert_combined_transformed_array_missing_item_fails() { let mut builder = TestBuilder::new(); // Clear the last item. @@ -162,7 +160,7 @@ mod tests { } #[test(should_fail_with = "hinted item in the commbined array does not match")] - fn assert_combined_transformed_array_unordered_fails() { + unconstrained fn assert_combined_transformed_array_unordered_fails() { let mut builder = TestBuilder::new(); // Swap the two items. diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_exposed_sorted_transformed_value_array/get_order_hints.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_exposed_sorted_transformed_value_array/get_order_hints.nr index 25071de486d..f06d0c488c2 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_exposed_sorted_transformed_value_array/get_order_hints.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_exposed_sorted_transformed_value_array/get_order_hints.nr @@ -98,19 +98,19 @@ mod tests { } impl TestBuilder { - pub fn asc_to_equal(self, expected: [OrderHint; N]) { - let hints = unsafe { get_order_hints_asc(self.array) }; + pub unconstrained fn asc_to_equal(self, expected: [OrderHint; N]) { + let hints = get_order_hints_asc(self.array); assert_eq(hints, expected); } - pub fn desc_to_equal(self, expected: [OrderHint; N]) { - let hints = unsafe { get_order_hints_desc(self.array) }; + pub unconstrained fn desc_to_equal(self, expected: [OrderHint; N]) { + let hints = get_order_hints_desc(self.array); assert_eq(hints, expected); } } #[test] - fn get_order_hints_asc_full_non_empty() { + unconstrained fn get_order_hints_asc_full_non_empty() { let builder = TestBuilder::new(); let expected_hints = [ OrderHint { counter: 3, sorted_index: 2 }, @@ -121,7 +121,7 @@ mod tests { } #[test] - fn get_order_hints_asc_padded_empty() { + unconstrained fn get_order_hints_asc_padded_empty() { let builder = TestBuilder::new_padded(); let expected_hints = [ OrderHint { counter: 3, sorted_index: 2 }, @@ -134,7 +134,7 @@ mod tests { } #[test] - fn get_order_hints_desc_full_non_empty() { + unconstrained fn get_order_hints_desc_full_non_empty() { let builder = TestBuilder::new(); let expected_hints = [ OrderHint { counter: 9, sorted_index: 0 }, @@ -145,7 +145,7 @@ mod tests { } #[test] - fn get_order_hints_desc_padded_empty() { + unconstrained fn get_order_hints_desc_padded_empty() { let builder = TestBuilder::new_padded(); let expected_hints = [ OrderHint { counter: 9, sorted_index: 0 }, diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_split_sorted_transformed_value_arrays/get_split_order_hints.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_split_sorted_transformed_value_arrays/get_split_order_hints.nr index 25a2d44ed9f..bd03e2a7289 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_split_sorted_transformed_value_arrays/get_split_order_hints.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_split_sorted_transformed_value_arrays/get_split_order_hints.nr @@ -131,13 +131,13 @@ mod tests { } impl TestBuilder { - pub fn asc_to_equal(self, expected: SplitOrderHints) { - let hints = unsafe { get_split_order_hints_asc(self.array, self.split_counter) }; + pub unconstrained fn asc_to_equal(self, expected: SplitOrderHints) { + let hints = get_split_order_hints_asc(self.array, self.split_counter); assert_eq(hints, expected); } - pub fn desc_to_equal(self, expected: SplitOrderHints) { - let hints = unsafe { get_split_order_hints_desc(self.array, self.split_counter) }; + pub unconstrained fn desc_to_equal(self, expected: SplitOrderHints) { + let hints = get_split_order_hints_desc(self.array, self.split_counter); assert_eq(hints, expected); } } @@ -145,7 +145,7 @@ mod tests { // asc #[test] - fn get_split_order_hints_asc_zero_split_counter_full() { + unconstrained fn get_split_order_hints_asc_zero_split_counter_full() { let builder = TestBuilder::new(); let expected_hints = SplitOrderHints { sorted_counters_lt: [0, 0, 0, 0, 0], @@ -156,7 +156,7 @@ mod tests { } #[test] - fn get_split_order_hints_asc_non_zero_split_counter_full() { + unconstrained fn get_split_order_hints_asc_non_zero_split_counter_full() { let mut builder = TestBuilder::new(); builder.split_counter = 9; @@ -169,7 +169,7 @@ mod tests { } #[test] - fn get_split_order_hints_asc_non_zero_split_counter_equal_full() { + unconstrained fn get_split_order_hints_asc_non_zero_split_counter_equal_full() { let mut builder = TestBuilder::new(); builder.split_counter = 11; @@ -182,7 +182,7 @@ mod tests { } #[test] - fn get_split_order_hints_asc_zero_split_counter_padded_empty() { + unconstrained fn get_split_order_hints_asc_zero_split_counter_padded_empty() { let builder = TestBuilder::new_padded(); let expected_hints = SplitOrderHints { @@ -194,7 +194,7 @@ mod tests { } #[test] - fn get_split_order_hints_asc_non_zero_split_counter_padded_empty() { + unconstrained fn get_split_order_hints_asc_non_zero_split_counter_padded_empty() { let mut builder = TestBuilder::new_padded(); builder.split_counter = 9; @@ -207,7 +207,7 @@ mod tests { } #[test] - fn get_split_order_hints_asc_non_zero_split_counter_equal_padded_empty() { + unconstrained fn get_split_order_hints_asc_non_zero_split_counter_equal_padded_empty() { let mut builder = TestBuilder::new_padded(); builder.split_counter = 11; @@ -222,7 +222,7 @@ mod tests { // desc #[test] - fn get_split_order_hints_desc_zero_split_counter_empty() { + unconstrained fn get_split_order_hints_desc_zero_split_counter_empty() { let builder = TestBuilder::new(); let expected_hints = SplitOrderHints { sorted_counters_lt: [0, 0, 0, 0, 0], @@ -233,7 +233,7 @@ mod tests { } #[test] - fn get_split_order_hints_desc_non_zero_split_counter_empty() { + unconstrained fn get_split_order_hints_desc_non_zero_split_counter_empty() { let mut builder = TestBuilder::new(); builder.split_counter = 9; @@ -246,7 +246,7 @@ mod tests { } #[test] - fn get_split_order_hints_desc_non_zero_split_counter_equal_empty() { + unconstrained fn get_split_order_hints_desc_non_zero_split_counter_equal_empty() { let mut builder = TestBuilder::new(); builder.split_counter = 11; @@ -259,7 +259,7 @@ mod tests { } #[test] - fn get_split_order_hints_desc_zero_split_counter_padded_empty() { + unconstrained fn get_split_order_hints_desc_zero_split_counter_padded_empty() { let builder = TestBuilder::new_padded(); let expected_hints = SplitOrderHints { sorted_counters_lt: [0, 0, 0, 0, 0, 0, 0], @@ -270,7 +270,7 @@ mod tests { } #[test] - fn get_split_order_hints_desc_non_zero_split_counter_padded_empty() { + unconstrained fn get_split_order_hints_desc_non_zero_split_counter_padded_empty() { let mut builder = TestBuilder::new_padded(); builder.split_counter = 9; @@ -283,7 +283,7 @@ mod tests { } #[test] - fn get_split_order_hints_desc_non_zero_split_counter_equal_padded_empty() { + unconstrained fn get_split_order_hints_desc_non_zero_split_counter_equal_padded_empty() { let mut builder = TestBuilder::new_padded(); builder.split_counter = 11; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_split_transformed_value_arrays.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_split_transformed_value_arrays.nr index 105ce5372b6..87b9c3ea0b2 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_split_transformed_value_arrays.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/assert_split_transformed_value_arrays.nr @@ -59,8 +59,10 @@ where T: Ordered + Empty + Eq, S: Empty + Eq, { - let num_non_revertibles = - unsafe { find_index_hint(sorted_array, |n: T| n.counter() >= split_counter) }; + let num_non_revertibles = unsafe { + //@safety The value is constrained by `assert_split_transformed_value_arrays_with_hint`. + find_index_hint(sorted_array, |n: T| n.counter() >= split_counter) + }; assert_split_transformed_value_arrays_with_hint( sorted_array, diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/get_sorted_result.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/get_sorted_result.nr index 6d0a31443b5..4bad623559d 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/get_sorted_result.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/get_sorted_result.nr @@ -24,37 +24,35 @@ pub unconstrained fn get_sorted_result( } #[test] -fn get_sorted_hints_asc_non_padded() { +unconstrained fn get_sorted_hints_asc_non_padded() { let values = [40, 60, 20, 50]; - let res = unsafe { get_sorted_result(values, |a: u32, b: u32| a < b) }; + let res = get_sorted_result(values, |a: u32, b: u32| a < b); assert_eq(res.sorted_array, [20, 40, 50, 60]); assert_eq(res.sorted_index_hints, [1, 3, 0, 2]); assert_eq(res.original_index_hints, [2, 0, 3, 1]); } #[test] -fn get_sorted_hints_desc_non_padded() { +unconstrained fn get_sorted_hints_desc_non_padded() { let values = [40, 20, 60, 50]; - let res = unsafe { get_sorted_result(values, |a: u32, b: u32| b < a) }; + let res = get_sorted_result(values, |a: u32, b: u32| b < a); assert_eq(res.sorted_array, [60, 50, 40, 20]); assert_eq(res.sorted_index_hints, [2, 3, 0, 1]); } #[test] -fn get_sorted_hints_asc_padded() { +unconstrained fn get_sorted_hints_asc_padded() { let values = [40, 60, 20, 50, 0, 0]; - let res = - unsafe { get_sorted_result(values, |a: u32, b: u32| (a != 0) & ((b == 0) | (a < b))) }; + let res = get_sorted_result(values, |a: u32, b: u32| (a != 0) & ((b == 0) | (a < b))); assert_eq(res.sorted_array, [20, 40, 50, 60, 0, 0]); assert_eq(res.sorted_index_hints, [1, 3, 0, 2, 4, 5]); assert_eq(res.original_index_hints, [2, 0, 3, 1, 4, 5]); } #[test] -fn get_sorted_hints_desc_padded() { +unconstrained fn get_sorted_hints_desc_padded() { let values = [40, 60, 20, 50, 0, 0]; - let res = - unsafe { get_sorted_result(values, |a: u32, b: u32| (a != 0) & ((b == 0) | (b < a))) }; + let res = get_sorted_result(values, |a: u32, b: u32| (a != 0) & ((b == 0) | (b < a))); assert_eq(res.sorted_array, [60, 50, 40, 20, 0, 0]); assert_eq(res.sorted_index_hints, [2, 0, 3, 1, 4, 5]); assert_eq(res.original_index_hints, [1, 3, 0, 2, 4, 5]); diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/get_sorted_tuple.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/get_sorted_tuple.nr index 569983a9344..05286963796 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/get_sorted_tuple.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/get_sorted_tuple.nr @@ -20,7 +20,7 @@ pub unconstrained fn get_sorted_tuple( } #[test] -fn get_sorted_tuple_asc() { +unconstrained fn get_sorted_tuple_asc() { let original = [3, 2, 9, 5]; let expected = [ SortedTuple { elem: 2, original_index: 1 }, @@ -28,7 +28,7 @@ fn get_sorted_tuple_asc() { SortedTuple { elem: 5, original_index: 3 }, SortedTuple { elem: 9, original_index: 2 }, ]; - let sorted = unsafe { get_sorted_tuple(original, |a: u64, b: u64| a < b) }; + let sorted = get_sorted_tuple(original, |a: u64, b: u64| a < b); for i in 0..4 { assert_eq(sorted[i].elem, expected[i].elem); assert_eq(sorted[i].original_index, expected[i].original_index); diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/sort_by.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/sort_by.nr index f8eaddc7a23..6a6c4ffe404 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/sort_by.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/sort_by.nr @@ -8,10 +8,10 @@ pub unconstrained fn sort_by( ordering: fn[Env](T, T) -> bool, ) -> [T; N] { let mut result = array; - let sorted_index = unsafe { get_sorting_index(array, ordering) }; + let sorted_index = get_sorting_index(array, ordering); // Ensure the indexes are correct for i in 0..N { - let pos = unsafe { find_index_hint(sorted_index, |index: u32| index == i) }; + let pos = find_index_hint(sorted_index, |index: u32| index == i); assert(sorted_index[pos] == i); } // Sort the array using the indexes diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/sort_by_counter.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/sort_by_counter.nr index d4cd7176d87..04b0d5ed7e3 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/sort_by_counter.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays/sort_by_counter.nr @@ -37,26 +37,26 @@ mod tests { }, }; - fn asc_values_to_equal(values: [u32; N], expected: [u32; N]) { + unconstrained fn asc_values_to_equal(values: [u32; N], expected: [u32; N]) { let items = values.map(|value| TestValue { value: value as Field, counter: value }); - let sorted = unsafe { sort_by_counter_asc(items).map(|item: TestValue| item.counter) }; + let sorted = sort_by_counter_asc(items).map(|item: TestValue| item.counter); assert_eq(sorted, expected); } - fn desc_values_to_equal(values: [u32; N], expected: [u32; N]) { + unconstrained fn desc_values_to_equal(values: [u32; N], expected: [u32; N]) { let items = values.map(|value| TestValue { value: value as Field, counter: value }); - let sorted = unsafe { sort_by_counter_desc(items).map(|item: TestValue| item.counter) }; + let sorted = sort_by_counter_desc(items).map(|item: TestValue| item.counter); assert_eq(sorted, expected); } - fn compare_test_items_asc(value_1: u32, value_2: u32) -> bool { + unconstrained fn compare_test_items_asc(value_1: u32, value_2: u32) -> bool { compare_by_counter_empty_padded_asc( TestValue { value: value_1 as Field, counter: value_1 }, TestValue { value: value_2 as Field, counter: value_2 }, ) } - fn compare_test_items_desc(value_1: u32, value_2: u32) -> bool { + unconstrained fn compare_test_items_desc(value_1: u32, value_2: u32) -> bool { compare_by_counter_empty_padded_desc( TestValue { value: value_1 as Field, counter: value_1 }, TestValue { value: value_2 as Field, counter: value_2 }, @@ -64,7 +64,7 @@ mod tests { } #[test] - fn compare_by_counter_empty_padded_asc_expected() { + unconstrained fn compare_by_counter_empty_padded_asc_expected() { assert_eq(compare_test_items_asc(1, 2), true); assert_eq(compare_test_items_asc(1, 1), false); assert_eq(compare_test_items_asc(2, 1), false); @@ -72,7 +72,7 @@ mod tests { } #[test] - fn compare_by_counter_empty_padded_desc_expected() { + unconstrained fn compare_by_counter_empty_padded_desc_expected() { assert_eq(compare_test_items_desc(1, 2), false); assert_eq(compare_test_items_desc(1, 1), true); assert_eq(compare_test_items_desc(2, 1), true); @@ -80,27 +80,27 @@ mod tests { } #[test] - fn sort_by_counter_asc_full_non_empty() { + unconstrained fn sort_by_counter_asc_full_non_empty() { asc_values_to_equal([4, 2, 1, 3, 5], [1, 2, 3, 4, 5]); } #[test] - fn sort_by_counter_desc_full_non_empty() { + unconstrained fn sort_by_counter_desc_full_non_empty() { desc_values_to_equal([4, 2, 1, 3, 5], [5, 4, 3, 2, 1]); } #[test] - fn sort_by_counter_asc_padded_empty() { + unconstrained fn sort_by_counter_asc_padded_empty() { asc_values_to_equal([4, 2, 0, 0, 1, 0, 3, 5], [1, 2, 3, 4, 5, 0, 0, 0]); } #[test] - fn sort_by_counter_desc_padded_empty() { + unconstrained fn sort_by_counter_desc_padded_empty() { desc_values_to_equal([4, 2, 0, 0, 1, 0, 3, 5], [5, 4, 3, 2, 1, 0, 0, 0]); } #[test] - fn sort_by_counter_asc_with_zero_counters() { + unconstrained fn sort_by_counter_asc_with_zero_counters() { let original = [ TestValue { value: 55, counter: 1 }, TestValue { value: 11, counter: 0 }, @@ -119,12 +119,12 @@ mod tests { TestValue::empty(), TestValue::empty(), ]; - let sorted = unsafe { sort_by_counter_asc(original) }; + let sorted = sort_by_counter_asc(original); assert_eq(sorted, expected); } #[test] - fn sort_by_counter_desc_with_zero_counters() { + unconstrained fn sort_by_counter_desc_with_zero_counters() { let original = [ TestValue { value: 55, counter: 1 }, TestValue { value: 11, counter: 0 }, @@ -143,7 +143,7 @@ mod tests { TestValue::empty(), TestValue::empty(), ]; - let sorted = unsafe { sort_by_counter_desc(original) }; + let sorted = sort_by_counter_desc(original); assert_eq(sorted, expected); } }