Merge c11a5f5 into 1516d7f

Author: dbanks12

barretenberg/cpp/pil/avm/gas.pil

@@ -64,10 +64,10 @@ namespace main(256);
     is_fake_row * (1 - is_fake_row) = 0; // Temporary
 
     //TODO(8945): clean up fake row related code
-    #[L2_GAS_NO_DECREMENT_FAKE_ROW]
-    is_fake_row * (l2_gas_remaining - l2_gas_remaining') = 0;
-    #[DA_GAS_NO_DECREMENT_FAKE_ROW]
-    is_fake_row * (da_gas_remaining - da_gas_remaining') = 0;
+    //#[L2_GAS_NO_DECREMENT_FAKE_ROW]
+    //is_fake_row * (l2_gas_remaining - l2_gas_remaining') = 0;
+    //#[DA_GAS_NO_DECREMENT_FAKE_ROW]
+    //is_fake_row * (da_gas_remaining - da_gas_remaining') = 0;
 
     // Constrain that the gas decrements correctly per instruction
     #[L2_GAS_REMAINING_DECREMENT_NOT_CALL]

barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/gas.hpp

@@ -10,7 +10,7 @@ template <typename FF_> class gasImpl {
   public:
     using FF = FF_;
 
-    static constexpr std::array<size_t, 12> SUBRELATION_PARTIAL_LENGTHS = { 3, 3, 3, 3, 3, 3, 5, 5, 4, 4, 2, 2 };
+    static constexpr std::array<size_t, 10> SUBRELATION_PARTIAL_LENGTHS = { 3, 3, 3, 3, 5, 5, 4, 4, 2, 2 };
 
     template <typename ContainerOverSubrelations, typename AllEntities>
     void static accumulate(ContainerOverSubrelations& evals,
@@ -46,67 +46,53 @@ template <typename FF_> class gasImpl {
         }
         {
             using Accumulator = typename std::tuple_element_t<4, ContainerOverSubrelations>;
-            auto tmp =
-                (new_term.main_is_fake_row * (new_term.main_l2_gas_remaining - new_term.main_l2_gas_remaining_shift));
-            tmp *= scaling_factor;
-            std::get<4>(evals) += typename Accumulator::View(tmp);
-        }
-        {
-            using Accumulator = typename std::tuple_element_t<5, ContainerOverSubrelations>;
-            auto tmp =
-                (new_term.main_is_fake_row * (new_term.main_da_gas_remaining - new_term.main_da_gas_remaining_shift));
-            tmp *= scaling_factor;
-            std::get<5>(evals) += typename Accumulator::View(tmp);
-        }
-        {
-            using Accumulator = typename std::tuple_element_t<6, ContainerOverSubrelations>;
             auto tmp = ((new_term.main_is_gas_accounted *
                          ((FF(1) - new_term.main_sel_op_external_call) - new_term.main_sel_op_static_call)) *
                         (((new_term.main_l2_gas_remaining_shift - new_term.main_l2_gas_remaining) +
                           new_term.main_base_l2_gas_op_cost) +
                          (new_term.main_dyn_l2_gas_op_cost * new_term.main_dyn_gas_multiplier)));
             tmp *= scaling_factor;
-            std::get<6>(evals) += typename Accumulator::View(tmp);
+            std::get<4>(evals) += typename Accumulator::View(tmp);
         }
         {
-            using Accumulator = typename std::tuple_element_t<7, ContainerOverSubrelations>;
+            using Accumulator = typename std::tuple_element_t<5, ContainerOverSubrelations>;
             auto tmp = ((new_term.main_is_gas_accounted *
                          ((FF(1) - new_term.main_sel_op_external_call) - new_term.main_sel_op_static_call)) *
                         (((new_term.main_da_gas_remaining_shift - new_term.main_da_gas_remaining) +
                           new_term.main_base_da_gas_op_cost) +
                          (new_term.main_dyn_da_gas_op_cost * new_term.main_dyn_gas_multiplier)));
             tmp *= scaling_factor;
-            std::get<7>(evals) += typename Accumulator::View(tmp);
+            std::get<5>(evals) += typename Accumulator::View(tmp);
         }
         {
-            using Accumulator = typename std::tuple_element_t<8, ContainerOverSubrelations>;
+            using Accumulator = typename std::tuple_element_t<6, ContainerOverSubrelations>;
             auto tmp = (new_term.main_is_gas_accounted *
                         (((FF(1) - (FF(2) * new_term.main_l2_out_of_gas)) * new_term.main_l2_gas_remaining_shift) -
                          new_term.main_abs_l2_rem_gas));
             tmp *= scaling_factor;
-            std::get<8>(evals) += typename Accumulator::View(tmp);
+            std::get<6>(evals) += typename Accumulator::View(tmp);
         }
         {
-            using Accumulator = typename std::tuple_element_t<9, ContainerOverSubrelations>;
+            using Accumulator = typename std::tuple_element_t<7, ContainerOverSubrelations>;
             auto tmp = (new_term.main_is_gas_accounted *
                         (((FF(1) - (FF(2) * new_term.main_da_out_of_gas)) * new_term.main_da_gas_remaining_shift) -
                          new_term.main_abs_da_rem_gas));
             tmp *= scaling_factor;
-            std::get<9>(evals) += typename Accumulator::View(tmp);
+            std::get<7>(evals) += typename Accumulator::View(tmp);
         }
         {
-            using Accumulator = typename std::tuple_element_t<10, ContainerOverSubrelations>;
+            using Accumulator = typename std::tuple_element_t<8, ContainerOverSubrelations>;
             auto tmp = (new_term.main_abs_l2_rem_gas -
                         (new_term.main_l2_gas_u16_r0 + (new_term.main_l2_gas_u16_r1 * FF(65536))));
             tmp *= scaling_factor;
-            std::get<10>(evals) += typename Accumulator::View(tmp);
+            std::get<8>(evals) += typename Accumulator::View(tmp);
         }
         {
-            using Accumulator = typename std::tuple_element_t<11, ContainerOverSubrelations>;
+            using Accumulator = typename std::tuple_element_t<9, ContainerOverSubrelations>;
             auto tmp = (new_term.main_abs_da_rem_gas -
                         (new_term.main_da_gas_u16_r0 + (new_term.main_da_gas_u16_r1 * FF(65536))));
             tmp *= scaling_factor;
-            std::get<11>(evals) += typename Accumulator::View(tmp);
+            std::get<9>(evals) += typename Accumulator::View(tmp);
         }
     }
 };
@@ -121,23 +107,17 @@ template <typename FF> class gas : public Relation<gasImpl<FF>> {
         case 0:
             return "IS_GAS_ACCOUNTED";
         case 4:
-            return "L2_GAS_NO_DECREMENT_FAKE_ROW";
-        case 5:
-            return "DA_GAS_NO_DECREMENT_FAKE_ROW";
-        case 6:
             return "L2_GAS_REMAINING_DECREMENT_NOT_CALL";
-        case 7:
+        case 5:
             return "DA_GAS_REMAINING_DECREMENT_NOT_CALL";
         }
         return std::to_string(index);
     }
 
     // Subrelation indices constants, to be used in tests.
     static constexpr size_t SR_IS_GAS_ACCOUNTED = 0;
-    static constexpr size_t SR_L2_GAS_NO_DECREMENT_FAKE_ROW = 4;
-    static constexpr size_t SR_DA_GAS_NO_DECREMENT_FAKE_ROW = 5;
-    static constexpr size_t SR_L2_GAS_REMAINING_DECREMENT_NOT_CALL = 6;
-    static constexpr size_t SR_DA_GAS_REMAINING_DECREMENT_NOT_CALL = 7;
+    static constexpr size_t SR_L2_GAS_REMAINING_DECREMENT_NOT_CALL = 4;
+    static constexpr size_t SR_DA_GAS_REMAINING_DECREMENT_NOT_CALL = 5;
 };
 
 } // namespace bb::avm

barretenberg/cpp/src/barretenberg/vm/avm/tests/arithmetic.test.cpp

@@ -227,8 +227,10 @@ class AvmArithmeticTests : public ::testing::Test {
                                                    .nested_returndata = {},
                                                    .last_pc = 0,
                                                    .success_offset = 0,
-                                                   .l2_gas = 0,
-                                                   .da_gas = 0,
+                                                   .start_l2_gas_left = 0,
+                                                   .start_da_gas_left = 0,
+                                                   .l2_gas_left = 0,
+                                                   .da_gas_left = 0,
                                                    .internal_return_ptr_stack = {} });
         trace_builder.current_ext_call_ctx = ext_call_ctx;
     }

barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp

@@ -193,8 +193,10 @@ TEST_F(AvmCastTests, truncationFFToU16ModMinus1)
                                                .nested_returndata = {},
                                                .last_pc = 0,
                                                .success_offset = 0,
-                                               .l2_gas = 0,
-                                               .da_gas = 0,
+                                               .start_l2_gas_left = 0,
+                                               .start_da_gas_left = 0,
+                                               .l2_gas_left = 0,
+                                               .da_gas_left = 0,
                                                .internal_return_ptr_stack = {} });
     trace_builder.current_ext_call_ctx = ext_call_ctx;
     trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
@@ -222,8 +224,10 @@ TEST_F(AvmCastTests, truncationFFToU16ModMinus2)
                                                .nested_returndata = {},
                                                .last_pc = 0,
                                                .success_offset = 0,
-                                               .l2_gas = 0,
-                                               .da_gas = 0,
+                                               .start_l2_gas_left = 0,
+                                               .start_da_gas_left = 0,
+                                               .l2_gas_left = 0,
+                                               .da_gas_left = 0,
                                                .internal_return_ptr_stack = {} });
     trace_builder.current_ext_call_ctx = ext_call_ctx;
 

barretenberg/cpp/src/barretenberg/vm/avm/tests/slice.test.cpp

@@ -37,8 +37,10 @@ class AvmSliceTests : public ::testing::Test {
                                                    .nested_returndata = {},
                                                    .last_pc = 0,
                                                    .success_offset = 0,
-                                                   .l2_gas = 0,
-                                                   .da_gas = 0,
+                                                   .start_l2_gas_left = 0,
+                                                   .start_da_gas_left = 0,
+                                                   .l2_gas_left = 0,
+                                                   .da_gas_left = 0,
                                                    .internal_return_ptr_stack = {} });
         trace_builder.current_ext_call_ctx = ext_call_ctx;
         this->calldata = calldata;

barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp

@@ -20,6 +20,7 @@ enum class AvmError : uint32_t {
     RADIX_OUT_OF_BOUNDS,
     DUPLICATE_NULLIFIER,
     SIDE_EFFECT_LIMIT_REACHED,
+    OUT_OF_GAS,
 };
 
 } // namespace bb::avm_trace

barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp

@@ -428,6 +428,8 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder,
     // These hints help us to set up first call ctx
     uint32_t clk = trace_builder.get_clk();
     auto context_id = static_cast<uint8_t>(clk);
+    uint32_t l2_gas_allocated_to_enqueued_call = trace_builder.get_l2_gas_left();
+    uint32_t da_gas_allocated_to_enqueued_call = trace_builder.get_da_gas_left();
     trace_builder.current_ext_call_ctx = AvmTraceBuilder::ExtCallCtx{
         .context_id = context_id,
         .parent_id = 0,
@@ -436,10 +438,13 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder,
         .nested_returndata = {},
         .last_pc = 0,
         .success_offset = 0,
-        .l2_gas = 0,
-        .da_gas = 0,
+        .start_l2_gas_left = l2_gas_allocated_to_enqueued_call,
+        .start_da_gas_left = da_gas_allocated_to_enqueued_call,
+        .l2_gas_left = l2_gas_allocated_to_enqueued_call,
+        .da_gas_left = da_gas_allocated_to_enqueued_call,
         .internal_return_ptr_stack = {},
     };
+    trace_builder.allocate_gas_for_call(l2_gas_allocated_to_enqueued_call, da_gas_allocated_to_enqueued_call);
     // Find the bytecode based on contract address of the public call request
     std::vector<uint8_t> bytecode =
         trace_builder.get_bytecode(trace_builder.current_ext_call_ctx.contract_address, check_bytecode_membership);
@@ -451,11 +456,13 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder,
     std::stack<uint32_t> debug_counter_stack;
     uint32_t counter = 0;
     trace_builder.set_call_ptr(context_id);
-    while (is_ok(error) && (pc = trace_builder.get_pc()) < bytecode.size()) {
+    while ((pc = trace_builder.get_pc()) < bytecode.size()) {
         auto [inst, parse_error] = Deserialization::parse(bytecode, pc);
-        error = parse_error;
 
+        // FIXME: properly handle case when an instruction fails parsing
+        // especially first instruction in bytecode
         if (!is_ok(error)) {
+            error = parse_error;
             break;
         }
 
@@ -848,9 +855,10 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder,
                                           std::get<uint16_t>(inst.operands.at(3)),
                                           std::get<uint16_t>(inst.operands.at(4)),
                                           std::get<uint16_t>(inst.operands.at(5)));
+            // TODO: what if an error is encountered on return or call which have already modified stack?
             // We hack it in here the logic to change contract address that we are processing
             bytecode = trace_builder.get_bytecode(trace_builder.current_ext_call_ctx.contract_address,
-                                                  check_bytecode_membership);
+                                                  /*check_membership=*/false);
             debug_counter_stack.push(counter);
             counter = 0;
             break;
@@ -864,7 +872,7 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder,
                                                  std::get<uint16_t>(inst.operands.at(5)));
             // We hack it in here the logic to change contract address that we are processing
             bytecode = trace_builder.get_bytecode(trace_builder.current_ext_call_ctx.contract_address,
-                                                  check_bytecode_membership);
+                                                  /*check_membership=*/false);
             debug_counter_stack.push(counter);
             counter = 0;
             break;
@@ -873,53 +881,58 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder,
             auto ret = trace_builder.op_return(std::get<uint8_t>(inst.operands.at(0)),
                                                std::get<uint16_t>(inst.operands.at(1)),
                                                std::get<uint16_t>(inst.operands.at(2)));
-            // We hack it in here the logic to change contract address that we are processing
+            // did the return opcode hit an exceptional halt?
+            error = ret.error;
             if (ret.is_top_level) {
-                error = ret.error;
                 returndata.insert(returndata.end(), ret.return_data.begin(), ret.return_data.end());
-
-            } else {
+            } else if (is_ok(error)) {
+                // switch back to caller's bytecode
                 bytecode = trace_builder.get_bytecode(trace_builder.current_ext_call_ctx.contract_address,
-                                                      check_bytecode_membership);
+                                                      /*check_membership=*/false);
                 counter = debug_counter_stack.top();
                 debug_counter_stack.pop();
             }
+            // on error/exceptional-halt, jumping back to parent code is handled at bottom of execution loop
             break;
         }
         case OpCode::REVERT_8: {
             info("HIT REVERT_8  ", "[PC=" + std::to_string(pc) + "] " + inst.to_string());
             auto ret = trace_builder.op_revert(std::get<uint8_t>(inst.operands.at(0)),
                                                std::get<uint8_t>(inst.operands.at(1)),
                                                std::get<uint8_t>(inst.operands.at(2)));
+            // error is only set here if the revert opcode hit an exceptional halt
+            // revert itself does not trigger "error"
+            error = ret.error;
             if (ret.is_top_level) {
-                error = ret.error;
                 returndata.insert(returndata.end(), ret.return_data.begin(), ret.return_data.end());
-            } else {
-                // change to the current ext call ctx
+            } else if (is_ok(error)) {
+                // switch back to caller's bytecode
                 bytecode = trace_builder.get_bytecode(trace_builder.current_ext_call_ctx.contract_address,
-                                                      check_bytecode_membership);
+                                                      /*check_membership=*/false);
                 counter = debug_counter_stack.top();
                 debug_counter_stack.pop();
             }
-
+            // on error/exceptional-halt, jumping back to parent code is handled at bottom of execution loop
             break;
         }
         case OpCode::REVERT_16: {
             info("HIT REVERT_16 ", "[PC=" + std::to_string(pc) + "] " + inst.to_string());
             auto ret = trace_builder.op_revert(std::get<uint8_t>(inst.operands.at(0)),
                                                std::get<uint16_t>(inst.operands.at(1)),
                                                std::get<uint16_t>(inst.operands.at(2)));
+            // error is only set here if the revert opcode hit an exceptional halt
+            // revert itself does not trigger "error"
+            error = ret.error;
             if (ret.is_top_level) {
-                error = ret.error;
                 returndata.insert(returndata.end(), ret.return_data.begin(), ret.return_data.end());
-            } else {
-                // change to the current ext call ctx
+            } else if (is_ok(error)) {
+                // switch back to caller's bytecode
                 bytecode = trace_builder.get_bytecode(trace_builder.current_ext_call_ctx.contract_address,
-                                                      check_bytecode_membership);
+                                                      /*check_membership=*/false);
                 counter = debug_counter_stack.top();
                 debug_counter_stack.pop();
             }
-
+            // on error/exceptional-halt, jumping back to parent code is handled at bottom of execution loop
             break;
         }
 
@@ -987,18 +1000,36 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder,
                            ".");
             break;
         }
-    }
-    if (!is_ok(error)) {
-        auto const error_ic = counter - 1; // Need adjustement as counter increment occurs in loop body
-        std::string reason_prefix = exceptionally_halted(error) ? "exceptional halt" : "REVERT opcode";
-        info("AVM enqueued call halted due to ",
-             reason_prefix,
-             ". Error: ",
-             to_name(error),
-             " at PC: ",
-             pc,
-             " IC: ",
-             error_ic);
+
+        if (!is_ok(error)) {
+            const bool is_top_level = trace_builder.current_ext_call_ctx.context_id == 0;
+
+            auto const error_ic = counter - 1; // Need adjustement as counter increment occurs in loop body
+            std::string call_type = is_top_level ? "enqueued" : "nested";
+            info("AVM ",
+                 call_type,
+                 " call exceptionally halted. Error: ",
+                 to_name(error),
+                 " at PC: ",
+                 pc,
+                 " IC: ",
+                 error_ic);
+
+            trace_builder.handle_exceptional_halt();
+
+            if (is_top_level) {
+                break;
+            }
+            // otherwise, handle exceptional halt and proceed with execution in caller/parent
+            // We hack it in here the logic to change contract address that we are processing
+            bytecode = trace_builder.get_bytecode(trace_builder.current_ext_call_ctx.contract_address,
+                                                  /*check_membership=*/false);
+            counter = debug_counter_stack.top();
+            debug_counter_stack.pop();
+
+            // reset error as we've now returned to caller
+            error = AvmError::NO_ERROR;
+        }
     }
     return error;
 }