1090 - address review's feedback

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

@@ -125,18 +125,18 @@ void common_validate_read_requests(DummyBuilder& builder,
  */
 void common_validate_arrays(DummyBuilder& builder, PrivateCircuitPublicInputs<NT> const& app_public_inputs)
 {
-    // The following arrays (new_commitments, etc...) are passed to push_array_to_array()
-    // routines which rely on the passed arrays to be well-formed.
+    // Each of the following arrays is expected to be zero-padded.
+    // In addition, some of the following arrays (new_commitments, etc...) are passed
+    // to push_array_to_array() routines which rely on the passed arrays to be well-formed.
+    validate_array(builder, app_public_inputs.return_values, "Return values");
+    validate_array(builder, app_public_inputs.read_requests, "Read requests");
     validate_array(builder, app_public_inputs.new_commitments, "New commitments");
     validate_array(builder, app_public_inputs.new_nullifiers, "New nullifiers");
     validate_array(builder, app_public_inputs.nullified_commitments, "Nullified commitments");
     validate_array(builder, app_public_inputs.private_call_stack, "Private Call Stack");
     validate_array(builder, app_public_inputs.public_call_stack, "Public Call Stack");
     validate_array(builder, app_public_inputs.new_l2_to_l1_msgs, "New L2 to L1 messages");
-    // return_values are not used in private kernel circuit and therefore not validated.
     // encrypted_logs_hash and unencrypted_logs_hash have their own integrity checks.
-    // read_requests are processed through a loop over the whole array and therefore do not
-    // rely on the array to be right-zero padded.
 }
 
 void common_validate_0th_nullifier(DummyBuilder& builder, CombinedAccumulatedData<NT> const& end)

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

@@ -117,6 +117,34 @@ TEST_F(native_private_kernel_init_tests, basic_contract_deployment)
     EXPECT_EQ(builder.get_first_failure().code, CircuitErrorCode::NO_ERROR);
 }
 
+TEST_F(native_private_kernel_init_tests, input_validation_malformed_arrays_return_values)
+{
+    auto private_inputs = do_private_call_get_kernel_inputs_init(true, constructor, standard_test_args());
+
+    std::array<fr, RETURN_VALUES_LENGTH> malformed_return_values{ fr(0), fr(0), fr(553) };
+    private_inputs.private_call.call_stack_item.public_inputs.return_values = malformed_return_values;
+
+    DummyBuilder builder = DummyBuilder("private_kernel_tests__input_validation_malformed_arrays_return_values");
+    native_private_kernel_circuit_initial(builder, private_inputs);
+
+    EXPECT_EQ(builder.failed(), true);
+    EXPECT_EQ(builder.get_first_failure().code, CircuitErrorCode::ARRAY_NOT_ZERO_RIGHT_PADDED);
+}
+
+TEST_F(native_private_kernel_init_tests, input_validation_malformed_arrays_read_requests)
+{
+    auto private_inputs = do_private_call_get_kernel_inputs_init(true, constructor, standard_test_args());
+
+    std::array<fr, MAX_READ_REQUESTS_PER_CALL> malformed_read_requests{ fr(0), fr(9123), fr(0), fr(12) };
+    private_inputs.private_call.call_stack_item.public_inputs.read_requests = malformed_read_requests;
+
+    DummyBuilder builder = DummyBuilder("private_kernel_tests__input_validation_malformed_arrays_read_requests");
+    native_private_kernel_circuit_initial(builder, private_inputs);
+
+    EXPECT_EQ(builder.failed(), true);
+    EXPECT_EQ(builder.get_first_failure().code, CircuitErrorCode::ARRAY_NOT_ZERO_RIGHT_PADDED);
+}
+
 TEST_F(native_private_kernel_init_tests, input_validation_malformed_arrays_commitments)
 {
     auto private_inputs = do_private_call_get_kernel_inputs_init(true, constructor, standard_test_args());

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

@@ -206,6 +206,34 @@ TEST_F(native_private_kernel_inner_tests, private_function_incorrect_call_stack_
               CircuitErrorCode::PRIVATE_KERNEL__CALCULATED_PRIVATE_CALL_HASH_AND_PROVIDED_PRIVATE_CALL_HASH_MISMATCH);
 }
 
+TEST_F(native_private_kernel_inner_tests, input_validation_malformed_arrays_return_values)
+{
+    auto private_inputs = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args());
+
+    std::array<fr, RETURN_VALUES_LENGTH> malformed_return_values{ fr(0), fr(0), fr(553) };
+    private_inputs.private_call.call_stack_item.public_inputs.return_values = malformed_return_values;
+
+    DummyBuilder builder = DummyBuilder("private_kernel_tests__input_validation_malformed_arrays_return_values");
+    native_private_kernel_circuit_inner(builder, private_inputs);
+
+    EXPECT_EQ(builder.failed(), true);
+    EXPECT_EQ(builder.get_first_failure().code, CircuitErrorCode::ARRAY_NOT_ZERO_RIGHT_PADDED);
+}
+
+TEST_F(native_private_kernel_inner_tests, input_validation_malformed_arrays_read_requests)
+{
+    auto private_inputs = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args());
+
+    std::array<fr, MAX_READ_REQUESTS_PER_CALL> malformed_read_requests{ fr(0), fr(9123), fr(0), fr(12) };
+    private_inputs.private_call.call_stack_item.public_inputs.read_requests = malformed_read_requests;
+
+    DummyBuilder builder = DummyBuilder("private_kernel_tests__input_validation_malformed_arrays_read_requests");
+    native_private_kernel_circuit_inner(builder, private_inputs);
+
+    EXPECT_EQ(builder.failed(), true);
+    EXPECT_EQ(builder.get_first_failure().code, CircuitErrorCode::ARRAY_NOT_ZERO_RIGHT_PADDED);
+}
+
 TEST_F(native_private_kernel_inner_tests, input_validation_malformed_arrays_commitments)
 {
     auto private_inputs = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args());
@@ -219,6 +247,7 @@ TEST_F(native_private_kernel_inner_tests, input_validation_malformed_arrays_comm
     EXPECT_EQ(builder.failed(), true);
     EXPECT_EQ(builder.get_first_failure().code, CircuitErrorCode::ARRAY_NOT_ZERO_RIGHT_PADDED);
 }
+
 TEST_F(native_private_kernel_inner_tests, input_validation_malformed_arrays_nullifiers)
 {
     auto private_inputs = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args());