feat: verify public validation requests (#8150)

Verify note hash read requests and l1tol2msg read requests in public kernel tail. --------- Co-authored-by: Ilyas Ridhuan <[email protected]>
AztecProtocol · Sep 5, 2024 · 2be1415 · 2be1415
1 parent 7f02900
commit 2be1415
Show file tree

Hide file tree

Showing 52 changed files with 790 additions and 196 deletions.
diff --git a/barretenberg/cpp/pil/avm/kernel.pil b/barretenberg/cpp/pil/avm/kernel.pil
@@ -164,8 +164,8 @@ namespace main(256);
 
     // When we encounter a state writing opcode
     // We increment the side effect counter by 1
-    #[SIDE_EFFECT_COUNTER_INCREMENT]
-    KERNEL_OUTPUT_SELECTORS * (side_effect_counter' - (side_effect_counter + 1)) = 0;
+    //#[SIDE_EFFECT_COUNTER_INCREMENT]
+    //KERNEL_OUTPUT_SELECTORS * (side_effect_counter' - (side_effect_counter + 1)) = 0;
 
     //===== LOOKUPS INTO THE PUBLIC INPUTS ===========================================
     pol KERNEL_INPUT_SELECTORS = sel_op_address + sel_op_storage_address + sel_op_sender
@@ -182,8 +182,9 @@ namespace main(256);
     #[KERNEL_OUTPUT_ACTIVE_CHECK]
     KERNEL_OUTPUT_SELECTORS * (1 - sel_q_kernel_output_lookup) = 0;
 
+    // TODO(#8287): Reintroduce constraints
     #[KERNEL_OUTPUT_LOOKUP]
-    sel_q_kernel_output_lookup {kernel_out_offset, ia, side_effect_counter, ib} in sel_kernel_out {clk, kernel_value_out, kernel_side_effect_out, kernel_metadata_out};
+    sel_q_kernel_output_lookup {kernel_out_offset, /*ia,*/ /*side_effect_counter,*/ ib } in sel_kernel_out {clk, /*kernel_value_out,*/ /*kernel_side_effect_out,*/ kernel_metadata_out};
 
     #[LOOKUP_INTO_KERNEL]
     sel_q_kernel_lookup { main.ia, kernel_in_offset } in sel_kernel_inputs { kernel_inputs, clk };
diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.cpp
@@ -730,24 +730,23 @@ AvmFlavor::AllConstRefValues::AllConstRefValues(
     , main_nullifier_non_exists_write_offset_shift(il[722])
     , main_pc_shift(il[723])
     , main_sel_execution_row_shift(il[724])
-    , main_side_effect_counter_shift(il[725])
-    , main_sload_write_offset_shift(il[726])
-    , main_sstore_write_offset_shift(il[727])
-    , mem_glob_addr_shift(il[728])
-    , mem_rw_shift(il[729])
-    , mem_sel_mem_shift(il[730])
-    , mem_tag_shift(il[731])
-    , mem_tsp_shift(il[732])
-    , mem_val_shift(il[733])
-    , slice_addr_shift(il[734])
-    , slice_clk_shift(il[735])
-    , slice_cnt_shift(il[736])
-    , slice_col_offset_shift(il[737])
-    , slice_sel_cd_cpy_shift(il[738])
-    , slice_sel_mem_active_shift(il[739])
-    , slice_sel_return_shift(il[740])
-    , slice_sel_start_shift(il[741])
-    , slice_space_id_shift(il[742])
+    , main_sload_write_offset_shift(il[725])
+    , main_sstore_write_offset_shift(il[726])
+    , mem_glob_addr_shift(il[727])
+    , mem_rw_shift(il[728])
+    , mem_sel_mem_shift(il[729])
+    , mem_tag_shift(il[730])
+    , mem_tsp_shift(il[731])
+    , mem_val_shift(il[732])
+    , slice_addr_shift(il[733])
+    , slice_clk_shift(il[734])
+    , slice_cnt_shift(il[735])
+    , slice_col_offset_shift(il[736])
+    , slice_sel_cd_cpy_shift(il[737])
+    , slice_sel_mem_active_shift(il[738])
+    , slice_sel_return_shift(il[739])
+    , slice_sel_start_shift(il[740])
+    , slice_space_id_shift(il[741])
 {}
 
 AvmFlavor::ProverPolynomials::ProverPolynomials(ProvingKey& proving_key)
@@ -1489,7 +1488,6 @@ AvmFlavor::AllConstRefValues AvmFlavor::ProverPolynomials::get_row(size_t row_id
                      main_nullifier_non_exists_write_offset_shift[row_idx],
                      main_pc_shift[row_idx],
                      main_sel_execution_row_shift[row_idx],
-                     main_side_effect_counter_shift[row_idx],
                      main_sload_write_offset_shift[row_idx],
                      main_sstore_write_offset_shift[row_idx],
                      mem_glob_addr_shift[row_idx],

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.hpp
diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/kernel.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/kernel.hpp
@@ -10,9 +10,9 @@ template <typename FF_> class kernelImpl {
   public:
     using FF = FF_;
 
-    static constexpr std::array<size_t, 44> SUBRELATION_PARTIAL_LENGTHS = { 3, 3, 4, 4, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
+    static constexpr std::array<size_t, 43> SUBRELATION_PARTIAL_LENGTHS = { 3, 3, 4, 4, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
                                                                             3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 4, 3, 3, 3,
-                                                                            3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3 };
+                                                                            3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3 };
 
     template <typename ContainerOverSubrelations, typename AllEntities>
     void static accumulate(ContainerOverSubrelations& evals,
@@ -363,22 +363,15 @@ template <typename FF_> class kernelImpl {
         }
         {
             using Accumulator = typename std::tuple_element_t<41, ContainerOverSubrelations>;
-            auto tmp = (main_KERNEL_OUTPUT_SELECTORS *
-                        (new_term.main_side_effect_counter_shift - (new_term.main_side_effect_counter + FF(1))));
+            auto tmp = (main_KERNEL_INPUT_SELECTORS * (FF(1) - new_term.main_sel_q_kernel_lookup));
             tmp *= scaling_factor;
             std::get<41>(evals) += typename Accumulator::View(tmp);
         }
         {
             using Accumulator = typename std::tuple_element_t<42, ContainerOverSubrelations>;
-            auto tmp = (main_KERNEL_INPUT_SELECTORS * (FF(1) - new_term.main_sel_q_kernel_lookup));
-            tmp *= scaling_factor;
-            std::get<42>(evals) += typename Accumulator::View(tmp);
-        }
-        {
-            using Accumulator = typename std::tuple_element_t<43, ContainerOverSubrelations>;
             auto tmp = (main_KERNEL_OUTPUT_SELECTORS * (FF(1) - new_term.main_sel_q_kernel_output_lookup));
             tmp *= scaling_factor;
-            std::get<43>(evals) += typename Accumulator::View(tmp);
+            std::get<42>(evals) += typename Accumulator::View(tmp);
         }
     }
 };
@@ -453,10 +446,8 @@ template <typename FF> class kernel : public Relation<kernelImpl<FF>> {
         case 39:
             return "SSTORE_KERNEL_OUTPUT";
         case 41:
-            return "SIDE_EFFECT_COUNTER_INCREMENT";
-        case 42:
             return "KERNEL_INPUT_ACTIVE_CHECK";
-        case 43:
+        case 42:
             return "KERNEL_OUTPUT_ACTIVE_CHECK";
         }
         return std::to_string(index);

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/kernel_output_lookup.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/kernel_output_lookup.hpp
@@ -14,7 +14,7 @@ class kernel_output_lookup_lookup_settings {
     static constexpr size_t WRITE_TERMS = 1;
     static constexpr size_t READ_TERM_TYPES[READ_TERMS] = { 0 };
     static constexpr size_t WRITE_TERM_TYPES[WRITE_TERMS] = { 0 };
-    static constexpr size_t LOOKUP_TUPLE_SIZE = 4;
+    static constexpr size_t LOOKUP_TUPLE_SIZE = 2;
     static constexpr size_t INVERSE_EXISTS_POLYNOMIAL_DEGREE = 4;
     static constexpr size_t READ_TERM_DEGREE = 0;
     static constexpr size_t WRITE_TERM_DEGREE = 0;
@@ -40,12 +40,8 @@ class kernel_output_lookup_lookup_settings {
                                      in.main_sel_q_kernel_output_lookup,
                                      in.main_sel_kernel_out,
                                      in.main_kernel_out_offset,
-                                     in.main_ia,
-                                     in.main_side_effect_counter,
                                      in.main_ib,
                                      in.main_clk,
-                                     in.main_kernel_value_out,
-                                     in.main_kernel_side_effect_out,
                                      in.main_kernel_metadata_out);
     }
 
@@ -56,12 +52,8 @@ class kernel_output_lookup_lookup_settings {
                                      in.main_sel_q_kernel_output_lookup,
                                      in.main_sel_kernel_out,
                                      in.main_kernel_out_offset,
-                                     in.main_ia,
-                                     in.main_side_effect_counter,
                                      in.main_ib,
                                      in.main_clk,
-                                     in.main_kernel_value_out,
-                                     in.main_kernel_side_effect_out,
                                      in.main_kernel_metadata_out);
     }
 };

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp
@@ -1771,7 +1771,8 @@ TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes)
     auto emit_note_hash_kernel_out_row = std::ranges::find_if(
         trace.begin(), trace.end(), [&](Row r) { return r.main_clk == emit_note_hash_out_offset; });
     EXPECT_EQ(emit_note_hash_kernel_out_row->main_kernel_value_out, 1);
-    EXPECT_EQ(emit_note_hash_kernel_out_row->main_kernel_side_effect_out, 0);
+    // TODO(#8287)
+    // EXPECT_EQ(emit_note_hash_kernel_out_row->main_kernel_side_effect_out, 0);
     feed_output(emit_note_hash_out_offset, 1, 0, 0);
 
     // CHECK EMIT NULLIFIER
@@ -2040,7 +2041,9 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes)
     std::vector<FF> returndata = {};
 
     // Generate Hint for hash exists operation
-    auto execution_hints = ExecutionHints().with_storage_value_hints({ { 0, 1 }, { 1, 1 }, { 2, 1 } });
+    auto execution_hints = ExecutionHints()
+                               .with_storage_value_hints({ { 0, 1 }, { 1, 1 }, { 2, 1 } })
+                               .with_note_hash_exists_hints({ { 0, 1 }, { 1, 1 }, { 2, 1 } });
 
     auto trace = Execution::gen_trace(instructions, returndata, calldata, public_inputs_vec, execution_hints);
 
@@ -2068,7 +2071,8 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes)
     auto nullifier_out_row = std::ranges::find_if(
         trace.begin(), trace.end(), [&](Row r) { return r.main_clk == START_NULLIFIER_EXISTS_OFFSET; });
     EXPECT_EQ(nullifier_out_row->main_kernel_value_out, 1); // value
-    EXPECT_EQ(nullifier_out_row->main_kernel_side_effect_out, 1);
+    // TODO(#8287)
+    // EXPECT_EQ(nullifier_out_row->main_kernel_side_effect_out, 1);
     EXPECT_EQ(nullifier_out_row->main_kernel_metadata_out, 1); // exists
     feed_output(START_NULLIFIER_EXISTS_OFFSET, 1, 1, 1);
 
@@ -2082,7 +2086,8 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes)
     auto msg_out_row = std::ranges::find_if(
         trace.begin(), trace.end(), [&](Row r) { return r.main_clk == START_L1_TO_L2_MSG_EXISTS_WRITE_OFFSET; });
     EXPECT_EQ(msg_out_row->main_kernel_value_out, 1); // value
-    EXPECT_EQ(msg_out_row->main_kernel_side_effect_out, 2);
+    // TODO(#8287)
+    // EXPECT_EQ(msg_out_row->main_kernel_side_effect_out, 2);
     EXPECT_EQ(msg_out_row->main_kernel_metadata_out, 1); // exists
     feed_output(START_L1_TO_L2_MSG_EXISTS_WRITE_OFFSET, 1, 2, 1);
 

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/kernel.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/kernel.test.cpp
@@ -1226,14 +1226,16 @@ TEST_F(AvmKernelOutputPositiveTests, kernelNoteHashExists)
 
     auto direct_apply_opcodes = [=](AvmTraceBuilder& trace_builder) {
         trace_builder.op_set(0, static_cast<uint128_t>(value), value_offset, AvmMemoryTag::FF);
-        trace_builder.op_note_hash_exists(/*indirect*/ false, value_offset, metadata_offset);
+        // TODO(#8287): Leaf index isnt constrained properly so we just set it to 0
+        trace_builder.op_note_hash_exists(/*indirect*/ false, value_offset, 0, metadata_offset);
     };
     // TODO: fix
     auto indirect_apply_opcodes = [=](AvmTraceBuilder& trace_builder) {
         trace_builder.op_set(0, static_cast<uint128_t>(value), value_offset, AvmMemoryTag::FF);
         trace_builder.op_set(0, value_offset, indirect_value_offset, AvmMemoryTag::U32);
         trace_builder.op_set(0, metadata_offset, indirect_metadata_offset, AvmMemoryTag::U32);
-        trace_builder.op_note_hash_exists(/*indirect*/ 3, indirect_value_offset, indirect_metadata_offset);
+        // TODO(#8287): Leaf index isnt constrained properly so we just set it to 0
+        trace_builder.op_note_hash_exists(/*indirect*/ 3, indirect_value_offset, 0, indirect_metadata_offset);
     };
     auto checks = [=](bool indirect, const std::vector<Row>& trace) {
         auto row = std::ranges::find_if(
@@ -1352,7 +1354,8 @@ TEST_F(AvmKernelOutputPositiveTests, kernelL1ToL2MsgExists)
 
     auto apply_opcodes = [=](AvmTraceBuilder& trace_builder) {
         trace_builder.op_set(0, static_cast<uint128_t>(value), value_offset, AvmMemoryTag::FF);
-        trace_builder.op_l1_to_l2_msg_exists(/*indirect*/ false, value_offset, metadata_offset);
+        // TODO(#8287): Leaf index isnt constrained properly so we just set it to 0
+        trace_builder.op_l1_to_l2_msg_exists(/*indirect*/ false, value_offset, 0, metadata_offset);
     };
     auto checks = [=]([[maybe_unused]] bool indirect, const std::vector<Row>& trace) {
         auto row = std::ranges::find_if(

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp
@@ -650,8 +650,7 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
         case OpCode::NOTEHASHEXISTS:
             trace_builder.op_note_hash_exists(std::get<uint8_t>(inst.operands.at(0)),
                                               std::get<uint32_t>(inst.operands.at(1)),
-                                              // TODO: leaf offset exists
-                                              // std::get<uint32_t>(inst.operands.at(2))
+                                              std::get<uint32_t>(inst.operands.at(2)),
                                               std::get<uint32_t>(inst.operands.at(3)));
             break;
         case OpCode::EMITNOTEHASH:
@@ -673,8 +672,7 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
         case OpCode::L1TOL2MSGEXISTS:
             trace_builder.op_l1_to_l2_msg_exists(std::get<uint8_t>(inst.operands.at(0)),
                                                  std::get<uint32_t>(inst.operands.at(1)),
-                                                 // TODO: leaf offset exists
-                                                 // std::get<uint32_t>(inst.operands.at(2))
+                                                 std::get<uint32_t>(inst.operands.at(2)),
                                                  std::get<uint32_t>(inst.operands.at(3)));
             break;
         case OpCode::GETCONTRACTINSTANCE:

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution_hints.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution_hints.hpp
@@ -105,8 +105,15 @@ struct ExecutionHints {
     {
         std::unordered_map<uint32_t, FF> hints_map;
         push_vec_into_map(hints_map, storage_value_hints);
-        push_vec_into_map(hints_map, note_hash_exists_hints);
         push_vec_into_map(hints_map, nullifier_exists_hints);
+        return hints_map;
+    }
+
+    // Leaf index -> exists
+    std::unordered_map<uint32_t, FF> get_leaf_index_hints() const
+    {
+        std::unordered_map<uint32_t, FF> hints_map;
+        push_vec_into_map(hints_map, note_hash_exists_hints);
         push_vec_into_map(hints_map, l1_to_l2_message_exists_hints);
         return hints_map;
     }
@@ -161,4 +168,4 @@ struct ExecutionHints {
     {}
 };
 
-} // namespace bb::avm_trace
+} // namespace bb::avm_trace
diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/kernel_trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/kernel_trace.cpp
@@ -173,7 +173,16 @@ void AvmKernelTraceBuilder::op_note_hash_exists(uint32_t clk,
 {
 
     uint32_t offset = START_NOTE_HASH_EXISTS_WRITE_OFFSET + note_hash_exists_offset;
-    perform_kernel_output_lookup(offset, side_effect_counter, note_hash, FF(result));
+    // TODO(#8287)Lookups are heavily underconstrained atm
+    if (result == 1) {
+        perform_kernel_output_lookup(offset, side_effect_counter, note_hash, FF(result));
+    } else {
+        // if the note_hash does NOT exist, the public inputs already contains the correct output value (i.e. the
+        // actual value at the index), so we don't try to overwrite the value
+        std::get<KERNEL_OUTPUTS_SIDE_EFFECT_COUNTER>(public_inputs)[offset] = side_effect_counter;
+        std::get<KERNEL_OUTPUTS_METADATA>(public_inputs)[offset] = FF(result);
+        kernel_output_selector_counter[offset]++;
+    }
     note_hash_exists_offset++;
 
     KernelTraceEntry entry = {
@@ -245,7 +254,16 @@ void AvmKernelTraceBuilder::op_l1_to_l2_msg_exists(uint32_t clk,
                                                    uint32_t result)
 {
     uint32_t offset = START_L1_TO_L2_MSG_EXISTS_WRITE_OFFSET + l1_to_l2_msg_exists_offset;
-    perform_kernel_output_lookup(offset, side_effect_counter, message, FF(result));
+    // TODO(#8287)Lookups are heavily underconstrained atm
+    if (result == 1) {
+        perform_kernel_output_lookup(offset, side_effect_counter, message, FF(result));
+    } else {
+        // if the l1_to_l2_msg_exists is false, the public inputs already contains the correct output value (i.e. the
+        // actual value at the index), so we don't try to overwrite the value
+        std::get<KERNEL_OUTPUTS_SIDE_EFFECT_COUNTER>(public_inputs)[offset] = side_effect_counter;
+        std::get<KERNEL_OUTPUTS_METADATA>(public_inputs)[offset] = FF(result);
+        kernel_output_selector_counter[offset]++;
+    }
     l1_to_l2_msg_exists_offset++;
 
     KernelTraceEntry entry = {