Merge pull request #5687 from ethereum/tq-maxdropped

Enforce 1024 max dropped transactions in the transaction queue

Author: halfalicious

CHANGELOG.md

@@ -41,6 +41,7 @@
 - Fixed: [#5662](https://github.com/ethereum/aleth/pull/5662) Correct depth value when aleth-interpreter invokes `evmc_host_interface::call` callback.
 - Fixed: [#5666](https://github.com/ethereum/aleth/pull/5666) aleth-interpreter returns `EVMC_INVALID_INSTRUCTION` when `INVALID` opcode is encountered and `EVMC_UNKNOWN_INSTRUCTION` for undefined opcodes.
 - Fixed: [#5706](https://github.com/ethereum/aleth/pull/5706) Stop tracking sent transactions after they've been imported into the blockchain.
+- Fixed: [#5687](https://github.com/ethereum/aleth/pull/5687) Limit transaction queue's dropped transaction history to 1024 transactions.
 
 ## [1.6.0] - 2019-04-16
 

libdevcore/CMakeLists.txt

@@ -33,6 +33,7 @@ add_library(
     Log.h
     LoggingProgramOptions.cpp
     LoggingProgramOptions.h
+    LruCache.h
     MemoryDB.cpp
     MemoryDB.h
     OverlayDB.cpp

libdevcore/LruCache.h

@@ -0,0 +1,96 @@
+// Aleth: Ethereum C++ client, tools and libraries.
+// Copyright 2019 Aleth Authors.
+// Licensed under the GNU General Public License, Version 3.
+
+#pragma once
+
+#include <list>
+#include <unordered_map>
+
+namespace dev
+{
+template <class Key, class Value>
+class LruCache
+{
+    using key_type = Key;
+    using value_type = Value;
+    using list_type = std::list<std::pair<key_type, value_type>>;
+    using map_type = std::unordered_map<key_type, typename list_type::const_iterator>;
+
+public:
+    explicit LruCache(size_t _capacity) : m_capacity(_capacity) {}
+
+    size_t insert(key_type const& _key, value_type const& _val)
+    {
+        auto const cIter = m_index.find(_key);
+        if (cIter == m_index.cend())
+        {
+            if (m_index.size() == m_capacity)
+            {
+                m_index.erase(m_data.back().first);
+                m_data.pop_back();
+            }
+            m_data.push_front({_key, _val});
+            m_index[_key] = m_data.begin();
+        }
+        else
+            m_data.splice(m_data.begin(), m_data, cIter->second);
+
+        return m_index.size();
+    }
+
+    size_t remove(key_type const& _key)
+    {
+        auto const cIter = m_index.find(_key);
+        if (cIter != m_index.cend())
+        {
+            m_data.erase(cIter->second);
+            m_index.erase(cIter);
+        }
+
+        return m_index.size();
+    }
+
+    bool touch(key_type const& _key)
+    {
+        auto const cIter = m_index.find(_key);
+        if (cIter != m_index.cend())
+        {
+            m_data.splice(m_data.begin(), m_data, cIter->second);
+            return true;
+        }
+        return false;
+    }
+
+    bool contains(key_type const& _key) const { return m_index.find(_key) != m_index.cend(); }
+
+    bool contains(key_type const& _key, value_type const& _value) const
+    {
+        auto const cIter = m_index.find(_key);
+        return cIter != m_index.cend() && (*(cIter->second)).second == _value;
+    }
+
+    bool empty() const noexcept { return m_index.empty(); }
+
+    size_t size() const noexcept { return m_index.size(); }
+
+    size_t capacity() const noexcept { return m_capacity; }
+
+    void clear() noexcept
+    {
+        m_index.clear();
+        m_data.clear();
+    }
+
+    // Expose data iterator for testing purposes
+    typename list_type::const_iterator cbegin() const noexcept { return m_data.cbegin(); }
+    typename list_type::iterator begin() noexcept { return m_data.begin(); }
+    typename list_type::const_iterator cend() const noexcept { return m_data.cend(); }
+    typename list_type::iterator end() noexcept { return m_data.end(); }
+
+private:
+    list_type m_data;
+    map_type m_index;
+    size_t m_capacity;
+};
+}  // namespace dev

libethereum/TransactionQueue.cpp

@@ -1,23 +1,6 @@
-/*
-    This file is part of cpp-ethereum.
-
-    cpp-ethereum is free software: you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation, either version 3 of the License, or
-    (at your option) any later version.
-
-    cpp-ethereum is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-
-    You should have received a copy of the GNU General Public License
-    along with cpp-ethereum.  If not, see <http://www.gnu.org/licenses/>.
-*/
-/** @file TransactionQueue.cpp
- * @author Gav Wood <i@gavwood.com>
- * @date 2014
- */
+// Aleth: Ethereum C++ client, tools and libraries.
+// Copyright 2019 Aleth Authors.
+// Licensed under the GNU General Public License, Version 3.
 
 #include "TransactionQueue.h"
 
@@ -28,12 +11,17 @@ using namespace std;
 using namespace dev;
 using namespace dev::eth;
 
-const size_t c_maxVerificationQueueSize = 8192;
-
-TransactionQueue::TransactionQueue(unsigned _limit, unsigned _futureLimit):
-    m_current(PriorityCompare { *this }),
-    m_limit(_limit),
-    m_futureLimit(_futureLimit)
+namespace
+{
+constexpr size_t c_maxVerificationQueueSize = 8192;
+constexpr size_t c_maxDroppedTransactionCount = 1024;
+}  // namespace
+
+TransactionQueue::TransactionQueue(unsigned _limit, unsigned _futureLimit)
+  : m_dropped{c_maxDroppedTransactionCount},
+    m_current{PriorityCompare{*this}},
+    m_limit{_limit},
+    m_futureLimit{_futureLimit}
 {
     unsigned verifierThreads = std::max(thread::hardware_concurrency(), 3U) - 2U;
     for (unsigned i = 0; i < verifierThreads; ++i)
@@ -70,7 +58,7 @@ ImportResult TransactionQueue::check_WITH_LOCK(h256 const& _h, IfDropped _ik)
     if (m_known.count(_h))
         return ImportResult::AlreadyKnown;
 
-    if (m_dropped.count(_h) && _ik == IfDropped::Ignore)
+    if (m_dropped.touch(_h) && _ik == IfDropped::Ignore)
         return ImportResult::AlreadyInChain;
 
     return ImportResult::Success;
@@ -328,7 +316,7 @@ void TransactionQueue::drop(h256 const& _txHash)
         return;
 
     UpgradeGuard ul(l);
-    m_dropped.insert(_txHash);
+    m_dropped.insert(_txHash, true /* placeholder value */);
     remove_WITH_LOCK(_txHash);
 }
 

libethereum/TransactionQueue.h

@@ -21,15 +21,16 @@
 
 #pragma once
 
-#include <functional>
-#include <condition_variable>
-#include <thread>
-#include <deque>
+#include "Transaction.h"
 #include <libdevcore/Common.h>
 #include <libdevcore/Guards.h>
 #include <libdevcore/Log.h>
+#include <libdevcore/LruCache.h>
 #include <libethcore/Common.h>
-#include "Transaction.h"
+#include <condition_variable>
+#include <deque>
+#include <functional>
+#include <thread>
 
 namespace dev
 {
@@ -189,7 +190,11 @@ class TransactionQueue
     h256Hash m_known;            ///< Headers of transactions in both sets.
 
     std::unordered_map<h256, std::function<void(ImportResult)>> m_callbacks;	///< Called once.
-    h256Hash m_dropped;															///< Transactions that have previously been dropped
+
+    ///< Transactions that have previously been dropped. We technically only need to store the tx
+    ///< hash, but we also store bool as a placeholder value so that we can use an LRU cache to cap
+    ///< the number of transaction hashes stored.
+    LruCache<h256, bool> m_dropped;
 
     PriorityQueue m_current;
     std::unordered_map<h256, PriorityQueue::iterator> m_currentByHash;			///< Transaction hash to set ref

test/CMakeLists.txt

@@ -10,6 +10,7 @@ set(unittest_sources
     unittests/libdevcore/CommonJS.cpp
     unittests/libdevcore/core.cpp
     unittests/libdevcore/FixedHash.cpp
+    unittests/libdevcore/LruCache.cpp
     unittests/libdevcore/RangeMask.cpp
     unittests/libdevcore/RLP.cpp
 

test/unittests/libdevcore/LruCache.cpp

@@ -0,0 +1,108 @@
+// Aleth: Ethereum C++ client, tools and libraries.
+// Copyright 2019 Aleth Authors.
+// Licensed under the GNU General Public License, Version 3.
+
+#include <libdevcore/LruCache.h>
+#include <test/tools/libtestutils/Common.h>
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace dev;
+using namespace dev::test;
+
+namespace
+{
+using LRU = LruCache<int, int>;
+using PAIR = pair<int, int>;
+using VEC = vector<PAIR>;
+
+constexpr size_t c_capacity = 10;
+
+mt19937_64 g_randomGenerator(random_device{}());
+
+int randomNumber(int _min = INT_MIN, int _max = INT_MAX)
+{
+    return std::uniform_int_distribution<int>{_min, _max}(g_randomGenerator);
+}
+
+VEC Populate(LRU& _lruCache, size_t _count)
+{
+    VEC ret;
+    for (size_t i = 0; i < _count; i++)
+    {
+        auto const item = PAIR{randomNumber(), randomNumber()};
+        ret.push_back(item);
+        _lruCache.insert(item.first, item.second);
+    }
+    reverse(ret.begin(), ret.end());
+
+    return ret;
+}
+
+void VerifyEquals(LRU& _lruCache, VEC& _data)
+{
+    EXPECT_EQ(_lruCache.size(), _data.size());
+    size_t i = 0;
+    auto iter = _lruCache.begin();
+    while (iter != _lruCache.cend() && i < _data.size())
+    {
+        EXPECT_EQ(*iter, _data[i]);
+        iter++;
+        i++;
+    }
+}
+}  // namespace
+
+TEST(LruCache, BasicOperations)
+{
+    LRU lruCache{c_capacity};
+    EXPECT_EQ(lruCache.capacity(), c_capacity);
+    EXPECT_TRUE(lruCache.empty());
+
+    // Populate and verify
+    VEC testData = Populate(lruCache, lruCache.capacity());
+    VerifyEquals(lruCache, testData);
+
+    // Reverse order and verify
+    for (size_t i = 0; i < testData.size(); i++)
+        lruCache.touch(testData[i].first);
+    reverse(testData.begin(), testData.end());
+    VerifyEquals(lruCache, testData);
+
+    // Remove elements and verify
+    auto size = lruCache.size();
+    for (PAIR item : testData)
+    {
+        lruCache.remove(item.first);
+        EXPECT_FALSE(lruCache.contains(item.first));
+        EXPECT_EQ(lruCache.size(), --size);
+    }
+}
+
+TEST(LruCache, AdvancedOperations)
+{
+    LRU lruCache{c_capacity};
+    VEC testData = Populate(lruCache, lruCache.capacity());
+    VerifyEquals(lruCache, testData);
+    testData = Populate(lruCache, lruCache.capacity());
+    VerifyEquals(lruCache, testData);
+    lruCache.clear();
+    EXPECT_TRUE(lruCache.empty());
+    EXPECT_EQ(lruCache.capacity(), c_capacity);
+}
+
+TEST(LruCache, Constructors)
+{
+    LRU lruCache{c_capacity};
+    VEC testData = Populate(lruCache, lruCache.capacity());
+    VerifyEquals(lruCache, testData);
+
+    LRU lruCacheCopy{lruCache};
+    VerifyEquals(lruCacheCopy, testData);
+    EXPECT_EQ(lruCache.capacity(), lruCacheCopy.capacity());
+
+    LRU lruCacheMove{move(lruCache)};
+    VerifyEquals(lruCacheMove, testData);
+    EXPECT_TRUE(lruCache.empty());
+    EXPECT_EQ(lruCache.capacity(), c_capacity);
+}