[5.0 -> main] Fix unlinked blocks caused by deferred trx removal

libraries/chain/controller.cpp

@@ -1595,12 +1595,14 @@ struct controller_impl {
                                                trx->packed_trx()->get_prunable_size() );
             }
 
+            trx_context.delay = fc::seconds(trn.delay_sec);
+
             if( check_auth ) {
                authorization.check_authorization(
                        trn.actions,
                        trx->recovered_keys(),
                        {},
-                       fc::seconds(trn.delay_sec),
+                       trx_context.delay,
                        [&trx_context](){ trx_context.checktime(); },
                        false,
                        trx->is_dry_run()
@@ -1613,7 +1615,9 @@ struct controller_impl {
 
             trx->billed_cpu_time_us = trx_context.billed_cpu_time_us;
             if (!trx->implicit() && !trx->is_read_only()) {
-               transaction_receipt::status_enum s = transaction_receipt::executed;
+               transaction_receipt::status_enum s = (trx_context.delay == fc::seconds(0))
+                                                    ? transaction_receipt::executed
+                                                    : transaction_receipt::delayed;
                trace->receipt = push_receipt(*trx->packed_trx(), s, trx_context.billed_cpu_time_us, trace->net_usage);
                std::get<building_block>(pending->_block_stage)._pending_trx_metas.emplace_back(trx);
             } else {

libraries/chain/include/eosio/chain/controller.hpp

@@ -370,7 +370,6 @@ namespace eosio { namespace chain {
       private:
          friend class apply_context;
          friend class transaction_context;
-         friend void modify_gto_for_canceldelay_test(controller& control, const transaction_id_type& trx_id); // canceldelay_test in delay_tests.cpp need access to mutable_db
 
          chainbase::database& mutable_db()const;
 

libraries/chain/include/eosio/chain/transaction_context.hpp

@@ -112,6 +112,7 @@ namespace eosio { namespace chain {
 
          void execute_action( uint32_t action_ordinal, uint32_t recurse_depth );
 
+         void schedule_transaction();
          void record_transaction( const transaction_id_type& id, fc::time_point_sec expire );
 
          void validate_cpu_usage_to_bill( int64_t billed_us, int64_t account_cpu_limit, bool check_minimum, int64_t subjective_billed_us )const;
@@ -142,6 +143,7 @@ namespace eosio { namespace chain {
          /// the maximum number of virtual CPU instructions of the transaction that can be safely billed to the billable accounts
          uint64_t                      initial_max_billable_cpu = 0;
 
+         fc::microseconds              delay;
          bool                          is_input           = false;
          bool                          apply_context_free = true;
          bool                          enforce_whiteblacklist = true;

libraries/chain/transaction_context.cpp

@@ -247,7 +247,12 @@ namespace eosio { namespace chain {
                                                  uint64_t packed_trx_prunable_size )
    {
       const transaction& trx = packed_trx.get_transaction();
-      EOS_ASSERT( trx.delay_sec.value == 0, transaction_exception, "transaction cannot be delayed" );
+      // delayed transactions are not allowed after protocol feature
+      // DISABLE_DEFERRED_TRXS_STAGE_1 is activated;
+      // read-only and dry-run transactions are not allowed to be delayed at any time
+      if( control.is_builtin_activated(builtin_protocol_feature_t::disable_deferred_trxs_stage_1) || is_transient() ) {
+         EOS_ASSERT( trx.delay_sec.value == 0, transaction_exception, "transaction cannot be delayed" );
+      }
       if( trx.transaction_extensions.size() > 0 ) {
          disallow_transaction_extensions( "no transaction extensions supported yet for input transactions" );
       }
@@ -266,6 +271,13 @@ namespace eosio { namespace chain {
       uint64_t initial_net_usage = static_cast<uint64_t>(cfg.base_per_transaction_net_usage)
                                     + packed_trx_unprunable_size + discounted_size_for_pruned_data;
 
+      if( trx.delay_sec.value > 0 ) {
+          // If delayed, also charge ahead of time for the additional net usage needed to retire the delayed transaction
+          // whether that be by successfully executing, soft failure, hard failure, or expiration.
+         initial_net_usage += static_cast<uint64_t>(cfg.base_per_transaction_net_usage)
+                               + static_cast<uint64_t>(config::transaction_id_net_usage);
+      }
+
       published = control.pending_block_time();
       is_input = true;
       if (!control.skip_trx_checks()) {
@@ -309,15 +321,21 @@ namespace eosio { namespace chain {
          }
       }
 
-      for( const auto& act : trx.actions ) {
-         schedule_action( act, act.account, false, 0, 0 );
+      if( delay == fc::microseconds() ) {
+         for( const auto& act : trx.actions ) {
+            schedule_action( act, act.account, false, 0, 0 );
+         }
       }
 
       auto& action_traces = trace->action_traces;
       uint32_t num_original_actions_to_execute = action_traces.size();
       for( uint32_t i = 1; i <= num_original_actions_to_execute; ++i ) {
          execute_action( i, 0 );
       }
+
+      if( delay != fc::microseconds() ) {
+         schedule_transaction();
+      }
    }
 
    void transaction_context::finalize() {
@@ -715,6 +733,42 @@ namespace eosio { namespace chain {
       acontext.exec();
    }
 
+   void transaction_context::schedule_transaction() {
+      // Charge ahead of time for the additional net usage needed to retire the delayed transaction
+      // whether that be by successfully executing, soft failure, hard failure, or expiration.
+      const transaction& trx = packed_trx.get_transaction();
+      if( trx.delay_sec.value == 0 ) { // Do not double bill. Only charge if we have not already charged for the delay.
+         const auto& cfg = control.get_global_properties().configuration;
+         add_net_usage( static_cast<uint64_t>(cfg.base_per_transaction_net_usage)
+                         + static_cast<uint64_t>(config::transaction_id_net_usage) ); // Will exit early if net usage cannot be payed.
+      }
+
+      auto first_auth = trx.first_authorizer();
+
+      uint32_t trx_size = 0;
+      const auto& cgto = control.mutable_db().create<generated_transaction_object>( [&]( auto& gto ) {
+        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 );
+        gto.published   = control.pending_block_time();
+        gto.delay_until = gto.published + delay;
+        gto.expiration  = gto.delay_until + fc::seconds(control.get_global_properties().configuration.deferred_trx_expiration_window);
+        trx_size = gto.set( trx );
+
+        if (auto dm_logger = control.get_deep_mind_logger(is_transient())) {
+           std::string event_id = RAM_EVENT_ID("${id}", ("id", gto.id));
+
+           dm_logger->on_create_deferred(deep_mind_handler::operation_qualifier::push, gto, packed_trx);
+           dm_logger->on_ram_trace(std::move(event_id), "deferred_trx", "push", "deferred_trx_pushed");
+        }
+      });
+
+      int64_t ram_delta = (config::billable_size_v<generated_transaction_object> + trx_size);
+      add_ram_usage( cgto.payer, ram_delta );
+      trace->account_ram_delta = account_delta( cgto.payer, ram_delta );
+   }
+
    void transaction_context::record_transaction( const transaction_id_type& id, fc::time_point_sec expire ) {
       try {
           control.mutable_db().create<transaction_object>([&](transaction_object& transaction) {

plugins/producer_plugin/producer_plugin.cpp

@@ -773,6 +773,10 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
                                       transaction_metadata::trx_type       trx_type,
                                       bool                                 return_failure_traces,
                                       next_function<transaction_trace_ptr> next) {
+
+      const transaction& t = trx->get_transaction();
+      EOS_ASSERT( t.delay_sec.value == 0, transaction_exception, "transaction cannot be delayed" );
+
       if (trx_type == transaction_metadata::trx_type::read_only) {
          assert(_ro_thread_pool_size > 0); // enforced by chain_plugin
          assert(app().executor().get_main_thread_id() != std::this_thread::get_id()); // should only be called from read only threads

plugins/producer_plugin/test/CMakeLists.txt

@@ -2,7 +2,8 @@ add_executable( test_producer_plugin
         test_trx_full.cpp
         test_options.cpp
         test_block_timing_util.cpp
+        test_disallow_delayed_trx.cpp
         main.cpp
         )
 target_link_libraries( test_producer_plugin producer_plugin eosio_testing eosio_chain_wrap )
-add_test(NAME test_producer_plugin COMMAND plugins/producer_plugin/test/test_producer_plugin WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
+add_test(NAME test_producer_plugin COMMAND plugins/producer_plugin/test/test_producer_plugin WORKING_DIRECTORY ${CMAKE_BINARY_DIR})

plugins/producer_plugin/test/test_disallow_delayed_trx.cpp

@@ -0,0 +1,101 @@
+#include <eosio/producer_plugin/producer_plugin.hpp>
+#include <eosio/testing/tester.hpp>
+#include <boost/test/unit_test.hpp>
+
+namespace eosio::test::detail {
+using namespace eosio::chain::literals;
+struct testit {
+   uint64_t id;
+
+   testit( uint64_t id = 0 ) :id(id){}
+
+   static account_name get_account() {
+      return chain::config::system_account_name;
+   }
+
+   static action_name get_name() {
+      return "testit"_n;
+   }
+};
+}
+FC_REFLECT( eosio::test::detail::testit, (id) )
+
+namespace {
+
+using namespace eosio;
+using namespace eosio::chain;
+using namespace eosio::test::detail;
+
+auto make_delayed_trx( const chain_id_type& chain_id ) {
+   account_name creator = config::system_account_name;
+
+   signed_transaction trx;
+   trx.actions.emplace_back( vector<permission_level>{{creator, config::active_name}}, testit{0} );
+   trx.delay_sec = 10;
+   auto priv_key = private_key_type::regenerate<fc::ecc::private_key_shim>(fc::sha256::hash(std::string("nathan")));
+   trx.sign( priv_key, chain_id );
+
+   return std::make_shared<packed_transaction>( std::move(trx) );
+}
+}
+
+BOOST_AUTO_TEST_SUITE(disallow_delayed_trx_test)
+
+// Verifies that incoming delayed transactions are blocked.
+BOOST_AUTO_TEST_CASE(delayed_trx) {
+   using namespace std::chrono_literals;
+   fc::temp_directory temp;
+   appbase::scoped_app app;
+   auto temp_dir_str = temp.path().string();
+
+   std::promise<std::tuple<producer_plugin*, chain_plugin*>> plugin_promise;
+   std::future<std::tuple<producer_plugin*, chain_plugin*>> plugin_fut = plugin_promise.get_future();
+   std::thread app_thread( [&]() {
+      try {
+         fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug);
+         std::vector<const char*> argv =
+            {"test", "--data-dir", temp_dir_str.c_str(), "--config-dir", temp_dir_str.c_str(),
+               "-p", "eosio", "-e", "--disable-subjective-p2p-billing=true" };
+         app->initialize<chain_plugin, producer_plugin>( argv.size(), (char**) &argv[0] );
+         app->startup();
+         plugin_promise.set_value(
+            {app->find_plugin<producer_plugin>(), app->find_plugin<chain_plugin>()} );
+         app->exec();
+         return;
+      } FC_LOG_AND_DROP()
+      BOOST_CHECK(!"app threw exception see logged error");
+   } );
+
+   auto[prod_plug, chain_plug] = plugin_fut.get();
+   auto chain_id = chain_plug->get_chain_id();
+
+   // create a delayed trx
+   auto ptrx = make_delayed_trx( chain_id );
+
+   // send it as incoming trx
+   app->post( priority::low, [ptrx, &app]() {
+      bool return_failure_traces = true;
+
+      // the delayed trx is blocked
+      BOOST_REQUIRE_EXCEPTION(
+         app->get_method<plugin_interface::incoming::methods::transaction_async>()(ptrx,
+            false,
+            transaction_metadata::trx_type::input,
+            return_failure_traces,
+            [ptrx, return_failure_traces] (const next_function_variant<transaction_trace_ptr>& result) {
+               elog( "trace with except ${e}", ("e", fc::json::to_pretty_string( *std::get<chain::transaction_trace_ptr>( result ) )) );
+            }
+         ),
+         fc::exception,
+         eosio::testing::fc_exception_message_starts_with("transaction cannot be delayed")
+      );
+   });
+
+   // leave time for transaction to be executed
+   std::this_thread::sleep_for( 2000ms );
+
+   app->quit();
+   app_thread.join();
+}
+
+BOOST_AUTO_TEST_SUITE_END()

tests/nodeos_chainbase_allocation_test.py

@@ -31,7 +31,6 @@
     # The following is the list of chainbase objects that need to be verified:
     # - account_object (bootstrap)
     # - code_object (bootstrap)
-    # - generated_transaction_object
     # - global_property_object
     # - key_value_object (bootstrap)
     # - protocol_state_object (bootstrap)
@@ -55,12 +54,6 @@
     irrNode = cluster.getNode(irrNodeId)
     nonProdNode = cluster.getNode(nonProdNodeId)
 
-    # Create delayed transaction to create "generated_transaction_object"
-    cmd = "create account -j eosio sample EOS6MRyAjQq8ud7hVNYcfnVPJqcVpscN5So8BhtHuGYqET5GDW5CV\
-         EOS6MRyAjQq8ud7hVNYcfnVPJqcVpscN5So8BhtHuGYqET5GDW5CV --delay-sec 600 -p eosio"
-    trans = producerNode.processCleosCmd(cmd, cmd, silentErrors=False)
-    assert trans
-
     # Schedule a new producer to trigger new producer schedule for "global_property_object"
     newProducerAcc = Account("newprod")
     newProducerAcc.ownerPublicKey = "EOS6MRyAjQq8ud7hVNYcfnVPJqcVpscN5So8BhtHuGYqET5GDW5CV"

unittests/api_tests.cpp

@@ -767,11 +767,11 @@ BOOST_FIXTURE_TEST_CASE(cfa_stateful_api, validating_tester)  try {
    BOOST_REQUIRE_EQUAL( validate(), true );
 } FC_LOG_AND_RETHROW()
 
-BOOST_FIXTURE_TEST_CASE(deferred_cfa_not_allowed, validating_tester)  try {
+BOOST_FIXTURE_TEST_CASE(deferred_cfa_failed, validating_tester_no_disable_deferred_trx)  try {
 
    create_account( "testapi"_n );
-   produce_blocks(1);
-   set_code( "testapi"_n, test_contracts::test_api_wasm() );
+	produce_blocks(1);
+	set_code( "testapi"_n, test_contracts::test_api_wasm() );
 
    account_name a = "testapi2"_n;
    account_name creator = config::system_account_name;
@@ -785,15 +785,58 @@ BOOST_FIXTURE_TEST_CASE(deferred_cfa_not_allowed, validating_tester)  try {
                                  .owner    = authority( get_public_key( a, "owner" ) ),
                                  .active   = authority( get_public_key( a, "active" ) )
                                  });
-   action act({}, test_api_action<TEST_METHOD("test_transaction", "context_free_api")>{});
+   action act({}, test_api_action<TEST_METHOD("test_transaction", "stateful_api")>{});
    trx.context_free_actions.push_back(act);
-   set_transaction_headers(trx, 10, 2); // set delay_sec to 2
+   set_transaction_headers(trx, 10, 2);
    trx.sign( get_private_key( creator, "active" ), control->get_chain_id()  );
+
    BOOST_CHECK_EXCEPTION(push_transaction( trx ), fc::exception,
       [&](const fc::exception &e) {
-         // any incoming trx is blocked
-         return expect_assert_message(e, "transaction cannot be delayed");
+         return expect_assert_message(e, "only context free api's can be used in this context");
+      });
+
+   produce_blocks(10);
+
+   // CFA failed, testapi2 not created
+   create_account( "testapi2"_n );
+
+   BOOST_REQUIRE_EQUAL( validate(), true );
+} FC_LOG_AND_RETHROW()
+
+BOOST_FIXTURE_TEST_CASE(deferred_cfa_success, validating_tester_no_disable_deferred_trx)  try {
+
+   create_account( "testapi"_n );
+	produce_blocks(1);
+	set_code( "testapi"_n, test_contracts::test_api_wasm() );
+
+   account_name a = "testapi2"_n;
+   account_name creator = config::system_account_name;
+   signed_transaction trx;
+   trx.actions.emplace_back( vector<permission_level>{{creator,config::active_name}},
+                                 newaccount{
+                                 .creator  = creator,
+                                 .name     = a,
+                                 .owner    = authority( get_public_key( a, "owner" ) ),
+                                 .active   = authority( get_public_key( a, "active" ) )
+                                 });
+   action act({}, test_api_action<TEST_METHOD("test_transaction", "context_free_api")>{});
+   trx.context_free_actions.push_back(act);
+   set_transaction_headers(trx, 10, 2);
+   trx.sign( get_private_key( creator, "active" ), control->get_chain_id()  );
+   auto trace = push_transaction( trx );
+   BOOST_REQUIRE(trace != nullptr);
+   if (trace) {
+      BOOST_REQUIRE_EQUAL(transaction_receipt_header::status_enum::delayed, trace->receipt->status);
+      BOOST_REQUIRE_EQUAL(1, trace->action_traces.size());
+   }
+   produce_blocks(10);
+
+   // CFA success, testapi2 created
+   BOOST_CHECK_EXCEPTION(create_account( "testapi2"_n ), fc::exception,
+      [&](const fc::exception &e) {
+         return expect_assert_message(e, "Cannot create account named testapi2, as that name is already taken");
       });
+   BOOST_REQUIRE_EQUAL( validate(), true );
 } FC_LOG_AND_RETHROW()
 
 BOOST_AUTO_TEST_CASE(light_validation_skip_cfa) try {