chore: Validate outputs in kernel circuits (#7706)

Kernel circuits have a validation step that checks their output, which is constructed in an unconstrained function earlier. This check should not be needed when running in unconstrained mode, so they were skipped. However, this means that errors in the unconstraiend construction of the output could go unnoticed in tests that run without real proofs enabled, since those use the `-simulated` versions of kernels which are unconstrained. To catch these errors, we now force kernel circuits to always validate this output. --------- Co-authored-by: Leila Wang <[email protected]>
AztecProtocol · Aug 1, 2024 · 9a98289 · 9a98289
1 parent 76ef298
commit 9a98289
Show file tree

Hide file tree

Showing 8 changed files with 15 additions and 6 deletions.
diff --git a/...ocol-circuits/crates/private-kernel-lib/src/components/tail_to_public_output_validator.nr b/...ocol-circuits/crates/private-kernel-lib/src/components/tail_to_public_output_validator.nr
@@ -178,7 +178,7 @@ impl TailToPublicOutputValidator {
         // unencrypted_logs_hashes
         assert_split_sorted_transformed_value_arrays_asc(
             prev_data.unencrypted_logs_hashes,
-            prev_data.unencrypted_logs_hashes,
+            prev_data.unencrypted_logs_hashes.map(|log: ScopedLogHash| log.expose_to_public()),
             split_counter,
             output_non_revertible.unencrypted_logs_hashes,
             output_revertible.unencrypted_logs_hashes,

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
@@ -40,7 +40,7 @@ impl PrivateKernelInitCircuitPrivateInputs {
         private_call_data_validator.validate(output.end.note_hashes);
 
         // Validate output.
-        if !std::runtime::is_unconstrained() {
+        if dep::types::validate::should_validate_output() {
             PrivateKernelCircuitOutputValidator::new(output).validate_as_first_call(
                 self.tx_request,
                 self.private_call.call_stack_item.public_inputs,

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
@@ -58,7 +58,7 @@ impl PrivateKernelInnerCircuitPrivateInputs {
         }
 
         // Validate output.
-        if !std::runtime::is_unconstrained() {
+        if dep::types::validate::should_validate_output() {
             PrivateKernelCircuitOutputValidator::new(output).validate_as_inner_call(
                 self.previous_kernel.public_inputs,
                 previous_kernel_array_lengths,

diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_reset.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_reset.nr
@@ -61,7 +61,7 @@ impl<NH_RR_PENDING, NH_RR_SETTLED, NLL_RR_PENDING, NLL_RR_SETTLED, KEY_VALIDATIO
         }.validate();
 
         // Validate output.
-        if !dep::std::runtime::is_unconstrained() {
+        if dep::types::validate::should_validate_output() {
             let transient_or_propagated_note_hash_indexes_for_logs = get_transient_or_propagated_note_hash_indexes_for_logs(
                 previous_public_inputs.end.note_encrypted_logs_hashes,
                 previous_public_inputs.end.note_hashes,

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
@@ -43,7 +43,7 @@ impl PrivateKernelTailCircuitPrivateInputs {
         }
 
         // Validate output.
-        if !std::runtime::is_unconstrained() {
+        if dep::types::validate::should_validate_output() {
             TailOutputValidator::new(
                 output,
                 self.previous_kernel.public_inputs,

diff --git a/...cts/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail_to_public.nr b/...cts/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail_to_public.nr
@@ -44,7 +44,7 @@ impl PrivateKernelTailToPublicCircuitPrivateInputs {
         }
 
         // Validate output.
-        if !dep::std::runtime::is_unconstrained() {
+        if dep::types::validate::should_validate_output() {
             TailToPublicOutputValidator::new(
                 output,
                 self.previous_kernel.public_inputs,

diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr b/noir-projects/noir-protocol-circuits/crates/types/src/lib.nr
@@ -34,3 +34,4 @@ use abis::kernel_circuit_public_inputs::{KernelCircuitPublicInputs, PrivateKerne
 mod recursion;
 mod data;
 mod storage;
+mod validate;
diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/validate.nr b/noir-projects/noir-protocol-circuits/crates/types/src/validate.nr
@@ -0,0 +1,8 @@
+// Returns whether to validate the output of the circuit. Eventually we'll want to change this
+// to `!dep::std::runtime::is_unconstrained`, so we skip unneeded validations when we
+// do not need to generate a proof. But for now, we always validate to make sure we catch
+// errors earlier, as most of our tests run with real proofs disabled, which means validation 
+// checks get skipped.
+pub fn should_validate_output() -> bool {
+    true
+}