Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: destroy_note(...) optimization #7103

Merged
merged 4 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions docs/docs/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,62 @@ keywords: [sandbox, aztec, notes, migration, updating, upgrading]

Aztec is in full-speed development. Literally every version breaks compatibility with the previous ones. This page attempts to target errors and difficulties you might encounter when upgrading, and how to resolve them.

## TBD

### [Aztec.nr] changes to `NoteInterface`
`compute_nullifier` function was renamed to `compute_note_hash_and_nullifier` and now the function has to return not only the nullifier but also the note hash used to compute the nullifier.
The same change was done to `compute_nullifier_without_context` function.
These changes were done because having the note hash exposed allowed us to not having to re-compute it again in `destroy_note` function of Aztec.nr which led to significant decrease in gate counts (see the [optimization PR](https://github.com/AztecProtocol/aztec-packages/pull/7103) for more details).


```diff
- impl NoteInterface<VALUE_NOTE_LEN, VALUE_NOTE_BYTES_LEN> for ValueNote {
- fn compute_nullifier(self, context: &mut PrivateContext) -> Field {
- let note_hash_for_nullify = compute_note_hash_for_consumption(self);
- let secret = context.request_nsk_app(self.npk_m_hash);
- poseidon2_hash([
- note_hash_for_nullify,
- secret,
- GENERATOR_INDEX__NOTE_NULLIFIER as Field,
- ])
- }
-
- fn compute_nullifier_without_context(self) -> Field {
- let note_hash_for_nullify = compute_note_hash_for_consumption(self);
- let secret = get_nsk_app(self.npk_m_hash);
- poseidon2_hash([
- note_hash_for_nullify,
- secret,
- GENERATOR_INDEX__NOTE_NULLIFIER as Field,
- ])
- }
- }
+ impl NoteInterface<VALUE_NOTE_LEN, VALUE_NOTE_BYTES_LEN> for ValueNote {
+ fn compute_note_hash_and_nullifier(self, context: &mut PrivateContext) -> (Field, Field) {
+ let note_hash_for_nullify = compute_note_hash_for_consumption(self);
+ let secret = context.request_nsk_app(self.npk_m_hash);
+ let nullifier = poseidon2_hash([
+ note_hash_for_nullify,
+ secret,
+ GENERATOR_INDEX__NOTE_NULLIFIER as Field,
+ ]);
+ (note_hash_for_nullify, nullifier)
+ }
+
+ fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field) {
+ let note_hash_for_nullify = compute_note_hash_for_consumption(self);
+ let secret = get_nsk_app(self.npk_m_hash);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On this. Since there are now a context for unconstrained, could we not get rid of this separate flow by updating it slightly? That way we don't need compute_note_hash_and_nullifier_without_context

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Created an issue for this and added it to the Aztec.nr tracking one.

+ let nullifier = poseidon2_hash([
+ note_hash_for_nullify,
+ secret,
+ GENERATOR_INDEX__NOTE_NULLIFIER as Field,
+ ]);
+ (note_hash_for_nullify, nullifier)
+ }
+ }
```


## 0.43.0

### [Aztec.nr] break `token.transfer()` into `transfer` and `transferFrom`
Expand Down
14 changes: 8 additions & 6 deletions noir-projects/aztec-nr/address-note/src/address_note.nr
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,26 @@ struct AddressNote {

impl NoteInterface<ADDRESS_NOTE_LEN, ADDRESS_NOTE_BYTES_LEN> for AddressNote {

fn compute_nullifier(self, context: &mut PrivateContext) -> Field {
fn compute_note_hash_and_nullifier(self, context: &mut PrivateContext) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = context.request_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}

fn compute_nullifier_without_context(self) -> Field {
fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned earlier. Think we should be able to get rid of this one.

let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = get_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}
}

Expand Down
2 changes: 2 additions & 0 deletions noir-projects/aztec-nr/aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ impl PrivateContext {
self.new_note_hashes.push(NoteHash { value: note_hash, counter: self.next_counter() });
}

// TODO(#7112): This function is called with non-zero note hash only in 1 of 25 cases in aztec-packages repo
// - consider creating a separate function with 1 arg for the zero note hash case.
fn push_new_nullifier(&mut self, nullifier: Field, nullified_note_hash: Field) {
self.new_nullifiers.push(Nullifier { value: nullifier, note_hash: nullified_note_hash, counter: self.next_counter() });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ mod test {

fn set_header(&mut self, header: NoteHeader) {self.header = header; }

fn compute_nullifier(self, context: &mut PrivateContext) -> Field {1}
fn compute_note_hash_and_nullifier(self, context: &mut PrivateContext) -> (Field, Field) {
(1, 1)
}

fn compute_nullifier_without_context(self) -> Field {1}
fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field) {(1,1)}

fn serialize_content(self) -> [Field; ADDRESS_NOTE_LEN] { [self.address.to_field(), self.owner.to_field(), self.randomness]}

Expand Down
32 changes: 15 additions & 17 deletions noir-projects/aztec-nr/aztec/src/note/lifecycle.nr
Original file line number Diff line number Diff line change
Expand Up @@ -57,25 +57,23 @@ pub fn destroy_note<Note, N, M>(
context: &mut PrivateContext,
note: Note
) where Note: NoteInterface<N, M> {
let mut nullifier = 0;
let mut consumed_note_hash: Field = 0;
nullifier = note.compute_nullifier(context);
let (note_hash, nullifier) = note.compute_note_hash_and_nullifier(context);

// We also need the note hash corresponding to the "nullifier"
let header = note.get_header();

// A non-zero note hash counter implies that we're nullifying a transient note (i.e. one that has not yet been
// persisted in the trees and is instead if the pending new commitments array). In such a case we compute its hash
// to inform the kernel which note we're nullifyng so that it can find it and squash both the note and the
// nullifier. This value is unused when nullifying non transient notes - in that case the kernel simply persists
// the nullifier in the tree.
if (header.note_hash_counter != 0) {
// TODO(1718): Can we reuse the note hash computed in `compute_nullifier`?
consumed_note_hash = compute_note_hash_for_consumption(note);
}
let note_hash_counter = note.get_header().note_hash_counter;
let note_hash_for_consumption = if (note_hash_counter == 0) {
// Counter is zero, so we're nullifying a non-transient note and we don't populate the note_hash with real
// value (if we did so the `notifyNullifiedNote` oracle would throw).
0
} else {
// A non-zero note hash counter implies that we're nullifying a transient note (i.e. one that has not yet been
// persisted in the trees and is instead in the pending new note hashes array). In such a case we populate its
// hash with real value to inform the kernel which note we're nullifyng so that it can find it and squash both
// the note and the nullifier.
note_hash
};

let nullifier_counter = context.side_effect_counter;
assert(notify_nullified_note(nullifier, consumed_note_hash, nullifier_counter) == 0);
assert(notify_nullified_note(nullifier, note_hash_for_consumption, nullifier_counter) == 0);

context.push_new_nullifier(nullifier, consumed_note_hash)
context.push_new_nullifier(nullifier, note_hash_for_consumption)
}
4 changes: 2 additions & 2 deletions noir-projects/aztec-nr/aztec/src/note/note_interface.nr
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use dep::protocol_types::grumpkin_point::GrumpkinPoint;

// docs:start:note_interface
trait NoteInterface<N, M> {
fn compute_nullifier(self, context: &mut PrivateContext) -> Field;
fn compute_note_hash_and_nullifier(self, context: &mut PrivateContext) -> (Field, Field);

fn compute_nullifier_without_context(self) -> Field;
fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field);

// Autogenerated by the #[aztec(note)] macro unless it is overridden by a custom implementation
fn serialize_content(self) -> [Field; N];
Expand Down
5 changes: 3 additions & 2 deletions noir-projects/aztec-nr/aztec/src/note/utils.nr
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub fn compute_siloed_nullifier<Note, N, M>(
context: &mut PrivateContext
) -> Field where Note: NoteInterface<N, M> {
let header = note_with_header.get_header();
let inner_nullifier = note_with_header.compute_nullifier(context);
let (_, inner_nullifier) = note_with_header.compute_note_hash_and_nullifier(context);

let input = [header.contract_address.to_field(), inner_nullifier];
pedersen_hash(input, GENERATOR_INDEX__OUTER_NULLIFIER)
Expand Down Expand Up @@ -128,7 +128,8 @@ pub fn compute_note_hash_and_optionally_a_nullifier<T, N, M, S>(
let siloed_note_hash = compute_siloed_hash(note_header.contract_address, unique_note_hash);

let inner_nullifier = if compute_nullifier {
note.compute_nullifier_without_context()
let (_, nullifier) = note.compute_note_hash_and_nullifier_without_context();
nullifier
} else {
0
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ impl<Note, Context> PrivateMutable<Note, Context> {
// Under such circumstances, such application developers might wish to _not_ use this state variable type.
// This is especially dangerous for initial assignment to elements of a `Map<AztecAddress, PrivateMutable>` type (for example), because the storage slot often also identifies an actor. e.g.
// the initial assignment to `my_map.at(msg.sender)` will leak: `msg.sender`, the fact that an element of `my_map` was assigned-to for the first time, and the contract_address.
// Note: subsequent nullification of this state variable, via the `replace` method will not be leaky, if the `compute_nullifier()` method of the underlying note is designed to ensure privacy.
// For example, if the `compute_nullifier()` method injects the secret key of a note owner into the computed nullifier's preimage.
// Note: subsequent nullification of this state variable, via the `replace` method will not be leaky, if the `compute_note_hash_and_nullifier()` method of the underlying note is designed to ensure privacy.
// For example, if the `compute_note_hash_and_nullifier()` method injects the secret key of a note owner into the computed nullifier's preimage.
pub fn compute_initialization_nullifier(self) -> Field {
pedersen_hash(
[self.storage_slot],
Expand Down
8 changes: 4 additions & 4 deletions noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ impl NoteInterface<MOCK_NOTE_LENGTH, MOCK_NOTE_BYTES_LENGTH> for MockNote {
0
}

fn compute_nullifier(self, _context: &mut PrivateContext) -> Field {
0
fn compute_note_hash_and_nullifier(self, _context: &mut PrivateContext) -> (Field, Field) {
(0, 0)
}

fn compute_nullifier_without_context(self) -> Field {
0
fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field) {
(0, 0)
}

fn to_be_bytes(self, storage_slot: Field) -> [u8; MOCK_NOTE_BYTES_LENGTH] {
Expand Down
14 changes: 8 additions & 6 deletions noir-projects/aztec-nr/value-note/src/value_note.nr
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,28 @@ struct ValueNote {
impl NoteInterface<VALUE_NOTE_LEN, VALUE_NOTE_BYTES_LEN> for ValueNote {
// docs:start:nullifier

fn compute_nullifier(self, context: &mut PrivateContext) -> Field {
fn compute_note_hash_and_nullifier(self, context: &mut PrivateContext) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = context.request_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}

// docs:end:nullifier

fn compute_nullifier_without_context(self) -> Field {
fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = get_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,26 @@ struct SubscriptionNote {
}

impl NoteInterface<SUBSCRIPTION_NOTE_LEN, SUBSCRIPTION_NOTE_BYTES_LEN> for SubscriptionNote {
fn compute_nullifier(self, context: &mut PrivateContext) -> Field {
fn compute_note_hash_and_nullifier(self, context: &mut PrivateContext) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = context.request_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}

fn compute_nullifier_without_context(self) -> Field {
fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = get_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ contract Claim {
// Note: Only the owner of the npk_m will be able to produce the nsk_app and compute this nullifier.
// The nullifier is unique to the note and THIS contract because the protocol siloes all nullifiers with
// the address of a contract it was emitted from.
context.push_new_nullifier(proof_note.compute_nullifier(&mut context), 0);
let (_, nullifier) = proof_note.compute_note_hash_and_nullifier(&mut context);
context.push_new_nullifier(nullifier, 0);

// 4) Finally we mint the reward token to the sender of the transaction
Token::at(storage.reward_token.read_private()).mint_public(recipient, proof_note.value).enqueue(&mut context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,26 @@ impl CardNote {
}

impl NoteInterface<CARD_NOTE_LEN, CARD_NOTE_BYTES_LEN> for CardNote {
fn compute_nullifier(self, context: &mut PrivateContext) -> Field {
fn compute_note_hash_and_nullifier(self, context: &mut PrivateContext) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = context.request_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}

fn compute_nullifier_without_context(self) -> Field {
fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = get_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,24 +65,26 @@ impl NoteInterface<ECDSA_PUBLIC_KEY_NOTE_LEN, ECDSA_PUBLIC_KEY_NOTE_BYTES_LEN> f
EcdsaPublicKeyNote { x, y, npk_m_hash: serialized_note[4], header: NoteHeader::empty() }
}

fn compute_nullifier(self, context: &mut PrivateContext) -> Field {
fn compute_note_hash_and_nullifier(self, context: &mut PrivateContext) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = context.request_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}

fn compute_nullifier_without_context(self) -> Field {
fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = get_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,26 @@ struct PublicKeyNote {
}

impl NoteInterface<PUBLIC_KEY_NOTE_LEN, PUBLIC_KEY_NOTE_BYTES_LEN> for PublicKeyNote {
fn compute_nullifier(self, context: &mut PrivateContext) -> Field {
fn compute_note_hash_and_nullifier(self, context: &mut PrivateContext) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = context.request_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}

fn compute_nullifier_without_context(self) -> Field {
fn compute_note_hash_and_nullifier_without_context(self) -> (Field, Field) {
let note_hash_for_nullify = compute_note_hash_for_consumption(self);
let secret = get_nsk_app(self.npk_m_hash);
poseidon2_hash([
let nullifier = poseidon2_hash([
note_hash_for_nullify,
secret,
GENERATOR_INDEX__NOTE_NULLIFIER as Field,
])
]);
(note_hash_for_nullify, nullifier)
}
}

Expand Down
Loading
Loading