flamenco: clean up instr info + trace

Author: mjain-jump

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

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

src/discof/exec/fd_exec_tile.c

@@ -128,8 +128,9 @@ returnable_frag( fd_exec_tile_ctx_t * ctx,
         fd_exec_txn_exec_msg_t * msg = fd_chunk_to_laddr( ctx->replay_in->mem, chunk );
         ctx->bank = fd_banks_bank_query( ctx->banks, msg->bank_idx );
         FD_TEST( ctx->bank );
-        ctx->txn_in.txn           = &msg->txn;
-        ctx->txn_in.exec_accounts = &ctx->exec_accounts;
+        ctx->txn_in.txn               = &msg->txn;
+        ctx->txn_in.exec_accounts     = &ctx->exec_accounts;
+        ctx->runtime->log.capture_ctx = ctx->capture_ctx;
 
         fd_runtime_prepare_and_execute_txn( ctx->runtime, ctx->bank, &ctx->txn_in, &ctx->txn_out );
 

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,
@@ -1160,11 +1143,13 @@ fd_txn_ctx_push( fd_runtime_t *      runtime,
     }
   }
 
-  /* 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/c4b42ab045860d7b13b3912eafb30e6d2f4e593f/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 */
   if( FD_UNLIKELY( runtime->instr.stack_sz>=FD_MAX_INSTRUCTION_STACK_DEPTH ) ) {
@@ -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,19 @@ 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. */
+    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_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

src/flamenco/runtime/info/fd_instr_info.c

@@ -28,6 +28,10 @@ fd_instr_info_init_from_txn_instr( fd_instr_info_t *      instr,
   fd_txn_t const * txn_descriptor = TXN( txn_in->txn );
   uchar *          instr_acc_idxs = (uchar *)txn_in->txn->payload + txn_instr->acct_off;
 
+  /* Set the stack height to 1 (since this is a top-level instruction) */
+  instr->stack_height = 1;
+
+  /* Set the program id */
   instr->program_id = txn_instr->program_id;
 
   /* See note in fd_instr_info.h.  TLDR: capping this value at 256
@@ -38,7 +42,7 @@ fd_instr_info_init_from_txn_instr( fd_instr_info_t *      instr,
      the instr info. */
   instr->acct_cnt = fd_ushort_min( txn_instr->acct_cnt, FD_INSTR_ACCT_MAX );
   instr->data_sz  = txn_instr->data_sz;
-  instr->data     = (uchar *)txn_in->txn->payload + txn_instr->data_off;
+  memcpy( instr->data, txn_in->txn->payload+txn_instr->data_off, instr->data_sz );
 
   uchar acc_idx_seen[ FD_INSTR_ACCT_MAX ] = {0};
 

src/flamenco/runtime/info/fd_instr_info.h

@@ -36,14 +36,17 @@ typedef struct fd_instruction_account fd_instruction_account_t;
 
 struct fd_instr_info {
   uchar                    program_id;
-  ushort                   data_sz;
   ushort                   acct_cnt;
 
-  uchar *                  data;
+  uchar                    data[ 10<<10 ]; // 10KB
+  ushort                   data_sz;
 
   fd_instruction_account_t accounts[ FD_INSTR_ACCT_MAX ];
   uchar                    is_duplicate[ FD_INSTR_ACCT_MAX ];
 
+  /* Stack height when this instruction was pushed onto the stack (including itself) */
+  uchar stack_height;
+
   /* TODO: convert to fd_uwide_t representation of uint_128 */
   ulong                    starting_lamports_h;
   ulong                    starting_lamports_l;
@@ -157,20 +160,6 @@ fd_instr_info_sum_account_lamports( fd_instr_info_t const * instr,
                                     ulong *                 total_lamports_h,
                                     ulong *                 total_lamports_l );
 
-static inline uchar
-fd_instr_get_acc_flags( fd_instr_info_t const * instr,
-                        ushort                  idx ) {
-  if( FD_UNLIKELY( idx>=instr->acct_cnt ) ) {
-    return 0;
-  }
-
-  uchar flags = 0;
-  if( instr->accounts[idx].is_signer )   flags |= FD_INSTR_ACCT_FLAGS_IS_SIGNER;
-  if( instr->accounts[idx].is_writable ) flags |= FD_INSTR_ACCT_FLAGS_IS_WRITABLE;
-
-  return flags;
-}
-
 FD_PROTOTYPES_END
 
 #endif /* HEADER_fd_src_flamenco_runtime_info_fd_instr_info_h */

src/flamenco/runtime/program/fd_bpf_loader_program.c

@@ -909,13 +909,11 @@ common_extend_program( fd_exec_instr_ctx_t * instr_ctx,
 /* https://github.com/anza-xyz/agave/blob/77daab497df191ef485a7ad36ed291c1874596e5/programs/bpf_loader/src/lib.rs#L566-L1444 */
 static int
 process_loader_upgradeable_instruction( fd_exec_instr_ctx_t * instr_ctx ) {
-  uchar const * data = instr_ctx->instr->data;
-
   uchar __attribute__((aligned(FD_BPF_UPGRADEABLE_LOADER_PROGRAM_INSTRUCTION_ALIGN))) instruction_mem[ FD_BPF_UPGRADEABLE_LOADER_PROGRAM_INSTRUCTION_FOOTPRINT ] = {0};
   fd_bpf_upgradeable_loader_program_instruction_t * instruction = fd_bincode_decode_static_limited_deserialize(
       bpf_upgradeable_loader_program_instruction,
       instruction_mem,
-      data,
+      instr_ctx->instr->data,
       instr_ctx->instr->data_sz,
       FD_TXN_MTU,
       NULL );

src/flamenco/runtime/program/fd_bpf_loader_serialization.c

@@ -391,8 +391,7 @@ fd_bpf_loader_input_serialize_aligned( fd_exec_instr_ctx_t *     ctx,
   *instr_data_offset = FD_VM_MEM_MAP_INPUT_REGION_START + region_vaddr_offset + (ulong)(serialized_params - curr_serialized_params_start);
 
   /* https://github.com/anza-xyz/agave/blob/v3.0.0/program-runtime/src/serialization.rs#L559 */
-  uchar * instr_data = ctx->instr->data;
-  fd_memcpy( serialized_params, instr_data, instr_data_len );
+  fd_memcpy( serialized_params, ctx->instr->data, instr_data_len );
   serialized_params += instr_data_len;
 
   /* https://github.com/anza-xyz/agave/blob/v3.0.0/program-runtime/src/serialization.rs#L560 */
@@ -641,8 +640,7 @@ fd_bpf_loader_input_serialize_unaligned( fd_exec_instr_ctx_t *     ctx,
   }
   *instr_data_offset = FD_VM_MEM_MAP_INPUT_REGION_START + region_vaddr_offset + (ulong)(serialized_params - curr_serialized_params_start);
 
-  uchar * instr_data = (uchar *)ctx->instr->data;
-  fd_memcpy( serialized_params, instr_data, instr_data_len );
+  fd_memcpy( serialized_params, ctx->instr->data, instr_data_len );
   serialized_params += instr_data_len;
 
   FD_STORE( fd_pubkey_t, serialized_params, txn_accs[ctx->instr->program_id] );