Skip to content

Commit

Permalink
fix: skip squashing of revertible nullifier and non-revertible note h…
Browse files Browse the repository at this point in the history
…ash (#7624)

#5212 

When a revertible nullifier is nullifying a non-revertible note hash in
a tx that contains public functions, they can't be squashed. Because if
the tx reverts, we will have to keep the note hash, and discard the
nullifier.

### Reset Circuit

It takes a `split_counter` from the private inputs, and uses this
counter to check that the revertible nullifiers being squashed do not
link to non-revertible note hashes.

It's provided from the private inputs instead of being fetched from the
previous kernel's public inputs because a reset circuit can be run
before processing the call that sets the
`min_revertible_side_effect_counter` to the public inputs.

The `split_counter` will be propagated to the `validation_requests` in
the public inputs. This value must be the same as the one in the
previous kernel, if it's already set in an earlier reset circuit.

### Private Tail(_to_Public) Circuit

It checks that each nullifier either has zero note hash, or is linked to
a note hash that has a smaller counter and the exact siloed note hash.
Making sure that no transient data remains in the final public inputs.

The `split_counter` in the `validation_requests` is verified:
- In `tail_to_public`, it must match the
`min_revertible_side_effect_counter`.
- In `tail`, it must be zero to allow all the transient pairs to be
squashed.

### PXE

PXE will assign nonces to all the pending note hashes when the
`end_setup` is called and `min_revertible_side_effect_counter` is set.
At this point, we know that all the pending note hashes that haven't
been squashed can't be squashed anymore, because the nullifiers emitted
afterwards will be revertible and won't be able to squash non-revertible
note hashes. So we can compute their nonces with the correct indexes.

Those notes will be returned to the `getNotes` oracle call with non-zero
nonces. Utils in aztec.nr will compute the siloed note hash as the
`note_hash_for_consumption`, emitted it along with the nullifier.

Currently only the non-revertible note hashes have non-zero nonces so
they can't be squashed. But if needs to, an oracle can assign nonces to
all the pending note hashes, keeping all of them from being squashed, to
have a complete record of everything happening in a tx.
  • Loading branch information
LeilaWang authored Aug 1, 2024
1 parent 2631fb5 commit 76ef298
Show file tree
Hide file tree
Showing 85 changed files with 1,411 additions and 668 deletions.
7 changes: 4 additions & 3 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,15 @@ library Constants {
uint256 internal constant AGGREGATION_OBJECT_LENGTH = 16;
uint256 internal constant SCOPED_READ_REQUEST_LEN = 3;
uint256 internal constant PUBLIC_DATA_READ_LENGTH = 2;
uint256 internal constant VALIDATION_REQUESTS_LENGTH = 1090;
uint256 internal constant PRIVATE_VALIDATION_REQUESTS_LENGTH = 772;
uint256 internal constant PUBLIC_VALIDATION_REQUESTS_LENGTH = 514;
uint256 internal constant PUBLIC_DATA_UPDATE_REQUEST_LENGTH = 3;
uint256 internal constant COMBINED_ACCUMULATED_DATA_LENGTH = 364;
uint256 internal constant COMBINED_CONSTANT_DATA_LENGTH = 41;
uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1336;
uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2483;
uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2165;
uint256 internal constant PUBLIC_ACCUMULATED_DATA_LENGTH = 1215;
uint256 internal constant PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 4011;
uint256 internal constant PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 3435;
uint256 internal constant KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 415;
uint256 internal constant CONSTANT_ROLLUP_DATA_LENGTH = 11;
uint256 internal constant BASE_OR_MERGE_PUBLIC_INPUTS_LENGTH = 28;
Expand Down
2 changes: 1 addition & 1 deletion noir-projects/aztec-nr/authwit/src/auth.nr
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ pub fn assert_inner_hash_valid_authwit(context: &mut PrivateContext, on_behalf_o
// Compute the nullifier, similar computation to the outer hash, but without the chain_id and version.
// Those should already be handled in the verification, so we just need something to nullify, that allow same inner_hash for multiple actors.
let nullifier = compute_authwit_nullifier(on_behalf_of, inner_hash);
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
}

/**
Expand Down
16 changes: 11 additions & 5 deletions noir-projects/aztec-nr/aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ use crate::{
key_validation_request::get_key_validation_request, arguments, returns::pack_returns,
call_private_function::call_private_function_internal, header::get_header_at,
logs::{emit_encrypted_note_log, emit_encrypted_event_log},
enqueue_public_function_call::{enqueue_public_function_call_internal, set_public_teardown_function_call_internal}
enqueue_public_function_call::{
enqueue_public_function_call_internal, notify_set_min_revertible_side_effect_counter,
set_public_teardown_function_call_internal
}
}
};
use dep::protocol_types::{
Expand Down Expand Up @@ -125,9 +128,11 @@ impl PrivateContext {
self.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_nullifier(&mut self, nullifier: Field, nullified_note_hash: Field) {
fn push_nullifier(&mut self, nullifier: Field) {
self.nullifiers.push(Nullifier { value: nullifier, note_hash: 0, counter: self.next_counter() });
}

fn push_nullifier_for_note_hash(&mut self, nullifier: Field, nullified_note_hash: Field) {
self.nullifiers.push(Nullifier { value: nullifier, note_hash: nullified_note_hash, counter: self.next_counter() });
}

Expand Down Expand Up @@ -186,6 +191,7 @@ impl PrivateContext {
// [self.side_effect_counter as Field]
// );
self.min_revertible_side_effect_counter = self.side_effect_counter;
notify_set_min_revertible_side_effect_counter(self.min_revertible_side_effect_counter);
}

// docs:start:max-block-number
Expand Down Expand Up @@ -254,7 +260,7 @@ impl PrivateContext {
);

// Push nullifier (and the "commitment" corresponding to this can be "empty")
self.push_nullifier(nullifier, 0)
self.push_nullifier(nullifier)
}
// docs:end:consume_l1_to_l2_message

Expand Down
6 changes: 2 additions & 4 deletions noir-projects/aztec-nr/aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ impl PublicContext {
self.l1_to_l2_msg_exists(message_hash, leaf_index), "Tried to consume nonexistent L1-to-L2 message"
);

// Push nullifier (and the "commitment" corresponding to this can be "empty")
self.push_nullifier(nullifier, 0);
self.push_nullifier(nullifier);
}

fn message_portal(_self: &mut Self, recipient: EthAddress, content: Field) {
Expand Down Expand Up @@ -115,8 +114,7 @@ impl PublicContext {
fn push_note_hash(_self: &mut Self, note_hash: Field) {
emit_note_hash(note_hash);
}
fn push_nullifier(_self: &mut Self, nullifier: Field, _nullified_commitment: Field) {
// Cannot nullify pending commitments in AVM, so `nullified_commitment` is not used
fn push_nullifier(_self: &mut Self, nullifier: Field) {
emit_nullifier(nullifier);
}

Expand Down
4 changes: 2 additions & 2 deletions noir-projects/aztec-nr/aztec/src/initializer.nr
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ use crate::{

pub fn mark_as_initialized_public(context: &mut PublicContext) {
let init_nullifier = compute_unsiloed_contract_initialization_nullifier((*context).this_address());
context.push_nullifier(init_nullifier, 0);
context.push_nullifier(init_nullifier);
}

pub fn mark_as_initialized_private(context: &mut PrivateContext) {
let init_nullifier = compute_unsiloed_contract_initialization_nullifier((*context).this_address());
context.push_nullifier(init_nullifier, 0);
context.push_nullifier(init_nullifier);
}

pub fn assert_is_initialized_public(context: &mut PublicContext) {
Expand Down
12 changes: 6 additions & 6 deletions noir-projects/aztec-nr/aztec/src/note/lifecycle.nr
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,19 @@ pub fn destroy_note<Note, N, M>(

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).
// Counter is zero, so we're nullifying a settled note and we don't populate the note_hash with real value.
0
} else {
// A non-zero note hash counter implies that we're nullifying a transient note (i.e. one that has not yet been
// A non-zero note hash counter implies that we're nullifying a pending 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.
// hash with real value to inform the kernel which note we're nullifyng so that it can either squash both
// the note and the nullifier if it's an inner note hash, or check that the it matches a pending note if it's
// a siloed note hash.
note_hash
};

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

context.push_nullifier(nullifier, note_hash_for_consumption)
context.push_nullifier_for_note_hash(nullifier, note_hash_for_consumption)
}
33 changes: 25 additions & 8 deletions noir-projects/aztec-nr/aztec/src/note/utils.nr
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,42 @@ fn compute_note_hash_for_read_request_from_slotted_and_nonce(slotted_note_hash:
pub fn compute_note_hash_for_read_request<Note, let N: u32, let M: u32>(note: Note) -> Field where Note: NoteInterface<N, M> {
let slotted_note_hash = compute_slotted_note_hash(note);
let nonce = note.get_header().nonce;
let counter = note.get_header().note_hash_counter;

compute_note_hash_for_read_request_from_slotted_and_nonce(slotted_note_hash, nonce)
if counter != 0 {
slotted_note_hash
} else {
compute_note_hash_for_read_request_from_slotted_and_nonce(slotted_note_hash, nonce)
}
}

pub fn compute_note_hash_for_consumption<Note, let N: u32, let M: u32>(note: Note) -> Field where Note: NoteInterface<N, M> {
let header = note.get_header();
// There are 3 cases for reading a note intended for consumption:
// 1. The note was inserted in this transaction, and is transient.
// 2. The note was inserted in a previous transaction, and was inserted in public
// 3. The note was inserted in a previous transaction, and was inserted in private
// There are 4 cases for reading a note intended for consumption:
// 1. The note was inserted in this transaction, is revertible, or is not nullified by a revertible nullifier in
// the same transaction: (note_hash_counter != 0) & (nonce == 0)
// 2. The note was inserted in this transaction, is non-revertible, and is nullified by a revertible nullifier in
// the same transaction: (note_hash_counter != 0) & (nonce != 0)
// 3. The note was inserted in a previous transaction, and was inserted in public: (note_hash_counter == 0) & (nonce == 0)
// 4. The note was inserted in a previous transaction, and was inserted in private: (note_hash_counter == 0) & (nonce != 0)

let slotted_note_hash = compute_slotted_note_hash(note);

if (header.note_hash_counter != 0) {
if ((header.note_hash_counter != 0) & (header.nonce == 0)) {
// Case 1.
// If a note is transient, we just read the slotted_note_hash (kernel will silo by contract address).
slotted_note_hash
} else {
// If a note is not transient, that means we are reading a settled note (from tree) created in a
// previous TX. So we need the siloed_note_hash which has already been hashed with
// Case 2: If a note is non-revertible, and is nullified by a revertible nullifier, we cannot squash them in the
// private reset circuit. Because if the tx reverts, we will have to keep the note hash and throw away the
// nullifier.
// And if the tx does not revert, both will be emitted. In which case, the nullifier must be created in the app
// from the siloed note hash.
// The kernel circuit will check that a nullifier with non-zero note_nonce is linked to a note hash, whose
// siloed note hash matches the note hash specified in the nullifier.

// Case 3 & 4: If a note is not from the current transaction, that means we are reading a settled note (from
// tree) created in a previous TX. So we need the siloed_note_hash which has already been hashed with
// nonce and then contract address. This hash will match the existing leaf in the note hash
// tree, so the kernel can just perform a membership check directly on this hash/leaf.
let unique_note_hash = compute_note_hash_for_read_request_from_slotted_and_nonce(slotted_note_hash, header.nonce);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,9 @@ unconstrained pub fn set_public_teardown_function_call_internal(
);
}

#[oracle(notifySetMinRevertibleSideEffectCounter)]
unconstrained fn notify_set_min_revertible_side_effect_counter_oracle(_counter: u32) {}

unconstrained pub fn notify_set_min_revertible_side_effect_counter(counter: u32) {
notify_set_min_revertible_side_effect_counter_oracle(counter);
}
1 change: 0 additions & 1 deletion noir-projects/aztec-nr/aztec/src/oracle/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ mod get_sibling_path;
mod unsafe_rand;
mod enqueue_public_function_call;
mod header;
mod public_call;
mod notes;
mod storage;
mod logs;
Expand Down
28 changes: 0 additions & 28 deletions noir-projects/aztec-nr/aztec/src/oracle/public_call.nr

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl<Note> PrivateImmutable<Note, &mut PrivateContext> {
) -> NoteEmission<Note> where Note: NoteInterface<N, M> {
// Nullify the storage slot.
let nullifier = self.compute_initialization_nullifier();
self.context.push_nullifier(nullifier, 0);
self.context.push_nullifier(nullifier);

create_note(self.context, self.storage_slot, note)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl<Note, N, M> PrivateMutable<Note, &mut PrivateContext> where Note: NoteInter
pub fn initialize(self, note: &mut Note) -> NoteEmission<Note> {
// Nullify the storage slot.
let nullifier = self.compute_initialization_nullifier();
self.context.push_nullifier(nullifier, 0);
self.context.push_nullifier(nullifier);

create_note(self.context, self.storage_slot, note)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ contract AvmTest {
// Use the standard context interface to emit a new nullifier
#[aztec(public)]
fn new_nullifier(nullifier: Field) {
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
}

// Use the standard context interface to check for a nullifier
Expand All @@ -390,17 +390,17 @@ contract AvmTest {
// Use the standard context interface to emit a new nullifier
#[aztec(public)]
fn emit_nullifier_and_check(nullifier: Field) {
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
let exists = context.nullifier_exists(nullifier, context.storage_address());
assert(exists, "Nullifier was just created, but its existence wasn't detected!");
}

// Create the same nullifier twice (shouldn't work!)
#[aztec(public)]
fn nullifier_collision(nullifier: Field) {
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
// Can't do this twice!
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
}

#[aztec(public)]
Expand Down Expand Up @@ -446,13 +446,13 @@ contract AvmTest {

#[aztec(public)]
fn create_same_nullifier_in_nested_call(nestedAddress: AztecAddress, nullifier: Field) {
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
AvmTest::at(nestedAddress).new_nullifier(nullifier).call(&mut context);
}

#[aztec(public)]
fn create_different_nullifier_in_nested_call(nestedAddress: AztecAddress, nullifier: Field) {
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
AvmTest::at(nestedAddress).new_nullifier(nullifier + 1).call(&mut context);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ contract Claim {
// 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.
let (_, nullifier) = proof_note.compute_note_hash_and_nullifier(&mut context);
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);

// 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 @@ -44,7 +44,7 @@ contract ContractClassRegisterer {

// Emit the contract class id as a nullifier to be able to prove that this class has been (not) registered
let event = ContractClassRegistered { contract_class_id, version: 1, artifact_hash, private_functions_root, packed_public_bytecode };
context.push_nullifier(contract_class_id.to_field(), 0);
context.push_nullifier(contract_class_id.to_field());

// Broadcast class info including public bytecode
dep::aztec::oracle::debug_log::debug_log_format(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ contract ContractInstanceDeployer {
let address = AztecAddress::compute(public_keys_hash, partial_address);

// Emit the address as a nullifier to be able to prove that this instance has been (not) deployed
context.push_nullifier(address.to_field(), 0);
context.push_nullifier(address.to_field());

// Broadcast the event
let event = ContractInstanceDeployed {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ contract EasyPrivateVoting {

let secret = context.request_nsk_app(msg_sender_npk_m_hash); // get secret key of caller of function
let nullifier = std::hash::pedersen_hash([context.msg_sender().to_field(), secret]); // derive nullifier from sender and secret
context.push_nullifier(nullifier, 0); // push nullifier
context.push_nullifier(nullifier);
EasyPrivateVoting::at(context.this_address()).add_to_tally_public(candidate).enqueue(&mut context);
}
// docs:end:cast_vote
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ contract InclusionProofs {

#[aztec(public)]
fn push_nullifier_public(nullifier: Field) {
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
}

// Proves nullifier existed at latest block
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ contract PrivateFPC {
fn fund_transaction_privately(amount: Field, asset: AztecAddress, user_randomness: Field) {
assert(asset == storage.other_asset.read_private());
// convince the FPC we are not cheating
context.push_nullifier(user_randomness, 0);
context.push_nullifier(user_randomness);

// We use different randomness for fee payer to prevent a potential privay leak (see impl
// of PrivatelyRefundable for TokenNote for details).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,13 @@ contract Test {
// Purely exists for testing
#[aztec(public)]
fn emit_nullifier_public(nullifier: Field) {
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
}

// Forcefully emits a nullifier (for testing purposes)
#[aztec(private)]
fn emit_nullifier(nullifier: Field) {
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
}

// For testing non-note encrypted logs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ contract Token {
fn cancel_authwit(inner_hash: Field) {
let on_behalf_of = context.msg_sender();
let nullifier = compute_authwit_nullifier(on_behalf_of, inner_hash);
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
}
// docs:end:cancel_authwit

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ contract TokenWithRefunds {
fn cancel_authwit(inner_hash: Field) {
let on_behalf_of = context.msg_sender();
let nullifier = compute_authwit_nullifier(on_behalf_of, inner_hash);
context.push_nullifier(nullifier, 0);
context.push_nullifier(nullifier);
}
// docs:end:cancel_authwit

Expand Down
Loading

0 comments on commit 76ef298

Please sign in to comment.