Better handling for missing transactions in history_plugin

plugins/history_plugin/history_plugin.cpp

@@ -380,63 +380,105 @@ namespace eosio {
 
       read_only::get_transaction_result read_only::get_transaction( const read_only::get_transaction_params& p )const {
          auto& chain = history->chain_plug->chain();
-
-         get_transaction_result result;
-
-         result.id = p.id;
-         result.last_irreversible_block = chain.last_irreversible_block_num();
+         auto short_id = fc::variant(p.id).as_string().substr(0,8);
 
          const auto& db = chain.db();
-
          const auto& idx = db.get_index<action_history_index, by_trx_id>();
          auto itr = idx.lower_bound( boost::make_tuple(p.id) );
-         if( itr == idx.end() ) {
-            return result;
+
+         bool in_history = (itr != idx.end() && fc::variant(itr->trx_id).as_string().substr(0,8) == short_id );
+
+         if( !in_history && !p.block_num_hint ) {
+            FC_THROW("Transaction ${id} not found in history and no block hint was given", ("id",p.id));
          }
-         result.id         = itr->trx_id;
-         result.block_num  = itr->block_num;
-         result.block_time = itr->block_time;
 
-         if( fc::variant(result.id).as_string().substr(0,8) != fc::variant(p.id).as_string().substr(0,8) )
-            return result;
+         get_transaction_result result;
 
-         while( itr != idx.end() && itr->trx_id == result.id ) {
+         if (in_history) {
+            result.id = p.id;
+            result.last_irreversible_block = chain.last_irreversible_block_num();
 
-           fc::datastream<const char*> ds( itr->packed_action_trace.data(), itr->packed_action_trace.size() );
-           action_trace t;
-           fc::raw::unpack( ds, t );
-           result.traces.emplace_back( chain.to_variant_with_abi(t) );
 
-           ++itr;
-         }
+            result.id         = itr->trx_id;
+            result.block_num  = itr->block_num;
+            result.block_time = itr->block_time;
 
-         auto blk = chain.fetch_block_by_number( result.block_num );
-         if( blk == nullptr ) { // still in pending
-             auto blk_state = chain.pending_block_state();
-             if( blk_state != nullptr ) {
-                 blk = blk_state->block;
-             }
-         }
-         if( blk != nullptr ) {
-             for (const auto &receipt: blk->transactions) {
-                 if (receipt.trx.contains<packed_transaction>()) {
-                     auto &pt = receipt.trx.get<packed_transaction>();
+            while( itr != idx.end() && itr->trx_id == result.id ) {
+
+              fc::datastream<const char*> ds( itr->packed_action_trace.data(), itr->packed_action_trace.size() );
+              action_trace t;
+              fc::raw::unpack( ds, t );
+              result.traces.emplace_back( chain.to_variant_with_abi(t) );
+
+              ++itr;
+            }
+
+            auto blk = chain.fetch_block_by_number( result.block_num );
+            if( blk == nullptr ) { // still in pending
+                auto blk_state = chain.pending_block_state();
+                if( blk_state != nullptr ) {
+                    blk = blk_state->block;
+                }
+            }
+            if( blk != nullptr ) {
+                for (const auto &receipt: blk->transactions) {
+                    if (receipt.trx.contains<packed_transaction>()) {
+                        auto &pt = receipt.trx.get<packed_transaction>();
+                        auto mtrx = transaction_metadata(pt);
+                        if (mtrx.id == result.id) {
+                            fc::mutable_variant_object r("receipt", receipt);
+                            r("trx", chain.to_variant_with_abi(mtrx.trx));
+                            result.trx = move(r);
+                            break;
+                        }
+                    } else {
+                        auto &id = receipt.trx.get<transaction_id_type>();
+                        if (id == result.id) {
+                            fc::mutable_variant_object r("receipt", receipt);
+                            result.trx = move(r);
+                            break;
+                        }
+                    }
+                }
+            }
+         } else {
+            auto blk = chain.fetch_block_by_number(*p.block_num_hint);
+            bool found = false;
+            if (blk) {
+               for (const auto& receipt: blk->transactions) {
+                  if (receipt.trx.contains<packed_transaction>()) {
+                     auto& pt = receipt.trx.get<packed_transaction>();
                      auto mtrx = transaction_metadata(pt);
-                     if (mtrx.id == result.id) {
-                         fc::mutable_variant_object r("receipt", receipt);
-                         r("trx", chain.to_variant_with_abi(mtrx.trx));
-                         result.trx = move(r);
-                         break;
+                     if (fc::variant(mtrx.id).as_string().substr(0, 8) == short_id) {
+                        result.id = mtrx.id;
+                        result.last_irreversible_block = chain.last_irreversible_block_num();
+                        result.block_num = *p.block_num_hint;
+                        result.block_time = blk->timestamp;
+                        fc::mutable_variant_object r("receipt", receipt);
+                        r("trx", chain.to_variant_with_abi(mtrx.trx));
+                        result.trx = move(r);
+                        found = true;
+                        break;
                      }
-                 } else {
-                     auto &id = receipt.trx.get<transaction_id_type>();
-                     if (id == result.id) {
-                         fc::mutable_variant_object r("receipt", receipt);
-                         result.trx = move(r);
-                         break;
+                  } else {
+                     auto& id = receipt.trx.get<transaction_id_type>();
+                     if (fc::variant(id).as_string().substr(0, 8) == short_id) {
+                        result.id = id;
+                        result.last_irreversible_block = chain.last_irreversible_block_num();
+                        result.block_num = *p.block_num_hint;
+                        result.block_time = blk->timestamp;
+                        fc::mutable_variant_object r("receipt", receipt);
+                        result.trx = move(r);
+                        found = true;
+                        break;
                      }
-                 }
-             }
+                  }
+               }
+            }
+
+            if (!found) {
+               FC_THROW("Transaction ${id} not found in history or in block number ${n}", ("id",p.id)("n", *p.block_num_hint));
+            }
          }
 
          return result;

plugins/history_plugin/include/eosio/history_plugin.hpp

@@ -67,7 +67,8 @@ class read_only {
 
 
       struct get_transaction_params {
-         transaction_id_type          id;
+         transaction_id_type           id;
+         optional<uint32_t>            block_num_hint;
       };
 
       struct get_transaction_result {
@@ -152,7 +153,7 @@ FC_REFLECT( eosio::history_apis::read_only::get_actions_params, (account_name)(p
 FC_REFLECT( eosio::history_apis::read_only::get_actions_result, (actions)(last_irreversible_block)(time_limit_exceeded_error) )
 FC_REFLECT( eosio::history_apis::read_only::ordered_action_result, (global_action_seq)(account_action_seq)(block_num)(block_time)(action_trace) )
 
-FC_REFLECT( eosio::history_apis::read_only::get_transaction_params, (id) )
+FC_REFLECT( eosio::history_apis::read_only::get_transaction_params, (id)(block_num_hint) )
 FC_REFLECT( eosio::history_apis::read_only::get_transaction_result, (id)(trx)(block_time)(block_num)(last_irreversible_block)(traces) )
 /*
 FC_REFLECT(eosio::history_apis::read_only::get_transaction_params, (transaction_id) )

programs/cleos/help_text.cpp

@@ -67,6 +67,13 @@ const char* unknown_abi_table_help_text = _(R"text(The ABI for the code on accou
 Please check the account and table name, and verify that the account has the expected code using:
   cleos get code ${1})text");
 
+const char* failed_to_find_transaction_text = _("Failed to fetch information for transaction: \033[1m${1}\033[0m from the history plugin\n\n"
+                                                "\033[32mIf you know the block number which included this transaction you providing it with the \033[2m--block-hint\033[22m option may help\033[0m");
+
+const char* failed_to_find_transaction_with_block_text = _("Failed to fetch information for transaction: \033[1m${1}\033[0m from the history plugin and the transaction was not present in block \033[1m${2}\033[0m\n");
+
+const char* history_plugin_advice_text = _("\033[32mPlease ensure that the \033[2meosio::history_plugin\033[22m is enabled on the RPC node you are connecting to and that an account involved in this transaction was configured in the \033[2mfilter-on\033[22m setting.\033[0m\n");
+
 const char* help_regex_error = _("Error locating help text: ${code} ${what}");
 
 const std::vector<std::pair<const char*, std::vector<const char *>>> error_help_text {
@@ -82,7 +89,9 @@ const std::vector<std::pair<const char*, std::vector<const char *>>> error_help_
    {"AES error[^\\x00]*wallet/unlock.*postdata\":\\[\"([^\"]*)\"", {bad_wallet_password_help_text}},
    {"Wallet is locked: ([\\S]*)", {locked_wallet_help_text}},
    {"Key already in wallet[^\\x00]*wallet/import_key.*postdata\":\\[\"([^\"]*)\"", {duplicate_key_import_help_text}},
-   {"ABI does not define table[^\\x00]*get_table_rows.*code\":\"([^\"]*)\",\"table\":\"([^\"]*)\"", {unknown_abi_table_help_text}}
+   {"ABI does not define table[^\\x00]*get_table_rows.*code\":\"([^\"]*)\",\"table\":\"([^\"]*)\"", {unknown_abi_table_help_text}},
+   {"Transaction ([^ ]{8})[^ ]* not found in history and no block hint was given", {failed_to_find_transaction_text, history_plugin_advice_text}},
+   {"Transaction ([^ ]{8})[^ ]* not found in history or in block number ([0-9]*)", {failed_to_find_transaction_with_block_text, history_plugin_advice_text}},
 };
 
 auto smatch_to_variant(const std::smatch& smatch) {

programs/cleos/main.cpp

@@ -1816,15 +1816,20 @@ int main( int argc, char** argv ) {
 
    // get transaction
    string transaction_id_str;
+   uint32_t block_num_hint = 0;
    auto getTransaction = get->add_subcommand("transaction", localized("Retrieve a transaction from the blockchain"), false);
    getTransaction->add_option("id", transaction_id_str, localized("ID of the transaction to retrieve"))->required();
+   getTransaction->add_option( "-b,--block-hint", block_num_hint, localized("the block number this transaction may be in") );
    getTransaction->set_callback([&] {
       transaction_id_type transaction_id;
       try {
          while( transaction_id_str.size() < 64 ) transaction_id_str += "0";
          transaction_id = transaction_id_type(transaction_id_str);
       } EOS_RETHROW_EXCEPTIONS(transaction_id_type_exception, "Invalid transaction ID: ${transaction_id}", ("transaction_id", transaction_id_str))
       auto arg= fc::mutable_variant_object( "id", transaction_id);
+      if ( block_num_hint > 0 ) {
+         arg = arg("block_num_hint", block_num_hint);
+      }
       std::cout << fc::json::to_pretty_string(call(get_transaction_func, arg)) << std::endl;
    });