flamenco: clean up instr info + trace (#7451)

Author: mjain-jump

contrib/test/test-vectors-commit-sha.txt

@@ -1 +1 @@
-3040262e57e97af58ade353ac3f32c9f7f181d8b
+155ea679befde81a71977eaf8ebf20a23d6d1fc3

src/discof/exec/fd_exec_tile.c

@@ -303,14 +303,16 @@ unprivileged_init( fd_topo_t *      topo,
   /* Runtime                                                          */
   /********************************************************************/
 
-  ctx->runtime->accdb = ctx->accdb;
-  ctx->runtime->funk = fd_accdb_user_v1_funk( ctx->accdb );
-  ctx->runtime->progcache = ctx->progcache;
-  ctx->runtime->status_cache = ctx->txncache;
-  ctx->runtime->log.log_collector = &ctx->log_collector;
+  ctx->runtime->accdb                    = ctx->accdb;
+  ctx->runtime->funk                     = fd_accdb_user_v1_funk( ctx->accdb );
+  ctx->runtime->progcache                = ctx->progcache;
+  ctx->runtime->status_cache             = ctx->txncache;
+  ctx->runtime->log.log_collector        = &ctx->log_collector;
   ctx->runtime->log.enable_log_collector = 0;
-  ctx->runtime->log.dumping_mem = ctx->dumping_mem;
-  ctx->runtime->log.tracing_mem = &ctx->tracing_mem[0][0];
+  ctx->runtime->log.dumping_mem          = ctx->dumping_mem;
+  ctx->runtime->log.enable_vm_tracing    = 0;
+  ctx->runtime->log.tracing_mem          = &ctx->tracing_mem[0][0];
+  ctx->runtime->log.capture_ctx          = ctx->capture_ctx;
 }
 
 /* Publish the next account update event buffered in the capture tile to the replay tile

src/flamenco/runtime/fd_executor.c

@@ -405,22 +405,6 @@ fd_executor_verify_transaction( fd_bank_t *         bank,
   return FD_RUNTIME_EXECUTE_SUCCESS;
 }
 
-static void
-fd_executor_setup_instr_infos_from_txn_instrs( fd_runtime_t *      runtime,
-                                               fd_bank_t *         bank,
-                                               fd_txn_in_t const * txn_in,
-                                               fd_txn_out_t *      txn_out ) {
-  ushort instr_cnt = TXN( txn_in->txn )->instr_cnt;
-
-  /* Set up the instr infos for the transaction */
-  for( ushort i=0; i<instr_cnt; i++ ) {
-    fd_txn_instr_t const * instr = &TXN( txn_in->txn )->instr[i];
-    fd_instr_info_init_from_txn_instr( &runtime->instr.infos[i], bank, txn_in, txn_out, instr );
-  }
-
-  runtime->instr.info_cnt = instr_cnt;
-}
-
 /* https://github.com/anza-xyz/agave/blob/v2.0.9/svm/src/account_loader.rs#L410-427 */
 static int
 accumulate_and_check_loaded_account_data_size( ulong   acc_size,
@@ -456,8 +440,7 @@ accumulate_and_check_loaded_account_data_size( ulong   acc_size,
 
    https://github.com/anza-xyz/agave/blob/v2.3.1/svm/src/account_loader.rs#L199-L228 */
 static ulong
-load_transaction_account( fd_runtime_t *      runtime,
-                          fd_bank_t *         bank,
+load_transaction_account( fd_bank_t *         bank,
                           fd_txn_in_t const * txn_in,
                           fd_txn_out_t *      txn_out,
                           fd_txn_account_t *  acct,
@@ -472,7 +455,7 @@ load_transaction_account( fd_runtime_t *      runtime,
        constructed by the SVM and modified within each transaction's
        instruction execution only, so it incurs a loaded size cost
        of 0. */
-    fd_sysvar_instructions_serialize_account( txn_in, txn_out, (fd_instr_info_t const *)runtime->instr.infos, TXN( txn_in->txn )->instr_cnt, txn_idx );
+    fd_sysvar_instructions_serialize_account( bank, txn_in, txn_out, txn_idx );
     return 0UL;
   }
 
@@ -549,7 +532,7 @@ fd_executor_load_transaction_accounts_old( fd_runtime_t *      runtime,
     }
 
     /* https://github.com/anza-xyz/agave/blob/v2.3.1/svm/src/account_loader.rs#L733-L740 */
-    ulong loaded_acc_size = load_transaction_account( runtime, bank, txn_in, txn_out, acct, is_writable, unknown_acc, i );
+    ulong loaded_acc_size = load_transaction_account( bank, txn_in, txn_out, acct, is_writable, unknown_acc, i );
     int err = accumulate_and_check_loaded_account_data_size( loaded_acc_size,
                                                              requested_loaded_accounts_data_size,
                                                              &txn_out->details.loaded_accounts_data_size );
@@ -816,7 +799,7 @@ fd_executor_load_transaction_accounts_simd_186( fd_runtime_t *      runtime,
 
     /* Load and collect any remaining accounts
        https://github.com/anza-xyz/agave/blob/v2.3.1/svm/src/account_loader.rs#L652-L659 */
-    ulong loaded_acc_size = load_transaction_account( runtime, bank, txn_in, txn_out, acct, is_writable, unknown_acc, i );
+    ulong loaded_acc_size = load_transaction_account( bank, txn_in, txn_out, acct, is_writable, unknown_acc, i );
     int err = fd_collect_loaded_account(
       runtime,
       txn_out,
@@ -1112,14 +1095,14 @@ fd_executor_setup_txn_alut_account_keys( fd_runtime_t *      runtime,
   return FD_RUNTIME_EXECUTE_SUCCESS;
 }
 
-/* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L319-L357 */
+/* https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L319-L357 */
 static inline int
 fd_txn_ctx_push( fd_runtime_t *      runtime,
                  fd_txn_in_t const * txn_in,
                  fd_txn_out_t *      txn_out,
                  fd_instr_info_t *   instr ) {
   /* Earlier checks in the permalink are redundant since Agave maintains instr stack and trace accounts separately
-     https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L327-L328 */
+     https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L327-L328 */
   ulong starting_lamports_h = 0UL;
   ulong starting_lamports_l = 0UL;
   int err = fd_instr_info_sum_account_lamports( instr,
@@ -1133,16 +1116,16 @@ fd_txn_ctx_push( fd_runtime_t *      runtime,
   instr->starting_lamports_l = starting_lamports_l;
 
   /* Check that the caller's lamport sum has not changed.
-     https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L329-L340 */
+     https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L329-L340 */
   if( runtime->instr.stack_sz>0 ) {
-    /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L330 */
+    /* https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L330 */
     fd_exec_instr_ctx_t const * caller_instruction_context = &runtime->instr.stack[ runtime->instr.stack_sz-1 ];
 
-    /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L331-L332 */
+    /* https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L331-L332 */
     ulong original_caller_lamport_sum_h = caller_instruction_context->instr->starting_lamports_h;
     ulong original_caller_lamport_sum_l = caller_instruction_context->instr->starting_lamports_l;
 
-    /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L333-L334 */
+    /* https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L333-L334 */
     ulong current_caller_lamport_sum_h = 0UL;
     ulong current_caller_lamport_sum_l = 0UL;
     int err = fd_instr_info_sum_account_lamports( caller_instruction_context->instr,
@@ -1153,20 +1136,22 @@ fd_txn_ctx_push( fd_runtime_t *      runtime,
       return err;
     }
 
-    /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L335-L339 */
+    /* https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L335-L339 */
     if( FD_UNLIKELY( current_caller_lamport_sum_h!=original_caller_lamport_sum_h ||
                      current_caller_lamport_sum_l!=original_caller_lamport_sum_l ) ) {
       return FD_EXECUTOR_INSTR_ERR_UNBALANCED_INSTR;
     }
   }
 
-  /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L347-L351 */
-  if( FD_UNLIKELY( runtime->instr.trace_length>=FD_MAX_INSTRUCTION_TRACE_LENGTH ) ) {
+  /* Note that we don't update the trace length here - since the caller
+     allocates out of the trace array, they are also responsible for
+     incrementing the trace length variable.
+     https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L347-L351 */
+  if( FD_UNLIKELY( runtime->instr.trace_length>FD_MAX_INSTRUCTION_TRACE_LENGTH ) ) {
     return FD_EXECUTOR_INSTR_ERR_MAX_INSN_TRACE_LENS_EXCEEDED;
   }
-  runtime->instr.trace_length++;
 
-  /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L352-L356 */
+  /* https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L352-L356 */
   if( FD_UNLIKELY( runtime->instr.stack_sz>=FD_MAX_INSTRUCTION_STACK_DEPTH ) ) {
     return FD_EXECUTOR_INSTR_ERR_CALL_DEPTH;
   }
@@ -1200,7 +1185,7 @@ fd_txn_ctx_push( fd_runtime_t *      runtime,
    and checks for reentrancy violations. If successful, simply increments the instruction stack and trace size and returns. It is
    the responsibility of the caller to populate the newly pushed instruction fields, which are undefined otherwise.
 
-   https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/program-runtime/src/invoke_context.rs#L246-L290 */
+   https://github.com/anza-xyz/agave/blob/v2.0.0/program-runtime/src/invoke_context.rs#L246-L290 */
 int
 fd_instr_stack_push( fd_runtime_t *      runtime,
                      fd_txn_in_t const * txn_in,
@@ -1224,9 +1209,9 @@ fd_instr_stack_push( fd_runtime_t *      runtime,
     return FD_EXECUTOR_INSTR_ERR_UNSUPPORTED_PROGRAM_ID;
   }
 
-  /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/program-runtime/src/invoke_context.rs#L256-L286 */
+  /* https://github.com/anza-xyz/agave/blob/v2.0.0/program-runtime/src/invoke_context.rs#L256-L286 */
   if( runtime->instr.stack_sz ) {
-    /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/program-runtime/src/invoke_context.rs#L261-L285 */
+    /* https://github.com/anza-xyz/agave/blob/v2.0.0/program-runtime/src/invoke_context.rs#L261-L285 */
     uchar contains = 0;
     uchar is_last  = 0;
 
@@ -1242,26 +1227,26 @@ fd_instr_stack_push( fd_runtime_t *      runtime,
         contains = 1;
       }
     }
-    /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/program-runtime/src/invoke_context.rs#L282-L285 */
+    /* https://github.com/anza-xyz/agave/blob/v2.0.0/program-runtime/src/invoke_context.rs#L282-L285 */
     if( FD_UNLIKELY( contains && !is_last ) ) {
       return FD_EXECUTOR_INSTR_ERR_REENTRANCY_NOT_ALLOWED;
     }
   }
   /* "Push" a new instruction onto the stack by simply incrementing the stack and trace size counters
-     https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/program-runtime/src/invoke_context.rs#L289 */
+     https://github.com/anza-xyz/agave/blob/v2.0.0/program-runtime/src/invoke_context.rs#L289 */
   return fd_txn_ctx_push( runtime, txn_in, txn_out, instr );
 }
 
 /* Pops an instruction from the instruction stack. Agave's implementation performs instruction balancing checks every time pop is called,
    but error codes returned from `pop` are only used if the program's execution was successful. Therefore, we can optimize our code by only
    checking for unbalanced instructions if the program execution was successful within fd_execute_instr.
 
-   https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/program-runtime/src/invoke_context.rs#L293-L298 */
+   https://github.com/anza-xyz/agave/blob/v2.0.0/program-runtime/src/invoke_context.rs#L293-L298 */
 int
 fd_instr_stack_pop( fd_runtime_t *          runtime,
                     fd_txn_out_t *          txn_out,
                     fd_instr_info_t const * instr ) {
-  /* https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L362-L364 */
+  /* https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L362-L364 */
   if( FD_UNLIKELY( runtime->instr.stack_sz==0 ) ) {
     return FD_EXECUTOR_INSTR_ERR_CALL_DEPTH;
   }
@@ -1279,7 +1264,7 @@ fd_instr_stack_pop( fd_runtime_t *          runtime,
   }
 
   /* Verify lamports are balanced before and after instruction
-     https://github.com/anza-xyz/agave/blob/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/sdk/src/transaction_context.rs#L366-L380 */
+     https://github.com/anza-xyz/agave/blob/v2.0.0/sdk/src/transaction_context.rs#L366-L380 */
   ulong ending_lamports_h = 0UL;
   ulong ending_lamports_l = 0UL;
   int err = fd_instr_info_sum_account_lamports( instr,
@@ -1342,11 +1327,6 @@ fd_execute_instr( fd_runtime_t *      runtime,
   };
   fd_base58_encode_32( txn_out->accounts.accounts[ instr->program_id ].pubkey->uc, NULL, ctx->program_id_base58 );
 
-  runtime->instr.trace[ runtime->instr.trace_length - 1 ] = (fd_exec_instr_trace_entry_t) {
-    .instr_info = instr,
-    .stack_height = runtime->instr.stack_sz,
-  };
-
   /* Look up the native program. We check for precompiles within the lookup function as well.
      https://github.com/anza-xyz/agave/blob/v2.1.6/svm/src/message_processor.rs#L88 */
   fd_exec_instr_fn_t native_prog_fn;
@@ -1588,9 +1568,6 @@ fd_executor_setup_accounts_for_txn( fd_runtime_t *      runtime,
 
   txn_out->accounts.nonce_idx_in_txn = ULONG_MAX;
   runtime->executable.cnt            = executable_idx;
-
-  /* Set up instr infos from the txn descriptor. No Agave equivalent to this function. */
-  fd_executor_setup_instr_infos_from_txn_instrs( runtime, bank, txn_in, txn_out );
 }
 
 int
@@ -1620,19 +1597,34 @@ fd_execute_txn( fd_runtime_t *      runtime,
   bool dump_insn = runtime->log.capture_ctx && fd_bank_slot_get( bank ) >= runtime->log.capture_ctx->dump_proto_start_slot && runtime->log.capture_ctx->dump_instr_to_pb;
   (void)dump_insn;
 
+  fd_txn_t const * txn = TXN( txn_in->txn );
+
   /* Initialize log collection. */
   fd_log_collector_init( runtime->log.log_collector, runtime->log.enable_log_collector );
 
   for( ushort i=0; i<TXN( txn_in->txn )->instr_cnt; i++ ) {
-    runtime->instr.current_idx = i;
+    /* Set up the instr info for the current instruction */
+    fd_instr_info_t * instr_info = &runtime->instr.trace[runtime->instr.trace_length++];
+    fd_instr_info_init_from_txn_instr(
+        instr_info,
+        bank,
+        txn_in,
+        txn_out,
+        &txn->instr[i]
+    );
+
 #   if FD_HAS_FLATCC
     if( FD_UNLIKELY( dump_insn ) ) {
       // Capture the input and convert it into a Protobuf message
-      fd_dump_instr_to_protobuf( runtime, bank, txn_in, txn_out, &runtime->instr.infos[i], i );
+      fd_dump_instr_to_protobuf( runtime, bank, txn_in, txn_out, instr_info, i );
     }
 #   endif
 
-    int instr_exec_result = fd_execute_instr( runtime, bank, txn_in, txn_out, &runtime->instr.infos[i] );
+    /* Update the current executing instruction index */
+    runtime->instr.current_idx = i;
+
+    /* Execute the current instruction */
+    int instr_exec_result = fd_execute_instr( runtime, bank, txn_in, txn_out, instr_info );
     if( FD_UNLIKELY( instr_exec_result!=FD_EXECUTOR_INSTR_SUCCESS ) ) {
       if( txn_out->err.exec_err_idx==INT_MAX ) {
         txn_out->err.exec_err_idx = i;

src/flamenco/runtime/fd_runtime.c

@@ -1341,7 +1341,6 @@ fd_runtime_prepare_and_execute_txn( fd_runtime_t *       runtime,
 
   runtime->executable.cnt = 0UL;
   runtime->log.enable_log_collector = 0;
-  runtime->instr.info_cnt     = 0UL;
   runtime->instr.trace_length = 0UL;
   runtime->instr.current_idx  = 0;
   runtime->instr.stack_sz     = 0;

src/flamenco/runtime/fd_runtime.h

@@ -13,22 +13,22 @@ struct fd_runtime {
     uchar                       stack_sz;                                /* Current depth of the instruction execution stack. */
     fd_exec_instr_ctx_t         stack[ FD_MAX_INSTRUCTION_STACK_DEPTH ]; /* Instruction execution stack. */
     /* The memory for all of the instructions in the transaction
-       (including CPI instructions) are preallocated.  However, the order
-       in which the instructions are executed does not match the order in
-       which they are allocated.  The instr_trace will instead be used to
-       track the order in which the instructions are executed.  We leave
-       space for an extra instruction to account for the case where the
-       transaction has too many instructions leading to
-       FD_EXECUTOR_INSTR_ERR_MAX_INSN_TRACE_LENS_EXCEEDED.
-       TODO: In reality, we should just be allocating instr_infos per
-       instruction and not up front.  The dependency on using instr_info
-       for the sysvar instruction setup is not needed and should be
-       removed.  At this point, instr_info and instr_trace should be
-       combined. */
-    fd_instr_info_t             infos[ FD_MAX_INSTRUCTION_TRACE_LENGTH * 2UL ];
-    ulong                       info_cnt;
-    fd_exec_instr_trace_entry_t trace[ FD_MAX_INSTRUCTION_TRACE_LENGTH ]; /* Instruction trace */
-    ulong                       trace_length;                           /* Number of instructions in the trace */
+       (including CPI instructions) are preallocated.  However, the
+       order in which the instructions are executed does not match the
+       order in which they are allocated.  The instr_trace will instead
+       be used to track the order in which the instructions are
+       executed. We add a +1 to allow any instructions past the max
+       instr trace limit to be safely allocated, so that we can fail
+       out like Agave does later at the stack push step within
+       fd_execute_instr.
+
+       The caller is responsible for updating the trace_length for the
+       callee. For CPI, the trace length is updated when preparing a
+       new instruction within cpi_common. For top-level instructions,
+       the trace length is updated within fd_execute_txn when preparing
+       an instruction for execution. */
+    fd_instr_info_t             trace[ FD_MAX_INSTRUCTION_TRACE_LENGTH+1UL ];
+    ulong                       trace_length;
     /* The current instruction index being executed */
     int current_idx;
   } instr;

src/flamenco/runtime/fd_runtime_const.h

@@ -96,6 +96,11 @@ static const fd_cluster_version_t FD_RUNTIME_CLUSTER_VERSION = {
 #define FD_RUNTIME_VM_TRACE_STATIC_FOOTPRINT (FD_RUNTIME_VM_TRACE_EVENT_MAX + sizeof(fd_vm_trace_t))
 #define FD_RUNTIME_VM_TRACE_STATIC_ALIGN     (8UL)
 
+/* Maximum CPI instruction data size. 10 KiB was chosen to ensure that
+   CPI instructions are not more limited than transaction instructions
+   if the size of transactions is doubled in the future.
+   https://github.com/anza-xyz/agave/blob/v3.1.1/transaction-context/src/lib.rs#L33 */
+#define FD_RUNTIME_CPI_MAX_INSTR_DATA_LEN (10240UL)
 
 /* The bpf loader's serialization footprint is bounded in the worst case
    by 64 unique writable accounts which are each 10MiB in size (bounded

src/flamenco/runtime/fd_runtime_helpers.h

@@ -32,16 +32,6 @@ typedef struct fd_txn_return_data fd_txn_return_data_t;
 
 /* fd_exec_txn_ctx_t is the context needed to execute a transaction. */
 
-/* An entry in the instruction trace */
-struct fd_exec_instr_trace_entry {
-  /* Metadata about the instruction */
-  fd_instr_info_t * instr_info;
-  /* Stack height when this instruction was pushed onto the stack (including itself)
-     https://github.com/anza-xyz/agave/blob/d87e23d8d91c32d5f2be2bb3557c730bee1e9434/sdk/src/transaction_context.rs#L475-L480 */
-  ulong stack_height;
-};
-typedef struct fd_exec_instr_trace_entry fd_exec_instr_trace_entry_t;
-
 FD_PROTOTYPES_BEGIN
 
 /* Returns 0 on success, and non zero otherwise.  On failure, the out