fix: AVM witgen track gas for nested calls and external halts (#10731)

Resolves #10033
Resolves #10374

This PR does the following:
- Witgen handles out-of-gas errors for all opcodes 
- all halts (return/revert/exceptional) work as follows:
- charge gas for the problematic instruction as always, adding a row to
the gas trace
    - pop the parent/caller's latest gas from the stack
- call a helper function on the gas trace to mutate that most recent gas
row, returning to the parent's latest gas minus any consumed gas (all
gas consumed on exceptional halt)
- `GasTraceEntry` includes a field `is_halt_or_first_row_in_nested_call`
which lets us break gas rules on a halt or when starting a nested call
because in both cases gas will jump.
- `constrain_gas` returns a bool `out_of_gas` so that opcode
implementations can handle out of gas
- `write_to_memory` now has an option to skip the "jump back to correct
pc" which was problematic when halting because the `jump` wouldn't
result in a next row with the right pc

Explanation on how gas works for calls:
- Parent snapshots its gas right before a nested call in
`ctx.*_gas_left`
- Nested call is given a `ctx.start_*_gas_left` and the gas trace is
forced to that same value
- throughout the nested call, the gas trace operates normally, charging
per instruction
- when any halt is encountered, the instruction that halted must have
its gas charged normally, but then we call a helper function on the gas
trace to mutate the most recent row, flagging it to eventually become a
sort of "fake" row that skips some constraints
- the mutation of the halting row resets the gas to the parents last gas
before the call (minus however much gas was consumed by the nested
call... if exceptional halt, that is _all_ allocated gas)


Follow-up work
- properly constrain gas for nested calls, returns, reverts and
exceptional halts
- if `jump` exceptionally halts (i.e. out of gas), it should be okay
that the next row doesn't have the target pc
- Handle the edge case when an error is encountered on
return/revert/call, but after the stack has already been modified

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;
 }