diff --git a/src/discof/bank/fd_bank_tile.c b/src/discof/bank/fd_bank_tile.c index edf5336d5a..fab22c8fda 100644 --- a/src/discof/bank/fd_bank_tile.c +++ b/src/discof/bank/fd_bank_tile.c @@ -161,88 +161,92 @@ handle_microblock( fd_bank_ctx_t * ctx, txn->flags &= ~FD_TXN_P_FLAGS_SANITIZE_SUCCESS; - int err = fd_runtime_prepare_and_execute_txn( ctx->banks, ctx->_bank_idx, txn_ctx, txn, ctx->exec_spad, NULL, 0 ); - if( FD_UNLIKELY( !(txn_ctx->flags & FD_TXN_P_FLAGS_SANITIZE_SUCCESS ) ) ) { - ctx->metrics.txn_result[ fd_bank_err_from_runtime_err( err ) ]++; - continue; - } + int err = FD_RUNTIME_EXECUTE_SUCCESS; + FD_SPAD_FRAME_BEGIN( ctx->exec_spad ) { + err = fd_runtime_prepare_and_execute_txn( ctx->banks, ctx->_bank_idx, txn_ctx, txn, NULL, 0 ); - /* The account keys in the transaction context are laid out such - that first the non-alt accounts are laid out, then the writable - alt accounts, and finally the read-only alt accounts. */ - fd_txn_t * txn_descriptor = TXN( &txn_ctx->txn ); - for( ushort i=txn_descriptor->acct_addr_cnt; iacct_addr_cnt+txn_descriptor->addr_table_adtl_writable_cnt; i++ ) { - writable_alt[ i-txn_descriptor->acct_addr_cnt ] = fd_type_pun_const( &txn_ctx->account_keys[ i ] ); - } + if( FD_UNLIKELY( !(txn_ctx->flags & FD_TXN_P_FLAGS_SANITIZE_SUCCESS ) ) ) { + ctx->metrics.txn_result[ fd_bank_err_from_runtime_err( err ) ]++; + continue; + } - txn->flags |= FD_TXN_P_FLAGS_SANITIZE_SUCCESS; + /* The account keys in the transaction context are laid out such + that first the non-alt accounts are laid out, then the writable + alt accounts, and finally the read-only alt accounts. */ + fd_txn_t * txn_descriptor = TXN( &txn_ctx->txn ); + for( ushort i=txn_descriptor->acct_addr_cnt; iacct_addr_cnt+txn_descriptor->addr_table_adtl_writable_cnt; i++ ) { + writable_alt[ i-txn_descriptor->acct_addr_cnt ] = fd_type_pun_const( &txn_ctx->account_keys[ i ] ); + } - uint requested_exec_plus_acct_data_cus = txn->pack_cu.requested_exec_plus_acct_data_cus; - uint non_execution_cus = txn->pack_cu.non_execution_cus; + txn->flags |= FD_TXN_P_FLAGS_SANITIZE_SUCCESS; - /* Assume failure, set below if success. If it doesn't land in the - block, rebate the non-execution CUs too. */ - txn->bank_cu.actual_consumed_cus = 0U; - txn->bank_cu.rebated_cus = requested_exec_plus_acct_data_cus + non_execution_cus; - txn->flags &= ~FD_TXN_P_FLAGS_EXECUTE_SUCCESS; + uint requested_exec_plus_acct_data_cus = txn->pack_cu.requested_exec_plus_acct_data_cus; + uint non_execution_cus = txn->pack_cu.non_execution_cus; - /* Stash the result in the flags value so that pack can inspect it. */ - /* TODO: Need to translate the err to a hacky Frankendancer style err - that pack and GUI expect ... */ - txn->flags = (txn->flags & 0x00FFFFFFU) | ((uint)(-err)<<24); + /* Assume failure, set below if success. If it doesn't land in the + block, rebate the non-execution CUs too. */ + txn->bank_cu.actual_consumed_cus = 0U; + txn->bank_cu.rebated_cus = requested_exec_plus_acct_data_cus + non_execution_cus; + txn->flags &= ~FD_TXN_P_FLAGS_EXECUTE_SUCCESS; - ctx->metrics.txn_result[ fd_bank_err_from_runtime_err( err ) ]++; + /* Stash the result in the flags value so that pack can inspect it. */ + /* TODO: Need to translate the err to a hacky Frankendancer style err + that pack and GUI expect ... */ + txn->flags = (txn->flags & 0x00FFFFFFU) | ((uint)(-err)<<24); - uint actual_execution_cus = (uint)(txn_ctx->compute_budget_details.compute_unit_limit - txn_ctx->compute_budget_details.compute_meter); - uint actual_acct_data_cus = (uint)(txn_ctx->loaded_accounts_data_size_cost); + ctx->metrics.txn_result[ fd_bank_err_from_runtime_err( err ) ]++; - int is_simple_vote = 0; - if( FD_UNLIKELY( is_simple_vote = fd_txn_is_simple_vote_transaction( TXN(txn), txn->payload ) ) ) { - /* Simple votes are charged fixed amounts of compute regardless of - the real cost they incur. Unclear what cost is returned by - fd_execute txn, however, so we override it here. */ - actual_execution_cus = FD_PACK_VOTE_DEFAULT_COMPUTE_UNITS; - actual_acct_data_cus = 0U; - } + uint actual_execution_cus = (uint)(txn_ctx->compute_budget_details.compute_unit_limit - txn_ctx->compute_budget_details.compute_meter); + uint actual_acct_data_cus = (uint)(txn_ctx->loaded_accounts_data_size_cost); - /* FeesOnly transactions are transactions that failed to load - before they even reach the VM stage. They have zero execution - cost but do charge for the account data they are able to load. - FeesOnly votes are charged the fixed voe cost. */ - txn->bank_cu.rebated_cus = requested_exec_plus_acct_data_cus - ( actual_execution_cus + actual_acct_data_cus ); - txn->bank_cu.actual_consumed_cus = non_execution_cus + actual_execution_cus + actual_acct_data_cus; - - /* TXN_P_FLAGS_EXECUTE_SUCCESS means that it should be included in - the block. It's a bit of a misnomer now that there are fee-only - transactions. */ - FD_TEST( txn_ctx->flags & FD_TXN_P_FLAGS_EXECUTE_SUCCESS ); - txn->flags |= FD_TXN_P_FLAGS_EXECUTE_SUCCESS; - - /* The VM will stop executing and fail an instruction immediately if - it exceeds its requested CUs. A transaction which requests less - account data than it actually consumes will fail in the account - loading stage. */ - if( FD_UNLIKELY( actual_execution_cus+actual_acct_data_cus>requested_exec_plus_acct_data_cus ) ) { - FD_LOG_HEXDUMP_WARNING(( "txn", txn->payload, txn->payload_sz )); - FD_LOG_ERR(( "Actual CUs unexpectedly exceeded requested amount. actual_execution_cus (%u) actual_acct_data_cus " - "(%u) requested_exec_plus_acct_data_cus (%u) is_simple_vote (%i) exec_failed (%i)", - actual_execution_cus, actual_acct_data_cus, requested_exec_plus_acct_data_cus, is_simple_vote, - err )); - } + int is_simple_vote = 0; + if( FD_UNLIKELY( is_simple_vote = fd_txn_is_simple_vote_transaction( TXN(txn), txn->payload ) ) ) { + /* Simple votes are charged fixed amounts of compute regardless of + the real cost they incur. Unclear what cost is returned by + fd_execute txn, however, so we override it here. */ + actual_execution_cus = FD_PACK_VOTE_DEFAULT_COMPUTE_UNITS; + actual_acct_data_cus = 0U; + } + + /* FeesOnly transactions are transactions that failed to load + before they even reach the VM stage. They have zero execution + cost but do charge for the account data they are able to load. + FeesOnly votes are charged the fixed voe cost. */ + txn->bank_cu.rebated_cus = requested_exec_plus_acct_data_cus - ( actual_execution_cus + actual_acct_data_cus ); + txn->bank_cu.actual_consumed_cus = non_execution_cus + actual_execution_cus + actual_acct_data_cus; + + /* TXN_P_FLAGS_EXECUTE_SUCCESS means that it should be included in + the block. It's a bit of a misnomer now that there are fee-only + transactions. */ + FD_TEST( txn_ctx->flags & FD_TXN_P_FLAGS_EXECUTE_SUCCESS ); + txn->flags |= FD_TXN_P_FLAGS_EXECUTE_SUCCESS; + + /* The VM will stop executing and fail an instruction immediately if + it exceeds its requested CUs. A transaction which requests less + account data than it actually consumes will fail in the account + loading stage. */ + if( FD_UNLIKELY( actual_execution_cus+actual_acct_data_cus>requested_exec_plus_acct_data_cus ) ) { + FD_LOG_HEXDUMP_WARNING(( "txn", txn->payload, txn->payload_sz )); + FD_LOG_ERR(( "Actual CUs unexpectedly exceeded requested amount. actual_execution_cus (%u) actual_acct_data_cus " + "(%u) requested_exec_plus_acct_data_cus (%u) is_simple_vote (%i) exec_failed (%i)", + actual_execution_cus, actual_acct_data_cus, requested_exec_plus_acct_data_cus, is_simple_vote, + err )); + } - /* Commit must succeed so no failure path. Once commit is called, - the transactions MUST be mixed into the PoH otherwise we will - fork and diverge, so the link from here til PoH mixin must be - completely reliable with nothing dropped. - - fd_runtime_finalize_txn checks if the transaction fits into the - block with the cost tracker. If it doesn't fit, flags is set to - zero. A key invariant of the leader pipeline is that pack - ensures all transactions must fit already, so it is a fatal error - if that happens. We cannot reject the transaction here as there - would be no way to undo the partially applied changes to the bank - in finalize anyway. */ - fd_runtime_finalize_txn( ctx->txn_ctx->funk, txn_ctx->status_cache, txn_ctx->xid, txn_ctx, bank, NULL ); + /* Commit must succeed so no failure path. Once commit is called, + the transactions MUST be mixed into the PoH otherwise we will + fork and diverge, so the link from here til PoH mixin must be + completely reliable with nothing dropped. + + fd_runtime_finalize_txn checks if the transaction fits into the + block with the cost tracker. If it doesn't fit, flags is set to + zero. A key invariant of the leader pipeline is that pack + ensures all transactions must fit already, so it is a fatal error + if that happens. We cannot reject the transaction here as there + would be no way to undo the partially applied changes to the bank + in finalize anyway. */ + fd_runtime_finalize_txn( ctx->txn_ctx->funk, txn_ctx->status_cache, txn_ctx->xid, txn_ctx, bank, NULL ); + } FD_SPAD_FRAME_END; FD_TEST( txn->flags ); } @@ -326,7 +330,7 @@ handle_bundle( fd_bank_ctx_t * ctx, fd_exec_txn_ctx_t txn_ctx[ 1 ]; // TODO ... bank manager ? txn->flags &= ~(FD_TXN_P_FLAGS_SANITIZE_SUCCESS | FD_TXN_P_FLAGS_EXECUTE_SUCCESS); - int err = fd_runtime_prepare_and_execute_txn( NULL, ULONG_MAX, txn_ctx, txn, NULL, NULL, 0 ); /* TODO ... */ + int err = fd_runtime_prepare_and_execute_txn( NULL, ULONG_MAX, txn_ctx, txn, NULL, 0 ); /* TODO ... */ transaction_err[ i ] = err; if( FD_UNLIKELY( err ) ) { diff --git a/src/discof/exec/fd_exec_tile.c b/src/discof/exec/fd_exec_tile.c index fd5efb02e3..6b828b69ee 100644 --- a/src/discof/exec/fd_exec_tile.c +++ b/src/discof/exec/fd_exec_tile.c @@ -115,12 +115,15 @@ during_frag( fd_exec_tile_ctx_t * ctx, if( FD_LIKELY( (sig>>32)==EXEC_NEW_TXN_SIG ) ) { fd_exec_txn_msg_t * txn = (fd_exec_txn_msg_t *)fd_chunk_to_laddr( ctx->replay_in->mem, chunk ); + + + /* Keep spad frame until txn is finalized (per txn spad frame) */ + fd_spad_push( ctx->exec_spad ); ctx->txn_ctx->exec_err = fd_runtime_prepare_and_execute_txn( ctx->banks, txn->bank_idx, ctx->txn_ctx, &txn->txn, - ctx->exec_spad, ctx->capture_ctx, 1 ); } else { @@ -184,6 +187,9 @@ after_frag( fd_exec_tile_ctx_t * ctx, fd_banks_mark_bank_dead( ctx->banks, bank ); } + /* Pop the per txn spad frame */ + fd_spad_pop( ctx->exec_spad ); + /* Notify the replay tile that we are done with this txn. */ ctx->pending_txn_finalized_msg = 1; } else { diff --git a/src/flamenco/runtime/fd_runtime.c b/src/flamenco/runtime/fd_runtime.c index a652160712..9eb80a9d22 100644 --- a/src/flamenco/runtime/fd_runtime.c +++ b/src/flamenco/runtime/fd_runtime.c @@ -1213,10 +1213,8 @@ fd_runtime_prepare_and_execute_txn( fd_banks_t * banks, ulong bank_idx, fd_exec_txn_ctx_t * txn_ctx, fd_txn_p_t * txn, - fd_spad_t * exec_spad, fd_capture_ctx_t * capture_ctx, uchar do_sigverify ) { - FD_SPAD_FRAME_BEGIN( exec_spad ) { int exec_res = 0; fd_bank_t * bank = fd_banks_bank_query( banks, bank_idx ); @@ -1268,7 +1266,6 @@ fd_runtime_prepare_and_execute_txn( fd_banks_t * banks, return exec_res; - } FD_SPAD_FRAME_END; } /* fd_executor_txn_verify and fd_runtime_pre_execute_check are responisble diff --git a/src/flamenco/runtime/fd_runtime.h b/src/flamenco/runtime/fd_runtime.h index 5661c9a380..8dd04ab96a 100644 --- a/src/flamenco/runtime/fd_runtime.h +++ b/src/flamenco/runtime/fd_runtime.h @@ -461,7 +461,6 @@ fd_runtime_prepare_and_execute_txn( fd_banks_t * banks, ulong bank_idx, fd_exec_txn_ctx_t * txn_ctx, fd_txn_p_t * txn, - fd_spad_t * exec_spad, fd_capture_ctx_t * capture_ctx, uchar do_sigverify ); diff --git a/src/flamenco/runtime/tests/fd_txn_harness.c b/src/flamenco/runtime/tests/fd_txn_harness.c index 152ab59323..e2b21850f7 100644 --- a/src/flamenco/runtime/tests/fd_txn_harness.c +++ b/src/flamenco/runtime/tests/fd_txn_harness.c @@ -353,7 +353,6 @@ fd_runtime_fuzz_txn_ctx_exec( fd_solfuzz_runner_t * runner, 0UL, txn_ctx, txn, - runner->spad, NULL, 0 );