From 84b9fcdc6ef011b593ccae09cbb17b9f38b389ef Mon Sep 17 00:00:00 2001 From: Leila Wang Date: Fri, 10 May 2024 20:22:31 +0100 Subject: [PATCH] refactor: validating private call data (#6316) Please read [contributing guidelines](CONTRIBUTING.md) and remove this line. --- .../crates/private-kernel-lib/src/common.nr | 190 ----- .../kernel_circuit_public_inputs_composer.nr | 4 +- .../crates/private-kernel-lib/src/lib.nr | 5 +- .../src/private_call_data_validator.nr | 230 +++++ ...e_kernel_circuit_public_inputs_composer.nr | 2 +- .../src/private_kernel_init.nr | 232 +---- .../src/private_kernel_inner.nr | 417 +-------- .../src/private_kernel_tail.nr | 7 - .../src/private_kernel_tail_to_public.nr | 7 - .../crates/private-kernel-lib/src/tests.nr | 1 + .../private_call_data_validator_builder.nr | 790 ++++++++++++++++++ .../crates/types/src/abis/call_request.nr | 6 - .../public_kernel_circuit_public_inputs.nr | 6 +- .../src/tests/private_call_data_builder.nr | 60 +- .../crates/types/src/utils/arrays.nr | 48 +- 15 files changed, 1118 insertions(+), 887 deletions(-) delete mode 100644 noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/common.nr create mode 100644 noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_call_data_validator.nr create mode 100644 noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests.nr create mode 100644 noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests/private_call_data_validator_builder.nr diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/common.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/common.nr deleted file mode 100644 index 8f828e9a6ca..00000000000 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/common.nr +++ /dev/null @@ -1,190 +0,0 @@ -use dep::std; -use dep::types::{ - abis::{ - call_request::CallRequest, accumulated_data::PrivateAccumulatedData, - private_circuit_public_inputs::PrivateCircuitPublicInputs, - private_kernel::private_call_data::PrivateCallData -}, - address::{AztecAddress, PartialAddress}, contract_class_id::ContractClassId, - hash::{private_functions_root_from_siblings, stdlib_recursion_verification_key_compress_native_vk}, - utils::{arrays::{array_length, validate_array}}, traits::{is_empty, is_empty_array} -}; - -fn validate_arrays(app_public_inputs: PrivateCircuitPublicInputs) { - // Each of the following arrays is expected to be zero-padded. - // In addition, some of the following arrays (new_note_hashes, etc...) are passed - // to extend_from_array_to_array() routines which rely on the passed arrays to be well-formed. - - validate_array(app_public_inputs.note_hash_read_requests); - validate_array(app_public_inputs.nullifier_read_requests); - validate_array(app_public_inputs.nullifier_key_validation_requests); - validate_array(app_public_inputs.new_note_hashes); - validate_array(app_public_inputs.new_nullifiers); - validate_array(app_public_inputs.private_call_stack_hashes); - validate_array(app_public_inputs.public_call_stack_hashes); - validate_array(app_public_inputs.new_l2_to_l1_msgs); - validate_array(app_public_inputs.encrypted_logs_hashes); - validate_array(app_public_inputs.unencrypted_logs_hashes); -} - -fn perform_static_call_checks(private_call: PrivateCallData) { - let public_inputs = private_call.call_stack_item.public_inputs; - let is_static_call = public_inputs.call_context.is_static_call; - if is_static_call { - // No state changes are allowed for static calls: - assert( - is_empty_array(public_inputs.new_note_hashes), "new_note_hashes must be empty for static calls" - ); - assert( - is_empty_array(public_inputs.new_nullifiers), "new_nullifiers must be empty for static calls" - ); - - let new_l2_to_l1_msgs_length = array_length(public_inputs.new_l2_to_l1_msgs); - assert(new_l2_to_l1_msgs_length == 0, "new_l2_to_l1_msgs must be empty for static calls"); - - // TODO: reevaluate when implementing https://github.com/AztecProtocol/aztec-packages/issues/1165 - // This 4 magical number is the minimum size of the buffer, since it has to store the total length of all the serialized logs. - assert( - public_inputs.encrypted_log_preimages_length == 4, "No encrypted logs are allowed for static calls" - ); - - assert( - public_inputs.unencrypted_log_preimages_length == 4, "No unencrypted logs are allowed for static calls" - ); - } -} - -fn is_valid_caller(request_from_stack: CallRequest, fn_being_verified: PrivateCallData) -> bool { - let call_context = fn_being_verified.call_stack_item.public_inputs.call_context; - - let valid_caller_context = request_from_stack.caller_context.msg_sender.eq(call_context.msg_sender) - & request_from_stack.caller_context.storage_contract_address.eq(call_context.storage_contract_address); - - request_from_stack.caller_contract_address.eq(fn_being_verified.call_stack_item.contract_address) - & (request_from_stack.caller_context.is_empty() | valid_caller_context) -} - -fn validate_call_request(request: CallRequest, hash: Field, private_call: PrivateCallData) { - if hash != 0 { - assert_eq(request.hash, hash, "call stack hash does not match call request hash"); - assert(is_valid_caller(request, private_call), "invalid caller"); - } else { - assert(is_empty(request), "call requests length does not match the expected length"); - } -} - -fn validate_call_requests(call_requests: [CallRequest; N], hashes: [Field; N], private_call: PrivateCallData) { - for i in 0..N { - let hash = hashes[i]; - let request = call_requests[i]; - validate_call_request(request, hash, private_call); - } -} - -// TODO: Move to a seperate file. -pub fn validate_private_call_data(private_call: PrivateCallData) { - let private_call_public_inputs = private_call.call_stack_item.public_inputs; - - validate_arrays(private_call_public_inputs); - - contract_logic(private_call); - - perform_static_call_checks(private_call); - - // Private call stack. - validate_call_requests( - private_call.private_call_stack, - private_call_public_inputs.private_call_stack_hashes, - private_call - ); - - // Public call stack. - validate_call_requests( - private_call.public_call_stack, - private_call_public_inputs.public_call_stack_hashes, - private_call - ); - - // Teardown call - validate_call_request( - private_call.public_teardown_call_request, - private_call_public_inputs.public_teardown_function_hash, - private_call - ); -} - -fn contract_logic(private_call: PrivateCallData) { - let contract_address = private_call.call_stack_item.contract_address; - - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/3062): Why is this using a hash function from the stdlib::recursion namespace - let private_call_vk_hash = stdlib_recursion_verification_key_compress_native_vk(private_call.vk); - - assert(!contract_address.is_zero(), "contract address cannot be zero"); - // std::println(f"contract_address={contract_address}"); - // std::println(f"private_call_vk_hash={private_call_vk_hash}"); - - // Recompute the contract class id - let computed_private_functions_root = private_functions_root_from_siblings( - private_call.call_stack_item.function_data.selector, - private_call_vk_hash, - private_call.function_leaf_membership_witness.leaf_index, - private_call.function_leaf_membership_witness.sibling_path - ); - // std::println(f"computed_private_functions_root={computed_private_functions_root}"); - - let computed_contract_class_id = ContractClassId::compute( - private_call.contract_class_artifact_hash, - computed_private_functions_root, - private_call.contract_class_public_bytecode_commitment - ); - // std::println(f"computed_contract_class_id={computed_contract_class_id}"); - - // Recompute contract address using the preimage which includes the class_id - let computed_partial_address = PartialAddress::compute_from_salted_initialization_hash( - computed_contract_class_id, - private_call.salted_initialization_hash - ); - // std::println(f"computed_partial_address={computed_partial_address}"); - - let computed_address = AztecAddress::compute(private_call.public_keys_hash, computed_partial_address); - // std::println(f"computed_address={computed_address}"); - - assert(computed_address.eq(contract_address), "computed contract address does not match expected one"); -} - -pub fn validate_previous_kernel_values(end: PrivateAccumulatedData) { - assert( - end.new_nullifiers[0].value() != 0, "The 0th nullifier in the accumulated nullifier array is zero" - ); -} - -pub fn validate_call_against_request(private_call: PrivateCallData, request: CallRequest) { - let call_stack_item = private_call.call_stack_item; - assert( - request.hash == call_stack_item.hash(), "calculated private_call_hash does not match provided private_call_hash at the top of the call stack" - ); - - let call_context = call_stack_item.public_inputs.call_context; - - if call_context.is_delegate_call { - let caller_context = request.caller_context; - assert(!caller_context.is_empty(), "caller context cannot be empty for delegate calls"); - assert( - call_context.msg_sender.eq(caller_context.msg_sender), "call stack msg_sender does not match expected msg_sender for delegate calls" - ); - assert( - call_context.storage_contract_address.eq(caller_context.storage_contract_address), "call stack storage address does not match expected contract address for delegate calls" - ); - assert( - !call_stack_item.contract_address.eq(call_context.storage_contract_address), "curent contract address must not match storage contract address for delegate calls" - ); - } else { - let caller_contract_address = request.caller_contract_address; - assert( - call_context.msg_sender.eq(caller_contract_address), "call stack msg_sender does not match caller contract address" - ); - assert( - call_context.storage_contract_address.eq(call_stack_item.contract_address), "call stack storage address does not match expected contract address" - ); - } -} diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/kernel_circuit_public_inputs_composer.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/kernel_circuit_public_inputs_composer.nr index f1b37f57318..2dec1046f68 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/kernel_circuit_public_inputs_composer.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/kernel_circuit_public_inputs_composer.nr @@ -129,7 +129,9 @@ impl KernelCircuitPublicInputsComposer { fn silo_note_hashes(&mut self) { let first_nullifier = self.public_inputs.end.new_nullifiers.get_unchecked(0).value(); - assert(first_nullifier != 0, "The 0th nullifier in the accumulated nullifier array is zero"); + + // This check is unnecessary. The 0th nullifier will always be set a non-zero value in private_kernel_init. + // assert(first_nullifier != 0, "The 0th nullifier in the accumulated nullifier array is zero"); let note_hashes = self.public_inputs.end.new_note_hashes.storage; for i in 0..MAX_NEW_NOTE_HASHES_PER_TX { diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/lib.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/lib.nr index 7ce19ad3c8e..88de299ebac 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/lib.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/lib.nr @@ -1,14 +1,13 @@ mod kernel_circuit_public_inputs_composer; +mod private_call_data_validator; mod private_kernel_circuit_public_inputs_composer; mod private_kernel_init; mod private_kernel_inner; mod private_kernel_tail; mod private_kernel_tail_to_public; +mod tests; use private_kernel_init::PrivateKernelInitCircuitPrivateInputs; use private_kernel_inner::PrivateKernelInnerCircuitPrivateInputs; use private_kernel_tail::PrivateKernelTailCircuitPrivateInputs; use private_kernel_tail_to_public::PrivateKernelTailToPublicCircuitPrivateInputs; - -// TODO: rename to be precise as to what its common to. -mod common; diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_call_data_validator.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_call_data_validator.nr new file mode 100644 index 00000000000..774b66b306e --- /dev/null +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_call_data_validator.nr @@ -0,0 +1,230 @@ +use dep::types::{ + abis::{ + call_context::CallContext, call_request::CallRequest, private_call_stack_item::PrivateCallStackItem, + private_kernel::private_call_data::PrivateCallData +}, + address::{AztecAddress, PartialAddress}, contract_class_id::ContractClassId, + hash::{private_functions_root_from_siblings, stdlib_recursion_verification_key_compress_native_vk}, + traits::{is_empty, is_empty_array}, transaction::tx_request::TxRequest, + utils::arrays::{array_length, validate_array} +}; + +fn validate_arrays(data: PrivateCallData) -> ArrayLengths { + let public_inputs = data.call_stack_item.public_inputs; + + // Each of the following arrays is expected to be zero-padded. + ArrayLengths { + note_hash_read_requests: validate_array(public_inputs.note_hash_read_requests), + nullifier_read_requests: validate_array(public_inputs.nullifier_read_requests), + nullifier_key_validation_requests: validate_array(public_inputs.nullifier_key_validation_requests), + new_note_hashes: validate_array(public_inputs.new_note_hashes), + new_nullifiers: validate_array(public_inputs.new_nullifiers), + new_l2_to_l1_msgs: validate_array(public_inputs.new_l2_to_l1_msgs), + private_call_stack_hashes: validate_array(public_inputs.private_call_stack_hashes), + public_call_stack_hashes: validate_array(public_inputs.public_call_stack_hashes), + encrypted_logs_hashes: validate_array(public_inputs.encrypted_logs_hashes), + unencrypted_logs_hashes: validate_array(public_inputs.unencrypted_logs_hashes) + } +} + +fn is_valid_caller(request: CallRequest, caller_address: AztecAddress, caller_context: CallContext) -> bool { + let valid_caller_context = request.caller_context.msg_sender.eq(caller_context.msg_sender) + & request.caller_context.storage_contract_address.eq(caller_context.storage_contract_address); + + request.caller_contract_address.eq(caller_address) + & (request.caller_context.is_empty() | valid_caller_context) +} + +fn validate_call_request(request: CallRequest, hash: Field, caller: PrivateCallStackItem) { + if hash != 0 { + assert_eq(request.hash, hash, "call stack hash does not match call request hash"); + assert( + is_valid_caller( + request, + caller.contract_address, + caller.public_inputs.call_context + ), "invalid caller" + ); + } else { + assert(is_empty(request), "call requests length does not match the expected length"); + } +} + +fn validate_call_requests(call_requests: [CallRequest; N], hashes: [Field; N], caller: PrivateCallStackItem) { + for i in 0..N { + validate_call_request(call_requests[i], hashes[i], caller); + } +} + +struct ArrayLengths { + note_hash_read_requests: u64, + nullifier_read_requests: u64, + nullifier_key_validation_requests: u64, + new_note_hashes: u64, + new_nullifiers: u64, + new_l2_to_l1_msgs: u64, + private_call_stack_hashes: u64, + public_call_stack_hashes: u64, + encrypted_logs_hashes: u64, + unencrypted_logs_hashes: u64, +} + +struct PrivateCallDataValidator { + data: PrivateCallData, + array_lengths: ArrayLengths, +} + +impl PrivateCallDataValidator { + pub fn new(data: PrivateCallData) -> Self { + let array_lengths = validate_arrays(data); + PrivateCallDataValidator { data, array_lengths } + } + + pub fn validate(self) { + self.validate_contract_address(); + self.validate_call(); + self.validate_private_call_requests(); + self.validate_public_call_requests(); + self.validate_teardown_call_request(); + } + + // Confirm that the TxRequest (user's intent) matches the private call being executed. + pub fn validate_against_tx_request(self, tx_request: TxRequest) { + let call_stack_item = self.data.call_stack_item; + assert_eq( + tx_request.origin, call_stack_item.contract_address, "origin address does not match call stack items contract address" + ); + assert_eq( + tx_request.function_data.hash(), call_stack_item.function_data.hash(), "tx_request function_data must match call_stack_item function_data" + ); + assert_eq( + tx_request.args_hash, call_stack_item.public_inputs.args_hash, "noir function args passed to tx_request must match args in the call_stack_item" + ); + assert_eq( + tx_request.tx_context, call_stack_item.public_inputs.tx_context, "tx_context in tx_request must match tx_context in call_stack_item" + ); + + // If checking against TxRequest, it must be the first call, which has the following restrictions. + let call_context = call_stack_item.public_inputs.call_context; + assert(call_context.is_delegate_call == false, "Users cannot make a delegatecall"); + assert(call_context.is_static_call == false, "Users cannot make a static call"); + } + + pub fn validate_against_call_request(self, request: CallRequest) { + let call_stack_item = self.data.call_stack_item; + + assert_eq( + request.hash, call_stack_item.hash(), "calculated private_call_hash does not match provided private_call_hash at the top of the call stack" + ); + + let call_context = call_stack_item.public_inputs.call_context; + + if call_context.is_delegate_call { + let caller_context = request.caller_context; + assert(!caller_context.is_empty(), "caller context cannot be empty for delegate calls"); + assert_eq( + call_context.msg_sender, caller_context.msg_sender, "call stack msg_sender does not match expected msg_sender for delegate calls" + ); + assert_eq( + call_context.storage_contract_address, caller_context.storage_contract_address, "call stack storage address does not match expected contract address for delegate calls" + ); + } else { + assert_eq( + call_context.msg_sender, request.caller_contract_address, "call stack msg_sender does not match caller contract address" + ); + } + } + + fn validate_contract_address(self) { + let contract_address = self.data.call_stack_item.contract_address; + + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/3062): Why is this using a hash function from the stdlib::recursion namespace + let private_call_vk_hash = stdlib_recursion_verification_key_compress_native_vk(self.data.vk); + + assert(!contract_address.is_zero(), "contract address cannot be zero"); + // std::println(f"contract_address={contract_address}"); + // std::println(f"private_call_vk_hash={private_call_vk_hash}"); + + // Recompute the contract class id + let computed_private_functions_root = private_functions_root_from_siblings( + self.data.call_stack_item.function_data.selector, + private_call_vk_hash, + self.data.function_leaf_membership_witness.leaf_index, + self.data.function_leaf_membership_witness.sibling_path + ); + // std::println(f"computed_private_functions_root={computed_private_functions_root}"); + + let computed_contract_class_id = ContractClassId::compute( + self.data.contract_class_artifact_hash, + computed_private_functions_root, + self.data.contract_class_public_bytecode_commitment + ); + // std::println(f"computed_contract_class_id={computed_contract_class_id}"); + + // Recompute contract address using the preimage which includes the class_id + let computed_partial_address = PartialAddress::compute_from_salted_initialization_hash( + computed_contract_class_id, + self.data.salted_initialization_hash + ); + // std::println(f"computed_partial_address={computed_partial_address}"); + + let computed_address = AztecAddress::compute(self.data.public_keys_hash, computed_partial_address); + // std::println(f"computed_address={computed_address}"); + + assert( + computed_address.eq(contract_address), "computed contract address does not match expected one" + ); + } + + fn validate_call(self) { + let call_context = self.data.call_stack_item.public_inputs.call_context; + + let is_own_storage = call_context.storage_contract_address == self.data.call_stack_item.contract_address; + if call_context.is_delegate_call { + assert( + !is_own_storage, "current contract address must not match storage contract address for delegate calls" + ); + } else { + assert(is_own_storage, "call stack storage address does not match expected contract address"); + } + + if call_context.is_static_call { + // No state changes are allowed for static calls: + assert_eq(self.array_lengths.new_note_hashes, 0, "new_note_hashes must be empty for static calls"); + assert_eq(self.array_lengths.new_nullifiers, 0, "new_nullifiers must be empty for static calls"); + assert_eq( + self.array_lengths.new_l2_to_l1_msgs, 0, "new_l2_to_l1_msgs must be empty for static calls" + ); + assert_eq( + self.array_lengths.encrypted_logs_hashes, 0, "encrypted_logs_hashes must be empty for static calls" + ); + assert_eq( + self.array_lengths.unencrypted_logs_hashes, 0, "unencrypted_logs_hashes must be empty for static calls" + ); + } + } + + fn validate_private_call_requests(self) { + validate_call_requests( + self.data.private_call_stack, + self.data.call_stack_item.public_inputs.private_call_stack_hashes, + self.data.call_stack_item + ); + } + + fn validate_public_call_requests(self) { + validate_call_requests( + self.data.public_call_stack, + self.data.call_stack_item.public_inputs.public_call_stack_hashes, + self.data.call_stack_item + ); + } + + fn validate_teardown_call_request(self) { + validate_call_request( + self.data.public_teardown_call_request, + self.data.call_stack_item.public_inputs.public_teardown_function_hash, + self.data.call_stack_item + ); + } +} diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_circuit_public_inputs_composer.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_circuit_public_inputs_composer.nr index ef4e6008eb9..b7f5dc96d85 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_circuit_public_inputs_composer.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_circuit_public_inputs_composer.nr @@ -215,7 +215,7 @@ impl PrivateKernelCircuitPublicInputsComposer { let call_request = source.public_teardown_call_request; if !is_empty(call_request) { assert( - self.public_inputs.public_teardown_call_request.is_empty(), "Public teardown call request already set" + is_empty(self.public_inputs.public_teardown_call_request), "Public teardown call request already set" ); self.public_inputs.public_teardown_call_request = call_request; } diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_init.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_init.nr index 4c3872bac03..da6ef0cc4ba 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_init.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_init.nr @@ -1,4 +1,7 @@ -use crate::{common, private_kernel_circuit_public_inputs_composer::PrivateKernelCircuitPublicInputsComposer}; +use crate::{ + private_call_data_validator::PrivateCallDataValidator, + private_kernel_circuit_public_inputs_composer::PrivateKernelCircuitPublicInputsComposer +}; use dep::types::{ abis::{ private_kernel::private_call_data::{PrivateCallData, verify_private_call}, @@ -20,63 +23,15 @@ struct PrivateKernelInitCircuitPrivateInputs { } impl PrivateKernelInitCircuitPrivateInputs { - // Confirm that the TxRequest (user's intent) - // matches the private call being executed - fn validate_this_private_call_against_tx_request(self) { - let tx_request = self.tx_request; - // Call stack item for the initial call - let call_stack_item = self.private_call.call_stack_item; - - // Checks to ensure that the user's intent matches the initial private call - // - // We use the word correct to denote whether it matches the user intent. - // - // Ensure we are calling the correct initial contract - let origin_address_matches = tx_request.origin.eq(call_stack_item.contract_address); - assert(origin_address_matches, "origin address does not match call stack items contract address"); - // - // Ensure we are calling the correct initial function in the contract - let entry_point_function_matches = tx_request.function_data.hash() == call_stack_item.function_data.hash(); - assert( - entry_point_function_matches, "tx_request function_data must match call_stack_item function_data" - ); - // - // Ensure we are passing the correct arguments to the function. - let args_match = tx_request.args_hash == call_stack_item.public_inputs.args_hash; - assert(args_match, "noir function args passed to tx_request must match args in the call_stack_item"); - // - // Ensure we are passing the correct tx context - let tx_context_matches = tx_request.tx_context == call_stack_item.public_inputs.tx_context; - assert(tx_context_matches, "tx_context in tx_request must match tx_context in call_stack_item"); - } - - fn validate_inputs(self) { - let call_stack_item = self.private_call.call_stack_item; - - let function_data = call_stack_item.function_data; - assert(function_data.is_private, "Private kernel circuit can only execute a private function"); - - let call_context = call_stack_item.public_inputs.call_context; - assert(call_context.is_delegate_call == false, "Users cannot make a delegatecall"); - assert(call_context.is_static_call == false, "Users cannot make a static call"); - // The below also prevents delegatecall/staticcall in the base case - assert( - call_context.storage_contract_address.eq(call_stack_item.contract_address), "Storage contract address must be that of the called contract" - ); - } - pub fn native_private_kernel_circuit_initial(self) -> PrivateKernelCircuitPublicInputs { - let private_call_public_inputs = self.private_call.call_stack_item.public_inputs; - // verify/aggregate the private call proof verify_private_call(self.private_call); - self.validate_inputs(); - - common::validate_private_call_data(self.private_call); - - self.validate_this_private_call_against_tx_request(); + let privateCallDataValidator = PrivateCallDataValidator::new(self.private_call); + privateCallDataValidator.validate(); + privateCallDataValidator.validate_against_tx_request(self.tx_request); + let private_call_public_inputs = self.private_call.call_stack_item.public_inputs; PrivateKernelCircuitPublicInputsComposer::new_from_tx_request(self.tx_request, private_call_public_inputs).compose( private_call_public_inputs, self.hints.note_hash_nullifier_counters, @@ -167,177 +122,6 @@ mod tests { assert_eq(public_inputs.end.unencrypted_logs_hashes[1].value, unencrypted_logs_hashes[1]); } - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_note_hash_read_requests() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - builder.private_call.public_inputs.note_hash_read_requests.extend_from_array( - [ - ReadRequest::empty(), - ReadRequest { value: 9123, counter: 1 } - ] - ); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_note_hashes() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - builder.private_call.public_inputs.new_note_hashes.extend_from_array( - [ - NoteHash { value: 0, counter: 0 }, - NoteHash { value: 9123, counter: 1 } - ] - ); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_nullifiers() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - builder.private_call.public_inputs.new_nullifiers.extend_from_array( - [ - Nullifier { value: 0, note_hash: 0, counter: 0 }, - Nullifier { value: 9123, note_hash: 0, counter: 1 } - ] - ); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_private_call_stack() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - builder.private_call.public_inputs.private_call_stack_hashes.extend_from_array([0, 9123]); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_public_call_stack() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - builder.private_call.public_inputs.public_call_stack_hashes.extend_from_array([0, 9123]); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_new_l2_to_l1_msgs() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - builder.private_call.public_inputs.new_l2_to_l1_msgs.extend_from_array( - [ - L2ToL1Message::empty(), - L2ToL1Message { recipient: EthAddress::from_field(6), content: 9123, counter: 0 } - ] - ); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_logs() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - builder.private_call.public_inputs.encrypted_logs_hashes.extend_from_array( - [ - SideEffect { value: 0, counter: 0 }, - SideEffect { value: 9123, counter: 1 } - ] - ); - - builder.failed(); - } - - #[test(should_fail_with="Private kernel circuit can only execute a private function")] - fn private_function_is_private_false_fails() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - // Set is_private in function data to false. - builder.private_call.function_data.is_private = false; - - builder.failed(); - } - - #[test(should_fail_with="Users cannot make a static call")] - fn private_function_static_call_fails() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - // Set is_static_call to true. - builder.private_call.public_inputs.call_context.is_static_call = true; - - builder.failed(); - } - - #[test(should_fail_with="Users cannot make a delegatecall")] - fn private_function_delegate_call_fails() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - // Set is_delegate_call to true. - builder.private_call.public_inputs.call_context.is_delegate_call = true; - - builder.failed(); - } - - #[test(should_fail_with="Storage contract address must be that of the called contract")] - fn private_function_incorrect_storage_contract_address_fails() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - // Set the storage_contract_address to a random scalar. - builder.private_call.public_inputs.call_context.storage_contract_address = AztecAddress::from_field(356); - - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_function_leaf_index_fails() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - // Set the leaf index of the function leaf to a wrong value (the correct value + 1). - let leaf_index = builder.private_call.function_leaf_membership_witness.leaf_index; - builder.private_call.function_leaf_membership_witness.leaf_index = leaf_index + 1; - - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_function_leaf_sibling_path_fails() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - - // Set the first value of the sibling path to a wrong value (the correct value + 1). - let sibling_path_0 = builder.private_call.function_leaf_membership_witness.sibling_path[0]; - builder.private_call.function_leaf_membership_witness.sibling_path[0] = sibling_path_0 + 1; - - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_contract_class_preimage_fails() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - builder.private_call.contract_class_artifact_hash = builder.private_call.contract_class_artifact_hash + 1; - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_partial_address_preimage_fails() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - builder.private_call.salted_initialization_hash.inner = builder.private_call.salted_initialization_hash.inner + 1; - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_address_preimage_fails() { - let mut builder = PrivateKernelInitInputsBuilder::new(); - builder.private_call.public_keys_hash.inner = builder.private_call.public_keys_hash.inner + 1; - builder.failed(); - } - #[test] fn default_max_block_number() { let mut builder = PrivateKernelInitInputsBuilder::new(); diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_inner.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_inner.nr index 07eabb0f6e0..20376e9b83a 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_inner.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_inner.nr @@ -1,10 +1,12 @@ -use crate::{common, private_kernel_circuit_public_inputs_composer::PrivateKernelCircuitPublicInputsComposer}; +use crate::{ + private_call_data_validator::PrivateCallDataValidator, + private_kernel_circuit_public_inputs_composer::PrivateKernelCircuitPublicInputsComposer +}; use dep::types::{ abis::{ private_kernel_data::{PrivateKernelData, verify_previous_kernel_proof}, private_kernel::private_call_data::{PrivateCallData, verify_private_call}, - kernel_circuit_public_inputs::{PrivateKernelCircuitPublicInputs, PrivateKernelCircuitPublicInputsBuilder}, - side_effect::SideEffect + kernel_circuit_public_inputs::PrivateKernelCircuitPublicInputs }, constants::MAX_NEW_NOTE_HASHES_PER_CALL, utils::arrays::array_length }; @@ -20,36 +22,24 @@ struct PrivateKernelInnerCircuitPrivateInputs { } impl PrivateKernelInnerCircuitPrivateInputs { - fn validate_inputs(self) { - let this_call_stack_item = self.private_call.call_stack_item; - let function_data = this_call_stack_item.function_data; - assert(function_data.is_private, "Private kernel circuit can only execute a private function"); - } - pub fn native_private_kernel_circuit_inner(self) -> PrivateKernelCircuitPublicInputs { - let private_call_public_inputs = self.private_call.call_stack_item.public_inputs; - let previous_kernel_public_inputs = self.previous_kernel.public_inputs; - // verify/aggregate the private call proof verify_private_call(self.private_call); // verify/aggregate the previous kernel verify_previous_kernel_proof(self.previous_kernel); - common::validate_previous_kernel_values(previous_kernel_public_inputs.end); + let privateCallDataValidator = PrivateCallDataValidator::new(self.private_call); + privateCallDataValidator.validate(); - self.validate_inputs(); - - common::validate_private_call_data(self.private_call); - - let mut private_call_stack = previous_kernel_public_inputs.end.private_call_stack; + let private_call_stack = self.previous_kernel.public_inputs.end.private_call_stack; // TODO: Should be a hint from private inputs. let private_call_stack_size = array_length(private_call_stack); let call_request = private_call_stack[private_call_stack_size - 1]; - common::validate_call_against_request(self.private_call, call_request); + privateCallDataValidator.validate_against_call_request(call_request); PrivateKernelCircuitPublicInputsComposer::new_from_previous_kernel(self.previous_kernel.public_inputs).compose( - private_call_public_inputs, + self.private_call.call_stack_item.public_inputs, self.hints.note_hash_nullifier_counters, self.private_call.private_call_stack, self.private_call.public_call_stack, @@ -122,357 +112,6 @@ mod tests { } } - #[test(should_fail_with = "contract address cannot be zero")] - fn private_function_zero_storage_contract_address_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - // Set (storage) contract_address to 0 - builder.private_call.contract_address = AztecAddress::zero(); - builder.private_call.public_inputs.call_context.storage_contract_address = AztecAddress::zero(); - - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_function_leaf_index_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - // Set the leaf index of the function leaf to a wrong value (the correct value + 1). - let leaf_index = builder.private_call.function_leaf_membership_witness.leaf_index; - builder.private_call.function_leaf_membership_witness.leaf_index = leaf_index + 1; - - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_function_leaf_sibling_path_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - // Set the first value of the sibling path to a wrong value (the correct value + 1). - let sibling_path_0 = builder.private_call.function_leaf_membership_witness.sibling_path[0]; - builder.private_call.function_leaf_membership_witness.sibling_path[0] = sibling_path_0 + 1; - - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_contract_class_preimage_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - builder.private_call.contract_class_artifact_hash = builder.private_call.contract_class_artifact_hash + 1; - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_partial_address_preimage_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - builder.private_call.salted_initialization_hash.inner = builder.private_call.salted_initialization_hash.inner + 1; - builder.failed(); - } - - #[test(should_fail_with="computed contract address does not match expected one")] - fn private_function_incorrect_address_preimage_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - builder.private_call.public_keys_hash.inner = builder.private_call.public_keys_hash.inner + 1; - builder.failed(); - } - - #[test(should_fail_with = "calculated private_call_hash does not match provided private_call_hash at the top of the call stack")] - fn private_function_incorrect_call_stack_item_hash_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - let private_call = builder.private_call.finish(); - let hash = private_call.call_stack_item.hash(); - // Set the first call stack hash to a wrong value (the correct hash + 1). - builder.previous_kernel.push_private_call_request(hash + 1, false); - let previous_kernel = builder.previous_kernel.to_private_kernel_data(); - - let kernel = PrivateKernelInnerCircuitPrivateInputs { previous_kernel, private_call, hints: builder.hints }; - - let _ = kernel.native_private_kernel_circuit_inner(); - } - - #[test(should_fail_with="call stack msg_sender does not match caller contract address")] - fn incorrect_msg_sender_for_regular_calls_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - // Set the msg_sender to a wrong value. - builder.private_call.public_inputs.call_context.msg_sender.inner += 1; - - builder.failed(); - } - - #[test(should_fail_with="call stack storage address does not match expected contract address")] - fn incorrect_storage_contract_for_regular_calls_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - // Set the storage contract address to a wrong value. - builder.private_call.public_inputs.call_context.storage_contract_address.inner += 1; - - builder.failed(); - } - - #[test] - fn delegate_call_succeeds() { - let mut builder = PrivateKernelInnerInputsBuilder::new().is_delegate_call(); - builder.succeeded(); - } - - #[test(should_fail_with="caller context cannot be empty for delegate calls")] - fn empty_caller_context_for_delegate_calls_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new().is_delegate_call(); - - let private_call = builder.private_call.finish(); - let hash = private_call.call_stack_item.hash(); - // Caller context is empty for regular calls. - let is_delegate_call = false; - builder.previous_kernel.push_private_call_request(hash, is_delegate_call); - let previous_kernel = builder.previous_kernel.to_private_kernel_data(); - - let kernel = PrivateKernelInnerCircuitPrivateInputs { previous_kernel, private_call, hints: builder.hints }; - - let _ = kernel.native_private_kernel_circuit_inner(); - } - - #[test(should_fail_with="call stack msg_sender does not match expected msg_sender for delegate calls")] - fn incorrect_msg_sender_for_delegate_calls_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new().is_delegate_call(); - - // Set the msg_sender to be the caller contract. - builder.private_call.public_inputs.call_context.msg_sender = builder.previous_kernel.contract_address; - - builder.failed(); - } - - #[test(should_fail_with="call stack storage address does not match expected contract address for delegate calls")] - fn incorrect_storage_address_for_delegate_calls_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new().is_delegate_call(); - - // Set the storage contract address to be the contract address. - builder.private_call.public_inputs.call_context.storage_contract_address = builder.private_call.contract_address; - - builder.failed(); - } - - #[test(should_fail_with="curent contract address must not match storage contract address for delegate calls")] - fn incorrect_storage_contract_for_delegate_calls_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new().is_delegate_call(); - - // Change the storage contract address to be the same as the contract address. - builder.private_call.public_inputs.call_context.storage_contract_address = builder.private_call.contract_address; - - let private_call = builder.private_call.finish(); - let hash = private_call.call_stack_item.hash(); - builder.previous_kernel.push_private_call_request(hash, true); - let mut call_request = builder.previous_kernel.private_call_stack.pop(); - // Change the caller's storage contract address to be the same as the contract address. - call_request.caller_context.storage_contract_address = builder.private_call.contract_address; - builder.previous_kernel.private_call_stack.push(call_request); - - let previous_kernel = builder.previous_kernel.to_private_kernel_data(); - let kernel = PrivateKernelInnerCircuitPrivateInputs { previous_kernel, private_call, hints: builder.hints }; - let _ = kernel.native_private_kernel_circuit_inner(); - } - - #[test] - fn call_requests_succeeds() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.append_private_call_requests(2, false); - builder.private_call.append_private_call_requests(1, true); - builder.private_call.append_public_call_requests(1, false); - builder.private_call.append_public_call_requests(2, true); - - builder.succeeded(); - } - - #[test(should_fail_with = "call requests length does not match the expected length")] - fn incorrect_private_call_requests_length_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.append_private_call_requests(2, false); - // Remove one call stack item hash. - let _ = builder.private_call.public_inputs.private_call_stack_hashes.pop(); - - builder.failed(); - } - - #[test(should_fail_with = "call requests length does not match the expected length")] - fn incorrect_public_call_requests_length_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.append_public_call_requests(2, false); - // Remove one call stack item hash. - let _ = builder.private_call.public_inputs.public_call_stack_hashes.pop(); - - builder.failed(); - } - - #[test(should_fail_with = "call stack hash does not match call request hash")] - fn incorrect_private_call_request_hash_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.append_private_call_requests(2, false); - let mut call_request = builder.private_call.private_call_stack.pop(); - // Change the hash to be a different value. - call_request.hash += 1; - builder.private_call.private_call_stack.push(call_request); - - builder.failed(); - } - - #[test(should_fail_with = "call stack hash does not match call request hash")] - fn incorrect_public_call_request_hash_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.append_public_call_requests(2, false); - let mut call_request = builder.private_call.public_call_stack.pop(); - // Change the hash to be a different value. - call_request.hash += 1; - builder.private_call.public_call_stack.push(call_request); - - builder.failed(); - } - - #[test(should_fail_with = "invalid caller")] - fn incorrect_caller_address_for_private_call_request_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.append_private_call_requests(1, false); - let mut call_request = builder.private_call.private_call_stack.pop(); - // Change the caller contract address to be a different value. - call_request.caller_contract_address.inner += 1; - builder.private_call.private_call_stack.push(call_request); - - builder.failed(); - } - - #[test(should_fail_with = "invalid caller")] - fn incorrect_caller_address_for_public_call_request_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.append_public_call_requests(1, false); - let mut call_request = builder.private_call.public_call_stack.pop(); - // Change the caller contract address to be a different value. - call_request.caller_contract_address.inner += 1; - builder.private_call.public_call_stack.push(call_request); - - builder.failed(); - } - - #[test(should_fail_with = "invalid caller")] - fn incorrect_caller_context_for_private_delegate_call_request_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.append_private_call_requests(1, true); - let mut call_request = builder.private_call.private_call_stack.pop(); - // Change the storage contract to be a different value. - call_request.caller_context.storage_contract_address.inner += 1; - builder.private_call.private_call_stack.push(call_request); - - builder.failed(); - } - - #[test(should_fail_with = "invalid caller")] - fn incorrect_caller_context_for_public_delegate_call_request_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.append_public_call_requests(1, true); - let mut call_request = builder.private_call.public_call_stack.pop(); - // Change the storage contract to be a different value. - call_request.caller_context.storage_contract_address.inner += 1; - builder.private_call.public_call_stack.push(call_request); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_read_requests() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.public_inputs.note_hash_read_requests.extend_from_array( - [ - ReadRequest::empty(), - ReadRequest { value: 9123, counter: 1 } - ] - ); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_note_hashes() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.public_inputs.new_note_hashes.extend_from_array( - [ - NoteHash { value: 0, counter: 0 }, - NoteHash { value: 9123, counter: 1 } - ] - ); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_nullifiers() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.public_inputs.new_nullifiers.extend_from_array( - [ - Nullifier { value: 0, note_hash: 0, counter: 0 }, - Nullifier { value: 12, note_hash: 0, counter: 1 } - ] - ); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_private_call_stack() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.public_inputs.private_call_stack_hashes.extend_from_array([0, 888]); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_public_call_stack() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.public_inputs.public_call_stack_hashes.extend_from_array([0, 888]); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_new_l2_to_l1_msgs() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.public_inputs.new_l2_to_l1_msgs.extend_from_array( - [ - L2ToL1Message::empty(), - L2ToL1Message { recipient: EthAddress::from_field(6), content: 888, counter: 0 } - ] - ); - - builder.failed(); - } - - #[test(should_fail_with = "invalid array")] - fn input_validation_malformed_arrays_logs() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.public_inputs.encrypted_logs_hashes.extend_from_array( - [ - SideEffect { value: 0, counter: 0 }, - SideEffect { value: 9123, counter: 1 } - ] - ); - - builder.failed(); - } - #[test(should_fail_with = "push out of bounds")] fn private_kernel_should_fail_if_aggregating_too_many_note_hashes() { let mut builder = PrivateKernelInnerInputsBuilder::new(); @@ -514,15 +153,6 @@ mod tests { builder.failed(); } - #[test(should_fail_with="Private kernel circuit can only execute a private function")] - fn private_function_is_private_false_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.private_call.function_data.is_private = false; - - builder.failed(); - } - #[test] fn propagate_previous_kernel_max_block_number() { let mut builder = PrivateKernelInnerInputsBuilder::new(); @@ -618,31 +248,4 @@ mod tests { assert_eq(public_inputs.end.encrypted_logs_hashes[1].value, encrypted_logs_hash); assert_eq(public_inputs.end.unencrypted_logs_hashes[1].value, unencrypted_logs_hash); } - - #[test(should_fail_with="new_note_hashes must be empty for static calls")] - fn creating_new_note_hashes_on_static_call_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new().is_static_call(); - - builder.private_call.public_inputs.new_note_hashes.push(NoteHash { value: 1, counter: 0 }); - - builder.failed(); - } - - #[test(should_fail_with="new_nullifiers must be empty for static calls")] - fn creating_new_nullifiers_on_static_call_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new().is_static_call(); - - builder.private_call.public_inputs.new_nullifiers.push(Nullifier { value: 1, note_hash: 0, counter: 0 }); - - builder.failed(); - } - - #[test(should_fail_with="The 0th nullifier in the accumulated nullifier array is zero")] - fn zero_0th_nullifier_fails() { - let mut builder = PrivateKernelInnerInputsBuilder::new(); - - builder.previous_kernel.new_nullifiers = BoundedVec::new(); - - builder.failed(); - } } diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr index 98b6b9c08fa..fabb95e9911 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr @@ -581,13 +581,6 @@ mod tests { builder.failed(); } - #[test(should_fail_with="The 0th nullifier in the accumulated nullifier array is zero")] - unconstrained fn zero_0th_nullifier_fails() { - let mut builder = PrivateKernelTailInputsBuilder::new(); - builder.previous_kernel.new_nullifiers = BoundedVec::new(); - builder.failed(); - } - #[test] unconstrained fn empty_tx_consumes_teardown_limits_plus_fixed_gas() { let mut builder = PrivateKernelTailInputsBuilder::new(); diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail_to_public.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail_to_public.nr index 42309b81435..fa73aaf18ae 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail_to_public.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail_to_public.nr @@ -483,13 +483,6 @@ mod tests { builder.succeeded(); } - #[test(should_fail_with="The 0th nullifier in the accumulated nullifier array is zero")] - unconstrained fn zero_0th_nullifier_fails() { - let mut builder = PrivateKernelTailToPublicInputsBuilder::new(); - builder.previous_kernel.new_nullifiers = BoundedVec::new(); - builder.failed(); - } - #[test] unconstrained fn split_nullifiers_into_non_revertible() { let mut builder = PrivateKernelTailToPublicInputsBuilder::new(); diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests.nr new file mode 100644 index 00000000000..e92b7e1016a --- /dev/null +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests.nr @@ -0,0 +1 @@ +mod private_call_data_validator_builder; diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests/private_call_data_validator_builder.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests/private_call_data_validator_builder.nr new file mode 100644 index 00000000000..8802b64d81e --- /dev/null +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/tests/private_call_data_validator_builder.nr @@ -0,0 +1,790 @@ +use crate::private_call_data_validator::PrivateCallDataValidator; +use dep::types::{ + abis::{ + call_request::CallRequest, caller_context::CallerContext, note_hash::NoteHash, nullifier::Nullifier, + nullifier_key_validation_request::NullifierKeyValidationRequest, read_request::ReadRequest, + side_effect::SideEffect +}, + address::{AztecAddress, EthAddress}, grumpkin_point::GrumpkinPoint, + messaging::l2_to_l1_message::L2ToL1Message, + tests::private_call_data_builder::PrivateCallDataBuilder, transaction::tx_request::TxRequest +}; + +struct PrivateCallDataValidatorBuilder { + private_call: PrivateCallDataBuilder, +} + +impl PrivateCallDataValidatorBuilder { + pub fn new() -> Self { + let private_call = PrivateCallDataBuilder::new(); + PrivateCallDataValidatorBuilder { private_call } + } + + pub fn is_delegate_call(&mut self) -> Self { + let _ = self.private_call.is_delegate_call(); + *self + } + + pub fn is_static_call(&mut self) -> Self { + let _ = self.private_call.is_static_call(); + *self + } + + pub fn validate(self) { + let private_call = self.private_call.finish(); + PrivateCallDataValidator::new(private_call).validate(); + } + + pub fn validate_against_tx_request(self, request: TxRequest) { + let private_call = self.private_call.finish(); + PrivateCallDataValidator::new(private_call).validate_against_tx_request(request); + } + + pub fn validate_against_call_request(self, request: CallRequest) { + let private_call = self.private_call.finish(); + PrivateCallDataValidator::new(private_call).validate_against_call_request(request); + } +} + +/** + * validate_arrays + */ + +#[test(should_fail_with="invalid array")] +fn validate_arrays_malformed_note_hash_read_requests_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_inputs.note_hash_read_requests.extend_from_array( + [ + ReadRequest::empty(), + ReadRequest { value: 9123, counter: 1 } + ] + ); + + builder.validate(); +} + +#[test(should_fail_with="invalid array")] +fn validate_arrays_malformed_nullifier_read_requests_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_inputs.nullifier_read_requests.extend_from_array( + [ + ReadRequest::empty(), + ReadRequest { value: 9123, counter: 1 } + ] + ); + + builder.validate(); +} + +// Enable this test if MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL is greater than 1. +// #[test(should_fail_with="invalid array")] +// fn validate_arrays_malformed_nullifier_key_validation_requests_fails() { +// let mut builder = PrivateCallDataValidatorBuilder::new(); + +// builder.private_call.public_inputs.nullifier_key_validation_requests.extend_from_array( +// [ +// NullifierKeyValidationRequest::empty(), +// NullifierKeyValidationRequest { master_nullifier_public_key: GrumpkinPoint { x: 12, y: 34 }, app_nullifier_secret_key: 5 } +// ] +// ); + +// builder.validate(); +// } + +#[test(should_fail_with="invalid array")] +fn validate_arrays_malformed_note_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_inputs.new_note_hashes.extend_from_array( + [ + NoteHash::empty(), + NoteHash { value: 9123, counter: 1 } + ] + ); + + builder.validate(); +} + +#[test(should_fail_with="invalid array")] +fn validate_arrays_malformed_nullifiers_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_inputs.new_nullifiers.extend_from_array( + [ + Nullifier::empty(), + Nullifier { value: 9123, note_hash: 0, counter: 1 } + ] + ); + + builder.validate(); +} + +#[test(should_fail_with="invalid array")] +fn validate_arrays_malformed_l2_to_l1_msgs_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_inputs.new_l2_to_l1_msgs.extend_from_array( + [ + L2ToL1Message::empty(), + L2ToL1Message { recipient: EthAddress::from_field(6), content: 9123, counter: 0 } + ] + ); + + builder.validate(); +} + +#[test(should_fail_with="invalid array")] +fn validate_arrays_malformed_private_call_stack_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_inputs.private_call_stack_hashes.extend_from_array([0, 9123]); + + builder.validate(); +} + +#[test(should_fail_with="invalid array")] +fn validate_arrays_malformed_public_call_stack_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_inputs.public_call_stack_hashes.extend_from_array([0, 9123]); + + builder.validate(); +} + +#[test(should_fail_with="invalid array")] +fn validate_arrays_malformed_encrypted_logs_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_inputs.encrypted_logs_hashes.extend_from_array( + [ + SideEffect { value: 0, counter: 0 }, + SideEffect { value: 9123, counter: 1 } + ] + ); + + builder.validate(); +} + +#[test(should_fail_with="invalid array")] +fn validate_arrays_malformed_unencrypted_logs_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_inputs.unencrypted_logs_hashes.extend_from_array( + [ + SideEffect { value: 0, counter: 0 }, + SideEffect { value: 9123, counter: 1 } + ] + ); + + builder.validate(); +} + +/** + * validate_contract_address + */ + +#[test(should_fail_with="contract address cannot be zero")] +fn validate_contract_address_zero_storage_contract_address_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + // Set (storage) contract_address to 0 + builder.private_call.contract_address = AztecAddress::zero(); + builder.private_call.public_inputs.call_context.storage_contract_address = AztecAddress::zero(); + + builder.validate(); +} + +#[test(should_fail_with="computed contract address does not match expected one")] +fn validate_contract_address_incorrect_function_leaf_index_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + // Set the leaf index of the function leaf to a wrong value (the correct value + 1). + let leaf_index = builder.private_call.function_leaf_membership_witness.leaf_index; + builder.private_call.function_leaf_membership_witness.leaf_index = leaf_index + 1; + + builder.validate(); +} + +#[test(should_fail_with="computed contract address does not match expected one")] +fn validate_contract_address_incorrect_function_leaf_sibling_path_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + // Set the first value of the sibling path to a wrong value (the correct value + 1). + let sibling_path_0 = builder.private_call.function_leaf_membership_witness.sibling_path[0]; + builder.private_call.function_leaf_membership_witness.sibling_path[0] = sibling_path_0 + 1; + + builder.validate(); +} + +#[test(should_fail_with="computed contract address does not match expected one")] +fn validate_contract_address_incorrect_contract_class_preimage_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.contract_class_artifact_hash = builder.private_call.contract_class_artifact_hash + 1; + + builder.validate(); +} + +#[test(should_fail_with="computed contract address does not match expected one")] +fn validate_contract_address_incorrect_partial_address_preimage_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.salted_initialization_hash.inner = builder.private_call.salted_initialization_hash.inner + 1; + + builder.validate(); +} + +#[test(should_fail_with="computed contract address does not match expected one")] +fn validate_contract_address_incorrect_address_preimage_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.public_keys_hash.inner = builder.private_call.public_keys_hash.inner + 1; + + builder.validate(); +} + +/** + * validate_call + */ + +#[test] +fn validate_call_is_regular_succeeds() { + let builder = PrivateCallDataValidatorBuilder::new(); + builder.validate(); +} + +#[test(should_fail_with="call stack storage address does not match expected contract address")] +fn validate_call_is_regular_mismatch_storage_contract_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + // Change the storage contract address to be a different value. + builder.private_call.public_inputs.call_context.storage_contract_address.inner += 1; + + builder.validate(); +} + +#[test] +fn validate_call_is_delegate_succeeds() { + let builder = PrivateCallDataValidatorBuilder::new().is_delegate_call(); + builder.validate(); +} + +#[test(should_fail_with="current contract address must not match storage contract address for delegate calls")] +fn validate_call_is_delegate_call_from_same_contract_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new().is_delegate_call(); + + // Change the caller's storage contract address to be the same as the contract address. + builder.private_call.public_inputs.call_context.storage_contract_address = builder.private_call.contract_address; + + builder.validate(); +} + +#[test] +fn validate_call_is_static_succeeds() { + let mut builder = PrivateCallDataValidatorBuilder::new().is_static_call(); + builder.validate(); +} + +#[test(should_fail_with="call stack storage address does not match expected contract address")] +fn validate_call_is_static_mismatch_storage_contract_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new().is_static_call(); + + // Change the storage contract address to be a different value. + builder.private_call.public_inputs.call_context.storage_contract_address.inner += 1; + + builder.validate(); +} + +#[test(should_fail_with="new_note_hashes must be empty for static calls")] +fn validate_call_is_static_creating_note_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new().is_static_call(); + + builder.private_call.public_inputs.new_note_hashes.push(NoteHash { value: 1, counter: 0 }); + + builder.validate(); +} + +#[test(should_fail_with="new_nullifiers must be empty for static calls")] +fn validate_call_is_static_creating_nullifiers_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new().is_static_call(); + + builder.private_call.public_inputs.new_nullifiers.push(Nullifier { value: 1, counter: 0, note_hash: 0 }); + + builder.validate(); +} + +#[test(should_fail_with="new_l2_to_l1_msgs must be empty for static calls")] +fn validate_call_is_static_creating_l2_to_l1_msgs_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new().is_static_call(); + + builder.private_call.public_inputs.new_l2_to_l1_msgs.push(L2ToL1Message { recipient: EthAddress::from_field(6), content: 9123, counter: 0 }); + + builder.validate(); +} + +#[test(should_fail_with="encrypted_logs_hashes must be empty for static calls")] +fn validate_call_is_static_creating_encrypted_logs_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new().is_static_call(); + + builder.private_call.public_inputs.encrypted_logs_hashes.push(SideEffect { value: 9123, counter: 1 }); + + builder.validate(); +} + +#[test(should_fail_with="unencrypted_logs_hashes must be empty for static calls")] +fn validate_call_is_static_creating_unencrypted_logs_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new().is_static_call(); + + builder.private_call.public_inputs.unencrypted_logs_hashes.push(SideEffect { value: 9123, counter: 1 }); + + builder.validate(); +} + +/** + * validate_private_call_requests + */ + +#[test] +fn validate_private_call_requests_succeeds() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_private_call_requests(2, false); + + builder.validate(); +} + +#[test] +fn validate_private_call_requests_delegate_calls_succeeds() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_private_call_requests(2, true); + + builder.validate(); +} + +#[test(should_fail_with="call stack hash does not match call request hash")] +fn validate_private_call_requests_incorrect_hash_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_private_call_requests(2, false); + let mut call_request = builder.private_call.private_call_stack.pop(); + // Change the hash to be a different value. + call_request.hash += 1; + builder.private_call.private_call_stack.push(call_request); + + builder.validate(); +} + +#[test(should_fail_with="invalid caller")] +fn validate_private_call_requests_incorrect_caller_address_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_private_call_requests(1, false); + let mut call_request = builder.private_call.private_call_stack.pop(); + // Change the caller contract address to be a different value. + call_request.caller_contract_address.inner += 1; + builder.private_call.private_call_stack.push(call_request); + + builder.validate(); +} + +#[test(should_fail_with="invalid caller")] +fn validate_private_call_requests_incorrect_caller_storage_contract_address_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_private_call_requests(1, true); + let mut call_request = builder.private_call.private_call_stack.pop(); + // Change the storage contract to be a different value. + call_request.caller_context.storage_contract_address.inner += 1; + builder.private_call.private_call_stack.push(call_request); + + builder.validate(); +} + +#[test(should_fail_with="invalid caller")] +fn validate_private_call_requests_incorrect_caller_msg_sender_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_private_call_requests(1, true); + let mut call_request = builder.private_call.private_call_stack.pop(); + // Change the msg_sender to be a different value. + call_request.caller_context.msg_sender.inner += 1; + builder.private_call.private_call_stack.push(call_request); + + builder.validate(); +} + +#[test(should_fail_with="call requests length does not match the expected length")] +fn validate_private_call_requests_fewer_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_private_call_requests(2, false); + // Remove one call stack item hash. + let _ = builder.private_call.public_inputs.private_call_stack_hashes.pop(); + + builder.validate(); +} + +#[test(should_fail_with="call stack hash does not match call request hash")] +fn validate_private_call_requests_more_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_private_call_requests(2, false); + // Add one random call stack item hash. + builder.private_call.public_inputs.private_call_stack_hashes.push(9123); + + builder.validate(); +} + +/** + * validate_public_call_requests + */ + +#[test] +fn validate_public_call_requests_succeeds() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_public_call_requests(2, false); + + builder.validate(); +} + +#[test] +fn validate_public_call_requests_delegate_calls_succeeds() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_public_call_requests(2, true); + + builder.validate(); +} + +#[test(should_fail_with="call stack hash does not match call request hash")] +fn validate_public_call_requests_incorrect_hash_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_public_call_requests(2, false); + let mut call_request = builder.private_call.public_call_stack.pop(); + // Change the hash to be a different value. + call_request.hash += 1; + builder.private_call.public_call_stack.push(call_request); + + builder.validate(); +} + +#[test(should_fail_with="invalid caller")] +fn validate_public_call_requests_incorrect_caller_address_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_public_call_requests(1, false); + let mut call_request = builder.private_call.public_call_stack.pop(); + // Change the caller contract address to be a different value. + call_request.caller_contract_address.inner += 1; + builder.private_call.public_call_stack.push(call_request); + + builder.validate(); +} + +#[test(should_fail_with="invalid caller")] +fn validate_public_call_requests_incorrect_caller_storage_contract_address_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_public_call_requests(1, true); + let mut call_request = builder.private_call.public_call_stack.pop(); + // Change the storage contract to be a different value. + call_request.caller_context.storage_contract_address.inner += 1; + builder.private_call.public_call_stack.push(call_request); + + builder.validate(); +} + +#[test(should_fail_with="invalid caller")] +fn validate_public_call_requests_incorrect_caller_msg_sender_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_public_call_requests(1, true); + let mut call_request = builder.private_call.public_call_stack.pop(); + // Change the msg_sender to be a different value. + call_request.caller_context.msg_sender.inner += 1; + builder.private_call.public_call_stack.push(call_request); + + builder.validate(); +} + +#[test(should_fail_with="call requests length does not match the expected length")] +fn validate_public_call_requests_fewer_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_public_call_requests(2, false); + // Remove one call stack item hash. + let _ = builder.private_call.public_inputs.public_call_stack_hashes.pop(); + + builder.validate(); +} + +#[test(should_fail_with="call stack hash does not match call request hash")] +fn validate_public_call_requests_more_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.append_public_call_requests(2, false); + // Add one random call stack item hash. + builder.private_call.public_inputs.public_call_stack_hashes.push(9123); + + builder.validate(); +} + +/** + * validate_teardown_call_request + */ + +#[test] +fn validate_teardown_call_request_succeeds() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.add_teaddown_call_request(false); + + builder.validate(); +} + +#[test] +fn validate_teardown_call_request_delegate_calls_succeeds() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.add_teaddown_call_request(true); + + builder.validate(); +} + +#[test(should_fail_with="call stack hash does not match call request hash")] +fn validate_teardown_call_request_incorrect_hash_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.add_teaddown_call_request(true); + // Change the hash to be a different value. + builder.private_call.public_teardown_call_request.hash += 1; + + builder.validate(); +} + +#[test(should_fail_with="invalid caller")] +fn validate_teardown_call_request_incorrect_caller_address_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.add_teaddown_call_request(true); + // Change the caller contract address to be a different value. + builder.private_call.public_teardown_call_request.caller_contract_address.inner += 1; + + builder.validate(); +} + +#[test(should_fail_with="invalid caller")] +fn validate_teardown_call_request_incorrect_caller_storage_contract_address_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.add_teaddown_call_request(true); + // Change the storage contract to be a different value. + builder.private_call.public_teardown_call_request.caller_context.storage_contract_address.inner += 1; + + builder.validate(); +} + +#[test(should_fail_with="invalid caller")] +fn validate_teardown_call_request_incorrect_caller_msg_sender_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.add_teaddown_call_request(true); + // Change the msg_sender to be a different value. + builder.private_call.public_teardown_call_request.caller_context.msg_sender.inner += 1; + + builder.validate(); +} + +#[test(should_fail_with="call requests length does not match the expected length")] +fn validate_teardown_call_request_fewer_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.add_teaddown_call_request(true); + // Remove the call stack item hash. + builder.private_call.public_inputs.public_teardown_function_hash = 0; + + builder.validate(); +} + +#[test(should_fail_with="call stack hash does not match call request hash")] +fn validate_teardown_call_request_more_hashes_fails() { + let mut builder = PrivateCallDataValidatorBuilder::new(); + + builder.private_call.add_teaddown_call_request(true); + // Remove the call request. + builder.private_call.public_teardown_call_request = CallRequest::empty(); + + builder.validate(); +} + +/** + * validate_against_tx_request + */ + +#[test] +fn validate_against_tx_request_succeeds() { + let builder = PrivateCallDataValidatorBuilder::new(); + + let request = builder.private_call.build_tx_request(); + + builder.validate_against_tx_request(request); +} + +#[test(should_fail_with="origin address does not match call stack items contract address")] +fn validate_against_tx_request_mismatch_contract_address_fails() { + let builder = PrivateCallDataValidatorBuilder::new(); + + let mut request = builder.private_call.build_tx_request(); + // Tweak the origin to be a different value. + request.origin.inner += 1; + + builder.validate_against_tx_request(request); +} + +#[test(should_fail_with="tx_request function_data must match call_stack_item function_data")] +fn validate_against_tx_request_mismatch_function_data_fails() { + let builder = PrivateCallDataValidatorBuilder::new(); + + let mut request = builder.private_call.build_tx_request(); + // Tweak the function selector to be a different value. + request.function_data.selector.inner += 1; + + builder.validate_against_tx_request(request); +} + +#[test(should_fail_with="noir function args passed to tx_request must match args in the call_stack_item")] +fn validate_against_tx_request_mismatch_args_hash_fails() { + let builder = PrivateCallDataValidatorBuilder::new(); + + let mut request = builder.private_call.build_tx_request(); + // Tweak the args hash to be a different value. + request.args_hash += 1; + + builder.validate_against_tx_request(request); +} + +#[test(should_fail_with="tx_context in tx_request must match tx_context in call_stack_item")] +fn validate_against_tx_request_mismatch_chain_id_fails() { + let builder = PrivateCallDataValidatorBuilder::new(); + + let mut request = builder.private_call.build_tx_request(); + // Tweak the chain id to be a different value. + request.tx_context.chain_id += 1; + + builder.validate_against_tx_request(request); +} + +#[test(should_fail_with="tx_context in tx_request must match tx_context in call_stack_item")] +fn validate_against_tx_request_mismatch_version_fails() { + let builder = PrivateCallDataValidatorBuilder::new(); + + let mut request = builder.private_call.build_tx_request(); + // Tweak the version to be a different value. + request.tx_context.version += 1; + + builder.validate_against_tx_request(request); +} + +#[test(should_fail_with="Users cannot make a static call")] +fn validate_against_tx_request_static_call_fails() { + let builder = PrivateCallDataValidatorBuilder::new().is_static_call(); + let request = builder.private_call.build_tx_request(); + builder.validate_against_tx_request(request); +} + +#[test(should_fail_with="Users cannot make a delegatecall")] +fn validate_against_tx_request_delegate_call_fails() { + let builder = PrivateCallDataValidatorBuilder::new().is_delegate_call(); + let request = builder.private_call.build_tx_request(); + builder.validate_against_tx_request(request); +} + +/** + * validate_against_call_request + */ + +#[test] +fn validate_against_call_request_succeeds() { + let builder = PrivateCallDataValidatorBuilder::new(); + + let request = builder.private_call.build_call_request(); + + builder.validate_against_call_request(request); +} + +#[test] +fn validate_against_call_request_delegate_call_succeeds() { + let builder = PrivateCallDataValidatorBuilder::new().is_delegate_call(); + + let request = builder.private_call.build_call_request(); + + builder.validate_against_call_request(request); +} + +#[test] +fn validate_against_call_request_static_call_succeeds() { + let builder = PrivateCallDataValidatorBuilder::new().is_static_call(); + + let request = builder.private_call.build_call_request(); + + builder.validate_against_call_request(request); +} + +#[test(should_fail_with="calculated private_call_hash does not match provided private_call_hash at the top of the call stack")] +fn validate_against_call_request_mismatch_hash_fails() { + let builder = PrivateCallDataValidatorBuilder::new(); + + let mut request = builder.private_call.build_call_request(); + // Tweak the hash to be a different value. + request.hash += 1; + + builder.validate_against_call_request(request); +} + +#[test(should_fail_with="caller context cannot be empty for delegate calls")] +fn validate_against_call_request_empty_caller_context_for_delegate_calls_fails() { + let builder = PrivateCallDataValidatorBuilder::new().is_delegate_call(); + + let mut request = builder.private_call.build_call_request(); + request.caller_context = CallerContext::empty(); + + builder.validate_against_call_request(request); +} + +#[test(should_fail_with="call stack msg_sender does not match expected msg_sender for delegate calls")] +fn validate_against_call_request_incorrect_msg_sender_for_delegate_call_fails() { + let builder = PrivateCallDataValidatorBuilder::new().is_delegate_call(); + + let mut request = builder.private_call.build_call_request(); + // Tweak the msg_sender to be a different value. + request.caller_context.msg_sender.inner += 1; + + builder.validate_against_call_request(request); +} + +#[test(should_fail_with="call stack storage address does not match expected contract address for delegate calls")] +fn validate_against_call_request_incorrect_storage_contract_address_for_delegate_call_fails() { + let builder = PrivateCallDataValidatorBuilder::new().is_delegate_call(); + + let mut request = builder.private_call.build_call_request(); + // Tweak the storage contract address to be a different value. + request.caller_context.storage_contract_address.inner += 1; + + builder.validate_against_call_request(request); +} + +#[test(should_fail_with="call stack msg_sender does not match caller contract address")] +fn validate_against_call_request_incorrect_msg_sender_for_regular_call_fails() { + let builder = PrivateCallDataValidatorBuilder::new(); + + let mut request = builder.private_call.build_call_request(); + // Tweak the caller's contract address to be a different value. + request.caller_contract_address.inner += 1; + + builder.validate_against_call_request(request); +} diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/call_request.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/call_request.nr index 140b1967ca7..2c667c25f08 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/call_request.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/call_request.nr @@ -34,12 +34,6 @@ impl Empty for CallRequest { } } -impl CallRequest { - pub fn is_empty(self) -> bool { - self.hash == 0 - } -} - impl Serialize for CallRequest { fn serialize(self) -> [Field; CALL_REQUEST_LENGTH] { let mut fields: BoundedVec = BoundedVec::new(); diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs.nr index c385a96f25a..06722dfce8e 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/kernel_circuit_public_inputs/public_kernel_circuit_public_inputs.nr @@ -17,17 +17,17 @@ impl PublicKernelCircuitPublicInputs { pub fn needs_setup(self) -> bool { // public calls for setup are deposited in the non-revertible public call stack. // if an element is present, we need to run setup - !self.end_non_revertible.public_call_stack[0].is_empty() + self.end_non_revertible.public_call_stack[0].hash != 0 } pub fn needs_app_logic(self) -> bool { // public calls for app logic are deposited in the revertible public call stack. // if an element is present, we need to run app logic - !self.end.public_call_stack[0].is_empty() + self.end.public_call_stack[0].hash != 0 } pub fn needs_teardown(self) -> bool { // the public call specified for teardown, if any, is placed in the teardown call stack - !self.public_teardown_call_stack[0].is_empty() + self.public_teardown_call_stack[0].hash != 0 } } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/tests/private_call_data_builder.nr b/noir-projects/noir-protocol-circuits/crates/types/src/tests/private_call_data_builder.nr index 999da287c85..68154e67806 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/tests/private_call_data_builder.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/tests/private_call_data_builder.nr @@ -75,7 +75,7 @@ impl PrivateCallDataBuilder { } pub fn build_tx_request(self) -> TxRequest { - let tx_context = self.build_tx_context(); + let tx_context = self.public_inputs.build_tx_context(); TxRequest { origin: self.contract_address, args_hash: self.public_inputs.args_hash, @@ -84,8 +84,24 @@ impl PrivateCallDataBuilder { } } - pub fn build_tx_context(self) -> TxContext { - self.public_inputs.build_tx_context() + pub fn build_call_request(self) -> CallRequest { + let hash = self.build_call_stack_item().hash(); + let is_delegate_call = self.public_inputs.call_context.is_delegate_call; + let caller_context = if is_delegate_call { + CallerContext { + msg_sender: fixtures::MSG_SENDER, + storage_contract_address: self.public_inputs.call_context.storage_contract_address + } + } else { + CallerContext::empty() + }; + CallRequest { + hash, + caller_contract_address: self.public_inputs.call_context.msg_sender, + caller_context, + start_side_effect_counter: 0, + end_side_effect_counter: 0 + } } pub fn append_private_call_requests(&mut self, num_requests: u64, is_delegate_call: bool) { @@ -100,6 +116,12 @@ impl PrivateCallDataBuilder { self.public_call_stack.extend_from_bounded_vec(call_requests); } + pub fn add_teaddown_call_request(&mut self, is_delegate_call: bool) { + let hash = 99887654; + self.public_inputs.public_teardown_function_hash = hash; + self.public_teardown_call_request = self.generate_call_request(hash, is_delegate_call); + } + fn generate_call_requests( self, requests: BoundedVec, @@ -107,12 +129,6 @@ impl PrivateCallDataBuilder { is_delegate_call: bool ) -> (BoundedVec, BoundedVec) { let value_offset = requests.len(); - let mut caller_context = CallerContext::empty(); - if is_delegate_call { - let call_context = self.public_inputs.call_context; - caller_context.msg_sender = call_context.msg_sender; - caller_context.storage_contract_address = call_context.storage_contract_address; - } let mut call_requests: BoundedVec = BoundedVec::new(); let mut hashes: BoundedVec = BoundedVec::new(); let mut exceeded_len = false; @@ -121,14 +137,7 @@ impl PrivateCallDataBuilder { if !exceeded_len { // The default hash is its index + 7788. let hash = (value_offset + 7788) as Field; - let request = CallRequest { - hash, - caller_contract_address: self.contract_address, - caller_context, - // TODO: populate these - start_side_effect_counter: 0, - end_side_effect_counter: 0 - }; + let request = self.generate_call_request(hash, is_delegate_call); hashes.push(hash); call_requests.push(request); } @@ -136,6 +145,23 @@ impl PrivateCallDataBuilder { (hashes, call_requests) } + fn generate_call_request(self, hash: Field, is_delegate_call: bool) -> CallRequest { + let mut caller_context = CallerContext::empty(); + if is_delegate_call { + let call_context = self.public_inputs.call_context; + caller_context.msg_sender = call_context.msg_sender; + caller_context.storage_contract_address = call_context.storage_contract_address; + } + CallRequest { + hash, + caller_contract_address: self.contract_address, + caller_context, + // TODO: populate these + start_side_effect_counter: 0, + end_side_effect_counter: 0 + } + } + pub fn set_tx_max_block_number(&mut self, max_block_number: u32) { self.public_inputs.max_block_number = MaxBlockNumber::new(max_block_number); } diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr index 52c277355c8..a84baf83a2a 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr @@ -16,21 +16,18 @@ pub fn array_to_bounded_vec(array: [T; N]) -> BoundedVec where T: Em // Routine which validates that all zero values of an array form a contiguous region at the end, i.e., // of the form: [*,*,*...,0,0,0,0] where any * is non-zero. Note that a full array of non-zero values is // valid. -pub fn validate_array(array: [T; N]) where T: Empty + Eq { - let array_length = array.len(); - - let mut first_zero_pos = array_length; - let mut last_non_zero_pos = 0; - - for i in 0..array_length { - let is_empty = is_empty(array[i]); - if !is_empty { - last_non_zero_pos = i; - } else if is_empty & (first_zero_pos == array_length) { - first_zero_pos = i; +pub fn validate_array(array: [T; N]) -> u64 where T: Empty + Eq { + let mut seen_empty = false; + let mut length = 0; + for i in 0..N { + if is_empty(array[i]) { + seen_empty = true; + } else { + assert(seen_empty == false, "invalid array"); + length += 1; } } - assert(last_non_zero_pos <= first_zero_pos, "invalid array"); + length } // Helper method to determine the number of non-zero/empty elements in a validated array (ie, validate_array(array) @@ -147,31 +144,40 @@ pub fn assert_sorted_array( #[test] fn smoke_validate_array() { let valid_array: [Field; 0] = []; - validate_array(valid_array); + assert(validate_array(valid_array) == 0); let valid_array = [0]; - validate_array(valid_array); + assert(validate_array(valid_array) == 0); + + let valid_array = [3]; + assert(validate_array(valid_array) == 1); let valid_array = [1, 2, 3]; - validate_array(valid_array); + assert(validate_array(valid_array) == 3); let valid_array = [1, 2, 3, 0]; - validate_array(valid_array); + assert(validate_array(valid_array) == 3); let valid_array = [1, 2, 3, 0, 0]; - validate_array(valid_array); + assert(validate_array(valid_array) == 3); } #[test(should_fail_with = "invalid array")] -fn smoke_validate_array_invalid() { +fn smoke_validate_array_invalid_case0() { let invalid_array = [0, 1]; - validate_array(invalid_array); + let _ = validate_array(invalid_array); +} + +#[test(should_fail_with = "invalid array")] +fn smoke_validate_array_invalid_case1() { + let invalid_array = [1, 0, 0, 1, 0]; + let _ = validate_array(invalid_array); } #[test(should_fail_with = "invalid array")] fn smoke_validate_array_invalid_case2() { let invalid_array = [0, 0, 0, 0, 1]; - validate_array(invalid_array); + let _ = validate_array(invalid_array); } #[test]