Skip to content

Commit

Permalink
feat: removing redundant key fetching (#8043)
Browse files Browse the repository at this point in the history
Fixes #7954

I replaced the use of `encode_and_encrypt_note` with
`encode_and_encrypt_note_with_keys` in most of the places as it allowed
for reusing the obtained keys. Note that there is only 1 legimate place
remaining where it made sense to keep on using `encode_and_encrypt_note`
and that is
[here](https://github.com/AztecProtocol/aztec-packages/blob/34ae51df5d45973deb3408075a50070c781f7a48/noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr#L47).
All the other places are either test contracts or the token blacklist
contract which is very outdated by now and hence it didn't seem to be
worth it to update it.

Given this I think we should nuke `encode_and_encrypt_note` to keep the
API simpler and to make devs write efficient code. Does the reviewer
agree? (possibly also `encode_and_encrypt_event`)

Token::transfer(...) gates before were 49296 and after 38903. Diff of
**10393 gates**.
  • Loading branch information
benesjan authored Aug 20, 2024
1 parent 4c568b0 commit 2bbcc7b
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 70 deletions.
30 changes: 23 additions & 7 deletions noir-projects/aztec-nr/easy-private-state/src/easy_private_uint.nr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use dep::aztec::{
context::PrivateContext, protocol_types::{address::AztecAddress},
note::note_getter_options::NoteGetterOptions, state_vars::PrivateSet,
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys,
keys::getters::get_current_public_keys
};
use dep::value_note::{filter::filter_notes_min_sum, value_note::ValueNote};
Expand All @@ -24,19 +24,28 @@ impl<Context> EasyPrivateUint<Context> {
impl<Context> EasyPrivateUint<&mut PrivateContext> {
// Very similar to `value_note::utils::increment`.
pub fn add(self, addend: u64, owner: AztecAddress, outgoing_viewer: AztecAddress) {
let owner_npk_m_hash = get_current_public_keys(self.context, owner).npk_m.hash();
let owner_keys = get_current_public_keys(self.context, owner);
let outgoing_viewer_keys = get_current_public_keys(self.context, outgoing_viewer);
// Creates new note for the owner.
let mut addend_note = ValueNote::new(addend as Field, owner_npk_m_hash);
let mut addend_note = ValueNote::new(addend as Field, owner_keys.npk_m.hash());

// Insert the new note to the owner's set of notes.
// docs:start:insert
self.set.insert(&mut addend_note).emit(encode_and_encrypt_note(self.context, outgoing_viewer, owner));
self.set.insert(&mut addend_note).emit(
encode_and_encrypt_note_with_keys(
self.context,
outgoing_viewer_keys.ovpk_m,
owner_keys.ivpk_m,
owner
)
);
// docs:end:insert
}

// Very similar to `value_note::utils::decrement`.
pub fn sub(self, subtrahend: u64, owner: AztecAddress, outgoing_viewer: AztecAddress) {
let owner_npk_m_hash = get_current_public_keys(self.context, owner).npk_m.hash();
let owner_keys = get_current_public_keys(self.context, owner);
let outgoing_viewer_keys = get_current_public_keys(self.context, outgoing_viewer);

// docs:start:pop_notes
let options = NoteGetterOptions::with_filter(filter_notes_min_sum, subtrahend as Field);
Expand All @@ -56,7 +65,14 @@ impl<Context> EasyPrivateUint<&mut PrivateContext> {

// Creates change note for the owner.
let result_value = minuend - subtrahend;
let mut result_note = ValueNote::new(result_value as Field, owner_npk_m_hash);
self.set.insert(&mut result_note).emit(encode_and_encrypt_note(self.context, outgoing_viewer, owner));
let mut result_note = ValueNote::new(result_value as Field, owner_keys.npk_m.hash());
self.set.insert(&mut result_note).emit(
encode_and_encrypt_note_with_keys(
self.context,
outgoing_viewer_keys.ovpk_m,
owner_keys.ivpk_m,
owner
)
);
}
}
16 changes: 12 additions & 4 deletions noir-projects/aztec-nr/value-note/src/utils.nr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use dep::aztec::prelude::{AztecAddress, PrivateContext, PrivateSet, NoteGetterOptions};
use dep::aztec::note::note_getter_options::SortOrder;
use dep::aztec::encrypted_logs::encrypted_note_emission::encode_and_encrypt_note;
use dep::aztec::encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys;
use dep::aztec::keys::getters::get_current_public_keys;
use crate::{filter::filter_notes_min_sum, value_note::{ValueNote, VALUE_NOTE_LEN, VALUE_NOTE_BYTES_LEN}};

Expand All @@ -19,11 +19,19 @@ pub fn increment(
recipient: AztecAddress,
outgoing_viewer: AztecAddress // docs:end:increment_args
) {
let recipient_npk_m_hash = get_current_public_keys(balance.context, recipient).npk_m.hash();
let recipient_keys = get_current_public_keys(balance.context, recipient);
let outgoing_viewer_ovpk_m = get_current_public_keys(balance.context, outgoing_viewer).ovpk_m;

let mut note = ValueNote::new(amount, recipient_npk_m_hash);
let mut note = ValueNote::new(amount, recipient_keys.npk_m.hash());
// Insert the new note to the owner's set of notes and emit the log if value is non-zero.
balance.insert(&mut note).emit(encode_and_encrypt_note(balance.context, outgoing_viewer, recipient));
balance.insert(&mut note).emit(
encode_and_encrypt_note_with_keys(
balance.context,
outgoing_viewer_ovpk_m,
recipient_keys.ivpk_m,
recipient
)
);
}

// Find some of the `owner`'s notes whose values add up to the `amount`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ contract AppSubscription {
AztecAddress, FunctionSelector, PrivateContext, NoteHeader, Map, PrivateMutable, PublicMutable,
SharedImmutable
},
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
encrypted_logs::encrypted_note_emission::{encode_and_encrypt_note, encode_and_encrypt_note_with_keys},
keys::getters::get_current_public_keys, protocol_types::constants::MAX_FIELD_VALUE
};
use authwit::{auth_witness::get_auth_witness, auth::assert_current_call_valid_authwit};
Expand Down Expand Up @@ -92,12 +92,7 @@ contract AppSubscription {
}

#[aztec(private)]
fn subscribe(
subscriber_address: AztecAddress,
nonce: Field,
expiry_block_number: Field,
tx_count: Field
) {
fn subscribe(subscriber: AztecAddress, nonce: Field, expiry_block_number: Field, tx_count: Field) {
assert(tx_count as u64 <= SUBSCRIPTION_TXS as u64);

Token::at(storage.subscription_token_address.read_private()).transfer_from(
Expand All @@ -109,13 +104,21 @@ contract AppSubscription {

// Assert that the given expiry_block_number < current_block_number + SUBSCRIPTION_DURATION_IN_BLOCKS.
AppSubscription::at(context.this_address()).assert_block_number(expiry_block_number).enqueue_view(&mut context);
let subscriber_npk_m_hash = get_current_public_keys(&mut context, subscriber_address).npk_m.hash();

let mut subscription_note = SubscriptionNote::new(subscriber_npk_m_hash, expiry_block_number, tx_count);
storage.subscriptions.at(subscriber_address).initialize_or_replace(&mut subscription_note).emit(encode_and_encrypt_note(&mut context, context.msg_sender(), subscriber_address));
let subscriber_keys = get_current_public_keys(&mut context, subscriber);
let msg_sender_ovpk_m = get_current_public_keys(&mut context, context.msg_sender()).ovpk_m;

let mut subscription_note = SubscriptionNote::new(subscriber_keys.npk_m.hash(), expiry_block_number, tx_count);
storage.subscriptions.at(subscriber).initialize_or_replace(&mut subscription_note).emit(
encode_and_encrypt_note_with_keys(
&mut context,
msg_sender_ovpk_m,
subscriber_keys.ivpk_m,
subscriber
)
);
}

unconstrained fn is_initialized(subscriber_address: AztecAddress) -> pub bool {
storage.subscriptions.at(subscriber_address).is_initialized()
unconstrained fn is_initialized(subscriber: AztecAddress) -> pub bool {
storage.subscriptions.at(subscriber).is_initialized()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ contract Child {
use dep::aztec::{
context::gas::GasOpts, protocol_types::{abis::call_context::CallContext},
note::{note_getter_options::NoteGetterOptions, note_header::NoteHeader},
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys,
keys::getters::get_current_public_keys
};
use dep::value_note::value_note::ValueNote;
Expand Down Expand Up @@ -52,10 +52,10 @@ contract Child {

#[aztec(private)]
fn private_set_value(new_value: Field, owner: AztecAddress) -> Field {
let owner_npk_m_hash = get_current_public_keys(&mut context, owner).npk_m.hash();
let owner_keys = get_current_public_keys(&mut context, owner);

let mut note = ValueNote::new(new_value, owner_npk_m_hash);
storage.a_map_with_private_values.at(owner).insert(&mut note).emit(encode_and_encrypt_note(&mut context, owner, owner));
let mut note = ValueNote::new(new_value, owner_keys.npk_m.hash());
storage.a_map_with_private_values.at(owner).insert(&mut note).emit(encode_and_encrypt_note_with_keys(&mut context, owner_keys.ovpk_m, owner_keys.ivpk_m, owner));
new_value
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ contract Crowdfunding {
// docs:start:all-deps
use dep::aztec::{
protocol_types::address::AztecAddress,
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys,
keys::getters::get_current_public_keys,
state_vars::{PrivateSet, PublicImmutable, SharedImmutable}
};
Expand Down Expand Up @@ -79,11 +79,11 @@ contract Crowdfunding {

// 3) Create a value note for the donor so that he can later on claim a rewards token in the Claim
// contract by proving that the hash of this note exists in the note hash tree.
let donor_npk_m_hash = get_current_public_keys(&mut context, donor).npk_m.hash();
let donor_keys = get_current_public_keys(&mut context, donor);
// docs:start:valuenote_new
let mut note = ValueNote::new(amount as Field, donor_npk_m_hash);
let mut note = ValueNote::new(amount as Field, donor_keys.npk_m.hash());
// docs:end:valuenote_new
storage.donation_receipts.insert(&mut note).emit(encode_and_encrypt_note(&mut context, donor, donor));
storage.donation_receipts.insert(&mut note).emit(encode_and_encrypt_note_with_keys(&mut context, donor_keys.ovpk_m, donor_keys.ivpk_m, donor));
}
// docs:end:donate

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
contract EcdsaKAccount {
use dep::aztec::prelude::{AztecAddress, FunctionSelector, NoteHeader, NoteGetterOptions, PrivateContext, PrivateImmutable};
use dep::aztec::{
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys,
keys::getters::get_current_public_keys
};

Expand All @@ -27,13 +27,13 @@ contract EcdsaKAccount {
#[aztec(initializer)]
fn constructor(signing_pub_key_x: [u8; 32], signing_pub_key_y: [u8; 32]) {
let this = context.this_address();
let this_npk_m_hash = get_current_public_keys(&mut context, this).npk_m.hash();
let this_keys = get_current_public_keys(&mut context, this);
// Not emitting outgoing for msg_sender here to not have to register keys for the contract through which we
// deploy this (typically MultiCallEntrypoint). I think it's ok here as I feel the outgoing here is not that
// important.

let mut pub_key_note = EcdsaPublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this_npk_m_hash);
storage.public_key.initialize(&mut pub_key_note).emit(encode_and_encrypt_note(&mut context, this, this));
let mut pub_key_note = EcdsaPublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this_keys.npk_m.hash());
storage.public_key.initialize(&mut pub_key_note).emit(encode_and_encrypt_note_with_keys(&mut context, this_keys.ovpk_m, this_keys.ivpk_m, this));
}

// Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
contract EcdsaRAccount {
use dep::aztec::prelude::{AztecAddress, FunctionSelector, NoteHeader, NoteGetterOptions, PrivateContext, PrivateImmutable};
use dep::aztec::{
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys,
keys::getters::get_current_public_keys
};

Expand All @@ -26,13 +26,13 @@ contract EcdsaRAccount {
#[aztec(initializer)]
fn constructor(signing_pub_key_x: [u8; 32], signing_pub_key_y: [u8; 32]) {
let this = context.this_address();
let this_npk_m_hash = get_current_public_keys(&mut context, this).npk_m.hash();
let this_keys = get_current_public_keys(&mut context, this);
// Not emitting outgoing for msg_sender here to not have to register keys for the contract through which we
// deploy this (typically MultiCallEntrypoint). I think it's ok here as I feel the outgoing here is not that
// important.

let mut pub_key_note = EcdsaPublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this_npk_m_hash);
storage.public_key.initialize(&mut pub_key_note).emit(encode_and_encrypt_note(&mut context, this, this));
let mut pub_key_note = EcdsaPublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this_keys.npk_m.hash());
storage.public_key.initialize(&mut pub_key_note).emit(encode_and_encrypt_note_with_keys(&mut context, this_keys.ovpk_m, this_keys.ivpk_m, this));
}

// Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
contract Escrow {
use dep::aztec::prelude::{AztecAddress, EthAddress, FunctionSelector, NoteHeader, PrivateContext, PrivateImmutable};
use dep::aztec::{
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys,
keys::getters::get_current_public_keys
};

Expand All @@ -20,11 +20,14 @@ contract Escrow {
#[aztec(private)]
#[aztec(initializer)]
fn constructor(owner: AztecAddress) {
let owner_npk_m_hash = get_current_public_keys(&mut context, owner).npk_m.hash();
let owner_keys = get_current_public_keys(&mut context, owner);
let msg_sender_keys = get_current_public_keys(&mut context, context.msg_sender());
// docs:start:addressnote_new
let mut note = AddressNote::new(owner, owner_npk_m_hash);
let mut note = AddressNote::new(owner, owner_keys.npk_m.hash());
// docs:end:addressnote_new
storage.owner.initialize(&mut note).emit(encode_and_encrypt_note(&mut context, context.msg_sender(), owner));
storage.owner.initialize(&mut note).emit(
encode_and_encrypt_note_with_keys(&mut context, msg_sender_keys.ovpk_m, owner_keys.ivpk_m, owner)
);
}

// Withdraws balance. Requires that msg.sender is the owner.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ contract PendingNoteHashes {

assert(notes.len() == 0);

let header = context.get_header();
let owner_npk_m_hash = get_current_public_keys(&mut context, owner).npk_m.hash();

// Insert note
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ contract SchnorrAccount {
use dep::std;

use dep::aztec::prelude::{AztecAddress, FunctionSelector, NoteHeader, PrivateContext, PrivateImmutable};
use dep::aztec::encrypted_logs::encrypted_note_emission::encode_and_encrypt_note;
use dep::aztec::encrypted_logs::encrypted_note_emission::encode_and_encrypt_note_with_keys;
use dep::authwit::{
entrypoint::{app::AppPayload, fee::FeePayload}, account::AccountActions,
auth_witness::get_auth_witness, auth::{compute_authwit_nullifier, compute_authwit_message_hash}
Expand All @@ -28,14 +28,14 @@ contract SchnorrAccount {
#[aztec(initializer)]
fn constructor(signing_pub_key_x: Field, signing_pub_key_y: Field) {
let this = context.this_address();
let this_npk_m_hash = get_current_public_keys(&mut context, this).npk_m.hash();
let this_keys = get_current_public_keys(&mut context, this);
// Not emitting outgoing for msg_sender here to not have to register keys for the contract through which we
// deploy this (typically MultiCallEntrypoint). I think it's ok here as I feel the outgoing here is not that
// important.

// docs:start:initialize
let mut pub_key_note = PublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this_npk_m_hash);
storage.signing_public_key.initialize(&mut pub_key_note).emit(encode_and_encrypt_note(&mut context, this, this));
let mut pub_key_note = PublicKeyNote::new(signing_pub_key_x, signing_pub_key_y, this_keys.npk_m.hash());
storage.signing_public_key.initialize(&mut pub_key_note).emit(encode_and_encrypt_note_with_keys(&mut context, this_keys.ovpk_m, this_keys.ivpk_m, this));
// docs:end:initialize
}

Expand Down
Loading

0 comments on commit 2bbcc7b

Please sign in to comment.