From ab5ffde84920a9821a20414dae7c9a14b3665b38 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Thu, 28 Sep 2023 11:08:04 +0000 Subject: [PATCH 1/3] 1329 - enforce that 0th nullifier is non-zero in private kernel circuit (inner and ordering) --- circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp | 7 +++++++ circuits/cpp/src/aztec3/circuits/kernel/private/common.hpp | 4 ++++ .../kernel/private/native_private_kernel_circuit_inner.cpp | 2 +- .../private/native_private_kernel_circuit_ordering.cpp | 3 ++- circuits/cpp/src/aztec3/utils/circuit_errors.hpp | 1 + 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp index 27e1f2ffddb..adf1d70353a 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp @@ -114,6 +114,13 @@ void common_validate_read_requests(DummyBuilder& builder, } } +void common_validate_0th_nullifier(DummyBuilder& builder, CombinedAccumulatedData const& end) +{ + builder.do_assert(end.new_nullifiers[0] != 0, + "The 0th nullifier in the accumulated nullifier array is zero", + CircuitErrorCode::PRIVATE_KERNEL__0TH_NULLLIFIER_IS_ZERO); +} + void common_update_end_values(DummyBuilder& builder, PrivateCallData const& private_call, KernelCircuitPublicInputs& public_inputs) diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/common.hpp b/circuits/cpp/src/aztec3/circuits/kernel/private/common.hpp index 52e26279d7b..32f231262cc 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/common.hpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/common.hpp @@ -2,6 +2,7 @@ #include "init.hpp" +#include "aztec3/circuits/abis/combined_accumulated_data.hpp" #include "aztec3/circuits/abis/contract_deployment_data.hpp" #include "aztec3/circuits/abis/function_data.hpp" #include "aztec3/circuits/abis/kernel_circuit_public_inputs.hpp" @@ -13,6 +14,7 @@ namespace aztec3::circuits::kernel::private_kernel { +using aztec3::circuits::abis::CombinedAccumulatedData; using aztec3::circuits::abis::ContractDeploymentData; using aztec3::circuits::abis::FunctionData; using aztec3::circuits::abis::KernelCircuitPublicInputs; @@ -32,6 +34,8 @@ void common_validate_read_requests(DummyBuilder& builder, std::array, MAX_READ_REQUESTS_PER_CALL> const& read_request_membership_witnesses); +void common_validate_0th_nullifier(DummyBuilder& builder, CombinedAccumulatedData const& end); + void common_update_end_values(DummyBuilder& builder, PrivateCallData const& private_call, KernelCircuitPublicInputs& public_inputs); 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 fa8337b6656..db7662394fc 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 @@ -112,7 +112,7 @@ void validate_inputs(DummyCircuitBuilder& builder, PrivateKernelInputsInner "Cannot execute private kernel circuit with an empty private call stack", CircuitErrorCode::PRIVATE_KERNEL__PRIVATE_CALL_STACK_EMPTY); - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/1329): validate that 0th nullifier is nonzero + common_validate_0th_nullifier(builder, private_inputs.previous_kernel.public_inputs.end); } // NOTE: THIS IS A VERY UNFINISHED WORK IN PROGRESS. diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.cpp index c1fd7362e45..3e1f39183bb 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.cpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.cpp @@ -161,7 +161,8 @@ KernelCircuitPublicInputsFinal native_private_kernel_circuit_ordering( // Do this before any functions can modify the inputs. initialise_end_values(private_inputs.previous_kernel, public_inputs); - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/1329): validate that 0th nullifier is nonzero + common_validate_0th_nullifier(builder, private_inputs.previous_kernel.public_inputs.end); + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/1486): validate that `len(new_nullifiers) == // len(nullified_commitments)` diff --git a/circuits/cpp/src/aztec3/utils/circuit_errors.hpp b/circuits/cpp/src/aztec3/utils/circuit_errors.hpp index ecb297c0321..f3a46976397 100644 --- a/circuits/cpp/src/aztec3/utils/circuit_errors.hpp +++ b/circuits/cpp/src/aztec3/utils/circuit_errors.hpp @@ -34,6 +34,7 @@ enum CircuitErrorCode : uint16_t { PRIVATE_KERNEL__UNRESOLVED_NON_TRANSIENT_READ_REQUEST = 2021, PRIVATE_KERNEL__IS_INTERNAL_BUT_NOT_SELF_CALL = 2022, PRIVATE_KERNEL__TRANSIENT_NEW_NULLIFIER_NO_MATCH = 2023, + PRIVATE_KERNEL__0TH_NULLLIFIER_IS_ZERO = 2024, // Public kernel related errors PUBLIC_KERNEL_CIRCUIT_FAILED = 3000, From 36969b8204e41c57389c2a6b1bfc835cb6c56ea1 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Thu, 28 Sep 2023 11:39:20 +0000 Subject: [PATCH 2/3] 1329 - fix unit tests --- .../native_private_kernel_circuit.test.cpp | 17 ++++++++++------- .../circuits/kernel/private/testing_harness.cpp | 2 ++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp index f9e0aef3d2f..c2e31a04af1 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.test.cpp @@ -165,6 +165,8 @@ TEST_F(native_private_kernel_tests, native_transient_read_requests_no_match) // Testing that the special value EMPTY_NULLIFIED_COMMITMENT keeps new_nullifiers aligned with nullified_commitments. TEST_F(native_private_kernel_tests, native_empty_nullified_commitment_respected) { + // The 0th nullifier (and corresponding nullified_commitment EMPTY_NULLIFIED_COMMITMENT) which is + // added by the init iteration is set into private_inputs_inner. auto private_inputs_inner = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args()); private_inputs_inner.private_call.call_stack_item.public_inputs.new_commitments[0] = fr(23); @@ -189,22 +191,23 @@ TEST_F(native_private_kernel_tests, native_empty_nullified_commitment_respected) << " with code: " << builder.get_first_failure().code; // EMPTY nullified commitment should keep new_nullifiers aligned with nullified_commitments - ASSERT_TRUE(public_inputs.end.nullified_commitments[0] == fr(EMPTY_NULLIFIED_COMMITMENT)); - ASSERT_TRUE(public_inputs.end.nullified_commitments[1] != fr(0) && - public_inputs.end.nullified_commitments[1] != fr(EMPTY_NULLIFIED_COMMITMENT)); + // We have to shift the offset by 1 due to the 0th nullifier/nullified_commitment pair. + ASSERT_TRUE(public_inputs.end.nullified_commitments[1] == fr(EMPTY_NULLIFIED_COMMITMENT)); + ASSERT_TRUE(public_inputs.end.nullified_commitments[2] != fr(0) && + public_inputs.end.nullified_commitments[2] != fr(EMPTY_NULLIFIED_COMMITMENT)); // Nothing squashed yet (until ordering circuit) - ASSERT_TRUE(array_length(public_inputs.end.new_nullifiers) == 2); + ASSERT_TRUE(array_length(public_inputs.end.new_nullifiers) == 3); // 0th nullifier to be taking into account ASSERT_TRUE(array_length(public_inputs.end.new_commitments) == 2); // explicitly EMPTY commitment respected in array - ASSERT_TRUE(array_length(public_inputs.end.nullified_commitments) == 2); + ASSERT_TRUE(array_length(public_inputs.end.nullified_commitments) == 3); auto& previous_kernel = private_inputs_inner.previous_kernel; previous_kernel.public_inputs = public_inputs; PrivateKernelInputsOrdering private_inputs{ .previous_kernel = previous_kernel, .nullifier_commitment_hints = - std::array{ 0, 1 } }; + std::array{ 0, 0, 1 } }; auto final_public_inputs = native_private_kernel_circuit_ordering(builder, private_inputs); @@ -212,7 +215,7 @@ TEST_F(native_private_kernel_tests, native_empty_nullified_commitment_respected) << " with code: " << builder.get_first_failure().code; ASSERT_TRUE(array_length(final_public_inputs.end.new_commitments) == 1); // 1/2 commitment squashed - ASSERT_TRUE(array_length(final_public_inputs.end.new_nullifiers) == 1); // 1/2 nullifier squashed + ASSERT_TRUE(array_length(final_public_inputs.end.new_nullifiers) == 2); // 1/2 nullifier squashed (+ 0th nullifier) } } // namespace aztec3::circuits::kernel::private_kernel diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/testing_harness.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/testing_harness.cpp index 66ed5e2774e..22cce3a6819 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/testing_harness.cpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/testing_harness.cpp @@ -501,6 +501,8 @@ PrivateKernelInputsInner do_private_call_get_kernel_inputs_inner( public_inputs_encrypted_log_preimages_length; mock_previous_kernel.public_inputs.end.unencrypted_log_preimages_length = public_inputs_unencrypted_log_preimages_length; + mock_previous_kernel.public_inputs.end.new_nullifiers[0] = 321; // 0th nullifier must be non-zero. + mock_previous_kernel.public_inputs.end.nullified_commitments[0] = EMPTY_NULLIFIED_COMMITMENT; //*************************************************************************** // Now we can construct the full private inputs to the kernel circuit From 7014f6c9b9956766fa154d3ec3f4ac2926e23dff Mon Sep 17 00:00:00 2001 From: jeanmon Date: Thu, 28 Sep 2023 11:53:51 +0000 Subject: [PATCH 3/3] 1329 - add unit test for ordering and inner native private kernel circuit on 0-th nullifier being non-zero --- ...native_private_kernel_circuit_inner.test.cpp | 16 ++++++++++++++++ ...ive_private_kernel_circuit_ordering.test.cpp | 17 +++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_inner.test.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_inner.test.cpp index 91372b05f09..77a8c7cfa65 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_inner.test.cpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_inner.test.cpp @@ -745,4 +745,20 @@ TEST_F(native_private_kernel_inner_tests, native_logs_are_hashed_as_expected) ASSERT_EQ(public_inputs.end.unencrypted_logs_hash, expected_unencrypted_logs_hash); } +TEST_F(native_private_kernel_inner_tests, 0th_nullifier_zero_fails) +{ + auto private_inputs = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args()); + + // Change the 0th nullifier to be zero. + private_inputs.previous_kernel.public_inputs.end.new_nullifiers[0] = 0; + + // Invoke the native private kernel circuit + DummyBuilder builder = DummyBuilder("private_kernel_tests__0th_nullifier_zero_fails"); + native_private_kernel_circuit_inner(builder, private_inputs); + + // Assertion checks + EXPECT_TRUE(builder.failed()); + EXPECT_EQ(builder.get_first_failure().code, CircuitErrorCode::PRIVATE_KERNEL__0TH_NULLLIFIER_IS_ZERO); +} + } // namespace aztec3::circuits::kernel::private_kernel diff --git a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.test.cpp b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.test.cpp index 829366423c8..3af7fcfd34c 100644 --- a/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.test.cpp +++ b/circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_ordering.test.cpp @@ -367,4 +367,21 @@ TEST_F(native_private_kernel_ordering_tests, native_empty_nullified_commitment_m ASSERT_TRUE(array_length(public_inputs.end.new_nullifiers) == 1); } +TEST_F(native_private_kernel_ordering_tests, 0th_nullifier_zero_fails) +{ + auto private_inputs_inner = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args()); + + std::array new_nullifiers{}; + auto& previous_kernel = private_inputs_inner.previous_kernel; + previous_kernel.public_inputs.end.new_nullifiers = new_nullifiers; + PrivateKernelInputsOrdering private_inputs{ .previous_kernel = previous_kernel }; + + DummyBuilder builder = DummyBuilder("native_private_kernel_ordering_tests__0th_nullifier_zero_fails"); + auto public_inputs = native_private_kernel_circuit_ordering(builder, private_inputs); + + ASSERT_TRUE(builder.failed()); + auto failure = builder.get_first_failure(); + ASSERT_EQ(failure.code, CircuitErrorCode::PRIVATE_KERNEL__0TH_NULLLIFIER_IS_ZERO); +} + } // namespace aztec3::circuits::kernel::private_kernel