Merge pull request #3205

bc61ae69 tx_pool: add a max pool size, settable with --max-txpool-size (moneromooo-monero)
3b4e6b35 txpool: increase unmined tx expiry to three days (moneromooo-monero)

src/cryptonote_config.h

@@ -92,7 +92,7 @@
 #define BLOCKS_SYNCHRONIZING_DEFAULT_COUNT_PRE_V4       100    //by default, blocks count in blocks downloading
 #define BLOCKS_SYNCHRONIZING_DEFAULT_COUNT              20     //by default, blocks count in blocks downloading
 
-#define CRYPTONOTE_MEMPOOL_TX_LIVETIME                    86400 //seconds, one day
+#define CRYPTONOTE_MEMPOOL_TX_LIVETIME                    (86400*3) //seconds, three days
 #define CRYPTONOTE_MEMPOOL_TX_FROM_ALT_BLOCK_LIVETIME     604800 //seconds, one week
 
 #define COMMAND_RPC_GET_BLOCKS_FAST_MAX_COUNT           1000
@@ -138,6 +138,7 @@
 
 #define HASH_OF_HASHES_STEP                     256
 
+#define DEFAULT_TXPOOL_MAX_SIZE                 648000000ull // 3 days at 300000, in bytes
 
 // New constants are intended to go here
 namespace config

src/cryptonote_core/cryptonote_core.cpp

@@ -137,6 +137,11 @@ namespace cryptonote
   , "Relay blocks as fluffy blocks where possible (automatic on testnet)"
   , false
   };
+  static const command_line::arg_descriptor<size_t> arg_max_txpool_size  = {
+    "max-txpool-size"
+  , "Set maximum txpool size in bytes."
+  , DEFAULT_TXPOOL_MAX_SIZE
+  };
 
   //-----------------------------------------------------------------------------------------------
   core::core(i_cryptonote_protocol* pprotocol):
@@ -241,6 +246,7 @@ namespace cryptonote
     command_line::add_arg(desc, arg_test_dbg_lock_sleep);
     command_line::add_arg(desc, arg_offline);
     command_line::add_arg(desc, arg_disable_dns_checkpoints);
+    command_line::add_arg(desc, arg_max_txpool_size);
 
     miner::init_options(desc);
     BlockchainDB::init_options(desc);
@@ -359,6 +365,7 @@ namespace cryptonote
     bool fast_sync = command_line::get_arg(vm, arg_fast_block_sync) != 0;
     uint64_t blocks_threads = command_line::get_arg(vm, arg_prep_blocks_threads);
     std::string check_updates_string = command_line::get_arg(vm, arg_check_updates);
+    size_t max_txpool_size = command_line::get_arg(vm, arg_max_txpool_size);
 
     boost::filesystem::path folder(m_config_folder);
     if (m_fakechain)
@@ -477,7 +484,7 @@ namespace cryptonote
 
     r = m_blockchain_storage.init(db.release(), m_testnet, m_offline, test_options);
 
-    r = m_mempool.init();
+    r = m_mempool.init(max_txpool_size);
     CHECK_AND_ASSERT_MES(r, false, "Failed to initialize memory pool");
 
     // now that we have a valid m_blockchain_storage, we can clean out any

src/cryptonote_core/tx_pool.cpp

@@ -102,7 +102,7 @@ namespace cryptonote
   }
   //---------------------------------------------------------------------------------
   //---------------------------------------------------------------------------------
-  tx_memory_pool::tx_memory_pool(Blockchain& bchs): m_blockchain(bchs)
+  tx_memory_pool::tx_memory_pool(Blockchain& bchs): m_blockchain(bchs), m_txpool_max_size(DEFAULT_TXPOOL_MAX_SIZE), m_txpool_size(0)
   {
 
   }
@@ -295,8 +295,12 @@ namespace cryptonote
     }
 
     tvc.m_verifivation_failed = false;
+    m_txpool_size += blob_size;
 
     MINFO("Transaction added to pool: txid " << id << " bytes: " << blob_size << " fee/byte: " << (fee / (double)blob_size));
+
+    prune(m_txpool_max_size);
+
     return true;
   }
   //---------------------------------------------------------------------------------
@@ -309,6 +313,72 @@ namespace cryptonote
     return add_tx(tx, h, blob_size, tvc, keeped_by_block, relayed, do_not_relay, version);
   }
   //---------------------------------------------------------------------------------
+  size_t tx_memory_pool::get_txpool_size() const
+  {
+    CRITICAL_REGION_LOCAL(m_transactions_lock);
+    return m_txpool_size;
+  }
+  //---------------------------------------------------------------------------------
+  void tx_memory_pool::set_txpool_max_size(size_t bytes)
+  {
+    CRITICAL_REGION_LOCAL(m_transactions_lock);
+    m_txpool_max_size = bytes;
+  }
+  //---------------------------------------------------------------------------------
+  void tx_memory_pool::prune(size_t bytes)
+  {
+    CRITICAL_REGION_LOCAL(m_transactions_lock);
+    if (bytes == 0)
+      bytes = m_txpool_max_size;
+    CRITICAL_REGION_LOCAL1(m_blockchain);
+    LockedTXN lock(m_blockchain);
+
+    // this will never remove the first one, but we don't care
+    auto it = --m_txs_by_fee_and_receive_time.end();
+    while (it != m_txs_by_fee_and_receive_time.begin())
+    {
+      if (m_txpool_size <= bytes)
+        break;
+      try
+      {
+        const crypto::hash &txid = it->second;
+        txpool_tx_meta_t meta;
+        if (!m_blockchain.get_txpool_tx_meta(txid, meta))
+        {
+          MERROR("Failed to find tx in txpool");
+          return;
+        }
+        // don't prune the kept_by_block ones, they're likely added because we're adding a block with those
+        if (meta.kept_by_block)
+        {
+          --it;
+          continue;
+        }
+        cryptonote::blobdata txblob = m_blockchain.get_txpool_tx_blob(txid);
+        cryptonote::transaction tx;
+        if (!parse_and_validate_tx_from_blob(txblob, tx))
+        {
+          MERROR("Failed to parse tx from txpool");
+          return;
+        }
+        // remove first, in case this throws, so key images aren't removed
+        MINFO("Pruning tx " << txid << " from txpool: size: " << it->first.second << ", fee/byte: " << it->first.first);
+        m_blockchain.remove_txpool_tx(txid);
+        m_txpool_size -= txblob.size();
+        remove_transaction_keyimages(tx);
+        MINFO("Pruned tx " << txid << " from txpool: size: " << it->first.second << ", fee/byte: " << it->first.first);
+        m_txs_by_fee_and_receive_time.erase(it--);
+      }
+      catch (const std::exception &e)
+      {
+        MERROR("Error while pruning txpool: " << e.what());
+        return;
+      }
+    }
+    if (m_txpool_size > bytes)
+      MINFO("Pool size after pruning is larger than limit: " << m_txpool_size << "/" << bytes);
+  }
+  //---------------------------------------------------------------------------------
   bool tx_memory_pool::insert_key_images(const transaction &tx, bool kept_by_block)
   {
     for(const auto& in: tx.vin)
@@ -391,6 +461,7 @@ namespace cryptonote
 
       // remove first, in case this throws, so key images aren't removed
       m_blockchain.remove_txpool_tx(id);
+      m_txpool_size -= blob_size;
       remove_transaction_keyimages(tx);
     }
     catch (const std::exception &e)
@@ -463,6 +534,7 @@ namespace cryptonote
           {
             // remove first, so we only remove key images if the tx removal succeeds
             m_blockchain.remove_txpool_tx(txid);
+            m_txpool_size -= bd.size();
             remove_transaction_keyimages(tx);
           }
         }
@@ -1125,7 +1197,9 @@ namespace cryptonote
     size_t tx_size_limit = get_transaction_size_limit(version);
     std::unordered_set<crypto::hash> remove;
 
+    m_txpool_size = 0;
     m_blockchain.for_all_txpool_txes([this, &remove, tx_size_limit](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata*) {
+      m_txpool_size += meta.blob_size;
       if (meta.blob_size >= tx_size_limit) {
         LOG_PRINT_L1("Transaction " << txid << " is too big (" << meta.blob_size << " bytes), removing it from pool");
         remove.insert(txid);
@@ -1154,6 +1228,7 @@ namespace cryptonote
           }
           // remove tx from db first
           m_blockchain.remove_txpool_tx(txid);
+          m_txpool_size -= txblob.size();
           remove_transaction_keyimages(tx);
           auto sorted_it = find_tx_in_sorted_container(txid);
           if (sorted_it == m_txs_by_fee_and_receive_time.end())
@@ -1176,13 +1251,15 @@ namespace cryptonote
     return n_removed;
   }
   //---------------------------------------------------------------------------------
-  bool tx_memory_pool::init()
+  bool tx_memory_pool::init(size_t max_txpool_size)
   {
     CRITICAL_REGION_LOCAL(m_transactions_lock);
     CRITICAL_REGION_LOCAL1(m_blockchain);
 
+    m_txpool_max_size = max_txpool_size ? max_txpool_size : DEFAULT_TXPOOL_MAX_SIZE;
     m_txs_by_fee_and_receive_time.clear();
     m_spent_key_images.clear();
+    m_txpool_size = 0;
     std::vector<crypto::hash> remove;
     bool r = m_blockchain.for_all_txpool_txes([this, &remove](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata *bd) {
       cryptonote::transaction tx;
@@ -1197,6 +1274,7 @@ namespace cryptonote
         return false;
       }
       m_txs_by_fee_and_receive_time.emplace(std::pair<double, time_t>(meta.fee / (double)meta.blob_size, meta.receive_time), txid);
+      m_txpool_size += meta.blob_size;
       return true;
     }, true);
     if (!r)

src/cryptonote_core/tx_pool.h

@@ -198,11 +198,11 @@ namespace cryptonote
     /**
      * @brief loads pool state (if any) from disk, and initializes pool
      *
-     * @param config_folder folder name where pool state will be
+     * @param max_txpool_size the max size in bytes
      *
      * @return true
      */
-    bool init();
+    bool init(size_t max_txpool_size = 0);
 
     /**
      * @brief attempts to save the transaction pool state to disk
@@ -362,6 +362,19 @@ namespace cryptonote
      */
     size_t validate(uint8_t version);
 
+    /**
+     * @brief get the cumulative txpool size in bytes
+     *
+     * @return the cumulative txpool size in bytes
+     */
+    size_t get_txpool_size() const;
+
+    /**
+     * @brief set the max cumulative txpool size in bytes
+     *
+     * @param bytes the max cumulative txpool size in bytes
+     */
+    void set_txpool_max_size(size_t bytes);
 
 #define CURRENT_MEMPOOL_ARCHIVE_VER    11
 #define CURRENT_MEMPOOL_TX_DETAILS_ARCHIVE_VER    12
@@ -496,6 +509,13 @@ namespace cryptonote
      */
     void mark_double_spend(const transaction &tx);
 
+    /**
+     * @brief prune lowest fee/byte txes till we're not above bytes
+     *
+     * if bytes is 0, use m_txpool_max_size
+     */
+    void prune(size_t bytes = 0);
+
     //TODO: confirm the below comments and investigate whether or not this
     //      is the desired behavior
     //! map key images to transactions which spent them
@@ -542,6 +562,9 @@ private:
     std::unordered_set<crypto::hash> m_timed_out_transactions;
 
     Blockchain& m_blockchain;  //!< reference to the Blockchain object
+
+    size_t m_txpool_max_size;
+    size_t m_txpool_size;
   };
 }