feat!: improves Credit Pool to protect from cumulative increasing amount of gaps

src/Makefile.am

@@ -326,7 +326,7 @@ BITCOIN_CORE_H = \
   util/underlying.h \
   util/serfloat.h \
   util/settings.h \
-  util/skip_set.h \
+  util/ranges_set.h \
   util/sock.h \
   util/string.h \
   util/time.h \
@@ -744,7 +744,7 @@ libbitcoin_util_a_SOURCES = \
   util/moneystr.cpp \
   util/readwritefile.cpp \
   util/settings.cpp \
-  util/skip_set.cpp \
+  util/ranges_set.cpp \
   util/spanparsing.cpp \
   util/strencodings.cpp \
   util/time.cpp \

src/evo/creditpool.cpp

@@ -70,8 +70,8 @@ static UnlockDataPerBlock GetDataFromUnlockTxes(const std::vector<CTransactionRe
 
 std::string CCreditPool::ToString() const
 {
-    return strprintf("CCreditPool(locked=%lld, currentLimit=%lld, nIndexes=%lld)",
-            locked, currentLimit, indexes.Size());
+    return strprintf("CCreditPool(locked=%lld, currentLimit=%lld)",
+            locked, currentLimit);
 }
 
 std::optional<CCreditPool> CCreditPoolManager::GetFromCache(const CBlockIndex* const block_index)
@@ -130,7 +130,7 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo
         // If reading of previous block is not successfully, but
         // prev contains credit pool related data, something strange happened
         assert(prev.locked == 0);
-        assert(prev.indexes.Size() == 0);
+        assert(prev.indexes.IsEmpty());
 
         CCreditPool emptyPool;
         AddToCache(block_index->GetBlockHash(), block_index->nHeight, emptyPool);
@@ -150,9 +150,9 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo
     // Indexes should not be duplicated since genesis block, but the Unlock Amount
     // of withdrawal transaction is limited only by this window
     UnlockDataPerBlock blockData = GetDataFromUnlockTxes(block->vtx);
-    CSkipSet indexes{std::move(prev.indexes)};
+    CRangesSet indexes{std::move(prev.indexes)};
     if (std::any_of(blockData.indexes.begin(), blockData.indexes.end(), [&](const uint64_t index) { return !indexes.Add(index); })) {
-        throw std::runtime_error(strprintf("%s: failed-getcreditpool-index-exceed", __func__));
+        throw std::runtime_error(strprintf("%s: failed-getcreditpool-index-duplicated", __func__));
     }
 
     const CBlockIndex* distant_block_index = block_index;
@@ -266,10 +266,6 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state)
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-duplicated-index");
     }
 
-    if (!pool.indexes.CanBeAdded(index)) {
-        return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-cant-add");
-    }
-
     newIndexes.insert(index);
     sessionUnlocked += toUnlock;
     return true;

src/evo/creditpool.h

@@ -15,7 +15,7 @@
 #include <sync.h>
 #include <threadsafety.h>
 #include <unordered_lru_cache.h>
-#include <util/skip_set.h>
+#include <util/ranges_set.h>
 
 #include <optional>
 #include <unordered_set>
@@ -34,7 +34,7 @@ struct CCreditPool {
     // needs for logic of limits of unlocks
     CAmount currentLimit{0};
     CAmount latelyUnlocked{0};
-    CSkipSet indexes{};
+    CRangesSet indexes{};
 
     std::string ToString() const;
 

src/test/util_tests.cpp

@@ -15,7 +15,7 @@
 #include <util/getuniquepath.h>
 #include <util/message.h> // For MessageSign(), MessageVerify(), MESSAGE_MAGIC
 #include <util/moneystr.h>
-#include <util/skip_set.h>
+#include <util/ranges_set.h>
 #include <util/spanparsing.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -2167,27 +2167,28 @@ BOOST_AUTO_TEST_CASE(test_Capitalize)
     BOOST_CHECK_EQUAL(Capitalize("\x00\xfe\xff"), "\x00\xfe\xff");
 }
 
-BOOST_AUTO_TEST_CASE(test_SkipSet)
+BOOST_AUTO_TEST_CASE(test_CRanges)
 {
     std::mt19937 gen;
     for (size_t test = 0; test < 17; ++test) {
         std::uniform_int_distribution<uint64_t> dist_value(0, (1 << test));
-        size_t skip_size = test ? (1 << (test - 1)) : 1;
-        CSkipSet set_1{skip_size};
+        CRangesSet ranges;
         std::unordered_set<uint64_t> set_2;
         for (size_t iter = 0; iter < (1 << test) * 2; ++iter) {
             uint64_t value = dist_value(gen);
-            BOOST_CHECK(set_1.Contains(value) == !!set_2.count(value));
-            if (!set_1.Contains(value) && set_1.CanBeAdded(value)) {
-                BOOST_CHECK(!set_1.Contains(value));
-                BOOST_CHECK(set_1.Add(value));
+            BOOST_CHECK_EQUAL(ranges.Contains(value), !!set_2.count(value));
+            if (!ranges.Contains(value)) {
+                BOOST_CHECK(ranges.Add(value));
                 set_2.insert(value);
+            } else {
+                BOOST_CHECK(ranges.Remove(value));
+                set_2.erase(set_2.find(value));
             }
-            BOOST_CHECK(set_1.Contains(value) == !!set_2.count(value));
-            BOOST_CHECK(set_1.Size() == set_2.size());
+            BOOST_CHECK_EQUAL(ranges.Contains(value), !!set_2.count(value));
+            BOOST_CHECK_EQUAL(ranges.Size(), set_2.size());
         }
         if (test > 4) {
-            BOOST_CHECK(set_1.Size() > ((1 << test) / 4));
+            BOOST_CHECK(ranges.Size() > ((1 << test) / 4));
         }
     }
 }

src/util/ranges_set.cpp

@@ -0,0 +1,98 @@
+// Copyright (c) 2023 The Dash Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <util/ranges_set.h>
+
+CRangesSet::Range::Range() : CRangesSet::Range::Range(0, 0) {}
+
+CRangesSet::Range::Range(uint64_t begin, uint64_t end) :
+    begin(begin),
+    end(end)
+{
+}
+
+bool CRangesSet::Add(uint64_t value)
+{
+    if (Contains(value)) return false;
+
+    // If element is not in CRangesSet, we need to add a new range [value, value + 1)
+    // But this operation can cause 2 operation of merge (total 3 cases):
+    // - if there're exist ranges [x, value) and [value + 1, y) than
+    //   all 3 of them should be merged in one range [x, y)
+    // - if there's exist a range [x, value) - we need to replace it to new range [x, value + 1)
+    // - if there's exist a range [value + 1, y) - we need to replace it to new range [value, y)
+    Range new_range{value, value + 1};
+    auto it = ranges.lower_bound({value, value});
+    if (it != ranges.begin()) {
+        auto prev = it;
+        --prev;
+
+        if (prev->end == value) {
+            new_range.begin = prev->begin;
+            it = ranges.erase(prev);
+        }
+    }
+    const auto next = it;
+    if (next != ranges.end()) {
+        if (next->begin == value + 1) {
+            new_range.end = next->end;
+            it = ranges.erase(next);
+        }
+    }
+    const auto ret = ranges.insert(new_range);
+    assert(ret.second);
+    return true;
+}
+
+bool CRangesSet::Remove(uint64_t value)
+{
+    if (!Contains(value)) return false;
+
+    // If element is in CRangesSet, there's possible 2 cases:
+    // - value in the range that is lower-bound (if begin of that range equals to `value`)
+    // - value in the previous range (otherwise)
+    auto it = ranges.lower_bound({value, value});
+
+    if (it == ranges.end() || it->begin != value) {
+        assert(it != ranges.begin());
+        --it;
+    }
+
+    const Range current_range = *it;
+    ranges.erase(it);
+    if (current_range.begin != value) {
+        const auto ret = ranges.insert({current_range.begin, value});
+        assert(ret.second);
+    }
+    if (value + 1 != current_range.end) {
+        const auto ret = ranges.insert({value + 1, current_range.end});
+        assert(ret.second);
+    }
+    return true;
+}
+
+bool CRangesSet::IsEmpty() const noexcept
+{
+    return ranges.empty();
+}
+
+size_t CRangesSet::Size() const noexcept
+{
+    size_t result{0};
+    for (auto i : ranges) {
+        result += i.end - i.begin;
+    }
+    return result;
+}
+
+bool CRangesSet::Contains(uint64_t value) const noexcept
+{
+    const auto it = ranges.lower_bound({value, value});
+    if (it != ranges.end() && it->begin == value) return true;
+    if (it == ranges.begin()) return false;
+    auto prev = it;
+    --prev;
+    return prev->begin <= value && prev->end > value;
+}
+

src/util/ranges_set.h

@@ -0,0 +1,84 @@
+// Copyright (c) 2023 The Dash Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_RANGES_SET_H
+#define BITCOIN_UTIL_RANGES_SET_H
+
+#include <hash.h>
+#include <saltedhasher.h>
+#include <serialize.h>
+
+#include <set>
+
+/**
+ * The CRangesSet is a datastructure that keeps efficiently numbers as set of
+ * continuous ranges of numbers.
+ * CRangesSet let's to keep elements with gaps of any size while CSkipList has
+ * limited capacity (total size of all gaps)
+ *
+ * The CRangesSet provides transaction guarantees: element can be added or
+ * removed and data structure will be consistent. For case if any of these
+ * operation failed (out-memory for example), the `assert` will be called to
+ * terminate program.
+ */
+class CRangesSet
+{
+    // internal datastructure, doesn't have a reason to be publicly available
+    struct Range
+    {
+        uint64_t begin;
+        uint64_t end;
+        Range();
+        Range(uint64_t begin, uint64_t end);
+        bool operator<(const Range& other) const
+        {
+            if (begin != other.begin) return begin < other.begin;
+            return end < other.end;
+        }
+
+        SERIALIZE_METHODS(Range, obj)
+        {
+            READWRITE(obj.begin);
+            READWRITE(obj.end);
+        }
+    };
+
+    std::set<Range> ranges;
+
+public:
+    /**
+     * this function adds `value` to the datastructure.
+     * it returns true if `add` succeed
+     */
+    [[nodiscard]] bool Add(uint64_t value);
+
+    /**
+     * this function returns true if `value` exists in the datastructure
+     */
+    [[nodiscard]] bool Contains(uint64_t value) const noexcept;
+
+    /**
+     * this function removes `value` from the datastructure.
+     * it returns `false` if element didn't existed or removing failed by any reason
+     */
+    [[nodiscard]] bool Remove(uint64_t value);
+
+    /**
+     * Size() works with complexity O(N) times, avoid calling it without a good reason
+     * Instead prefer to use IsEmpty()
+     */
+    [[nodiscard]] size_t Size() const noexcept;
+
+    /**
+     * IsEmpty() returns true if there's no any elements added
+     */
+    [[nodiscard]] bool IsEmpty() const noexcept;
+
+    SERIALIZE_METHODS(CRangesSet, obj)
+    {
+        READWRITE(obj.ranges);
+    }
+};
+
+#endif // BITCOIN_UTIL_RANGES_SET_H