From 8e2faa607068692feb488373658817ede9f68f62 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 6 Feb 2025 14:24:39 -0600 Subject: [PATCH 1/6] GH-1145 Do not interrupt_oc for implicit trxs as deferred onerror is run outside of transaction_context retry of interrupt_oc. --- libraries/chain/controller.cpp | 2 ++ libraries/chain/include/eosio/chain/transaction_context.hpp | 1 + .../chain/include/eosio/chain/wasm_interface_private.hpp | 5 ++++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index fb877b0201..b688b08f75 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -2690,6 +2690,8 @@ struct controller_impl { } catch ( const boost::interprocess::bad_alloc& ) { throw; } catch( const fc::exception& e ) { + // apply_onerror for deferred trxs is implicit so interrupt oc not allowed + assert(e.code() != interrupt_oc_exception::code_value); handle_exception(e); } catch ( const std::exception& e ) { auto wrapper = fc::std_exception_wrapper::from_current_exception(e); diff --git a/libraries/chain/include/eosio/chain/transaction_context.hpp b/libraries/chain/include/eosio/chain/transaction_context.hpp index 44f3c26dbd..24da69c29f 100644 --- a/libraries/chain/include/eosio/chain/transaction_context.hpp +++ b/libraries/chain/include/eosio/chain/transaction_context.hpp @@ -162,6 +162,7 @@ namespace eosio::chain { bool is_dry_run()const { return trx_type == transaction_metadata::trx_type::dry_run; }; bool is_read_only()const { return trx_type == transaction_metadata::trx_type::read_only; }; bool is_transient()const { return trx_type == transaction_metadata::trx_type::read_only || trx_type == transaction_metadata::trx_type::dry_run; }; + bool is_implicit()const { return trx_type == transaction_metadata::trx_type::implicit; }; bool has_undo()const; int64_t set_proposed_producers(vector producers); diff --git a/libraries/chain/include/eosio/chain/wasm_interface_private.hpp b/libraries/chain/include/eosio/chain/wasm_interface_private.hpp index a351abdc8d..2d6fa2cb05 100644 --- a/libraries/chain/include/eosio/chain/wasm_interface_private.hpp +++ b/libraries/chain/include/eosio/chain/wasm_interface_private.hpp @@ -164,7 +164,10 @@ struct eosvmoc_tier { } } #endif - const bool allow_oc_interrupt = attempt_tierup && context.is_applying_block() && context.trx_context.has_undo(); + // do not allow oc interrupt if no undo as the transaction needs to be undone to restart it. + // do not allow oc interrupt if implicit as deferred trx onerror execute outside the transaction retry in transaction_context. + const bool allow_oc_interrupt = attempt_tierup && context.is_applying_block() && + context.trx_context.has_undo() && !context.trx_context.is_implicit(); auto ex = fc::make_scoped_exit([&]() { if (allow_oc_interrupt) { eos_vm_oc_compile_interrupt = false; From 232971977f792df961d15378ccb424962f968347 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 6 Feb 2025 14:25:24 -0600 Subject: [PATCH 2/6] GH-1145 Move checktime call since it can call, make sure cleanup is setup first. --- libraries/chain/webassembly/runtimes/eos-vm-oc/executor.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/chain/webassembly/runtimes/eos-vm-oc/executor.cpp b/libraries/chain/webassembly/runtimes/eos-vm-oc/executor.cpp index 4e6be901d9..0079d936a2 100644 --- a/libraries/chain/webassembly/runtimes/eos-vm-oc/executor.cpp +++ b/libraries/chain/webassembly/runtimes/eos-vm-oc/executor.cpp @@ -231,7 +231,6 @@ void executor::execute(const code_descriptor& code, memory& mem, apply_context& syscall(SYS_mprotect, self->code_mapping, self->code_mapping_size, PROT_NONE); self->mapping_is_executable = false; }, this); - context.trx_context.checktime(); //catch any expiration that might have occurred before setting up callback auto cleanup = fc::make_scoped_exit([cb, &tt=context.trx_context.transaction_timer, &mem=mem](){ cb->is_running = false; @@ -245,6 +244,8 @@ void executor::execute(const code_descriptor& code, memory& mem, apply_context& } }); + context.trx_context.checktime(); //catch any expiration that might have occurred before setting up callback + void(*apply_func)(uint64_t, uint64_t, uint64_t) = (void(*)(uint64_t, uint64_t, uint64_t))(cb->running_code_base + code.apply_offset); switch(sigsetjmp(*cb->jmp, 0)) { From 46660b3f88b2ee49b246214cd25ed4fc84bc7187 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 6 Feb 2025 14:26:08 -0600 Subject: [PATCH 3/6] GH-1145 Call stop on timer so timer is stopped() before start() as expected. --- libraries/chain/transaction_context.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/chain/transaction_context.cpp b/libraries/chain/transaction_context.cpp index d45110c76a..91d0131a79 100644 --- a/libraries/chain/transaction_context.cpp +++ b/libraries/chain/transaction_context.cpp @@ -59,6 +59,7 @@ namespace eosio::chain { undo(); *trace = transaction_trace{}; // reset trace initialize(); + transaction_timer.stop(); resume_billing_timer(start); auto sw = executed_action_receipts.store_which(); From 0170e0201430dfea40b9d6e6143f8ce083fe5542 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 6 Feb 2025 15:56:23 -0600 Subject: [PATCH 4/6] GH-1145 Make sure correct transaction type is passed to transaction_context. --- benchmark/bls.cpp | 3 ++- libraries/chain/controller.cpp | 6 ++++-- libraries/chain/include/eosio/chain/transaction_context.hpp | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/benchmark/bls.cpp b/benchmark/bls.cpp index 4665448780..e0155ce9c3 100644 --- a/benchmark/bls.cpp +++ b/benchmark/bls.cpp @@ -78,7 +78,8 @@ struct interface_in_benchmark { timer = std::make_unique(); trx_timer = std::make_unique(*timer); trx_ctx = std::make_unique(*chain->control.get(), *ptrx, ptrx->id(), std::move(*trx_timer), - action_digests_t::store_which_t::legacy); + action_digests_t::store_which_t::legacy, fc::time_point::now(), + transaction_metadata::trx_type::input); trx_ctx->max_transaction_time_subjective = fc::microseconds::maximum(); trx_ctx->init_for_input_trx( ptrx->get_unprunable_size(), ptrx->get_prunable_size() ); trx_ctx->exec(); // this is required to generate action traces to be used by apply_context constructor diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index b688b08f75..85e67c17f6 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -2644,7 +2644,8 @@ struct controller_impl { transaction_checktime_timer trx_timer(timer); const packed_transaction trx( std::move( etrx ) ); - transaction_context trx_context( self, trx, trx.id(), std::move(trx_timer), bb.action_receipt_digests().store_which(), start ); + transaction_context trx_context( self, trx, trx.id(), std::move(trx_timer), bb.action_receipt_digests().store_which(), + start, transaction_metadata::trx_type::implicit ); if (auto dm_logger = get_deep_mind_logger(trx_context.is_transient())) { dm_logger->on_onerror(etrx); @@ -2817,7 +2818,8 @@ struct controller_impl { auto& bb = std::get(pending->_block_stage); transaction_checktime_timer trx_timer( timer ); - transaction_context trx_context( self, *trx->packed_trx(), gtrx.trx_id, std::move(trx_timer), bb.action_receipt_digests().store_which() ); + transaction_context trx_context( self, *trx->packed_trx(), gtrx.trx_id, std::move(trx_timer), bb.action_receipt_digests().store_which(), + start, transaction_metadata::trx_type::scheduled ); trx_context.leeway = fc::microseconds(0); // avoid stealing cpu resource trx_context.block_deadline = block_deadline; trx_context.max_transaction_time_subjective = max_transaction_time; diff --git a/libraries/chain/include/eosio/chain/transaction_context.hpp b/libraries/chain/include/eosio/chain/transaction_context.hpp index 24da69c29f..a9f9fe1d1f 100644 --- a/libraries/chain/include/eosio/chain/transaction_context.hpp +++ b/libraries/chain/include/eosio/chain/transaction_context.hpp @@ -114,8 +114,8 @@ namespace eosio::chain { const transaction_id_type& trx_id, // trx_id diff than t.id() before replace_deferred transaction_checktime_timer&& timer, action_digests_t::store_which_t sad, - fc::time_point start = fc::time_point::now(), - transaction_metadata::trx_type type = transaction_metadata::trx_type::input); + fc::time_point start, + transaction_metadata::trx_type type); ~transaction_context(); void init_for_implicit_trx(); From f0d379a83309387d4e64e000823c928e20257396 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 6 Feb 2025 17:15:17 -0600 Subject: [PATCH 5/6] GH-1145 Do not allow interrupt of scheduled transactions. --- libraries/chain/include/eosio/chain/transaction_context.hpp | 1 + .../chain/include/eosio/chain/wasm_interface_private.hpp | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/chain/include/eosio/chain/transaction_context.hpp b/libraries/chain/include/eosio/chain/transaction_context.hpp index a9f9fe1d1f..364e04ec3e 100644 --- a/libraries/chain/include/eosio/chain/transaction_context.hpp +++ b/libraries/chain/include/eosio/chain/transaction_context.hpp @@ -163,6 +163,7 @@ namespace eosio::chain { bool is_read_only()const { return trx_type == transaction_metadata::trx_type::read_only; }; bool is_transient()const { return trx_type == transaction_metadata::trx_type::read_only || trx_type == transaction_metadata::trx_type::dry_run; }; bool is_implicit()const { return trx_type == transaction_metadata::trx_type::implicit; }; + bool is_scheduled()const { return trx_type == transaction_metadata::trx_type::scheduled; }; bool has_undo()const; int64_t set_proposed_producers(vector producers); diff --git a/libraries/chain/include/eosio/chain/wasm_interface_private.hpp b/libraries/chain/include/eosio/chain/wasm_interface_private.hpp index 2d6fa2cb05..43a320e9c8 100644 --- a/libraries/chain/include/eosio/chain/wasm_interface_private.hpp +++ b/libraries/chain/include/eosio/chain/wasm_interface_private.hpp @@ -165,9 +165,10 @@ struct eosvmoc_tier { } #endif // do not allow oc interrupt if no undo as the transaction needs to be undone to restart it. - // do not allow oc interrupt if implicit as deferred trx onerror execute outside the transaction retry in transaction_context. + // do not allow oc interrupt if implicit as deferred trx onerror execute as implicit outside the transaction + // retry in transaction_context. Also scheduled manage the undo stack explicitly. const bool allow_oc_interrupt = attempt_tierup && context.is_applying_block() && - context.trx_context.has_undo() && !context.trx_context.is_implicit(); + context.trx_context.has_undo() && !context.trx_context.is_implicit() && !context.trx_context.is_scheduled(); auto ex = fc::make_scoped_exit([&]() { if (allow_oc_interrupt) { eos_vm_oc_compile_interrupt = false; From 845576c52b8cabecf6b315dbc44988debd6b1924 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 7 Feb 2025 12:42:24 -0600 Subject: [PATCH 6/6] GH-1145 Attempt to make comment clearer --- .../chain/include/eosio/chain/wasm_interface_private.hpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libraries/chain/include/eosio/chain/wasm_interface_private.hpp b/libraries/chain/include/eosio/chain/wasm_interface_private.hpp index 43a320e9c8..3f329936f6 100644 --- a/libraries/chain/include/eosio/chain/wasm_interface_private.hpp +++ b/libraries/chain/include/eosio/chain/wasm_interface_private.hpp @@ -164,9 +164,12 @@ struct eosvmoc_tier { } } #endif - // do not allow oc interrupt if no undo as the transaction needs to be undone to restart it. - // do not allow oc interrupt if implicit as deferred trx onerror execute as implicit outside the transaction - // retry in transaction_context. Also scheduled manage the undo stack explicitly. + // Do not allow oc interrupt if no undo as the transaction needs to be undone to restart it. + // Do not allow oc interrupt if implicit or scheduled. There are two implicit trxs: onblock and onerror. + // The onerror trx of deferred trxs is implicit. Interrupt needs to be disabled for deferred trxs because + // they capture all exceptions, explicitly handle undo stack, and directly call trx_context.execute_action. + // Not allowing interrupt for onblock seems rather harmless, so instead of distinguishing between onerror and + // onblock, just disallow for all implicit. const bool allow_oc_interrupt = attempt_tierup && context.is_applying_block() && context.trx_context.has_undo() && !context.trx_context.is_implicit() && !context.trx_context.is_scheduled(); auto ex = fc::make_scoped_exit([&]() {