Skip to content

Commit

Permalink
feat!: rename unsafe_rand, misc log unsafe changes (#8844)
Browse files Browse the repository at this point in the history
I renamed `unsafe_rand` to `random` since we now have `unsafe` blocks,
and added notes on why it's safe to call in each callsite. I reworked
the encrypted event emission module a bit so that there'd be a single
callsite, which might cause conflicts with some of @benesjan's
refactors.

I also marked some log oracles as safe, though I don't really understand
what it is they do. We need to do some cleanup and document these.
  • Loading branch information
nventuro authored Oct 2, 2024
1 parent 8031ef4 commit 81a4d74
Show file tree
Hide file tree
Showing 27 changed files with 243 additions and 118 deletions.
21 changes: 17 additions & 4 deletions docs/docs/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,20 @@ Aztec is in full-speed development. Literally every version breaks compatibility

## TBD

### [Aztec.nr] Renamed `unsafe_rand` to `random`

Since this is an `unconstrained` function, callers are already supposed to include an `unsafe` block, so this function has been renamed for reduced verbosity.

```diff
-use aztec::oracle::unsafe_rand::unsafe_rand;
+use aztec::oracle::random::random;

-let random_value = unsafe { unsafe_rand() };
+let random_value = unsafe { random() };
```

### [Aztec.js] Removed `L2Block.fromFields`

`L2Block.fromFields` was a syntactic sugar which is causing [issues](https://github.com/AztecProtocol/aztec-packages/issues/8340) so we've removed it.

```diff
Expand Down Expand Up @@ -43,11 +56,11 @@ All of `TestEnvironment`'s functions are now `unconstrained`, preventing acciden

### [Aztec.nr] removed `encode_and_encrypt_note` and renamed `encode_and_encrypt_note_with_keys` to `encode_and_encrypt_note`

````diff
```diff
contract XYZ {
- use dep::aztec::encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys;
+ use dep::aztec::encrypted_logs::encrypted_note_emission::encode_and_encrypt_note;
....
...

- numbers.at(owner).initialize(&mut new_number).emit(encode_and_encrypt_note_with_keys(&mut context, owner_ovpk_m, owner_ivpk_m, owner));
+ numbers.at(owner).initialize(&mut new_number).emit(encode_and_encrypt_note(&mut context, owner_ovpk_m, owner_ivpk_m, owner));
Expand Down Expand Up @@ -190,7 +203,7 @@ export LOG_LEVEL="debug"

`is_valid_impl` method in account contract asserted if signature was true. Instead now we will return the verification to give flexibility to developers to handle it as they please.

````diff
```diff
- let verification = std::ecdsa_secp256k1::verify_signature(public_key.x, public_key.y, signature, hashed_message);
- assert(verification == true);
- true
Expand All @@ -211,7 +224,7 @@ Public keys (ivpk, ovpk, npk, tpk) should no longer be fetched using the old `ge
+let owner_keys = get_current_public_keys(&mut context, owner);
+let owner_ivpk_m = owner_keys.ivpk_m;
+let owner_ovpk_m = owner_keys.ovpk_m;
````
```

If using more than one key per account, this will result in very large circuit gate count reductions.

Expand Down
11 changes: 8 additions & 3 deletions noir-projects/aztec-nr/address-note/src/address_note.nr
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use dep::aztec::{
protocol_types::{address::AztecAddress, constants::GENERATOR_INDEX__NOTE_NULLIFIER, hash::poseidon2_hash_with_separator},
note::{note_header::NoteHeader, note_interface::NullifiableNote, utils::compute_note_hash_for_nullify},
oracle::unsafe_rand::unsafe_rand, keys::getters::get_nsk_app, context::PrivateContext,
macros::notes::note
oracle::random::random, keys::getters::get_nsk_app, context::PrivateContext, macros::notes::note
};

// docs:start:address_note_def
Expand Down Expand Up @@ -45,7 +44,13 @@ impl NullifiableNote for AddressNote {

impl AddressNote {
pub fn new(address: AztecAddress, npk_m_hash: Field) -> Self {
let randomness = unsafe_rand();
// 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()
};
AddressNote { address, npk_m_hash, randomness, header: NoteHeader::empty() }
}
// docs:end:address_note_def
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
context::PrivateContext, event::event_interface::EventInterface,
encrypted_logs::payload::compute_encrypted_log,
keys::{getters::get_ovsk_app, public_keys::{OvpkM, IvpkM}}, oracle::unsafe_rand::unsafe_rand
keys::{getters::get_ovsk_app, public_keys::{OvpkM, IvpkM}}, oracle::random::random
};
use dep::protocol_types::{address::AztecAddress, hash::sha256_to_field};

Expand Down Expand Up @@ -40,8 +40,12 @@ pub fn encode_and_encrypt_event<Event, let N: u32>(
recipient: AztecAddress
) -> fn[(&mut PrivateContext, OvpkM, IvpkM, AztecAddress)](Event) -> () where Event: EventInterface<N> {
| e: Event | {
let randomness = unsafe {
unsafe_rand()
// We use the randomness to preserve function privacy by making it non brute-forceable, so a malicious sender could
// use non-random values to reveal the plaintext. But they already know it themselves anyway, and is presumably not
// interested in disclosing this information. We can therefore assume that the sender will cooperate in the random
// value generation.
let randomness = unsafe {
random()
};
let ovsk_app: Field = context.request_ovsk_app(ovpk.hash());
let (encrypted_log, log_hash) = compute_raw_event_log(*context, e, randomness, ovsk_app, ovpk, ivpk, recipient);
Expand All @@ -56,8 +60,12 @@ pub fn encode_and_encrypt_event_unconstrained<Event, let N: u32>(
recipient: AztecAddress
) -> fn[(&mut PrivateContext, OvpkM, IvpkM, AztecAddress)](Event) -> () where Event: EventInterface<N> {
| e: Event | {
let randomness = unsafe {
unsafe_rand()
// We use the randomness to preserve function privacy by making it non brute-forceable, so a malicious sender could
// use non-random values to reveal the plaintext. But they already know it themselves anyway, and is presumably not
// interested in disclosing this information. We can therefore assume that the sender will cooperate in the random
// value generation.
let randomness = unsafe {
random()
};
let (encrypted_log, log_hash) = unsafe {
compute_raw_event_log_unconstrained(*context, e, randomness, ovpk, ivpk, recipient)
Expand Down
15 changes: 10 additions & 5 deletions noir-projects/aztec-nr/aztec/src/encrypted_logs/payload.nr
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ use std::{
};

use crate::{
oracle::unsafe_rand::unsafe_rand, utils::point::point_to_bytes,
encrypted_logs::{header::EncryptedLogHeader},
oracle::random::random, utils::point::point_to_bytes, encrypted_logs::{header::EncryptedLogHeader},
keys::{point_to_symmetric_key::point_to_symmetric_key, public_keys::{OvpkM, IvpkM}}
};

Expand Down Expand Up @@ -73,11 +72,17 @@ 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 unsafe version of `fr_to_fq` because multi_scalar_mul (called by derive_public_key) will constrain
// the scalars.

// 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 {
unsafe_rand()
random()
};

// We use the unsafe version of `fr_to_fq` because multi_scalar_mul (called by derive_public_key) will constrain
// the scalars.
let eph_sk = fr_to_fq_unsafe(randomness);
let eph_pk = derive_public_key(eph_sk);

Expand Down
92 changes: 75 additions & 17 deletions noir-projects/aztec-nr/aztec/src/oracle/logs.nr
Original file line number Diff line number Diff line change
@@ -1,13 +1,74 @@
use dep::protocol_types::address::AztecAddress;

// = 480 + 32 * N bytes
#[oracle(emitEncryptedNoteLog)]
unconstrained fn emit_encrypted_note_log_oracle<let M: u32>(_note_hash_counter: u32, _encrypted_note: [u8; M], _counter: u32) {}
// This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call.

pub fn emit_encrypted_note_log<let M: u32>(
note_hash_counter: u32,
encrypted_note: [u8; M],
counter: u32
) {
// This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call.
unsafe {
emit_encrypted_note_log_oracle_wrapper(note_hash_counter, encrypted_note, counter)
}
}

pub fn emit_encrypted_event_log<let M: u32>(
contract_address: AztecAddress,
randomness: Field,
encrypted_event: [u8; M],
counter: u32
) {
// This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call.
unsafe {
emit_encrypted_event_log_oracle_wrapper(contract_address, randomness, encrypted_event, counter)
}
}

pub fn emit_unencrypted_log_private_internal<T>(
contract_address: AztecAddress,
message: T,
counter: u32
) {
// This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call.
unsafe {
emit_unencrypted_log_private_internal_oracle_wrapper(contract_address, message, counter)
}
}

unconstrained pub fn emit_encrypted_note_log<let M: u32>(note_hash_counter: u32, encrypted_note: [u8; M], counter: u32) {
unconstrained fn emit_encrypted_note_log_oracle_wrapper<let M: u32>(note_hash_counter: u32, encrypted_note: [u8; M], counter: u32) {
emit_encrypted_note_log_oracle(note_hash_counter, encrypted_note, counter)
}

unconstrained fn emit_encrypted_event_log_oracle_wrapper<let M: u32>(
contract_address: AztecAddress,
randomness: Field,
encrypted_event: [u8; M],
counter: u32
) {
emit_encrypted_event_log_oracle(contract_address, randomness, encrypted_event, counter)
}

unconstrained fn emit_unencrypted_log_private_internal_oracle_wrapper<T>(contract_address: AztecAddress, message: T, counter: u32) {
let _ = emit_unencrypted_log_oracle_private(contract_address, message, counter);
}

unconstrained pub fn emit_contract_class_unencrypted_log_private_internal<let N: u32>(
contract_address: AztecAddress,
message: [Field; N],
counter: u32
) -> Field {
emit_contract_class_unencrypted_log_private(contract_address, message, counter)
}

// = 480 + 32 * N bytes
#[oracle(emitEncryptedNoteLog)]
unconstrained fn emit_encrypted_note_log_oracle<let M: u32>(
_note_hash_counter: u32,
_encrypted_note: [u8; M],
_counter: u32
) {}

#[oracle(emitEncryptedEventLog)]
unconstrained fn emit_encrypted_event_log_oracle<let M: u32>(
_contract_address: AztecAddress,
Expand All @@ -16,19 +77,16 @@ unconstrained fn emit_encrypted_event_log_oracle<let M: u32>(
_counter: u32
) {}

unconstrained pub fn emit_encrypted_event_log<let M: u32>(contract_address: AztecAddress, randomness: Field, encrypted_event: [u8; M], counter: u32) {
emit_encrypted_event_log_oracle(contract_address, randomness, encrypted_event, counter)
}
#[oracle(emitUnencryptedLog)]
unconstrained fn emit_unencrypted_log_oracle_private<T>(_contract_address: AztecAddress, _message: T, _counter: u32) -> Field {}

unconstrained pub fn emit_unencrypted_log_private_internal<T>(contract_address: AztecAddress, message: T, counter: u32) -> Field {
emit_unencrypted_log_oracle_private(contract_address, message, counter)
}
unconstrained fn emit_unencrypted_log_oracle_private<T>(
_contract_address: AztecAddress,
_message: T,
_counter: u32
) -> Field {}

#[oracle(emitContractClassUnencryptedLog)]
unconstrained fn emit_contract_class_unencrypted_log_private<let N: u32>(contract_address: AztecAddress, message: [Field; N], counter: u32) -> Field {}

unconstrained pub fn emit_contract_class_unencrypted_log_private_internal<let N: u32>(contract_address: AztecAddress, message: [Field; N], counter: u32) -> Field {
emit_contract_class_unencrypted_log_private(contract_address, message, counter)
}
unconstrained fn emit_contract_class_unencrypted_log_private<let N: u32>(
contract_address: AztecAddress,
message: [Field; N],
counter: u32
) -> Field {}
2 changes: 1 addition & 1 deletion noir-projects/aztec-nr/aztec/src/oracle/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ mod get_membership_witness;
mod keys;
mod key_validation_request;
mod get_sibling_path;
mod unsafe_rand;
mod random;
mod enqueue_public_function_call;
mod header;
mod notes;
Expand Down
10 changes: 10 additions & 0 deletions noir-projects/aztec-nr/aztec/src/oracle/random.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// Returns an unconstrained random value. Note that it is not possible to constrain this value to prove that it is
/// truly random: we assume that the oracle is cooperating and returning random values.
/// In some applications this behavior might not be acceptable and other techniques might be more suitable, such as
/// producing pseudo-random values by hashing values outside of user control (like block hashes) or secrets.
unconstrained pub fn random() -> Field {
rand_oracle()
}

#[oracle(getRandomField)]
unconstrained fn rand_oracle() -> Field {}
8 changes: 0 additions & 8 deletions noir-projects/aztec-nr/aztec/src/oracle/unsafe_rand.nr

This file was deleted.

10 changes: 8 additions & 2 deletions noir-projects/aztec-nr/value-note/src/value_note.nr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use dep::aztec::{
protocol_types::{traits::Serialize, constants::GENERATOR_INDEX__NOTE_NULLIFIER, hash::poseidon2_hash_with_separator},
macros::notes::note,
note::{note_header::NoteHeader, note_interface::NullifiableNote, utils::compute_note_hash_for_nullify},
oracle::unsafe_rand::unsafe_rand, keys::getters::get_nsk_app, context::PrivateContext
oracle::random::random, keys::getters::get_nsk_app, context::PrivateContext
};

global VALUE_NOTE_LEN: u32 = 3; // 3 plus a header.
Expand Down Expand Up @@ -51,7 +51,13 @@ impl NullifiableNote for ValueNote {

impl ValueNote {
pub fn new(value: Field, npk_m_hash: Field) -> Self {
let randomness = unsafe_rand();
// 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 header = NoteHeader::empty();
ValueNote { value, npk_m_hash, randomness, header }
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use dep::aztec::{
hash::poseidon2_hash_with_separator, note::utils::compute_note_hash_for_nullify,
keys::getters::get_nsk_app, oracle::unsafe_rand::unsafe_rand,
keys::getters::get_nsk_app, oracle::random::random,
prelude::{PrivateContext, NoteHeader, NullifiableNote},
protocol_types::constants::GENERATOR_INDEX__NOTE_NULLIFIER, macros::notes::note
};
Expand Down Expand Up @@ -42,8 +42,12 @@ impl NullifiableNote for SubscriptionNote {

impl SubscriptionNote {
pub fn new(npk_m_hash: Field, 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 {
unsafe_rand()
random()
};
Self { npk_m_hash, expiry_block_number, remaining_txs, randomness, header: NoteHeader::empty() }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,6 @@ contract ContractInstanceDeployer {
let side_effect = LogHash { value: log_hash, counter, length: len };
context.unencrypted_logs_hashes.push(side_effect);

let _void = emit_unencrypted_log_private_internal(contract_address, payload, counter);
emit_unencrypted_log_private_internal(contract_address, payload, counter);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::test::utils;
use dep::aztec::test::helpers::cheatcodes;
use aztec::oracle::unsafe_rand::unsafe_rand;
use aztec::oracle::random::random;
use dep::authwit::cheatcodes as authwit_cheatcodes;
use crate::NFT;

Expand Down Expand Up @@ -76,7 +76,7 @@ unconstrained fn transfer_in_private_failure_on_behalf_of_self_non_zero_nonce()
let (env, nft_contract_address, sender, recipient) = utils::setup(/* with_account_contracts */ false);

// We set random value for the token_id as the nonce check is before we use the value.
let token_id = unsafe_rand();
let token_id = random();

// Try transferring the NFT
env.call_private_void(NFT::at(nft_contract_address).transfer_in_private(sender, recipient, token_id, 1));
Expand All @@ -90,7 +90,7 @@ unconstrained fn transfer_in_private_failure_on_behalf_of_other_without_approval
let (env, nft_contract_address, sender, recipient) = utils::setup(/* with_account_contracts */ true);

// We set random value for the token_id as the nonce check is before we use the value.
let token_id = unsafe_rand();
let token_id = random();

// Impersonate recipient to perform the call
env.impersonate(recipient);
Expand All @@ -106,7 +106,7 @@ unconstrained fn transfer_in_private_failure_on_behalf_of_other_wrong_caller() {
let (env, nft_contract_address, sender, recipient) = utils::setup(/* with_account_contracts */ true);

// We set random value for the token_id as the nonce check is before we use the value.
let token_id = unsafe_rand();
let token_id = random();

let transfer_in_private_from_call_interface = NFT::at(nft_contract_address).transfer_in_private(sender, recipient, token_id, 1);
authwit_cheatcodes::add_private_authwit_from_call_interface(sender, sender, transfer_in_private_from_call_interface);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::test::utils;
use dep::aztec::oracle::unsafe_rand::unsafe_rand;
use dep::aztec::oracle::random::random;
use dep::authwit::cheatcodes as authwit_cheatcodes;
use crate::NFT;

Expand Down Expand Up @@ -50,10 +50,10 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_self_non_zero_nonce() {
let (env, nft_contract_address, sender, recipient) = utils::setup(/* with_account_contracts */ false);

// We set random value for the token_id as the nonce check is before we use the value.
let token_id = unsafe_rand();
let token_id = random();

// Try to transfer the NFT
env.call_public(NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, unsafe_rand()));
env.call_public(NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, random()));
}

#[test(should_fail_with="invalid owner")]
Expand Down
Loading

0 comments on commit 81a4d74

Please sign in to comment.