better way of rejecting disallowed transaction extensions #6115

Allows the `num_failed` tracker and blacklist of producer_plugin to work as intended. Preserves the current pattern of not retiring (except for case with expired status) deferred transaction with invalid extensions even after NO_DUPLICATE_DEFERRED_ID activation.
EOSIO · Apr 4, 2019 · aa83af6 · aa83af6
1 parent 3633119
commit aa83af6
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 11 deletions.
diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp
@@ -984,6 +984,8 @@ struct controller_impl {
          trx_context.squash();
          restore.cancel();
          return trace;
+      } catch( const disallowed_transaction_extensions_bad_block_exception& ) {
+         throw;
       } catch( const protocol_feature_bad_block_exception& ) {
          throw;
       } catch( const fc::exception& e ) {
@@ -1118,6 +1120,8 @@ struct controller_impl {
          restore.cancel();
 
          return trace;
+      } catch( const disallowed_transaction_extensions_bad_block_exception& ) {
+         throw;
       } catch( const protocol_feature_bad_block_exception& ) {
          throw;
       } catch( const fc::exception& e ) {

diff --git a/libraries/chain/include/eosio/chain/exceptions.hpp b/libraries/chain/include/eosio/chain/exceptions.hpp
@@ -200,6 +200,8 @@ namespace eosio { namespace chain {
                                     3040015, "Invalid transaction extension" )
       FC_DECLARE_DERIVED_EXCEPTION( ill_formed_deferred_transaction_generation_context, transaction_exception,
                                     3040016, "Transaction includes an ill-formed deferred transaction generation context extension" )
+      FC_DECLARE_DERIVED_EXCEPTION( disallowed_transaction_extensions_bad_block_exception, transaction_exception,
+                                    3250002, "Transaction includes disallowed extensions (invalid block)" )
 
 
    FC_DECLARE_DERIVED_EXCEPTION( action_validate_exception, chain_exception,

diff --git a/libraries/chain/include/eosio/chain/transaction_context.hpp b/libraries/chain/include/eosio/chain/transaction_context.hpp
@@ -73,6 +73,8 @@ namespace eosio { namespace chain {
 
          void validate_cpu_usage_to_bill( int64_t u, bool check_minimum = true )const;
 
+         void disallow_transaction_extensions( const char* error_msg )const;
+
       /// Fields:
       public:
 

diff --git a/libraries/chain/transaction_context.cpp b/libraries/chain/transaction_context.cpp
@@ -166,10 +166,14 @@ namespace bacc = boost::accumulators;
       trace->block_time = c.pending_block_time();
       trace->producer_block_id = c.pending_producer_block_id();
       executed.reserve( trx.total_actions() );
-      EOS_ASSERT( trx.transaction_extensions.size() == 0
-                     || control.is_builtin_activated( builtin_protocol_feature_t::no_duplicate_deferred_id ),
-                  unsupported_feature, "we don't support any extensions yet"
-      ); // This assert may not be necessary. Consider removing.
+   }
+
+   void transaction_context::disallow_transaction_extensions( const char* error_msg )const {
+      if( control.is_producing_block() ) {
+         EOS_THROW( subjective_block_production_exception, error_msg );
+      } else {
+         EOS_THROW( disallowed_transaction_extensions_bad_block_exception, error_msg );
+      }
    }
 
    void transaction_context::init(uint64_t initial_net_usage)
@@ -281,8 +285,10 @@ namespace bacc = boost::accumulators;
 
    void transaction_context::init_for_implicit_trx( uint64_t initial_net_usage  )
    {
-      EOS_ASSERT( trx.transaction_extensions.size() == 0, unsupported_feature,
-                  "no transaction extensions supported yet for implicit transactions" );
+      if( trx.transaction_extensions.size() > 0 ) {
+         disallow_transaction_extensions( "no transaction extensions supported yet for implicit transactions" );
+      }
+
       published = control.pending_block_time();
       init( initial_net_usage);
    }
@@ -291,8 +297,9 @@ namespace bacc = boost::accumulators;
                                                  uint64_t packed_trx_prunable_size,
                                                  bool skip_recording )
    {
-      EOS_ASSERT( trx.transaction_extensions.size() == 0, unsupported_feature,
-                  "no transaction extensions supported yet for input transactions" );
+      if( trx.transaction_extensions.size() > 0 ) {
+         disallow_transaction_extensions( "no transaction extensions supported yet for input transactions" );
+      }
 
       const auto& cfg = control.get_global_properties().configuration;
 
@@ -330,9 +337,13 @@ namespace bacc = boost::accumulators;
 
    void transaction_context::init_for_deferred_trx( fc::time_point p )
    {
-      EOS_ASSERT( (trx.expiration.sec_since_epoch() == 0) || (trx.transaction_extensions.size() == 0), unsupported_feature,
-                  "no transaction extensions supported yet for deferred transactions"
-      );
+      if( (trx.expiration.sec_since_epoch() != 0) && (trx.transaction_extensions.size() > 0) ) {
+         disallow_transaction_extensions( "no transaction extensions supported yet for deferred transactions" );
+      }
+      // If (trx.expiration.sec_since_epoch() == 0) then it was created after NO_DUPLICATE_DEFERRED_ID activation,
+      // and so validation of its extensions was done either in:
+      //   * apply_context::schedule_deferred_transaction for contract-generated transactions;
+      //   * or transaction_context::init_for_input_trx for delayed input transactions.
 
       published = p;
       trace->scheduled = true;