From a6b6c7b0e7b156b72462259b7ea8ead7f42f428b Mon Sep 17 00:00:00 2001 From: Jean M <132435771+jeanmon@users.noreply.github.com> Date: Fri, 29 Sep 2023 08:12:49 +0200 Subject: [PATCH] chore(circuits): Remove obsolete comments in native private kernel circuit (#2570) Clean up comments # Checklist: Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge. - [x] If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag. - [x] I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code. - [x] Every change is related to the PR description. - [ ] I have [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this pull request to relevant issues (if any exist). --- .../native_private_kernel_circuit_init.cpp | 34 ++----------------- .../native_private_kernel_circuit_init.hpp | 1 - .../native_private_kernel_circuit_inner.cpp | 32 ++--------------- 3 files changed, 4 insertions(+), 63 deletions(-) diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.cpp index 53df785c288..9e70e541a0a 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.cpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.cpp @@ -41,28 +41,6 @@ void initialise_end_values(PrivateKernelInputsInit<NT> const& private_inputs, namespace aztec3::circuits::kernel::private_kernel { -// using plonk::stdlib::merkle_tree:: - -// // TODO: NEED TO RECONCILE THE `proof`'s public inputs (which are uint8's) with the -// // private_call.call_stack_item.public_inputs! -// CT::AggregationObject verify_proofs(Builder& builder, -// PrivateKernelInputsInit<CT> const& private_inputs, -// size_t const& num_private_call_public_inputs, -// size_t const& num_private_kernel_public_inputs) -// { -// CT::AggregationObject aggregation_object = Aggregator::aggregate( -// &builder, private_inputs.private_call.vk, private_inputs.private_call.proof, -// num_private_call_public_inputs); - -// Aggregator::aggregate(&builder, -// private_inputs.previous_kernel.vk, -// private_inputs.previous_kernel.proof, -// num_private_kernel_public_inputs, -// aggregation_object); - -// return aggregation_object; -// } - void validate_this_private_call_against_tx_request(DummyCircuitBuilder& builder, PrivateKernelInputsInit<NT> const& private_inputs) { @@ -171,9 +149,6 @@ void update_end_values(DummyCircuitBuilder& builder, // https://github.com/AztecProtocol/aztec-packages/issues/660 } -// NOTE: THIS IS A VERY UNFINISHED WORK IN PROGRESS. -// TODO(mike): is there a way to identify whether an input has not been used by ths circuit? This would help us -// more-safely ensure we're constraining everything. KernelCircuitPublicInputs<NT> native_private_kernel_circuit_initial(DummyCircuitBuilder& builder, PrivateKernelInputsInit<NT> const& private_inputs) { @@ -205,13 +180,8 @@ KernelCircuitPublicInputs<NT> native_private_kernel_circuit_initial(DummyCircuit private_inputs.tx_request.tx_context.contract_deployment_data, private_inputs.tx_request.function_data); - // We'll skip any verification in this native implementation, because for a Local Developer Testnet, there won't - // _be_ a valid proof to verify!!! auto aggregation_object = verify_proofs(builder, - // private_inputs, - // _private_inputs.private_call.vk->num_public_inputs, - // _private_inputs.previous_kernel.vk->num_public_inputs); - - // TODO(dbanks12): kernel vk membership check! + // This is where a real circuit would perform recursive verification of the previous kernel proof and private call + // proof. // In the native version, as there is no verify_proofs call, we can initialize aggregation object with the default // constructor. diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.hpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.hpp index b5f23f4a928..8baabb77f1f 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.hpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.hpp @@ -10,7 +10,6 @@ namespace aztec3::circuits::kernel::private_kernel { using aztec3::circuits::abis::KernelCircuitPublicInputs; using aztec3::circuits::abis::private_kernel::PrivateKernelInputsInit; -// using abis::private_kernel::PublicInputs; using DummyBuilder = aztec3::utils::DummyCircuitBuilder; KernelCircuitPublicInputs<NT> native_private_kernel_circuit_initial(DummyBuilder& builder, diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_inner.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_inner.cpp index db7662394fc..4312d82c04d 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_inner.cpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_inner.cpp @@ -35,26 +35,6 @@ void initialise_end_values(PreviousKernelData<NT> const& previous_kernel, Kernel namespace aztec3::circuits::kernel::private_kernel { -// // TODO: NEED TO RECONCILE THE `proof`'s public inputs (which are uint8's) with the -// // private_call.call_stack_item.public_inputs! -// CT::AggregationObject verify_proofs(Builder& builder, -// PrivateInputs<CT> const& private_inputs, -// size_t const& num_private_call_public_inputs, -// size_t const& num_private_kernel_public_inputs) -// { -// CT::AggregationObject aggregation_object = Aggregator::aggregate( -// &builder, private_inputs.private_call.vk, private_inputs.private_call.proof, -// num_private_call_public_inputs); - -// Aggregator::aggregate(&builder, -// private_inputs.previous_kernel.vk, -// private_inputs.previous_kernel.proof, -// num_private_kernel_public_inputs, -// aggregation_object); - -// return aggregation_object; -// } - void pop_and_validate_this_private_call_hash( DummyCircuitBuilder& builder, PrivateCallData<NT> const& private_call, @@ -115,9 +95,6 @@ void validate_inputs(DummyCircuitBuilder& builder, PrivateKernelInputsInner<NT> common_validate_0th_nullifier(builder, private_inputs.previous_kernel.public_inputs.end); } -// NOTE: THIS IS A VERY UNFINISHED WORK IN PROGRESS. -// TODO(mike): is there a way to identify whether an input has not been used by ths circuit? This would help us -// more-safely ensure we're constraining everything. KernelCircuitPublicInputs<NT> native_private_kernel_circuit_inner(DummyCircuitBuilder& builder, PrivateKernelInputsInner<NT> const& private_inputs) { @@ -153,13 +130,8 @@ KernelCircuitPublicInputs<NT> native_private_kernel_circuit_inner(DummyCircuitBu private_call_stack_item.public_inputs.contract_deployment_data, private_call_stack_item.function_data); - // We'll skip any verification in this native implementation, because for a Local Developer Testnet, there won't - // _be_ a valid proof to verify!!! auto aggregation_object = verify_proofs(builder, - // private_inputs, - // _private_inputs.private_call.vk->num_public_inputs, - // _private_inputs.previous_kernel.vk->num_public_inputs); - - // TODO(dbanks12): kernel vk membership check! + // This is where a real circuit would perform recursive verification of the previous kernel proof and private call + // proof. // Note: given that we skipped the verify_proof function, the aggregation object we get at the end will just be // the same as we had at the start. public_inputs.end.aggregation_object = aggregation_object;