From 97aa4b80316b9a615846ab266da942de062010be Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 7 Jul 2023 11:17:05 -0500 Subject: [PATCH 1/2] GH-1354 Add test case for deferred trx id before/after protocol feature replace_deferred --- unittests/protocol_feature_tests.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/unittests/protocol_feature_tests.cpp b/unittests/protocol_feature_tests.cpp index 7447e9ceef..a6b9d556df 100644 --- a/unittests/protocol_feature_tests.cpp +++ b/unittests/protocol_feature_tests.cpp @@ -412,6 +412,14 @@ BOOST_AUTO_TEST_CASE( replace_deferred_test ) try { cfg.disable_all_subjective_mitigations = true; c.init( cfg ); + transaction_trace_ptr trace; + auto h = c.control->applied_transaction.connect( [&](std::tuple x) { + auto& t = std::get<0>(x); + if( t && !eosio::chain::is_onblock(*t)) { + trace = t; + } + } ); + BOOST_CHECK_EQUAL( c.control->get_resource_limits_manager().get_account_ram_usage( "alice"_n ), alice_ram_usage0 ); c.push_action( "test"_n, "defercall"_n, "alice"_n, fc::mutable_variant_object() @@ -448,6 +456,8 @@ BOOST_AUTO_TEST_CASE( replace_deferred_test ) try { dtrxs = c.get_scheduled_transactions(); BOOST_CHECK_EQUAL( dtrxs.size(), 0 ); + // must be equal before builtin_protocol_feature_t::replace_deferred to support replay of blocks before activation + BOOST_CHECK( first_dtrx_id.str() == trace->id.str() ); c.produce_block(); @@ -507,6 +517,13 @@ BOOST_AUTO_TEST_CASE( replace_deferred_test ) try { BOOST_CHECK_EQUAL( dtrxs.size(), 1 ); BOOST_CHECK_EQUAL( first_dtrx_id2, dtrxs[0] ); + c.produce_block(); + + dtrxs = c.get_scheduled_transactions(); + BOOST_CHECK_EQUAL( dtrxs.size(), 0 ); + // Not equal after builtin_protocol_feature_t::replace_deferred activated + BOOST_CHECK( first_dtrx_id2.str() != trace->id.str() ); + } FC_LOG_AND_RETHROW() BOOST_AUTO_TEST_CASE( no_duplicate_deferred_id_test ) try { From 71ecec44f88c6a93a0d04dfb04cbf3a8880f00ff Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 7 Jul 2023 11:20:58 -0500 Subject: [PATCH 2/2] GH-1354 Provide transaction_context trx id for trace since it can differ from provided packed_transaction for deferred trxs before replace_deferred protocol feature is activated. --- libraries/chain/controller.cpp | 6 +++--- .../chain/include/eosio/chain/transaction_context.hpp | 2 ++ libraries/chain/transaction_context.cpp | 8 +++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 65d679bb03..23e4077ebe 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1154,7 +1154,7 @@ struct controller_impl { transaction_checktime_timer trx_timer(timer); const packed_transaction trx( std::move( etrx ) ); - transaction_context trx_context( self, trx, std::move(trx_timer), start ); + transaction_context trx_context( self, trx, trx.id(), std::move(trx_timer), start ); trx_context.block_deadline = block_deadline; trx_context.max_transaction_time_subjective = max_transaction_time; @@ -1315,7 +1315,7 @@ struct controller_impl { uint32_t cpu_time_to_bill_us = billed_cpu_time_us; transaction_checktime_timer trx_timer(timer); - transaction_context trx_context( self, *trx->packed_trx(), std::move(trx_timer) ); + transaction_context trx_context( self, *trx->packed_trx(), gtrx.trx_id, std::move(trx_timer) ); 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; @@ -1528,7 +1528,7 @@ struct controller_impl { const signed_transaction& trn = trx->packed_trx()->get_signed_transaction(); transaction_checktime_timer trx_timer(timer); - transaction_context trx_context(self, *trx->packed_trx(), std::move(trx_timer), start, trx->read_only); + transaction_context trx_context(self, *trx->packed_trx(), trx->id(), std::move(trx_timer), start, trx->read_only); if ((bool)subjective_cpu_leeway && pending->_block_status == controller::block_status::incomplete) { trx_context.leeway = *subjective_cpu_leeway; } diff --git a/libraries/chain/include/eosio/chain/transaction_context.hpp b/libraries/chain/include/eosio/chain/transaction_context.hpp index 0126dee936..83a428c84d 100644 --- a/libraries/chain/include/eosio/chain/transaction_context.hpp +++ b/libraries/chain/include/eosio/chain/transaction_context.hpp @@ -37,6 +37,7 @@ namespace eosio { namespace chain { transaction_context( controller& c, const packed_transaction& t, + const transaction_id_type& trx_id, // trx_id diff than t.id() before replace_deferred transaction_checktime_timer&& timer, fc::time_point start = fc::time_point::now(), bool read_only=false); @@ -121,6 +122,7 @@ namespace eosio { namespace chain { controller& control; const packed_transaction& packed_trx; + const transaction_id_type& id; std::optional undo_session; transaction_trace_ptr trace; fc::time_point start; diff --git a/libraries/chain/transaction_context.cpp b/libraries/chain/transaction_context.cpp index bd7d32ff30..cde37e5731 100644 --- a/libraries/chain/transaction_context.cpp +++ b/libraries/chain/transaction_context.cpp @@ -46,11 +46,13 @@ namespace eosio { namespace chain { transaction_context::transaction_context( controller& c, const packed_transaction& t, + const transaction_id_type& trx_id, transaction_checktime_timer&& tmr, fc::time_point s, bool read_only) :control(c) ,packed_trx(t) + ,id(trx_id) ,undo_session() ,trace(std::make_shared()) ,start(s) @@ -62,7 +64,7 @@ namespace eosio { namespace chain { if (!c.skip_db_sessions()) { undo_session.emplace(c.mutable_db().start_undo_session(true)); } - trace->id = packed_trx.id(); + trace->id = id; trace->block_num = c.head_block_num() + 1; trace->block_time = c.pending_block_time(); trace->producer_block_id = c.pending_producer_block_id(); @@ -271,7 +273,7 @@ namespace eosio { namespace chain { validate_referenced_accounts( trx, enforce_whiteblacklist && control.is_producing_block() ); } init( initial_net_usage); - record_transaction( packed_trx.id(), trx.expiration ); /// checks for dupes + record_transaction( id, trx.expiration ); /// checks for dupes } void transaction_context::init_for_deferred_trx( fc::time_point p ) @@ -700,7 +702,7 @@ namespace eosio { namespace chain { uint32_t trx_size = 0; const auto& cgto = control.mutable_db().create( [&]( auto& gto ) { - gto.trx_id = packed_trx.id(); + gto.trx_id = id; gto.payer = first_auth; gto.sender = account_name(); /// delayed transactions have no sender gto.sender_id = transaction_id_to_sender_id( gto.trx_id );