Relay transactions when they linger too long in the pool

The last relayed time of a transaction is maintained, and
transactions will be relayed again if they are still in the
pool after a certain amount of time, which increases with
the transaction's age. All such transactions are resent,
whether or not they originated on the local node.

6 files changed, 123 insertions, 26 deletions

diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp
index e6a233868..e2249d21b 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -334,11 +334,12 @@ bool Blockchain::init(BlockchainDB* db, const bool testnet)
 
                 size_t blob_size;
                 uint64_t fee;
+                bool relayed;
                 transaction pool_tx;
                 for(const transaction &tx : txs)
                 {
                     crypto::hash tx_hash = get_transaction_hash(tx);
-                    m_tx_pool.take_tx(tx_hash, pool_tx, blob_size, fee);
+                    m_tx_pool.take_tx(tx_hash, pool_tx, blob_size, fee, relayed);
                 }
             }
         }
@@ -452,7 +453,12 @@ block Blockchain::pop_block_from_blockchain()
         if (!is_coinbase(tx))
         {
             cryptonote::tx_verification_context tvc = AUTO_VAL_INIT(tvc);
-            bool r = m_tx_pool.add_tx(tx, tvc, true);
+            // We assume that if they were in a block, the transactions are already
+            // known to the network as a whole. However, if we had mined that block,
+            // that might not be always true. Unlikely though, and always relaying
+            // these again might cause a spike of traffic as many nodes re-relay
+            // all the transactions in a popped block when a reorg happens.
+            bool r = m_tx_pool.add_tx(tx, tvc, true, true);
             if (!r)
             {
                 LOG_ERROR("Error returning transaction to tx_pool");
@@ -2464,6 +2470,7 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash&
         transaction tx;
         size_t blob_size = 0;
         uint64_t fee = 0;
+        bool relayed = false;
         TIME_MEASURE_START(aa);
 
 // XXX old code does not check whether tx exists
@@ -2479,7 +2486,7 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash&
         TIME_MEASURE_START(bb);
 
         // get transaction with hash <tx_id> from tx_pool
-        if(!m_tx_pool.take_tx(tx_id, tx, blob_size, fee))
+        if(!m_tx_pool.take_tx(tx_id, tx, blob_size, fee, relayed))
         {
             LOG_PRINT_L1("Block with id: " << id  << " has at least one unknown transaction with id: " << tx_id);
             bvc.m_verifivation_failed = true;
@@ -2597,7 +2604,12 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash&
         for (auto& tx : txs)
         {
             cryptonote::tx_verification_context tvc = AUTO_VAL_INIT(tvc);
-            if (!m_tx_pool.add_tx(tx, tvc, true))
+            // We assume that if they were in a block, the transactions are already
+            // known to the network as a whole. However, if we had mined that block,
+            // that might not be always true. Unlikely though, and always relaying
+            // these again might cause a spike of traffic as many nodes re-relay
+            // all the transactions in a popped block when a reorg happens.
+            if (!m_tx_pool.add_tx(tx, tvc, true, true))
             {
                 LOG_PRINT_L0("Failed to return taken transaction with hash: " << get_transaction_hash(tx) << " to tx_pool");
             }
diff --git a/src/cryptonote_core/blockchain_storage.cpp b/src/cryptonote_core/blockchain_storage.cpp
index f7c9ffb78..72bd05f6a 100644
--- a/src/cryptonote_core/blockchain_storage.cpp
+++ b/src/cryptonote_core/blockchain_storage.cpp
@@ -301,7 +301,7 @@ bool blockchain_storage::purge_transaction_from_blockchain(const crypto::hash& t
   if(!is_coinbase(tx))
   {
     cryptonote::tx_verification_context tvc = AUTO_VAL_INIT(tvc);
-    bool r = m_tx_pool->add_tx(tx, tvc, true);
+    bool r = m_tx_pool->add_tx(tx, tvc, true, true);
     CHECK_AND_ASSERT_MES(r, false, "purge_block_data_from_blockchain: failed to add transaction to transaction pool");
   }
 
@@ -1686,7 +1686,8 @@ bool blockchain_storage::handle_block_to_main_chain(const block& bl, const crypt
     transaction tx;
     size_t blob_size = 0;
     uint64_t fee = 0;
-    if(!m_tx_pool->take_tx(tx_id, tx, blob_size, fee))
+    bool relayed = false;
+    if(!m_tx_pool->take_tx(tx_id, tx, blob_size, fee, relayed))
     {
       LOG_PRINT_L1("Block with id: " << id  << "has at least one unknown transaction with id: " << tx_id);
       purge_block_data_from_blockchain(bl, tx_processed_count);
@@ -1698,7 +1699,7 @@ bool blockchain_storage::handle_block_to_main_chain(const block& bl, const crypt
     {
       LOG_PRINT_L1("Block with id: " << id  << "has at least one transaction (id: " << tx_id << ") with wrong inputs.");
       cryptonote::tx_verification_context tvc = AUTO_VAL_INIT(tvc);
-      bool add_res = m_tx_pool->add_tx(tx, tvc, true);
+      bool add_res = m_tx_pool->add_tx(tx, tvc, true, relayed);
       CHECK_AND_ASSERT_MES2(add_res, "handle_block_to_main_chain: failed to add transaction back to transaction pool");
       purge_block_data_from_blockchain(bl, tx_processed_count);
       add_block_as_invalid(bl, id);
@@ -1711,7 +1712,7 @@ bool blockchain_storage::handle_block_to_main_chain(const block& bl, const crypt
     {
        LOG_PRINT_L1("Block with id: " << id << " failed to add transaction to blockchain storage");
        cryptonote::tx_verification_context tvc = AUTO_VAL_INIT(tvc);
-       bool add_res = m_tx_pool->add_tx(tx, tvc, true);
+       bool add_res = m_tx_pool->add_tx(tx, tvc, true, relayed);
        CHECK_AND_ASSERT_MES2(add_res, "handle_block_to_main_chain: failed to add transaction back to transaction pool");
        purge_block_data_from_blockchain(bl, tx_processed_count);
        bvc.m_verifivation_failed = true;
diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp
index 4ade50cde..9881ae34f 100644
--- a/src/cryptonote_core/cryptonote_core.cpp
+++ b/src/cryptonote_core/cryptonote_core.cpp
@@ -401,7 +401,7 @@ namespace cryptonote
 	  return false;
   }
   //-----------------------------------------------------------------------------------------------
-  bool core::handle_incoming_tx(const blobdata& tx_blob, tx_verification_context& tvc, bool keeped_by_block)
+  bool core::handle_incoming_tx(const blobdata& tx_blob, tx_verification_context& tvc, bool keeped_by_block, bool relayed)
   {
     tvc = boost::value_initialized<tx_verification_context>();
     //want to process all transactions sequentially
@@ -440,7 +440,7 @@ namespace cryptonote
       return false;
     }
 
-    bool r = add_new_tx(tx, tx_hash, tx_prefixt_hash, tx_blob.size(), tvc, keeped_by_block);
+    bool r = add_new_tx(tx, tx_hash, tx_prefixt_hash, tx_blob.size(), tvc, keeped_by_block, relayed);
     if(tvc.m_verifivation_failed)
     {LOG_PRINT_RED_L1("Transaction verification failed: " << tx_hash);}
     else if(tvc.m_verifivation_impossible)
@@ -542,13 +542,13 @@ namespace cryptonote
     return true;
   }
   //-----------------------------------------------------------------------------------------------
-  bool core::add_new_tx(const transaction& tx, tx_verification_context& tvc, bool keeped_by_block)
+  bool core::add_new_tx(const transaction& tx, tx_verification_context& tvc, bool keeped_by_block, bool relayed)
   {
     crypto::hash tx_hash = get_transaction_hash(tx);
     crypto::hash tx_prefix_hash = get_transaction_prefix_hash(tx);
     blobdata bl;
     t_serializable_object_to_blob(tx, bl);
-    return add_new_tx(tx, tx_hash, tx_prefix_hash, bl.size(), tvc, keeped_by_block);
+    return add_new_tx(tx, tx_hash, tx_prefix_hash, bl.size(), tvc, keeped_by_block, relayed);
   }
   //-----------------------------------------------------------------------------------------------
   size_t core::get_blockchain_total_transactions() const
@@ -561,7 +561,7 @@ namespace cryptonote
   //  return m_blockchain_storage.get_outs(amount, pkeys);
   //}
   //-----------------------------------------------------------------------------------------------
-  bool core::add_new_tx(const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prefix_hash, size_t blob_size, tx_verification_context& tvc, bool keeped_by_block)
+  bool core::add_new_tx(const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prefix_hash, size_t blob_size, tx_verification_context& tvc, bool keeped_by_block, bool relayed)
   {
     if(m_mempool.have_tx(tx_hash))
     {
@@ -575,7 +575,28 @@ namespace cryptonote
       return true;
     }
 
-    return m_mempool.add_tx(tx, tx_hash, blob_size, tvc, keeped_by_block);
+    return m_mempool.add_tx(tx, tx_hash, blob_size, tvc, keeped_by_block, relayed);
+  }
+  //-----------------------------------------------------------------------------------------------
+  bool core::relay_txpool_transactions()
+  {
+    // we attempt to relay txes that should be relayed, but were not
+    std::list<std::pair<crypto::hash, cryptonote::transaction>> txs;
+    if (m_mempool.get_relayable_transactions(txs))
+    {
+      cryptonote_connection_context fake_context = AUTO_VAL_INIT(fake_context);
+      tx_verification_context tvc = AUTO_VAL_INIT(tvc);
+      NOTIFY_NEW_TRANSACTIONS::request r;
+      blobdata bl;
+      for (auto it = txs.begin(); it != txs.end(); ++it)
+      {
+        t_serializable_object_to_blob(it->second, bl);
+        r.txs.push_back(bl);
+      }
+      get_protocol()->relay_transactions(r, fake_context);
+      m_mempool.set_relayed(txs);
+    }
+    return true;
   }
   //-----------------------------------------------------------------------------------------------
   bool core::get_block_template(block& b, const account_public_address& adr, difficulty_type& diffic, uint64_t& height, const blobdata& ex_nonce)
@@ -831,6 +852,7 @@ namespace cryptonote
     m_store_blockchain_interval.do_call(boost::bind(&blockchain_storage::store_blockchain, &m_blockchain_storage));
 #endif
     m_fork_moaner.do_call(boost::bind(&core::check_fork_time, this));
+    m_txpool_auto_relayer.do_call(boost::bind(&core::relay_txpool_transactions, this));
     m_miner.on_idle();
     m_mempool.on_idle();
     return true;
diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h
index 634340a3f..e7a6c4154 100644
--- a/src/cryptonote_core/cryptonote_core.h
+++ b/src/cryptonote_core/cryptonote_core.h
@@ -64,7 +64,7 @@ namespace cryptonote
      core(i_cryptonote_protocol* pprotocol);
      bool handle_get_objects(NOTIFY_REQUEST_GET_OBJECTS::request& arg, NOTIFY_RESPONSE_GET_OBJECTS::request& rsp, cryptonote_connection_context& context);
      bool on_idle();
-     bool handle_incoming_tx(const blobdata& tx_blob, tx_verification_context& tvc, bool keeped_by_block);
+     bool handle_incoming_tx(const blobdata& tx_blob, tx_verification_context& tvc, bool keeped_by_block, bool relayed);
      bool handle_incoming_block(const blobdata& block_blob, block_verification_context& bvc, bool update_miner_blocktemplate = true);
      bool prepare_handle_incoming_blocks(const std::list<block_complete_entry>  &blocks);
      bool cleanup_handle_incoming_blocks(bool force_sync = false);
@@ -152,8 +152,8 @@ namespace cryptonote
      bool are_key_images_spent(const std::vector<crypto::key_image>& key_im, std::vector<bool> &spent) const;
 
    private:
-     bool add_new_tx(const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prefix_hash, size_t blob_size, tx_verification_context& tvc, bool keeped_by_block);
-     bool add_new_tx(const transaction& tx, tx_verification_context& tvc, bool keeped_by_block);
+     bool add_new_tx(const transaction& tx, const crypto::hash& tx_hash, const crypto::hash& tx_prefix_hash, size_t blob_size, tx_verification_context& tvc, bool keeped_by_block, bool relayed);
+     bool add_new_tx(const transaction& tx, tx_verification_context& tvc, bool keeped_by_block, bool relayed);
      bool add_new_block(const block& b, block_verification_context& bvc);
      bool load_state_data();
      bool parse_tx_from_blob(transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash, const blobdata& blob) const;
@@ -171,6 +171,8 @@ namespace cryptonote
      bool check_tx_inputs_keyimages_diff(const transaction& tx) const;
      void graceful_exit();
      bool check_fork_time();
+     bool relay_txpool_transactions();
+
      static std::atomic<bool> m_fast_exit;
      bool m_test_drop_download = true;
 	 uint64_t m_test_drop_download_height = 0;
@@ -190,6 +192,7 @@ namespace cryptonote
      cryptonote_protocol_stub m_protocol_stub;
      epee::math_helper::once_a_time_seconds<60*60*12, false> m_store_blockchain_interval;
      epee::math_helper::once_a_time_seconds<60*60*2, false> m_fork_moaner;
+     epee::math_helper::once_a_time_seconds<60*2, false> m_txpool_auto_relayer; //!< interval for checking re-relaying txpool transactions
      friend class tx_validate_inputs;
      std::atomic<bool> m_starter_message_showed;
 
diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp
index dce64db99..defbeb3b3 100644
--- a/src/cryptonote_core/tx_pool.cpp
+++ b/src/cryptonote_core/tx_pool.cpp
@@ -55,6 +55,17 @@ namespace cryptonote
   namespace
   {
     size_t const TRANSACTION_SIZE_LIMIT = (((CRYPTONOTE_BLOCK_GRANTED_FULL_REWARD_ZONE * 125) / 100) - CRYPTONOTE_COINBASE_BLOB_RESERVED_SIZE);
+    time_t const MIN_RELAY_TIME = (60 * 5); // only start re-relaying transactions after that many seconds
+    time_t const MAX_RELAY_TIME = (60 * 60 * 4); // at most that many seconds between resends
+
+    // a kind of increasing backoff within min/max bounds
+    time_t get_relay_delay(time_t now, time_t received)
+    {
+      time_t d = (now - received + MIN_RELAY_TIME) / MIN_RELAY_TIME * MIN_RELAY_TIME;
+      if (d > MAX_RELAY_TIME)
+        d = MAX_RELAY_TIME;
+      return d;
+    }
   }
   //---------------------------------------------------------------------------------
 #if BLOCKCHAIN_DB == DB_LMDB
@@ -70,7 +81,7 @@ namespace cryptonote
   }
 #endif
   //---------------------------------------------------------------------------------
-  bool tx_memory_pool::add_tx(const transaction &tx, /*const crypto::hash& tx_prefix_hash,*/ const crypto::hash &id, size_t blob_size, tx_verification_context& tvc, bool kept_by_block)
+  bool tx_memory_pool::add_tx(const transaction &tx, /*const crypto::hash& tx_prefix_hash,*/ const crypto::hash &id, size_t blob_size, tx_verification_context& tvc, bool kept_by_block, bool relayed)
   {
 
 
@@ -154,6 +165,8 @@ namespace cryptonote
         txd_p.first->second.max_used_block_height = 0;
         txd_p.first->second.kept_by_block = kept_by_block;
         txd_p.first->second.receive_time = time(nullptr);
+        txd_p.first->second.last_relayed_time = time(NULL);
+        txd_p.first->second.relayed = relayed;
         tvc.m_verifivation_impossible = true;
         tvc.m_added_to_pool = true;
       }else
@@ -176,6 +189,8 @@ namespace cryptonote
       txd_p.first->second.last_failed_height = 0;
       txd_p.first->second.last_failed_id = null_hash;
       txd_p.first->second.receive_time = time(nullptr);
+      txd_p.first->second.last_relayed_time = time(NULL);
+      txd_p.first->second.relayed = relayed;
       tvc.m_added_to_pool = true;
 
       if(txd_p.first->second.fee > 0)
@@ -202,12 +217,12 @@ namespace cryptonote
     return true;
   }
   //---------------------------------------------------------------------------------
-  bool tx_memory_pool::add_tx(const transaction &tx, tx_verification_context& tvc, bool keeped_by_block)
+  bool tx_memory_pool::add_tx(const transaction &tx, tx_verification_context& tvc, bool keeped_by_block, bool relayed)
   {
     crypto::hash h = null_hash;
     size_t blob_size = 0;
     get_transaction_hash(tx, h, blob_size);
-    return add_tx(tx, h, blob_size, tvc, keeped_by_block);
+    return add_tx(tx, h, blob_size, tvc, keeped_by_block, relayed);
   }
   //---------------------------------------------------------------------------------
   bool tx_memory_pool::remove_transaction_keyimages(const transaction& tx)
@@ -240,7 +255,7 @@ namespace cryptonote
     return true;
   }
   //---------------------------------------------------------------------------------
-  bool tx_memory_pool::take_tx(const crypto::hash &id, transaction &tx, size_t& blob_size, uint64_t& fee)
+  bool tx_memory_pool::take_tx(const crypto::hash &id, transaction &tx, size_t& blob_size, uint64_t& fee, bool &relayed)
   {
     CRITICAL_REGION_LOCAL(m_transactions_lock);
     auto it = m_transactions.find(id);
@@ -255,6 +270,7 @@ namespace cryptonote
     tx = it->second.tx;
     blob_size = it->second.blob_size;
     fee = it->second.fee;
+    relayed = it->second.relayed;
     remove_transaction_keyimages(it->second.tx);
     m_transactions.erase(it);
     m_txs_by_fee.erase(sorted_it);
@@ -304,6 +320,41 @@ namespace cryptonote
     return true;
   }
   //---------------------------------------------------------------------------------
+  bool tx_memory_pool::get_relayable_transactions(std::list<std::pair<crypto::hash, cryptonote::transaction>> &txs) const
+  {
+    CRITICAL_REGION_LOCAL(m_transactions_lock);
+    const time_t now = time(NULL);
+    for(auto it = m_transactions.begin(); it!= m_transactions.end();)
+    {
+      // 0 fee transactions are never relayed
+      if(it->second.fee > 0 && now - it->second.last_relayed_time > get_relay_delay(now, it->second.receive_time))
+      {
+        // if the tx is older than half the max lifetime, we don't re-relay it, to avoid a problem
+        // mentioned by smooth where nodes would flush txes at slightly different times, causing
+        // flushed txes to be re-added when received from a node which was just about to flush it
+        time_t max_age = it->second.kept_by_block ? CRYPTONOTE_MEMPOOL_TX_FROM_ALT_BLOCK_LIVETIME : CRYPTONOTE_MEMPOOL_TX_LIVETIME;
+        if (now - it->second.receive_time <= max_age / 2)
+        {
+          txs.push_back(std::make_pair(it->first, it->second.tx));
+        }
+      }
+      ++it;
+    }
+    return true;
+  }
+  //---------------------------------------------------------------------------------
+  void tx_memory_pool::set_relayed(const std::list<std::pair<crypto::hash, cryptonote::transaction>> &txs)
+  {
+    CRITICAL_REGION_LOCAL(m_transactions_lock);
+    const time_t now = time(NULL);
+    for (auto it = txs.begin(); it != txs.end(); ++it)
+    {
+      auto i = m_transactions.find(it->first);
+      if (i != m_transactions.end())
+        i->second.last_relayed_time = now;
+    }
+  }
+  //---------------------------------------------------------------------------------
   size_t tx_memory_pool::get_transactions_count() const
   {
     CRITICAL_REGION_LOCAL(m_transactions_lock);
diff --git a/src/cryptonote_core/tx_pool.h b/src/cryptonote_core/tx_pool.h
index 0110bb8cd..a72c331a7 100644
--- a/src/cryptonote_core/tx_pool.h
+++ b/src/cryptonote_core/tx_pool.h
@@ -81,10 +81,10 @@ namespace cryptonote
 #else
     tx_memory_pool(blockchain_storage& bchs);
 #endif
-    bool add_tx(const transaction &tx, const crypto::hash &id, size_t blob_size, tx_verification_context& tvc, bool keeped_by_block);
-    bool add_tx(const transaction &tx, tx_verification_context& tvc, bool keeped_by_block);
+    bool add_tx(const transaction &tx, const crypto::hash &id, size_t blob_size, tx_verification_context& tvc, bool keeped_by_block, bool relayed);
+    bool add_tx(const transaction &tx, tx_verification_context& tvc, bool keeped_by_block, bool relayed);
     //gets tx and remove it from pool
-    bool take_tx(const crypto::hash &id, transaction &tx, size_t& blob_size, uint64_t& fee);
+    bool take_tx(const crypto::hash &id, transaction &tx, size_t& blob_size, uint64_t& fee, bool &relayed);
 
     bool have_tx(const crypto::hash &id) const;
     bool on_blockchain_inc(uint64_t new_block_height, const crypto::hash& top_block_id);
@@ -101,13 +101,15 @@ namespace cryptonote
     void get_transactions(std::list<transaction>& txs) const;
     bool get_transactions_and_spent_keys_info(std::vector<tx_info>& tx_infos, std::vector<spent_key_image_info>& key_image_infos) const;
     bool get_transaction(const crypto::hash& h, transaction& tx) const;
+    bool get_relayable_transactions(std::list<std::pair<crypto::hash, cryptonote::transaction>>& txs) const;
+    void set_relayed(const std::list<std::pair<crypto::hash, cryptonote::transaction>>& txs);
     size_t get_transactions_count() const;
     std::string print_pool(bool short_format) const;
 
     /*bool flush_pool(const std::strig& folder);
     bool inflate_pool(const std::strig& folder);*/
 
-#define CURRENT_MEMPOOL_ARCHIVE_VER    8
+#define CURRENT_MEMPOOL_ARCHIVE_VER    9
 
     template<class archive_t>
     void serialize(archive_t & a, const unsigned int version)
@@ -131,6 +133,9 @@ namespace cryptonote
       uint64_t last_failed_height;
       crypto::hash last_failed_id;
       time_t receive_time;
+
+      time_t last_relayed_time;
+      bool relayed;
     };
 
   private:
@@ -230,7 +235,10 @@ namespace boost
       ar & td.last_failed_height;
       ar & td.last_failed_id;
       ar & td.receive_time;
-
+      if (version < 9)
+        return;
+      ar & td.last_relayed_time;
+      ar & td.relayed;
     }
   }
 }