feat!: replaced CSkipList to CRangesSet in credit pool

By design we can have more and more and more gaps in indexes list so far as
we can not re-sign expired transaction of asset-unlock. CRangesList is protected from this situation

Author: knst

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;
 

test/functional/feature_asset_locks.py

@@ -284,14 +284,6 @@ def run_test(self):
         self.validate_credit_pool_balance(locked_1)
 
         self.log.info("Testing asset unlock...")
-        asset_unlock_tx_index_too_far = self.create_assetunlock(10001, COIN, pubkey)
-        tx_too_far_index = self.send_tx(asset_unlock_tx_index_too_far)
-        node.generate(1)
-        self.sync_all()
-        self.mempool_size += 1
-        self.check_mempool_size()
-        self.log.info("Checking that `asset_unlock_tx_index_too_far` not mined yet...")
-        self.ensure_tx_is_not_mined(tx_too_far_index)
 
         self.log.info("Generating several txes by same quorum....")
         self.validate_credit_pool_balance(locked_1)
@@ -323,17 +315,11 @@ def run_test(self):
         self.mempool_size += 1
         self.check_mempool_size()
         self.validate_credit_pool_balance(locked_1)
-        self.log.info("Mining one block - index '10001' can't be included in this block")
         node.generate(1)
         self.sync_all()
         self.validate_credit_pool_balance(locked_1 - COIN)
         self.mempool_size -= 1
         self.check_mempool_size()
-        self.log.info("Tx should not be mined yet... mine one more block")
-        node.generate(1)
-        self.sync_all()
-        self.mempool_size -= 1
-        self.check_mempool_size()
         block_asset_unlock = node.getrawtransaction(asset_unlock_tx.rehash(), 1)['blockhash']
 
         self.send_tx(asset_unlock_tx,
@@ -346,21 +332,17 @@ def run_test(self):
             expected_error = "bad-assetunlock-duplicated-index",
             reason = "double index")
 
-        self.log.info("Checking tx with too far index is mined too - it is not too far anymore...")
-        self.validate_credit_pool_balance(locked_1 - 2 * COIN)
-        self.nodes[0].getrawtransaction(tx_too_far_index, 1)['blockhash']
-
         self.log.info("Mining next quorum to check tx 'asset_unlock_tx_late' is still valid...")
         self.mine_quorum()
         self.log.info("Checking credit pool amount is same...")
-        self.validate_credit_pool_balance(locked_1 - 2 * COIN)
+        self.validate_credit_pool_balance(locked_1 - 1 * COIN)
         self.check_mempool_result(tx=asset_unlock_tx_late, result_expected={'allowed': True})
         self.log.info("Checking credit pool amount still is same...")
-        self.validate_credit_pool_balance(locked_1 - 2 * COIN)
+        self.validate_credit_pool_balance(locked_1 - 1 * COIN)
         self.send_tx(asset_unlock_tx_late)
         node.generate(1)
         self.sync_all()
-        self.validate_credit_pool_balance(locked_1 - 3 * COIN)
+        self.validate_credit_pool_balance(locked_1 - 2 * COIN)
 
         self.log.info("Generating many blocks to make quorum far behind (even still active)...")
         self.slowly_generate_batch(too_late_height - node.getblock(node.getbestblockhash())["height"] - 1)
@@ -385,15 +367,15 @@ def run_test(self):
         self.validate_credit_pool_balance(locked_1)
         for inode in self.nodes:
             inode.reconsiderblock(block_to_reconsider)
-        self.validate_credit_pool_balance(locked_1 - 3 * COIN)
+        self.validate_credit_pool_balance(locked_1 - 2 * COIN)
 
         self.log.info("Forcibly mining asset_unlock_tx_too_late and ensure block is invalid...")
         self.create_and_check_block([asset_unlock_tx_too_late], expected_error = "bad-assetunlock-not-active-quorum")
 
         node.generate(1)
         self.sync_all()
 
-        self.validate_credit_pool_balance(locked_1 - 3 * COIN)
+        self.validate_credit_pool_balance(locked_1 - 2 * COIN)
         self.validate_credit_pool_balance(block_hash=block_hash_1, expected=locked_1)
 
         self.log.info("Checking too big withdrawal... expected to not be mined")