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

feat: Nullifier non membership #5152

Merged
merged 10 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 9 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
8 changes: 4 additions & 4 deletions l1-contracts/slither_output.md
Original file line number Diff line number Diff line change
Expand Up @@ -321,15 +321,15 @@ src/core/messagebridge/Inbox.sol#L148-L153
Impact: Informational
Confidence: Medium
- [ ] ID-35
Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L129) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L122)
Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L131) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L124)

src/core/libraries/ConstantsGen.sol#L129
src/core/libraries/ConstantsGen.sol#L131


- [ ] ID-36
Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L109) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L110)
Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L111) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L112)

src/core/libraries/ConstantsGen.sol#L109
src/core/libraries/ConstantsGen.sol#L111


- [ ] ID-37
Expand Down
4 changes: 3 additions & 1 deletion l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ library Constants {
uint256 internal constant MAX_PUBLIC_DATA_READS_PER_CALL = 16;
uint256 internal constant MAX_NOTE_HASH_READ_REQUESTS_PER_CALL = 32;
uint256 internal constant MAX_NULLIFIER_READ_REQUESTS_PER_CALL = 2;
uint256 internal constant MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL = 2;
uint256 internal constant MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL = 1;
uint256 internal constant MAX_NEW_NOTE_HASHES_PER_TX = 64;
uint256 internal constant MAX_NON_REVERTIBLE_NOTE_HASHES_PER_TX = 8;
Expand All @@ -45,6 +46,7 @@ library Constants {
uint256 internal constant MAX_NEW_L2_TO_L1_MSGS_PER_TX = 2;
uint256 internal constant MAX_NOTE_HASH_READ_REQUESTS_PER_TX = 128;
uint256 internal constant MAX_NULLIFIER_READ_REQUESTS_PER_TX = 8;
uint256 internal constant MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_TX = 8;
uint256 internal constant MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_TX = 4;
uint256 internal constant NUM_ENCRYPTED_LOGS_HASHES_PER_TX = 1;
uint256 internal constant NUM_UNENCRYPTED_LOGS_HASHES_PER_TX = 1;
Expand Down Expand Up @@ -113,7 +115,7 @@ library Constants {
uint256 internal constant PARTIAL_STATE_REFERENCE_LENGTH = 6;
uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 214;
uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 209;
uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 196;
uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 200;
uint256 internal constant STATE_REFERENCE_LENGTH = 8;
uint256 internal constant TX_CONTEXT_DATA_LENGTH = 4;
uint256 internal constant TX_REQUEST_LENGTH = 10;
Expand Down
3 changes: 2 additions & 1 deletion noir-projects/aztec-nr/aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use dep::protocol_types::{
MAX_NEW_NOTE_HASHES_PER_CALL, MAX_NEW_L2_TO_L1_MSGS_PER_CALL, MAX_NEW_NULLIFIERS_PER_CALL,
MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL,
MAX_PUBLIC_DATA_READS_PER_CALL, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL,
MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_READ_REQUESTS_PER_CALL,
MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL,
MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL, NUM_FIELDS_PER_SHA256, RETURN_VALUES_LENGTH
},
contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
Expand Down Expand Up @@ -451,6 +451,7 @@ impl PrivateContext {
args_hash: reader.read(),
return_values: [0; RETURN_VALUES_LENGTH],
nullifier_read_requests: [ReadRequest::empty(); MAX_NULLIFIER_READ_REQUESTS_PER_CALL],
nullifier_non_existent_read_requests: [ReadRequest::empty(); MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL],
contract_storage_update_requests: [StorageUpdateRequest::empty(); MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL],
contract_storage_reads: [StorageRead::empty(); MAX_PUBLIC_DATA_READS_PER_CALL],
public_call_stack_hashes: [0; MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL],
Expand Down
12 changes: 11 additions & 1 deletion noir-projects/aztec-nr/aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ use dep::protocol_types::{
MAX_NEW_NOTE_HASHES_PER_CALL, MAX_NEW_L2_TO_L1_MSGS_PER_CALL, MAX_NEW_NULLIFIERS_PER_CALL,
MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL, MAX_PUBLIC_DATA_READS_PER_CALL,
MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL,
MAX_NULLIFIER_READ_REQUESTS_PER_CALL, NUM_FIELDS_PER_SHA256, RETURN_VALUES_LENGTH
MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL,
NUM_FIELDS_PER_SHA256, RETURN_VALUES_LENGTH
},
contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
hash::hash_args, header::Header, messaging::l2_to_l1_message::L2ToL1Message, utils::reader::Reader
Expand All @@ -29,6 +30,7 @@ struct PublicContext {
return_values : BoundedVec<Field, RETURN_VALUES_LENGTH>,

nullifier_read_requests: BoundedVec<ReadRequest, MAX_NULLIFIER_READ_REQUESTS_PER_CALL>,
nullifier_non_existent_read_requests: BoundedVec<ReadRequest, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL>,
contract_storage_update_requests: BoundedVec<StorageUpdateRequest, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL>,
contract_storage_reads: BoundedVec<StorageRead, MAX_PUBLIC_DATA_READS_PER_CALL>,
public_call_stack_hashes: BoundedVec<Field, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL>,
Expand Down Expand Up @@ -102,6 +104,7 @@ impl PublicContext {
args_hash,
return_values: BoundedVec::new(),
nullifier_read_requests: BoundedVec::new(),
nullifier_non_existent_read_requests: BoundedVec::new(),
contract_storage_update_requests: BoundedVec::new(),
contract_storage_reads: BoundedVec::new(),
public_call_stack_hashes: BoundedVec::new(),
Expand Down Expand Up @@ -143,6 +146,7 @@ impl PublicContext {
call_context: self.inputs.call_context, // Done
args_hash: self.args_hash, // Done
nullifier_read_requests: self.nullifier_read_requests.storage,
nullifier_non_existent_read_requests: self.nullifier_non_existent_read_requests.storage,
contract_storage_update_requests: self.contract_storage_update_requests.storage,
contract_storage_reads: self.contract_storage_reads.storage,
return_values: self.return_values.storage,
Expand All @@ -165,6 +169,12 @@ impl PublicContext {
self.side_effect_counter = self.side_effect_counter + 1;
}

pub fn push_nullifier_non_existent_read_request(&mut self, nullifier: Field) {
let request = ReadRequest { value: nullifier, counter: self.side_effect_counter };
self.nullifier_non_existent_read_requests.push(request);
self.side_effect_counter = self.side_effect_counter + 1;
}

pub fn message_portal(&mut self, recipient: EthAddress, content: Field) {
let message = L2ToL1Message { recipient, content };
self.new_l2_to_l1_msgs.push(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ use dep::types::{
hash::{
compute_constructor_hash, compute_l2_to_l1_hash, compute_logs_hash,
compute_new_contract_address_hash, function_tree_root_from_siblings, pedersen_hash,
private_functions_root_from_siblings, root_from_sibling_path, silo_note_hash, silo_nullifier,
private_functions_root_from_siblings, silo_note_hash, silo_nullifier,
stdlib_recursion_verification_key_compress_native_vk
},
merkle_tree::check_membership,
utils::{arrays::{array_length, array_to_bounded_vec, validate_array}},
traits::{is_empty, is_empty_array}
};
Expand Down Expand Up @@ -72,8 +73,14 @@ pub fn validate_note_hash_read_requests(
// but we use the leaf index as a placeholder to detect a 'pending note read'.

if (read_request != 0) & (witness.is_transient == false) {
let root_for_read_request = root_from_sibling_path(read_request, witness.leaf_index, witness.sibling_path);
assert(root_for_read_request == historical_note_hash_tree_root, "note hash tree root mismatch");
assert(
check_membership(
read_request,
witness.leaf_index,
witness.sibling_path,
historical_note_hash_tree_root
), "note hash tree root mismatch"
);
// TODO(https://github.com/AztecProtocol/aztec-packages/issues/1354): do we need to enforce
// that a non-transient read_request was derived from the proper/current contract address?
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use crate::common;
use dep::std::{cmp::Eq, option::Option, unsafe};
use dep::reset_kernel_lib::{NullifierReadRequestResetHints, reset_read_requests};
use dep::reset_kernel_lib::{NullifierReadRequestHints, reset_read_requests};
use dep::types::{
abis::{
call_request::CallRequest, nullifier_key_validation_request::NullifierKeyValidationRequestContext,
kernel_data::{PrivateKernelInnerData, PrivateKernelTailData},
kernel_circuit_public_inputs::{PrivateKernelCircuitPublicInputsBuilder, PrivateKernelTailCircuitPublicInputs},
membership_witness::{MembershipWitness, NullifierMembershipWitness},
side_effect::{SideEffect, SideEffectLinkedToNoteHash, Ordered}
},
constants::{
Expand All @@ -26,7 +25,7 @@ struct PrivateKernelTailCircuitPrivateInputs {
read_commitment_hints: [u64; MAX_NOTE_HASH_READ_REQUESTS_PER_TX],
sorted_new_nullifiers: [SideEffectLinkedToNoteHash; MAX_NEW_NULLIFIERS_PER_TX],
sorted_new_nullifiers_indexes: [u64; MAX_NEW_NULLIFIERS_PER_TX],
nullifier_read_request_reset_hints: NullifierReadRequestResetHints,
nullifier_read_request_hints: NullifierReadRequestHints,
nullifier_commitment_hints: [u64; MAX_NEW_NULLIFIERS_PER_TX],
master_nullifier_secret_keys: [GrumpkinPrivateKey; MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_TX],
}
Expand All @@ -43,7 +42,7 @@ impl PrivateKernelTailCircuitPrivateInputs {

let pending_nullifiers = self.previous_kernel.public_inputs.end.new_nullifiers;

let hints = self.nullifier_read_request_reset_hints;
let hints = self.nullifier_read_request_hints;

let nullifier_tree_root = public_inputs.constants.historical_header.state.partial.nullifier_tree.root;

Expand Down Expand Up @@ -256,7 +255,7 @@ mod tests {
use dep::std::{cmp::Eq, unsafe};
use crate::{private_kernel_tail::PrivateKernelTailCircuitPrivateInputs};
use dep::reset_kernel_lib::{
NullifierReadRequestResetHintsBuilder,
tests::nullifier_read_request_hints_builder::NullifierReadRequestHintsBuilder,
read_request_reset::{PendingReadHint, ReadRequestState, ReadRequestStatus}
};
use dep::types::constants::{
Expand All @@ -277,7 +276,7 @@ mod tests {
previous_kernel: PreviousKernelDataBuilder,
read_commitment_hints: [u64; MAX_NOTE_HASH_READ_REQUESTS_PER_TX],
nullifier_commitment_hints: [u64; MAX_NEW_NULLIFIERS_PER_TX],
nullifier_read_request_reset_hints_builder: NullifierReadRequestResetHintsBuilder,
nullifier_read_request_hints_builder: NullifierReadRequestHintsBuilder,
}

impl PrivateKernelTailInputsBuilder {
Expand All @@ -286,7 +285,7 @@ mod tests {
previous_kernel: PreviousKernelDataBuilder::new(false),
read_commitment_hints: [0; MAX_NOTE_HASH_READ_REQUESTS_PER_TX],
nullifier_commitment_hints: [0; MAX_NEW_NULLIFIERS_PER_TX],
nullifier_read_request_reset_hints_builder: NullifierReadRequestResetHintsBuilder::new(MAX_NULLIFIER_READ_REQUESTS_PER_TX)
nullifier_read_request_hints_builder: NullifierReadRequestHintsBuilder::new(MAX_NULLIFIER_READ_REQUESTS_PER_TX)
}
}

Expand Down Expand Up @@ -326,10 +325,10 @@ mod tests {
pub fn add_nullifier_pending_read(&mut self, nullifier_index_offset_one: u64) {
let nullifier_index = nullifier_index_offset_one + 1; // + 1 is for the first nullifier
let read_request_index = self.previous_kernel.add_read_request_for_pending_nullifier(nullifier_index);
let hint_index = self.nullifier_read_request_reset_hints_builder.pending_read_hints.len();
let hint_index = self.nullifier_read_request_hints_builder.pending_read_hints.len();
let hint = PendingReadHint { read_request_index, pending_value_index: nullifier_index };
self.nullifier_read_request_reset_hints_builder.pending_read_hints.push(hint);
self.nullifier_read_request_reset_hints_builder.read_request_statuses[read_request_index] = ReadRequestStatus { state: ReadRequestState.PENDING, hint_index };
self.nullifier_read_request_hints_builder.pending_read_hints.push(hint);
self.nullifier_read_request_hints_builder.read_request_statuses[read_request_index] = ReadRequestStatus { state: ReadRequestState.PENDING, hint_index };
}

pub fn nullify_transient_commitment(&mut self, nullifier_index: Field, commitment_index: u64) {
Expand Down Expand Up @@ -383,7 +382,7 @@ mod tests {
read_commitment_hints: sorted_read_commitment_hints,
sorted_new_nullifiers,
sorted_new_nullifiers_indexes,
nullifier_read_request_reset_hints: self.nullifier_read_request_reset_hints_builder.to_hints(),
nullifier_read_request_hints: self.nullifier_read_request_hints_builder.to_hints(),
nullifier_commitment_hints: sorted_nullifier_commitment_hints,
master_nullifier_secret_keys: unsafe::zeroed()
};
Expand Down Expand Up @@ -480,10 +479,10 @@ mod tests {

builder.append_nullifiers(3);
builder.add_nullifier_pending_read(1);
let mut hint = builder.nullifier_read_request_reset_hints_builder.pending_read_hints.pop();
let mut hint = builder.nullifier_read_request_hints_builder.pending_read_hints.pop();
assert(hint.pending_value_index == 2);
hint.pending_value_index = 1;
builder.nullifier_read_request_reset_hints_builder.pending_read_hints.push(hint);
builder.nullifier_read_request_hints_builder.pending_read_hints.push(hint);

builder.failed();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ use dep::types::{
contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
constants::{
MAX_NEW_L2_TO_L1_MSGS_PER_CALL, MAX_NEW_NOTE_HASHES_PER_CALL, MAX_NEW_NULLIFIERS_PER_CALL,
MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL,
MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, MAX_PUBLIC_DATA_READS_PER_CALL, NUM_FIELDS_PER_SHA256,
MAX_REVERTIBLE_PUBLIC_DATA_READS_PER_TX, MAX_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX,
MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL,
MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX,
MAX_PUBLIC_DATA_READS_PER_CALL, NUM_FIELDS_PER_SHA256, MAX_REVERTIBLE_PUBLIC_DATA_READS_PER_TX,
MAX_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX,
MAX_NON_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, MAX_NON_REVERTIBLE_PUBLIC_DATA_READS_PER_TX
},
hash::{silo_note_hash, silo_nullifier, compute_l2_to_l1_hash, accumulate_sha256},
Expand Down Expand Up @@ -89,6 +90,7 @@ pub fn initialize_end_values(
let start_non_revertible = previous_kernel.public_inputs.end_non_revertible;
circuit_outputs.end_non_revertible.public_call_stack = array_to_bounded_vec(start_non_revertible.public_call_stack);
circuit_outputs.end_non_revertible.nullifier_read_requests = array_to_bounded_vec(start_non_revertible.nullifier_read_requests);
circuit_outputs.end_non_revertible.nullifier_non_existent_read_requests = array_to_bounded_vec(start_non_revertible.nullifier_non_existent_read_requests);
}

fn perform_static_call_checks(public_call: PublicCallData) {
Expand Down Expand Up @@ -161,6 +163,7 @@ pub fn update_public_end_non_revertible_values(
circuit_outputs.end_non_revertible.public_call_stack.extend_from_bounded_vec(public_call_requests);

propagate_nullifier_read_requests_non_revertible(public_call, circuit_outputs);
propagate_nullifier_non_existent_read_requests_non_revertible(public_call, circuit_outputs);
propagate_new_nullifiers_non_revertible(public_call, circuit_outputs);
propagate_new_note_hashes_non_revertible(public_call, circuit_outputs);
propagate_valid_non_revertible_public_data_update_requests(public_call, circuit_outputs);
Expand All @@ -182,6 +185,8 @@ pub fn update_public_end_values(public_call: PublicCallData, circuit_outputs: &m
circuit_outputs.end.public_call_stack.extend_from_bounded_vec(public_call_requests);

propagate_nullifier_read_requests_revertible(public_call, circuit_outputs);
propagate_nullifier_non_existent_read_requests_non_revertible(public_call, circuit_outputs); // TODO - Requests are not revertible and should be propagated to "validation_requests".

propagate_new_nullifiers(public_call, circuit_outputs);
propagate_new_note_hashes(public_call, circuit_outputs);

Expand Down Expand Up @@ -224,6 +229,22 @@ fn propagate_nullifier_read_requests_revertible<T>(
}
}

fn propagate_nullifier_non_existent_read_requests_non_revertible<T>(
public_call: PublicCallData,
circuit_outputs: &mut PublicKernelCircuitPublicInputsBuilder
) {
let public_call_public_inputs = public_call.call_stack_item.public_inputs;
let nullifier_non_existent_read_requests = public_call_public_inputs.nullifier_non_existent_read_requests;
let storage_contract_address = public_call_public_inputs.call_context.storage_contract_address;

for i in 0..MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL {
let request = nullifier_non_existent_read_requests[i];
if !is_empty(request) {
circuit_outputs.end_non_revertible.nullifier_non_existent_read_requests.push(request.to_context(storage_contract_address));
}
}
}

fn propagate_valid_public_data_update_requests(
public_call: PublicCallData,
circuit_outputs: &mut PublicKernelCircuitPublicInputsBuilder
Expand Down
Loading
Loading