chore: refactor avm tx execution to generate "empty phase" events (#16676)

This simplifies tracegen so that it doesn't need to know about setup
snapshots and rollbacks anymore, doesn't need special handling for
situations when a phase is empty, and omits rows for phases skipped due
to reverts.

Note that there are two different relevant scenarios:
1. A phase is just "empty". The phase is encountered normally, but has
no contents, like when there are no revertible nullifiers to insert, or
when app logic has no enqueued calls to execute. **In this case, the
phase gets a padding row**.
2. A phase is "skipped" entirely because of a revert. Like if revertible
nullifier insertions fail, app-logic won't ever be encountered. **In
this case, app-logic doesn't even get a row.**

This PR specifically generates events for scenario 1, when a phase is
encountered, but "empty". This "empty" event drives tracegen of a
"padding" row.

Creation of an event that drives padding rows means that tracegen no
longer needs to know about setup snapshots and rollbacks to ensure that
a padding row after a revert gets the post-revert state! Because now the
post-revert state will be in the `EmptyPhaseEvent`, so tracegen can just
generate a row from the phase event as it does for other rows.

Also note that this now lets us easily differentiate between "empty"
phases versus skipped phases. Now, in tracegen, if a phase has no
events, it means that it was _skipped_ because if it was "empty" in
simulation, it would've generated an `EmptyPhaseEvent`. So, this PR also
stops generating padding rows for _skipped_ phases, which was the
originally intended mechanism proposed by @IlyasRidhuan, but it was not
implemented that way.

Author: dbanks12

barretenberg/cpp/src/barretenberg/vm2/common/aztec_types.hpp

@@ -29,6 +29,7 @@ enum TransactionPhase {
     COLLECT_GAS_FEES = 10,
     TREE_PADDING = 11,
     CLEANUP = 12,
+    LAST = CLEANUP,
 };
 
 using InternalCallId = uint32_t;

barretenberg/cpp/src/barretenberg/vm2/simulation/events/tx_events.hpp

@@ -48,12 +48,15 @@ struct PadTreesEvent {};
 
 struct CleanupEvent {};
 
+struct EmptyPhaseEvent {};
+
 using TxPhaseEventType = std::variant<EnqueuedCallEvent,
                                       PrivateAppendTreeEvent,
                                       PrivateEmitL2L1MessageEvent,
                                       CollectGasFeeEvent,
                                       PadTreesEvent,
-                                      CleanupEvent>;
+                                      CleanupEvent,
+                                      EmptyPhaseEvent>;
 
 struct TxPhaseEvent {
     TransactionPhase phase;

barretenberg/cpp/src/barretenberg/vm2/simulation/tx_execution.cpp

@@ -83,50 +83,11 @@ void TxExecution::simulate(const Tx& tx)
     insert_non_revertibles(tx);
 
     // Setup.
-    for (const auto& call : tx.setupEnqueuedCalls) {
-        info("[SETUP] Executing enqueued call to ", call.request.contractAddress);
-        TxContextEvent state_before = tx_context.serialize_tx_context_event();
-        Gas start_gas = tx_context.gas_used;
-        auto context = context_provider.make_enqueued_context(call.request.contractAddress,
-                                                              call.request.msgSender,
-                                                              /*transaction_fee=*/FF(0),
-                                                              call.calldata,
-                                                              call.request.isStaticCall,
-                                                              gas_limit,
-                                                              start_gas,
-                                                              tx_context.side_effect_states,
-                                                              TransactionPhase::SETUP);
-        // This call should not throw unless it's an unexpected unrecoverable failure.
-        ExecutionResult result = call_execution.execute(std::move(context));
-        tx_context.side_effect_states = result.side_effect_states;
-        tx_context.gas_used = result.gas_used;
-        emit_public_call_request(call,
-                                 TransactionPhase::SETUP,
-                                 /*transaction_fee=*/FF(0),
-                                 result.success,
-                                 start_gas,
-                                 tx_context.gas_used,
-                                 state_before,
-                                 tx_context.serialize_tx_context_event());
-        if (!result.success) {
-            // This will result in an unprovable tx.
-            throw TxExecutionException(
-                format("[SETUP] UNRECOVERABLE ERROR! Enqueued call to ", call.request.contractAddress, " failed"));
-        }
-    }
-    SideEffectStates end_setup_side_effect_states = tx_context.side_effect_states;
-
-    // The checkpoint we should go back to if anything from now on reverts.
-    merkle_db.create_checkpoint();
-
-    try {
-        // Insert revertibles. This can throw if there is a nullifier collision.
-        // Such an exception should be handled and the tx be provable.
-        insert_revertibles(tx);
-
-        // App logic.
-        for (const auto& call : tx.appLogicEnqueuedCalls) {
-            info("[APP_LOGIC] Executing enqueued call to ", call.request.contractAddress);
+    if (tx.setupEnqueuedCalls.empty()) {
+        emit_empty_phase(TransactionPhase::SETUP);
+    } else {
+        for (const auto& call : tx.setupEnqueuedCalls) {
+            info("[SETUP] Executing enqueued call to ", call.request.contractAddress);
             TxContextEvent state_before = tx_context.serialize_tx_context_event();
             Gas start_gas = tx_context.gas_used;
             auto context = context_provider.make_enqueued_context(call.request.contractAddress,
@@ -137,23 +98,70 @@ void TxExecution::simulate(const Tx& tx)
                                                                   gas_limit,
                                                                   start_gas,
                                                                   tx_context.side_effect_states,
-                                                                  TransactionPhase::APP_LOGIC);
+                                                                  TransactionPhase::SETUP);
             // This call should not throw unless it's an unexpected unrecoverable failure.
             ExecutionResult result = call_execution.execute(std::move(context));
             tx_context.side_effect_states = result.side_effect_states;
             tx_context.gas_used = result.gas_used;
             emit_public_call_request(call,
-                                     TransactionPhase::APP_LOGIC,
+                                     TransactionPhase::SETUP,
                                      /*transaction_fee=*/FF(0),
                                      result.success,
                                      start_gas,
                                      tx_context.gas_used,
                                      state_before,
                                      tx_context.serialize_tx_context_event());
             if (!result.success) {
-                // This exception should be handled and the tx be provable.
+                // This will result in an unprovable tx.
                 throw TxExecutionException(
-                    format("[APP_LOGIC] Enqueued call to ", call.request.contractAddress, " failed"));
+                    format("[SETUP] UNRECOVERABLE ERROR! Enqueued call to ", call.request.contractAddress, " failed"));
+            }
+        }
+    }
+    SideEffectStates end_setup_side_effect_states = tx_context.side_effect_states;
+
+    // The checkpoint we should go back to if anything from now on reverts.
+    merkle_db.create_checkpoint();
+
+    try {
+        // Insert revertibles. This can throw if there is a nullifier collision.
+        // Such an exception should be handled and the tx be provable.
+        insert_revertibles(tx);
+
+        // App logic.
+        if (tx.appLogicEnqueuedCalls.empty()) {
+            emit_empty_phase(TransactionPhase::APP_LOGIC);
+        } else {
+            for (const auto& call : tx.appLogicEnqueuedCalls) {
+                info("[APP_LOGIC] Executing enqueued call to ", call.request.contractAddress);
+                TxContextEvent state_before = tx_context.serialize_tx_context_event();
+                Gas start_gas = tx_context.gas_used;
+                auto context = context_provider.make_enqueued_context(call.request.contractAddress,
+                                                                      call.request.msgSender,
+                                                                      /*transaction_fee=*/FF(0),
+                                                                      call.calldata,
+                                                                      call.request.isStaticCall,
+                                                                      gas_limit,
+                                                                      start_gas,
+                                                                      tx_context.side_effect_states,
+                                                                      TransactionPhase::APP_LOGIC);
+                // This call should not throw unless it's an unexpected unrecoverable failure.
+                ExecutionResult result = call_execution.execute(std::move(context));
+                tx_context.side_effect_states = result.side_effect_states;
+                tx_context.gas_used = result.gas_used;
+                emit_public_call_request(call,
+                                         TransactionPhase::APP_LOGIC,
+                                         /*transaction_fee=*/FF(0),
+                                         result.success,
+                                         start_gas,
+                                         tx_context.gas_used,
+                                         state_before,
+                                         tx_context.serialize_tx_context_event());
+                if (!result.success) {
+                    // This exception should be handled and the tx be provable.
+                    throw TxExecutionException(
+                        format("[APP_LOGIC] Enqueued call to ", call.request.contractAddress, " failed"));
+                }
             }
         }
     } catch (const TxExecutionException& e) {
@@ -175,7 +183,9 @@ void TxExecution::simulate(const Tx& tx)
 
     // Teardown.
     try {
-        if (tx.teardownEnqueuedCall) {
+        if (!tx.teardownEnqueuedCall) {
+            emit_empty_phase(TransactionPhase::TEARDOWN);
+        } else {
             info("[TEARDOWN] Executing enqueued call to ", tx.teardownEnqueuedCall->request.contractAddress);
             // Teardown has its own gas limit and usage.
             Gas start_gas = { 0, 0 };
@@ -334,18 +344,30 @@ void TxExecution::insert_non_revertibles(const Tx& tx)
          tx.hash);
 
     // 1. Write the already siloed nullifiers.
-    for (const auto& nullifier : tx.nonRevertibleAccumulatedData.nullifiers) {
-        emit_nullifier(false, nullifier);
+    if (tx.nonRevertibleAccumulatedData.nullifiers.empty()) {
+        emit_empty_phase(TransactionPhase::NR_NULLIFIER_INSERTION);
+    } else {
+        for (const auto& nullifier : tx.nonRevertibleAccumulatedData.nullifiers) {
+            emit_nullifier(false, nullifier);
+        }
     }
 
     // 2. Write already unique note hashes.
-    for (const auto& unique_note_hash : tx.nonRevertibleAccumulatedData.noteHashes) {
-        emit_note_hash(false, unique_note_hash);
+    if (tx.nonRevertibleAccumulatedData.noteHashes.empty()) {
+        emit_empty_phase(TransactionPhase::NR_NOTE_INSERTION);
+    } else {
+        for (const auto& unique_note_hash : tx.nonRevertibleAccumulatedData.noteHashes) {
+            emit_note_hash(false, unique_note_hash);
+        }
     }
 
     // 3. Write l2_l1 messages
-    for (const auto& l2_to_l1_msg : tx.nonRevertibleAccumulatedData.l2ToL1Messages) {
-        emit_l2_to_l1_message(false, l2_to_l1_msg);
+    if (tx.nonRevertibleAccumulatedData.l2ToL1Messages.empty()) {
+        emit_empty_phase(TransactionPhase::NR_L2_TO_L1_MESSAGE);
+    } else {
+        for (const auto& l2_to_l1_msg : tx.nonRevertibleAccumulatedData.l2ToL1Messages) {
+            emit_l2_to_l1_message(false, l2_to_l1_msg);
+        }
     }
 }
 
@@ -362,18 +384,30 @@ void TxExecution::insert_revertibles(const Tx& tx)
          tx.hash);
 
     // 1. Write the already siloed nullifiers.
-    for (const auto& siloed_nullifier : tx.revertibleAccumulatedData.nullifiers) {
-        emit_nullifier(true, siloed_nullifier);
+    if (tx.revertibleAccumulatedData.nullifiers.empty()) {
+        emit_empty_phase(TransactionPhase::R_NULLIFIER_INSERTION);
+    } else {
+        for (const auto& siloed_nullifier : tx.revertibleAccumulatedData.nullifiers) {
+            emit_nullifier(true, siloed_nullifier);
+        }
     }
 
     // 2. Write the siloed non uniqued note hashes
-    for (const auto& siloed_note_hash : tx.revertibleAccumulatedData.noteHashes) {
-        emit_note_hash(true, siloed_note_hash);
+    if (tx.revertibleAccumulatedData.noteHashes.empty()) {
+        emit_empty_phase(TransactionPhase::R_NOTE_INSERTION);
+    } else {
+        for (const auto& siloed_note_hash : tx.revertibleAccumulatedData.noteHashes) {
+            emit_note_hash(true, siloed_note_hash);
+        }
     }
 
     // 3. Write L2 to L1 messages.
-    for (const auto& l2_to_l1_msg : tx.revertibleAccumulatedData.l2ToL1Messages) {
-        emit_l2_to_l1_message(true, l2_to_l1_msg);
+    if (tx.revertibleAccumulatedData.l2ToL1Messages.empty()) {
+        emit_empty_phase(TransactionPhase::R_L2_TO_L1_MESSAGE);
+    } else {
+        for (const auto& l2_to_l1_msg : tx.revertibleAccumulatedData.l2ToL1Messages) {
+            emit_l2_to_l1_message(true, l2_to_l1_msg);
+        }
     }
 }
 
@@ -426,4 +460,14 @@ void TxExecution::cleanup()
                               .event = CleanupEvent{} });
 }
 
+void TxExecution::emit_empty_phase(TransactionPhase phase)
+{
+    TxContextEvent current_state = tx_context.serialize_tx_context_event();
+    events.emit(TxPhaseEvent{ .phase = phase,
+                              .state_before = current_state,
+                              .state_after = current_state,
+                              .reverted = false,
+                              .event = EmptyPhaseEvent{} });
+}
+
 } // namespace bb::avm2::simulation

barretenberg/cpp/src/barretenberg/vm2/simulation/tx_execution.hpp

@@ -66,6 +66,8 @@ class TxExecution final {
     void pad_trees();
 
     void cleanup();
+
+    void emit_empty_phase(TransactionPhase phase);
 };
 
 } // namespace bb::avm2::simulation

barretenberg/cpp/src/barretenberg/vm2/tracegen/tx_trace.cpp

@@ -28,7 +28,7 @@ template <class... Ts> struct overloaded : Ts... {
 // explicit deduction guide (not needed as of C++20)
 template <class... Ts> overloaded(Ts...) -> overloaded<Ts...>;
 
-constexpr size_t NUM_PHASES = 12; // See TransactionPhase enum
+constexpr size_t NUM_PHASES = static_cast<size_t>(TransactionPhase::LAST);
 
 bool is_revertible(TransactionPhase phase)
 {
@@ -529,7 +529,6 @@ void TxTraceBuilder::process(const simulation::EventEmitterInterface<simulation:
 
     // This is the tree state we will use during the "skipped" phases
     TxContextEvent propagated_state = startup_event_data.state;
-    TxContextEvent end_setup_snapshot = startup_event_data.state;
     // Used to track the gas limit for the "padded" phases.
     Gas current_gas_limit = startup_event_data.gas_limit;
     Gas teardown_gas_limit = startup_event_data.teardown_gas_limit;
@@ -538,6 +537,12 @@ void TxTraceBuilder::process(const simulation::EventEmitterInterface<simulation:
     // Go through each phase except startup and process the events in the phase
     for (uint32_t i = 0; i < NUM_PHASES; i++) {
         const auto& phase_events = phase_buckets[i];
+        if (phase_events.empty()) {
+            // There will be no events for a phase if it is skipped (jumped over) due to a revert.
+            // This is different from a phase that has an EmptyPhaseEvent, which is a phase that has no contents to
+            // process, like when app logic starts but has no enqueued calls.
+            continue;
+        }
 
         TransactionPhase phase = phase_array[i];
 
@@ -555,24 +560,6 @@ void TxTraceBuilder::process(const simulation::EventEmitterInterface<simulation:
             current_gas_limit = teardown_gas_limit;
         }
 
-        if (phase_events.empty()) {
-            trace.set(row, insert_state(propagated_state, propagated_state));
-            trace.set(row, handle_padded_row(phase, gas_used, discard));
-            trace.set(row, handle_pi_read(phase, /*phase_length=*/0, /*read_counter*/ 0));
-            trace.set(row, handle_prev_gas_used(gas_used));
-            trace.set(row, handle_next_gas_used(gas_used));
-            trace.set(row, handle_gas_limit(current_gas_limit));
-            trace.set(row, handle_state_change_selectors(phase));
-            if (row == 1) {
-                trace.set(row, handle_first_row());
-            }
-            if (phase == TransactionPhase::SETUP) {
-                // If setup is empty, the end-setup-snapshot should just be current/propagated state
-                end_setup_snapshot = propagated_state;
-            }
-            row++;
-            continue;
-        }
         // Count the number of steps in this phase
         uint32_t phase_counter = 0;
         uint32_t phase_length = static_cast<uint32_t>(phase_events.size());
@@ -591,7 +578,7 @@ void TxTraceBuilder::process(const simulation::EventEmitterInterface<simulation:
                     { C::tx_discard, discard ? 1 : 0 },
                     { C::tx_phase_value, static_cast<uint8_t>(tx_phase_event->phase) },
                     { Column::tx_setup_phase_value, static_cast<uint8_t>(TransactionPhase::SETUP) },
-                    { C::tx_is_padded, 0 },
+                    { C::tx_is_padded, 0 }, // overidden below if this is a skipped phase event
                     { C::tx_start_phase, phase_counter == 0 ? 1 : 0 },
                     { C::tx_sel_read_phase_length, phase_counter == 0 && !is_one_shot_phase(tx_phase_event->phase) },
                     { C::tx_is_revertible, is_revertible(tx_phase_event->phase) ? 1 : 0 },
@@ -640,40 +627,23 @@ void TxTraceBuilder::process(const simulation::EventEmitterInterface<simulation:
                             [&](const simulation::CleanupEvent&) {
                                 trace.set(row, handle_pi_read(tx_phase_event->phase, 1, 0));
                                 trace.set(row, handle_cleanup());
+                            },
+                            [&](const simulation::EmptyPhaseEvent&) {
+                                // EmptyPhaseEvent represents a phase that is not explicitly skipped because of a
+                                // revert, but just has no contents to process, like when app logic starts but has no
+                                // enqueued calls.
+                                trace.set(row, handle_pi_read(tx_phase_event->phase, 0, 0));
+                                trace.set(row, handle_padded_row(tx_phase_event->phase, gas_used, discard));
                             } },
                 tx_phase_event->event);
             trace.set(row, handle_next_gas_used(gas_used));
             trace.set(row, handle_gas_limit(current_gas_limit));
 
-            // Handle a potential phase jump due to a revert, we dont need to check if we are in a revertible phase
-            // since our witgen will have exited for any reverts in a non-revertible phase.
-            // If we revert in a phase that isnt TEARDOWN, we jump to TEARDOWN
-            if (tx_phase_event->reverted && tx_phase_event->phase != TransactionPhase::TEARDOWN) {
-                // Jump to the TEARDOWN phase
-                // we need to -2 because of the loop increment and because the enum is 1-indexed
-                i = static_cast<uint8_t>(TransactionPhase::TEARDOWN) - 2;
-            }
             phase_counter++;
             row++;
         }
         // In case we encounter another skip row
         propagated_state = phase_events.back()->state_after;
-
-        if (phase == TransactionPhase::SETUP) {
-            // Store off the state at the end of setup to rollback to later on revert
-            end_setup_snapshot = phase_events.back()->state_after;
-        }
-        if (phase_events.back()->reverted) {
-            // On revert, roll back to end-setup-snapshot
-            // Even though tx-execution events should already do this,
-            // we need to update propagated state here so that any padded rows
-            // get the correct rolled-back state rather then the pre-rollback state.
-            propagated_state.tree_states = end_setup_snapshot.tree_states;
-            propagated_state.written_public_data_slots_tree_snapshot =
-                end_setup_snapshot.written_public_data_slots_tree_snapshot;
-            propagated_state.side_effect_states = end_setup_snapshot.side_effect_states;
-            // Note: we only rollback tree/side-effect states, not gas used or next_context_id.
-        }
     }
 }