Merge pull request #630

bf6d147 new flush_txpool command, and associated RPC call (moneromooo-monero)
6288295 rpc: add missing return on error when getting a tx (moneromooo-monero)
b7e37b7 simplewallet: show_transfers can now show just failed txes (moneromooo-monero)
b11539f wallet: detect and handle failed outgoing transfers (moneromooo-monero)
4b23714 tx_pool: serialize missing kept_by_block flag (moneromooo-monero)

src/cryptonote_core/blockchain.cpp

@@ -2386,6 +2386,25 @@ void Blockchain::return_tx_to_pool(const std::vector<transaction> &txs)
   }
 }
 //------------------------------------------------------------------
+bool Blockchain::flush_txes_from_pool(const std::list<crypto::hash> &txids)
+{
+  bool res = true;
+  for (const auto &txid: txids)
+  {
+    cryptonote::transaction tx;
+    size_t blob_size;
+    uint64_t fee;
+    bool relayed;
+    LOG_PRINT_L1("Removing txid " << txid << " from the pool");
+    if(!m_tx_pool.take_tx(txid, tx, blob_size, fee, relayed))
+    {
+      LOG_PRINT_L0("Failed to remove txid " << txid << " from the pool");
+      res = false;
+    }
+  }
+  return res;
+}
+//------------------------------------------------------------------
 //      Needs to validate the block and acquire each transaction from the
 //      transaction mem_pool, then pass the block and transactions to
 //      m_db->add_block()

src/cryptonote_core/blockchain.h

@@ -164,6 +164,8 @@ namespace cryptonote
 
     bool get_hard_fork_voting_info(uint8_t version, uint32_t &window, uint32_t &votes, uint32_t &threshold, uint64_t &earliest_height, uint8_t &voting) const;
 
+    bool flush_txes_from_pool(const std::list<crypto::hash> &txids);
+
     bool for_all_key_images(std::function<bool(const crypto::key_image&)>) const;
     bool for_all_blocks(std::function<bool(uint64_t, const crypto::hash&, const block&)>) const;
     bool for_all_transactions(std::function<bool(const crypto::hash&, const cryptonote::transaction&)>) const;

src/cryptonote_core/blockchain_storage.cpp

@@ -1892,6 +1892,25 @@ void blockchain_storage::set_enforce_dns_checkpoints(bool enforce_checkpoints)
   m_enforce_dns_checkpoints = enforce_checkpoints;
 }
 //------------------------------------------------------------------
+bool blockchain_storage::flush_txes_from_pool(const std::list<crypto::hash> &txids)
+{
+  bool res = true;
+  for (const auto &txid: txids)
+  {
+    cryptonote::transaction tx;
+    size_t blob_size;
+    uint64_t fee;
+    bool relayed;
+    LOG_PRINT_L1("Removing txid " << txid << " from the pool");
+    if(!m_tx_pool->take_tx(txid, tx, blob_size, fee, relayed))
+    {
+      LOG_PRINT_L0("Failed to remove txid " << txid << " from the pool");
+      res = false;
+    }
+  }
+  return res;
+}
+//------------------------------------------------------------------
 bool blockchain_storage::for_all_key_images(std::function<bool(const crypto::key_image&)> f) const
 {
   for (key_images_container::const_iterator i = m_spent_keys.begin(); i != m_spent_keys.end(); ++i) {

src/cryptonote_core/blockchain_storage.h

@@ -130,6 +130,7 @@ namespace cryptonote
     bool is_storing_blockchain()const{return m_is_blockchain_storing;}
     uint64_t block_difficulty(size_t i) const;
     double get_avg_block_size( size_t count) const;
+    bool flush_txes_from_pool(const std::list<crypto::hash> &txids);
 
     template<class t_ids_container, class t_blocks_container, class t_missed_container>
     bool get_blocks(const t_ids_container& block_ids, t_blocks_container& blocks, t_missed_container& missed_bs) const

src/cryptonote_core/tx_pool.h

@@ -110,7 +110,7 @@ namespace cryptonote
     /*bool flush_pool(const std::strig& folder);
     bool inflate_pool(const std::strig& folder);*/
 
-#define CURRENT_MEMPOOL_ARCHIVE_VER    10
+#define CURRENT_MEMPOOL_ARCHIVE_VER    11
 
     template<class archive_t>
     void serialize(archive_t & a, const unsigned int version)
@@ -243,6 +243,9 @@ namespace boost
         return;
       ar & td.last_relayed_time;
       ar & td.relayed;
+      if (version < 11)
+        return;
+      ar & td.kept_by_block;
     }
   }
 }

src/daemon/command_parser_executor.cpp

@@ -421,5 +421,23 @@ bool t_command_parser_executor::unban(const std::vector<std::string>& args)
   return m_executor.unban(ip);
 }
 
+bool t_command_parser_executor::flush_txpool(const std::vector<std::string>& args)
+{
+  if (args.size() > 1) return false;
+
+  std::string txid;
+  if (args.size() == 1)
+  {
+    crypto::hash hash;
+    if (!parse_hash256(args[0], hash))
+    {
+      std::cout << "failed to parse tx id" << std::endl;
+      return true;
+    }
+    txid = args[0];
+  }
+  return m_executor.flush_txpool(txid);
+}
+
 
 } // namespace daemonize

src/daemon/command_parser_executor.h

@@ -112,6 +112,8 @@ public:
   bool ban(const std::vector<std::string>& args);
 
   bool unban(const std::vector<std::string>& args);
+
+  bool flush_txpool(const std::vector<std::string>& args);
 };
 
 } // namespace daemonize

src/daemon/command_server.cpp

@@ -209,6 +209,11 @@ t_command_server::t_command_server(
     , std::bind(&t_command_parser_executor::unban, &m_parser, p::_1)
     , "Unban a given IP"
     );
+    m_command_lookup.set_handler(
+      "flush_txpool"
+    , std::bind(&t_command_parser_executor::flush_txpool, &m_parser, p::_1)
+    , "Flush a transaction from the tx pool by its txid, or the whole tx pool"
+    );
 }
 
 bool t_command_server::process_command_str(const std::string& cmd)

src/daemon/rpc_command_executor.cpp

@@ -1161,4 +1161,34 @@ bool t_rpc_command_executor::unban(const std::string &ip)
     return true;
 }
 
+bool t_rpc_command_executor::flush_txpool(const std::string &txid)
+{
+    cryptonote::COMMAND_RPC_FLUSH_TRANSACTION_POOL::request req;
+    cryptonote::COMMAND_RPC_FLUSH_TRANSACTION_POOL::response res;
+    std::string fail_message = "Unsuccessful";
+    epee::json_rpc::error error_resp;
+
+    if (!txid.empty())
+      req.txids.push_back(txid);
+
+    if (m_is_rpc)
+    {
+        if (!m_rpc_client->json_rpc_request(req, res, "flush_txpool", fail_message.c_str()))
+        {
+            return true;
+        }
+    }
+    else
+    {
+        if (!m_rpc_server->on_flush_txpool(req, res, error_resp))
+        {
+            tools::fail_msg_writer() << fail_message.c_str();
+            return true;
+        }
+    }
+
+    return true;
+}
+
+
 }// namespace daemonize

src/daemon/rpc_command_executor.h

@@ -130,6 +130,8 @@ public:
   bool ban(const std::string &ip, time_t seconds);
 
   bool unban(const std::string &ip);
+
+  bool flush_txpool(const std::string &txid);
 };
 
 } // namespace daemonize

src/rpc/core_rpc_server.cpp

@@ -224,6 +224,7 @@ namespace cryptonote
       if(b.size() != sizeof(crypto::hash))
       {
         res.status = "Failed, size of data mismatch";
+        return true;
       }
       vh.push_back(*reinterpret_cast<const crypto::hash*>(b.data()));
     }
@@ -983,6 +984,60 @@ namespace cryptonote
     return true;
   }
   //------------------------------------------------------------------------------------------------------------------------------
+  bool core_rpc_server::on_flush_txpool(const COMMAND_RPC_FLUSH_TRANSACTION_POOL::request& req, COMMAND_RPC_FLUSH_TRANSACTION_POOL::response& res, epee::json_rpc::error& error_resp)
+  {
+    if(!check_core_busy())
+    {
+      error_resp.code = CORE_RPC_ERROR_CODE_CORE_BUSY;
+      error_resp.message = "Core is busy.";
+      return false;
+    }
+
+    bool failed = false;
+    std::list<crypto::hash> txids;
+    if (req.txids.empty())
+    {
+      std::list<transaction> pool_txs;
+      bool r = m_core.get_pool_transactions(pool_txs);
+      if (!r)
+      {
+        res.status = "Failed to get txpool contents";
+        return true;
+      }
+      for (const auto &tx: pool_txs)
+      {
+        txids.push_back(cryptonote::get_transaction_hash(tx));
+      }
+    }
+    else
+    {
+      for (const auto &str: req.txids)
+      {
+        cryptonote::blobdata txid_data;
+        if(!epee::string_tools::parse_hexstr_to_binbuff(str, txid_data))
+        {
+          failed = true;
+        }
+        crypto::hash txid = *reinterpret_cast<const crypto::hash*>(txid_data.data());
+        txids.push_back(txid);
+      }
+    }
+    if (!m_core.get_blockchain_storage().flush_txes_from_pool(txids))
+    {
+      res.status = "Failed to remove one more tx";
+      return false;
+    }
+
+    if (failed)
+    {
+      res.status = "Failed to parse txid";
+      return false;
+    }
+
+    res.status = CORE_RPC_STATUS_OK;
+    return true;
+  }
+  //------------------------------------------------------------------------------------------------------------------------------
   bool core_rpc_server::on_fast_exit(const COMMAND_RPC_FAST_EXIT::request& req, COMMAND_RPC_FAST_EXIT::response& res)
   {
 	  cryptonote::core::set_fast_exit();

src/rpc/core_rpc_server.h

@@ -108,6 +108,7 @@ namespace cryptonote
         MAP_JON_RPC_WE("hard_fork_info",         on_hard_fork_info,             COMMAND_RPC_HARD_FORK_INFO)
         MAP_JON_RPC_WE("setbans",                on_set_bans,                   COMMAND_RPC_SETBANS)
         MAP_JON_RPC_WE("getbans",                on_get_bans,                   COMMAND_RPC_GETBANS)
+        MAP_JON_RPC_WE("flush_txpool",           on_flush_txpool,               COMMAND_RPC_FLUSH_TRANSACTION_POOL)
       END_JSON_RPC_MAP()
     END_URI_MAP2()
 
@@ -147,6 +148,7 @@ namespace cryptonote
     bool on_hard_fork_info(const COMMAND_RPC_HARD_FORK_INFO::request& req, COMMAND_RPC_HARD_FORK_INFO::response& res, epee::json_rpc::error& error_resp);
     bool on_set_bans(const COMMAND_RPC_SETBANS::request& req, COMMAND_RPC_SETBANS::response& res, epee::json_rpc::error& error_resp);
     bool on_get_bans(const COMMAND_RPC_GETBANS::request& req, COMMAND_RPC_GETBANS::response& res, epee::json_rpc::error& error_resp);
+    bool on_flush_txpool(const COMMAND_RPC_FLUSH_TRANSACTION_POOL::request& req, COMMAND_RPC_FLUSH_TRANSACTION_POOL::response& res, epee::json_rpc::error& error_resp);
     //-----------------------
 
 private:

src/rpc/core_rpc_server_commands_defs.h

@@ -965,5 +965,26 @@ namespace cryptonote
       END_KV_SERIALIZE_MAP()
     };
   };
+
+  struct COMMAND_RPC_FLUSH_TRANSACTION_POOL
+  {
+    struct request
+    {
+      std::list<std::string> txids;
+
+      BEGIN_KV_SERIALIZE_MAP()
+        KV_SERIALIZE(txids)
+      END_KV_SERIALIZE_MAP()
+    };
+
+    struct response
+    {
+      std::string status;
+
+      BEGIN_KV_SERIALIZE_MAP()
+        KV_SERIALIZE(status)
+      END_KV_SERIALIZE_MAP()
+    };
+  };
 }
 

src/simplewallet/simplewallet.cpp

@@ -2194,18 +2194,19 @@ bool simple_wallet::show_transfers(const std::vector<std::string> &args_)
   bool in = true;
   bool out = true;
   bool pending = true;
+  bool failed = true;
   uint64_t min_height = 0;
   uint64_t max_height = (uint64_t)-1;
 
   if(local_args.size() > 3) {
-    fail_msg_writer() << tr("usage: show_transfers [in|out|all|pending] [<min_height> [<max_height>]]");
+    fail_msg_writer() << tr("usage: show_transfers [in|out|all|pending|failed] [<min_height> [<max_height>]]");
     return true;
   }
 
   // optional in/out selector
   if (local_args.size() > 0) {
     if (local_args[0] == "in" || local_args[0] == "incoming") {
-      out = pending = false;
+      out = pending = failed = false;
       local_args.erase(local_args.begin());
     }
     else if (local_args[0] == "out" || local_args[0] == "outgoing") {
@@ -2213,7 +2214,11 @@ bool simple_wallet::show_transfers(const std::vector<std::string> &args_)
       local_args.erase(local_args.begin());
     }
     else if (local_args[0] == "pending") {
-      in = out = false;
+      in = out = failed = false;
+      local_args.erase(local_args.begin());
+    }
+    else if (local_args[0] == "failed") {
+      in = out = pending = false;
       local_args.erase(local_args.begin());
     }
     else if (local_args[0] == "all" || local_args[0] == "both") {
@@ -2287,7 +2292,7 @@ bool simple_wallet::show_transfers(const std::vector<std::string> &args_)
   }
 
   // print unconfirmed last
-  if (pending) {
+  if (pending || failed) {
     std::list<std::pair<crypto::hash, tools::wallet2::unconfirmed_transfer_details>> upayments;
     m_wallet->get_unconfirmed_payments_out(upayments);
     for (std::list<std::pair<crypto::hash, tools::wallet2::unconfirmed_transfer_details>>::const_iterator i = upayments.begin(); i != upayments.end(); ++i) {
@@ -2298,7 +2303,10 @@ bool simple_wallet::show_transfers(const std::vector<std::string> &args_)
       std::string payment_id = string_tools::pod_to_hex(i->second.m_payment_id);
       if (payment_id.substr(16).find_first_not_of('0') == std::string::npos)
         payment_id = payment_id.substr(0,16);
-      message_writer() << (boost::format("%8.8s %6.6s %20.20s %s %s %14.14s") % tr("pending") % tr("out") % print_money(amount - pd.m_change) % string_tools::pod_to_hex(i->first) % payment_id % print_money(fee)).str();
+      bool is_failed = pd.m_state == tools::wallet2::unconfirmed_transfer_details::failed;
+      if ((failed && is_failed) || (!is_failed && pending)) {
+        message_writer() << (boost::format("%8.8s %6.6s %20.20s %s %s %14.14s") % (is_failed ? tr("failed") : tr("pending")) % tr("out") % print_money(amount - pd.m_change) % string_tools::pod_to_hex(i->first) % payment_id % print_money(fee)).str();
+      }
     }
   }
 

src/wallet/wallet2.cpp

@@ -674,6 +674,58 @@ void wallet2::pull_next_blocks(uint64_t start_height, uint64_t &blocks_start_hei
   }
 }
 //----------------------------------------------------------------------------------------------------
+void wallet2::check_pending_txes()
+{
+  // if we don't have any pending txes, we don't need to check anything
+  if (m_unconfirmed_txs.empty())
+    return;
+
+  // we have at least one pending tx, so get the pool state
+  cryptonote::COMMAND_RPC_GET_TRANSACTION_POOL::request req;
+  cryptonote::COMMAND_RPC_GET_TRANSACTION_POOL::response res;
+  m_daemon_rpc_mutex.lock();
+  bool r = epee::net_utils::invoke_http_json_remote_command2(m_daemon_address + "/get_transaction_pool", req, res, m_http_client, 200000);
+  m_daemon_rpc_mutex.unlock();
+  THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "check_pending_txes");
+  THROW_WALLET_EXCEPTION_IF(res.status == CORE_RPC_STATUS_BUSY, error::daemon_busy, "is_key_image_spent");
+  THROW_WALLET_EXCEPTION_IF(res.status != CORE_RPC_STATUS_OK, error::get_tx_pool_error);
+
+  // remove any pending tx that's not in the pool
+  std::unordered_map<crypto::hash, wallet2::unconfirmed_transfer_details>::iterator it = m_unconfirmed_txs.begin();
+  while (it != m_unconfirmed_txs.end())
+  {
+    const std::string txid = epee::string_tools::pod_to_hex(cryptonote::get_transaction_hash(it->second.m_tx));
+    bool found = false;
+    for (auto it2: res.transactions)
+    {
+      if (it2.id_hash == txid)
+      {
+        found = true;
+        break;
+      }
+    }
+    auto pit = it++;
+    if (!found)
+    {
+      // we want to avoid a false positive when we ask for the pool just after
+      // a tx is removed from the pool due to being found in a new block, but
+      // just before the block is visible by refresh. So we keep a boolean, so
+      // that the first time we don't see the tx, we set that boolean, and only
+      // delete it the second time it is checked
+      if (pit->second.m_state == wallet2::unconfirmed_transfer_details::pending)
+      {
+        LOG_PRINT_L1("Pending txid " << txid << " not in pool, marking as not in pool");
+        pit->second.m_state = wallet2::unconfirmed_transfer_details::pending_not_in_pool;
+      }
+      else if (pit->second.m_state == wallet2::unconfirmed_transfer_details::pending_not_in_pool)
+      {
+        LOG_PRINT_L1("Pending txid " << txid << " not in pool, marking as failed");
+        pit->second.m_state = wallet2::unconfirmed_transfer_details::failed;
+      }
+    }
+  }
+}
+//----------------------------------------------------------------------------------------------------
 void wallet2::refresh(uint64_t start_height, uint64_t & blocks_fetched, bool& received_money)
 {
   received_money = false;
@@ -735,6 +787,15 @@ void wallet2::refresh(uint64_t start_height, uint64_t & blocks_fetched, bool& re
   if(last_tx_hash_id != (m_transfers.size() ? get_transaction_hash(m_transfers.back().m_tx) : null_hash))
     received_money = true;
 
+  try
+  {
+    check_pending_txes();
+  }
+  catch (...)
+  {
+    LOG_PRINT_L1("Failed to check pending transactions");
+  }
+
   LOG_PRINT_L1("Refresh done, blocks received: " << blocks_fetched << ", balance: " << print_money(balance()) << ", unlocked: " << print_money(unlocked_balance()));
 }
 //----------------------------------------------------------------------------------------------------
@@ -1342,6 +1403,7 @@ uint64_t wallet2::balance() const
 
 
   BOOST_FOREACH(auto& utx, m_unconfirmed_txs)
+    if (utx.second.m_state != wallet2::unconfirmed_transfer_details::failed)
       amount+= utx.second.m_change;
 
   return amount;
@@ -1565,6 +1627,7 @@ void wallet2::add_unconfirmed_tx(const cryptonote::transaction& tx, const std::v
   utd.m_tx = tx;
   utd.m_dests = dests;
   utd.m_payment_id = payment_id;
+  utd.m_state = wallet2::unconfirmed_transfer_details::pending;
 }
 
 //----------------------------------------------------------------------------------------------------

src/wallet/wallet2.h

@@ -119,6 +119,7 @@ namespace tools
       time_t m_sent_time;
       std::vector<cryptonote::tx_destination_entry> m_dests;
       crypto::hash m_payment_id;
+      enum { pending, pending_not_in_pool, failed } m_state;
     };
 
     struct confirmed_transfer_details
@@ -375,6 +376,7 @@ namespace tools
     void parse_block_round(const cryptonote::blobdata &blob, cryptonote::block &bl, crypto::hash &bl_id, bool &error) const;
     bool use_fork_rules(uint8_t version);
     uint64_t get_upper_tranaction_size_limit();
+    void check_pending_txes();
 
     cryptonote::account_base m_account;
     std::string m_daemon_address;
@@ -412,7 +414,7 @@ namespace tools
 }
 BOOST_CLASS_VERSION(tools::wallet2, 10)
 BOOST_CLASS_VERSION(tools::wallet2::payment_details, 0)
-BOOST_CLASS_VERSION(tools::wallet2::unconfirmed_transfer_details, 1)
+BOOST_CLASS_VERSION(tools::wallet2::unconfirmed_transfer_details, 2)
 BOOST_CLASS_VERSION(tools::wallet2::confirmed_transfer_details, 1)
 
 namespace boost
@@ -440,6 +442,9 @@ namespace boost
         return;
       a & x.m_dests;
       a & x.m_payment_id;
+      if (ver < 2)
+        return;
+      a & x.m_state;
     }
 
     template <class Archive>

src/wallet/wallet_errors.h

@@ -61,6 +61,7 @@ namespace tools
     //         get_blocks_error
     //         get_out_indexes_error
     //         tx_parse_error
+    //         get_tx_pool_error
     //       transfer_error *
     //         get_random_outs_general_error
     //         not_enough_money
@@ -307,6 +308,16 @@ namespace tools
       cryptonote::blobdata m_tx_blob;
     };
     //----------------------------------------------------------------------------------------------------
+    struct get_tx_pool_error : public refresh_error
+    {
+      explicit get_tx_pool_error(std::string&& loc)
+        : refresh_error(std::move(loc), "error getting tranaction pool")
+      {
+      }
+
+      std::string to_string() const { return refresh_error::to_string(); }
+    };
+    //----------------------------------------------------------------------------------------------------
     struct transfer_error : public wallet_logic_error
     {
     protected: