feat!: remove dependency of Asset Lock txes on CCreditPool

There's no more check when tx is accepted in mempool.
Functional and unit tests updated accordingly

Author: knst

src/evo/assetlocktx.cpp

@@ -4,7 +4,6 @@
 
 #include <evo/assetlocktx.h>
 #include <evo/specialtx.h>
-#include <evo/creditpool.h>
 
 #include <consensus/params.h>
 
@@ -17,18 +16,20 @@
 #include <llmq/utils.h>
 #include <llmq/quorums.h>
 
+#include <util/ranges_set.h>
+
 #include <algorithm>
 
 /**
  *  Common code for Asset Lock and Asset Unlock
  */
-bool CheckAssetLockUnlockTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCreditPool& creditPool, TxValidationState& state)
+bool CheckAssetLockUnlockTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const std::optional<CRangesSet>& indexes, TxValidationState& state)
 {
     switch (tx.nType) {
     case TRANSACTION_ASSET_LOCK:
         return CheckAssetLockTx(tx, state);
     case TRANSACTION_ASSET_UNLOCK:
-        return CheckAssetUnlockTx(tx, pindexPrev, creditPool, state);
+        return CheckAssetUnlockTx(tx, pindexPrev, indexes, state);
     default:
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-not-asset-locks-at-all");
     }
@@ -142,8 +143,10 @@ bool CAssetUnlockPayload::VerifySig(const uint256& msgHash, const CBlockIndex* p
     return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-not-verified");
 }
 
-bool CheckAssetUnlockTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCreditPool& creditPool, TxValidationState& state)
+bool CheckAssetUnlockTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const std::optional<CRangesSet>& indexes, TxValidationState& state)
 {
+    // Some checks depends from blockchain status also, such as `known indexes` and `withdrawal limits`
+    // They are omitted here and done by CCreditPool
     if (tx.nType != TRANSACTION_ASSET_UNLOCK) {
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-type");
     }
@@ -165,7 +168,7 @@ bool CheckAssetUnlockTx(const CTransaction& tx, const CBlockIndex* pindexPrev, c
         return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetunlocktx-version");
     }
 
-    if (creditPool.indexes.Contains(assetUnlockTx.getIndex())) {
+    if (indexes != std::nullopt && indexes->Contains(assetUnlockTx.getIndex())) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-duplicated-index");
     }
 

src/evo/assetlocktx.h

@@ -14,8 +14,10 @@
 #include <tinyformat.h>
 #include <univalue.h>
 
+#include <optional>
+
 class CBlockIndex;
-struct CCreditPool;
+class CRangesSet;
 
 class CAssetLockPayload
 {
@@ -166,8 +168,8 @@ class CAssetUnlockPayload
 };
 
 bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state);
-bool CheckAssetUnlockTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCreditPool& creditPool, TxValidationState& state);
-bool CheckAssetLockUnlockTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCreditPool& creditPool, TxValidationState& state);
+bool CheckAssetUnlockTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const std::optional<CRangesSet>& indexes, TxValidationState& state);
+bool CheckAssetLockUnlockTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const std::optional<CRangesSet>& indexes, TxValidationState& state);
 bool GetAssetUnlockFee(const CTransaction& tx, CAmount& txfee, TxValidationState& state);
 
 #endif // BITCOIN_EVO_ASSETLOCKTX_H

src/evo/creditpool.cpp

@@ -278,7 +278,7 @@ bool CCreditPoolDiff::ProcessTransaction(const CTransaction& tx, TxValidationSta
 
     if (tx.nType != TRANSACTION_ASSET_LOCK && tx.nType != TRANSACTION_ASSET_UNLOCK) return true;
 
-    if (!CheckAssetLockUnlockTx(tx, pindex, this->pool, state)) {
+    if (!CheckAssetLockUnlockTx(tx, pindex, pool.indexes, state)) {
         // pass the state returned by the function above
         return false;
     }

src/evo/specialtxman.cpp

@@ -19,7 +19,7 @@
 #include <primitives/block.h>
 #include <validation.h>
 
-bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, const CCreditPool& creditPool, bool check_sigs, TxValidationState& state)
+static bool CheckSpecialTxInner(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, const std::optional<CRangesSet>& indexes, bool check_sigs, TxValidationState& state)
 {
     AssertLockHeld(cs_main);
 
@@ -54,7 +54,7 @@ bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const
             if (!llmq::utils::IsV20Active(pindexPrev)) {
                 return state.Invalid(TxValidationResult::TX_CONSENSUS, "assetlocks-before-v20");
             }
-            return CheckAssetLockUnlockTx(tx, pindexPrev, creditPool, state);
+            return CheckAssetLockUnlockTx(tx, pindexPrev, indexes, state);
         }
     } catch (const std::exception& e) {
         LogPrintf("%s -- failed: %s\n", __func__, e.what());
@@ -64,6 +64,12 @@ bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const
     return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-tx-type-check");
 }
 
+bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs, TxValidationState& state)
+{
+    AssertLockHeld(cs_main);
+    return CheckSpecialTxInner(tx, pindexPrev, view, std::nullopt, check_sigs, state);
+}
+
 bool ProcessSpecialTx(const CTransaction& tx, const CBlockIndex* pindex, TxValidationState& state)
 {
     if (tx.nVersion != 3 || tx.nType == TRANSACTION_NORMAL) {
@@ -142,7 +148,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll
             TxValidationState tx_state;
             // At this moment CheckSpecialTx() and ProcessSpecialTx() may fail by 2 possible ways:
             // consensus failures and "TX_BAD_SPECIAL"
-            if (!CheckSpecialTx(*ptr_tx, pindex->pprev, view, creditPool, fCheckCbTxMerleRoots, tx_state)) {
+            if (!CheckSpecialTxInner(*ptr_tx, pindex->pprev, view, creditPool.indexes, fCheckCbTxMerleRoots, tx_state)) {
                 assert(tx_state.GetResult() == TxValidationResult::TX_CONSENSUS || tx_state.GetResult() == TxValidationResult::TX_BAD_SPECIAL);
                 return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(),
                                  strprintf("Special Transaction check failed (tx hash %s) %s", ptr_tx->GetHash().ToString(), tx_state.GetDebugMessage()));

src/evo/specialtxman.h

@@ -12,7 +12,6 @@
 class BlockValidationState;
 class CBlock;
 class CBlockIndex;
-struct CCreditPool;
 class CCoinsViewCache;
 class TxValidationState;
 namespace llmq {
@@ -25,7 +24,7 @@ struct Params;
 
 extern RecursiveMutex cs_main;
 
-bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, const CCreditPool& pool, bool check_sigs,
+bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs,
                     TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, const llmq::CChainLocksHandler& chainlock_handler,
                             const Consensus::Params& consensusParams, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots,

src/rpc/evo.cpp

@@ -7,7 +7,6 @@
 #include <chainparams.h>
 #include <consensus/validation.h>
 #include <core_io.h>
-#include <evo/creditpool.h>
 #include <evo/deterministicmns.h>
 #include <evo/dmn_types.h>
 #include <evo/providertx.h>
@@ -329,10 +328,8 @@ static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, const Cha
     {
     LOCK(cs_main);
 
-    CCreditPool creditPool = creditPoolManager->GetCreditPool(chainman.ActiveChain().Tip(), Params().GetConsensus());
-
     TxValidationState state;
-    if (!CheckSpecialTx(CTransaction(tx), chainman.ActiveChain().Tip(), chainman.ActiveChainstate().CoinsTip(), creditPool, true, state)) {
+    if (!CheckSpecialTx(CTransaction(tx), chainman.ActiveChain().Tip(), chainman.ActiveChainstate().CoinsTip(), true, state)) {
         throw std::runtime_error(state.ToString());
     }
     } // cs_main

src/test/evo_assetlocks_tests.cpp

@@ -7,14 +7,16 @@
 #include <amount.h>
 #include <consensus/tx_check.h>
 #include <evo/assetlocktx.h>
-#include <evo/creditpool.h>
 #include <policy/settings.h>
 #include <script/script.h>
 #include <script/signingprovider.h>
+#include <util/ranges_set.h>
 #include <validation.h> // for ::ChainActive()
 
 #include <boost/test/unit_test.hpp>
 
+
+#include <optional>
 //
 // Helper: create two dummy transactions, each with
 // two outputs.  The first has 11 and 50 CENT outputs
@@ -314,8 +316,7 @@ BOOST_FIXTURE_TEST_CASE(evo_assetunlock, TestChain100Setup)
     BOOST_CHECK(tx_state.IsValid());
 
     const CBlockIndex *block_index = ::ChainActive().Tip();
-    CCreditPool pool;
-    BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(tx), block_index, pool, tx_state));
+    BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(tx), block_index, std::nullopt, tx_state));
     BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetunlock-quorum-hash");
 
     {
@@ -333,7 +334,7 @@ BOOST_FIXTURE_TEST_CASE(evo_assetunlock, TestChain100Setup)
         std::string reason;
         BOOST_CHECK(IsStandardTx(CTransaction(tx), reason));
 
-        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txNonemptyInput), block_index, pool, tx_state));
+        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txNonemptyInput), block_index, std::nullopt, tx_state));
         BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetunlocktx-have-input");
     }
 
@@ -348,7 +349,7 @@ BOOST_FIXTURE_TEST_CASE(evo_assetunlock, TestChain100Setup)
         // Wrong type "Asset Lock TX" instead "Asset Unlock TX"
         CMutableTransaction txWrongType = tx;
         txWrongType.nType = TRANSACTION_ASSET_LOCK;
-        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txWrongType), block_index, pool, tx_state));
+        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txWrongType), block_index, std::nullopt, tx_state));
         BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetunlocktx-type");
 
         // Check version of tx and payload
@@ -363,10 +364,10 @@ BOOST_FIXTURE_TEST_CASE(evo_assetunlock, TestChain100Setup)
             CMutableTransaction txWrongVersion = tx;
             SetTxPayload(txWrongVersion, unlockPayload_tmp);
             if (payload_version != 1) {
-                BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txWrongVersion), block_index, pool, tx_state));
+                BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txWrongVersion), block_index, std::nullopt, tx_state));
                 BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetunlocktx-version");
             } else {
-                BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txWrongVersion), block_index, pool, tx_state));
+                BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txWrongVersion), block_index, std::nullopt, tx_state));
                 BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetunlock-quorum-hash");
             }
         }
@@ -382,14 +383,23 @@ BOOST_FIXTURE_TEST_CASE(evo_assetunlock, TestChain100Setup)
             out.scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
         }
 
-        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txManyOutputs), block_index, pool, tx_state));
+        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txManyOutputs), block_index, std::nullopt, tx_state));
+        BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetunlock-quorum-hash");
+
+        // Basic checks for CRangesSet
+        CRangesSet indexes;
+        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txManyOutputs), block_index, indexes, tx_state));
         BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetunlock-quorum-hash");
+        BOOST_CHECK(indexes.Add(0x001122334455667788L));
+        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txManyOutputs), block_index, indexes, tx_state));
+        BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetunlock-duplicated-index");
+
 
         // Should not be more than 32 withdrawal in one transaction
         txManyOutputs.vout.resize(outputsLimit + 1);
         txManyOutputs.vout.back().nValue = CENT;
         txManyOutputs.vout.back().scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
-        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txManyOutputs), block_index, pool, tx_state));
+        BOOST_CHECK(!CheckAssetUnlockTx(CTransaction(txManyOutputs), block_index, std::nullopt, tx_state));
         BOOST_CHECK(tx_state.GetRejectReason() == "bad-assetunlocktx-too-many-outs");
     }
 

src/validation.cpp

@@ -52,7 +52,6 @@
 #include <masternode/payments.h>
 #include <masternode/sync.h>
 
-#include <evo/creditpool.h>
 #include <evo/evodb.h>
 #include <evo/specialtx.h>
 #include <evo/specialtxman.h>
@@ -712,7 +711,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
             }
         }
     }
-    CCreditPool creditPool = creditPoolManager->GetCreditPool(::ChainActive().Tip(), chainparams.GetConsensus());
     LockPoints lp;
     m_view.SetBackend(m_viewmempool);
 
@@ -827,7 +825,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     // DoS scoring a node for non-critical errors, e.g. duplicate keys because a TX is received that was already
     // mined
     // NOTE: we use UTXO here and do NOT allow mempool txes as masternode collaterals
-    if (!CheckSpecialTx(tx, m_active_chainstate.m_chain.Tip(), m_active_chainstate.CoinsTip(), creditPool, true, state))
+    if (!CheckSpecialTx(tx, m_active_chainstate.m_chain.Tip(), m_active_chainstate.CoinsTip(), true, state))
         return false;
 
     if (m_pool.existsProviderTxConflict(tx)) {

test/functional/feature_asset_locks.py

@@ -298,6 +298,7 @@ def run_test(self):
         asset_unlock_tx_too_big_fee = self.create_assetunlock(104, COIN, pubkey, fee=int(Decimal("0.1") * COIN))
         asset_unlock_tx_zero_fee = self.create_assetunlock(105, COIN, pubkey, fee=0)
         asset_unlock_tx_duplicate_index = copy.deepcopy(asset_unlock_tx)
+        # modify this tx with duplicated index to make a hash of tx different, otherwise tx would be refused too early
         asset_unlock_tx_duplicate_index.vout[0].nValue += COIN
         too_late_height = node.getblock(node.getbestblockhash())["height"] + 48
 
@@ -331,12 +332,6 @@ def run_test(self):
             expected_error = "Transaction already in block chain",
             reason = "double copy")
 
-        self.check_mempool_result(tx=asset_unlock_tx_duplicate_index,
-                result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-duplicated-index'})
-        self.send_tx(asset_unlock_tx_duplicate_index,
-            expected_error = "bad-assetunlock-duplicated-index",
-            reason = "double index")
-
         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...")
@@ -411,11 +406,6 @@ def run_test(self):
         assert txid_in_block in block['tx']
         self.validate_credit_pool_balance(0)
 
-        self.log.info("After many blocks duplicated tx still should not be mined")
-        self.send_tx(asset_unlock_tx_duplicate_index,
-                expected_error = "bad-assetunlock-duplicated-index",
-                reason = "double index")
-
         self.log.info("Forcibly mine asset_unlock_tx_full and ensure block is invalid...")
         self.create_and_check_block([asset_unlock_tx_duplicate_index], expected_error = "bad-assetunlock-duplicated-index")
 

test/lint/lint-circular-dependencies.sh

@@ -81,10 +81,8 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
     "evo/deterministicmns -> llmq/utils -> net -> evo/deterministicmns"
     "policy/policy -> policy/settings -> policy/policy"
     "evo/specialtxman -> validation -> evo/specialtxman"
-    "evo/creditpool -> validation -> evo/creditpool"
     "consensus/tx_verify -> evo/assetlocktx -> validation -> consensus/tx_verify"
     "consensus/tx_verify -> evo/assetlocktx -> llmq/quorums -> net_processing -> txmempool -> consensus/tx_verify"
-    "evo/assetlocktx -> evo/creditpool -> evo/assetlocktx"
     "evo/assetlocktx -> llmq/quorums -> net_processing -> txmempool -> evo/assetlocktx"
 
     "evo/simplifiedmns -> llmq/blockprocessor -> net_processing -> llmq/snapshot -> evo/simplifiedmns"