various cleanup tasks in kernel circuits and tests (#1091)

circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.cpp

@@ -129,13 +129,28 @@ void update_end_values(DummyBuilder& builder,
 {
     // We only initialized constants member of public_inputs so far. Therefore, there must not be any
     // new nullifiers or logs as part of public_inputs.
-    ASSERT(is_array_empty(public_inputs.end.new_nullifiers));
-    ASSERT(is_array_empty(public_inputs.end.encrypted_logs_hash));
-    ASSERT(is_array_empty(public_inputs.end.unencrypted_logs_hash));
-    ASSERT(is_array_empty(public_inputs.end.read_requests));
-    ASSERT(is_array_empty(public_inputs.end.read_request_membership_witnesses));
-    ASSERT(public_inputs.end.encrypted_log_preimages_length == fr(0));
-    ASSERT(public_inputs.end.unencrypted_log_preimages_length == fr(0));
+    builder.do_assert(is_array_empty(public_inputs.end.new_nullifiers),
+                      "public_inputs.end.new_nullifiers must start as empty in initial kernel iteration",
+                      CircuitErrorCode::PRIVATE_KERNEL__UNSUPPORTED_OP);
+    builder.do_assert(is_array_empty(public_inputs.end.encrypted_logs_hash),
+                      "public_inputs.end.encrypted_logs_hash must start as empty in initial kernel iteration",
+                      CircuitErrorCode::PRIVATE_KERNEL__UNSUPPORTED_OP);
+    builder.do_assert(is_array_empty(public_inputs.end.unencrypted_logs_hash),
+                      "public_inputs.end.unencrypted_logs_hash must start as empty in initial kernel iteration",
+                      CircuitErrorCode::PRIVATE_KERNEL__UNSUPPORTED_OP);
+    builder.do_assert(is_array_empty(public_inputs.end.read_requests),
+                      "public_inputs.end.read_requests must start as empty in initial kernel iteration",
+                      CircuitErrorCode::PRIVATE_KERNEL__UNSUPPORTED_OP);
+    builder.do_assert(
+        is_array_empty(public_inputs.end.read_request_membership_witnesses),
+        "public_inputs.end.read_request_membership_witnesses must start as empty in initial kernel iteration",
+        CircuitErrorCode::PRIVATE_KERNEL__UNSUPPORTED_OP);
+    builder.do_assert(public_inputs.end.encrypted_log_preimages_length == fr(0),
+                      "public_inputs.end.encrypted_log_preimages_length must start as 0 in initial kernel iteration",
+                      CircuitErrorCode::PRIVATE_KERNEL__UNSUPPORTED_OP);
+    builder.do_assert(public_inputs.end.unencrypted_log_preimages_length == fr(0),
+                      "public_inputs.end.unencrypted_log_preimages_length must start as 0 in initial kernel iteration",
+                      CircuitErrorCode::PRIVATE_KERNEL__UNSUPPORTED_OP);
 
     // Since it's the first iteration, we need to push the the tx hash nullifier into the `new_nullifiers` array
     array_push(builder, public_inputs.end.new_nullifiers, private_inputs.tx_request.hash());

circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.test.cpp

@@ -319,7 +319,7 @@ TEST_F(native_private_kernel_init_tests, native_read_request_bad_request)
 
     validate_no_new_deployed_contract(public_inputs);
 
-    ASSERT(builder.failed());
+    ASSERT_TRUE(builder.failed());
     ASSERT_EQ(builder.get_first_failure().code,
               CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_PRIVATE_DATA_ROOT_MISMATCH);
 
@@ -347,7 +347,7 @@ TEST_F(native_private_kernel_init_tests, native_read_request_bad_leaf_index)
 
     validate_no_new_deployed_contract(public_inputs);
 
-    ASSERT(builder.failed());
+    ASSERT_TRUE(builder.failed());
     ASSERT_EQ(builder.get_first_failure().code,
               CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_PRIVATE_DATA_ROOT_MISMATCH);
 
@@ -375,7 +375,7 @@ TEST_F(native_private_kernel_init_tests, native_read_request_bad_sibling_path)
 
     validate_no_new_deployed_contract(public_inputs);
 
-    ASSERT(builder.failed());
+    ASSERT_TRUE(builder.failed());
     ASSERT_EQ(builder.get_first_failure().code,
               CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_PRIVATE_DATA_ROOT_MISMATCH);
 
@@ -541,6 +541,54 @@ TEST_F(native_private_kernel_init_tests, native_max_read_requests_works)
 // Check enforcement that inner iterations' read_requests match root in constants
 // https://github.com/AztecProtocol/aztec-packages/issues/786
 
+TEST_F(native_private_kernel_init_tests, native_read_requests_less_than_witnesses)
+{
+    auto private_inputs = do_private_call_get_kernel_inputs_init(false, deposit, standard_test_args());
+
+    auto const& contract_address =
+        private_inputs.private_call.call_stack_item.public_inputs.call_context.storage_contract_address;
+
+    auto [read_requests, read_request_membership_witnesses, root] =
+        get_random_reads(contract_address, MAX_READ_REQUESTS_PER_CALL);
+
+    read_requests[MAX_READ_REQUESTS_PER_CALL - 1] = fr(0);
+    private_inputs.private_call.call_stack_item.public_inputs.historic_private_data_tree_root = root;
+    private_inputs.private_call.call_stack_item.public_inputs.read_requests = read_requests;
+    private_inputs.private_call.read_request_membership_witnesses = read_request_membership_witnesses;
+
+    DummyBuilder builder = DummyBuilder("native_private_kernel_init_tests__native_read_requests_less_than_witnesses");
+    auto const& public_inputs = native_private_kernel_circuit_initial(builder, private_inputs);
+
+    ASSERT_TRUE(builder.failed());
+    ASSERT_EQ(builder.get_first_failure().code,
+              CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_WITNESSES_ARRAY_LENGTH_MISMATCH);
+}
+
+TEST_F(native_private_kernel_init_tests, native_read_requests_more_than_witnesses)
+{
+    auto private_inputs = do_private_call_get_kernel_inputs_init(false, deposit, standard_test_args());
+
+    auto const& contract_address =
+        private_inputs.private_call.call_stack_item.public_inputs.call_context.storage_contract_address;
+
+    auto [read_requests, read_request_membership_witnesses, root] =
+        get_random_reads(contract_address, MAX_READ_REQUESTS_PER_CALL);
+
+    read_request_membership_witnesses[MAX_READ_REQUESTS_PER_CALL - 1] =
+        ReadRequestMembershipWitness<NT, PRIVATE_DATA_TREE_HEIGHT>{};
+
+    private_inputs.private_call.call_stack_item.public_inputs.historic_private_data_tree_root = root;
+    private_inputs.private_call.call_stack_item.public_inputs.read_requests = read_requests;
+    private_inputs.private_call.read_request_membership_witnesses = read_request_membership_witnesses;
+
+    DummyBuilder builder = DummyBuilder("native_private_kernel_init_tests__native_read_requests_more_than_witnesses");
+    auto const& public_inputs = native_private_kernel_circuit_initial(builder, private_inputs);
+
+    ASSERT_TRUE(builder.failed());
+    ASSERT_EQ(builder.get_first_failure().code,
+              CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_WITNESSES_ARRAY_LENGTH_MISMATCH);
+}
+
 TEST_F(native_private_kernel_init_tests, native_one_transient_read_requests_works)
 {
     // one transient read request should work

circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_inner.test.cpp

@@ -240,7 +240,7 @@ TEST_F(native_private_kernel_inner_tests, native_read_request_bad_request)
 
     validate_no_new_deployed_contract(public_inputs);
 
-    ASSERT(builder.failed());
+    ASSERT_TRUE(builder.failed());
     ASSERT_EQ(builder.get_first_failure().code,
               CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_PRIVATE_DATA_ROOT_MISMATCH);
 }
@@ -267,7 +267,7 @@ TEST_F(native_private_kernel_inner_tests, native_read_request_bad_leaf_index)
 
     validate_no_new_deployed_contract(public_inputs);
 
-    ASSERT(builder.failed());
+    ASSERT_TRUE(builder.failed());
     ASSERT_EQ(builder.get_first_failure().code,
               CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_PRIVATE_DATA_ROOT_MISMATCH);
 }
@@ -294,7 +294,7 @@ TEST_F(native_private_kernel_inner_tests, native_read_request_bad_sibling_path)
 
     validate_no_new_deployed_contract(public_inputs);
 
-    ASSERT(builder.failed());
+    ASSERT_TRUE(builder.failed());
     ASSERT_EQ(builder.get_first_failure().code,
               CircuitErrorCode::PRIVATE_KERNEL__READ_REQUEST_PRIVATE_DATA_ROOT_MISMATCH);
 }

circuits/cpp/src/aztec3/circuits/kernel/public/.test.cpp

@@ -461,14 +461,17 @@ void validate_public_kernel_outputs_correctly_propagated(const KernelInput& inpu
     }
 }
 
-void validate_private_data_propagation(const PublicKernelInputs<NT>& inputs,
+void validate_private_data_propagation(DummyBuilder& builder,
+                                       const PublicKernelInputs<NT>& inputs,
                                        const KernelCircuitPublicInputs<NT>& public_inputs)
 {
-    ASSERT_TRUE(source_arrays_are_in_target(inputs.previous_kernel.public_inputs.end.private_call_stack,
+    ASSERT_TRUE(source_arrays_are_in_target(builder,
+                                            inputs.previous_kernel.public_inputs.end.private_call_stack,
                                             std::array<NT::fr, MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX>{},
                                             public_inputs.end.private_call_stack));
 
-    ASSERT_TRUE(source_arrays_are_in_target(inputs.previous_kernel.public_inputs.end.new_contracts,
+    ASSERT_TRUE(source_arrays_are_in_target(builder,
+                                            inputs.previous_kernel.public_inputs.end.new_contracts,
                                             std::array<NewContractData<NT>, MAX_NEW_CONTRACTS_PER_TX>(),
                                             public_inputs.end.new_contracts));
 
@@ -878,7 +881,7 @@ TEST(public_kernel_tests, circuit_outputs_should_be_correctly_populated_with_pre
     auto public_inputs = native_public_kernel_circuit_private_previous_kernel(dummyBuilder, inputs);
 
     // test that the prior set of private kernel public inputs were copied to the outputs
-    validate_private_data_propagation(inputs, public_inputs);
+    validate_private_data_propagation(dummyBuilder, inputs, public_inputs);
 
     validate_public_kernel_outputs_correctly_propagated(inputs, public_inputs);
     ASSERT_FALSE(dummyBuilder.failed());
@@ -1040,7 +1043,7 @@ TEST(public_kernel_tests, circuit_outputs_should_be_correctly_populated_with_pre
     auto public_inputs = native_public_kernel_circuit_public_previous_kernel(dummyBuilder, inputs);
 
     // test that the prior set of private kernel public inputs were copied to the outputs
-    validate_private_data_propagation(inputs, public_inputs);
+    validate_private_data_propagation(dummyBuilder, inputs, public_inputs);
 
     // this call should have been popped from the public call stack and the stack of call pre images pushed on
     for (size_t i = 0; i < MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL; i++) {
@@ -1090,30 +1093,34 @@ TEST(public_kernel_tests, circuit_outputs_should_be_correctly_populated_with_pre
     ASSERT_EQ(public_inputs.end.unencrypted_log_preimages_length,
               unencrypted_log_preimages_length + public_inputs_unencrypted_log_preimages_length);
 
-    ASSERT_TRUE(source_arrays_are_in_target(inputs.previous_kernel.public_inputs.end.public_data_update_requests,
+    ASSERT_TRUE(source_arrays_are_in_target(dummyBuilder,
+                                            inputs.previous_kernel.public_inputs.end.public_data_update_requests,
                                             expected_new_writes,
                                             public_inputs.end.public_data_update_requests));
 
     std::array<PublicDataRead<NT>, MAX_PUBLIC_DATA_READS_PER_CALL> const expected_new_reads =
         public_data_reads_from_contract_storage_reads(
             inputs.public_call.call_stack_item.public_inputs.contract_storage_reads, contract_address);
 
-    ASSERT_TRUE(source_arrays_are_in_target(inputs.previous_kernel.public_inputs.end.public_data_reads,
+    ASSERT_TRUE(source_arrays_are_in_target(dummyBuilder,
+                                            inputs.previous_kernel.public_inputs.end.public_data_reads,
                                             expected_new_reads,
                                             public_inputs.end.public_data_reads));
 
     std::array<NT::fr, MAX_NEW_COMMITMENTS_PER_CALL> const expected_new_commitments =
         new_commitments_as_siloed_commitments(inputs.public_call.call_stack_item.public_inputs.new_commitments,
                                               contract_address);
 
-    ASSERT_TRUE(source_arrays_are_in_target(inputs.previous_kernel.public_inputs.end.new_commitments,
+    ASSERT_TRUE(source_arrays_are_in_target(dummyBuilder,
+                                            inputs.previous_kernel.public_inputs.end.new_commitments,
                                             expected_new_commitments,
                                             public_inputs.end.new_commitments));
 
     std::array<NT::fr, MAX_NEW_NULLIFIERS_PER_CALL> const expected_new_nullifiers = new_nullifiers_as_siloed_nullifiers(
         inputs.public_call.call_stack_item.public_inputs.new_nullifiers, contract_address);
 
-    ASSERT_TRUE(source_arrays_are_in_target(inputs.previous_kernel.public_inputs.end.new_nullifiers,
+    ASSERT_TRUE(source_arrays_are_in_target(dummyBuilder,
+                                            inputs.previous_kernel.public_inputs.end.new_nullifiers,
                                             expected_new_nullifiers,
                                             public_inputs.end.new_nullifiers));
 
@@ -1128,7 +1135,8 @@ TEST(public_kernel_tests, circuit_outputs_should_be_correctly_populated_with_pre
                                      chain_id,
                                      version);
 
-    ASSERT_TRUE(source_arrays_are_in_target(inputs.previous_kernel.public_inputs.end.new_l2_to_l1_msgs,
+    ASSERT_TRUE(source_arrays_are_in_target(dummyBuilder,
+                                            inputs.previous_kernel.public_inputs.end.new_l2_to_l1_msgs,
                                             expected_new_messages,
                                             public_inputs.end.new_l2_to_l1_msgs));
 

circuits/cpp/src/aztec3/utils/array.hpp

@@ -188,15 +188,18 @@ void push_array_to_array(Builder& builder, std::array<T, size_1> const& source,
  * @param The `target` array
  * @return Whether the source arrays are indeed in the target
  */
-template <size_t size_1, size_t size_2, size_t size_3, typename T>
-bool source_arrays_are_in_target(std::array<T, size_1> const& source1,
+template <size_t size_1, size_t size_2, size_t size_3, typename T, typename Builder>
+bool source_arrays_are_in_target(Builder& builder,
+                                 std::array<T, size_1> const& source1,
                                  std::array<T, size_2> const& source2,
                                  std::array<T, size_3> const& target)
 {
     // Check if the `source` arrays are too large vs the size of the `target` array
     size_t const source1_size = array_length(source1);
     size_t const source2_size = array_length(source2);
-    ASSERT(source1_size + source2_size <= size_3);
+    builder.do_assert(source1_size + source2_size <= size_3,
+                      "source_arrays_are_in_target: source arrays are too large vs the size of the target",
+                      CircuitErrorCode::ARRAY_OVERFLOW);
 
     // first ensure that all non-empty items in the first source are in the target
     size_t target_index = 0;