deprecate non-deterministic islocks

dashpay · Oct 24, 2023 · d2b1bec · d2b1bec
1 parent 297b50a
commit d2b1bec
Show file tree

Hide file tree

Showing 21 changed files with 165 additions and 354 deletions.
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
@@ -108,7 +108,6 @@ BITCOIN_TESTS =\
   test/dynamic_activation_thresholds_tests.cpp \
   test/evo_assetlocks_tests.cpp \
   test/evo_deterministicmns_tests.cpp \
-  test/evo_instantsend_tests.cpp \
   test/evo_mnhf_tests.cpp \
   test/evo_simplifiedmns_tests.cpp \
   test/evo_trivialvalidation.cpp \

diff --git a/src/llmq/instantsend.cpp b/src/llmq/instantsend.cpp
@@ -697,23 +697,18 @@ void CInstantSendManager::TrySignInstantSendLock(const CTransaction& tx)
     LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s: got all recovered sigs, creating CInstantSendLock\n", __func__,
             tx.GetHash().ToString());
 
-    CInstantSendLock islock(llmqType == Params().GetConsensus().llmqTypeDIP0024InstantSend ?
-                CInstantSendLock::isdlock_version:
-                CInstantSendLock::islock_version);
+    CInstantSendLock islock;
     islock.txid = tx.GetHash();
     for (const auto& in : tx.vin) {
         islock.inputs.emplace_back(in.prevout);
     }
 
-    // compute and set cycle hash if islock is deterministic
-    if (islock.IsDeterministic()) {
-        const auto& llmq_params_opt = GetLLMQParams(llmqType);
-        assert(llmq_params_opt);
-        LOCK(cs_main);
-        const auto dkgInterval = llmq_params_opt->dkgInterval;
-        const auto quorumHeight = m_chainstate.m_chain.Height() - (m_chainstate.m_chain.Height() % dkgInterval);
-        islock.cycleHash = m_chainstate.m_chain[quorumHeight]->GetBlockHash();
-    }
+    const auto& llmq_params_opt = GetLLMQParams(llmqType);
+    assert(llmq_params_opt);
+    LOCK(cs_main);
+    const auto dkgInterval = llmq_params_opt->dkgInterval;
+    const auto quorumHeight = m_chainstate.m_chain.Height() - (m_chainstate.m_chain.Height() % dkgInterval);
+    islock.cycleHash = m_chainstate.m_chain[quorumHeight]->GetBlockHash();
 
     auto id = islock.GetRequestId();
 
@@ -771,9 +766,8 @@ void CInstantSendManager::ProcessMessage(const CNode& pfrom, const std::string&
         return;
     }
 
-    if (msg_type == NetMsgType::ISLOCK || msg_type == NetMsgType::ISDLOCK) {
-        const auto islock_version = msg_type == NetMsgType::ISLOCK ? CInstantSendLock::islock_version : CInstantSendLock::isdlock_version;
-        const auto islock = std::make_shared<CInstantSendLock>(islock_version);
+    if (msg_type == NetMsgType::ISDLOCK) {
+        const auto islock = std::make_shared<CInstantSendLock>(CInstantSendLock::isdlock_version);
         vRecv >> *islock;
         ProcessMessageInstantSendLock(pfrom, islock);
     }
@@ -786,7 +780,7 @@ void CInstantSendManager::ProcessMessageInstantSendLock(const CNode& pfrom, cons
     bool fDIP0024IsActive = false;
     {
         LOCK(cs_main);
-        EraseObjectRequest(pfrom.GetId(), CInv(islock->IsDeterministic() ? MSG_ISDLOCK : MSG_ISLOCK, hash));
+        EraseObjectRequest(pfrom.GetId(), CInv(MSG_ISDLOCK, hash));
         fDIP0024IsActive = utils::IsDIP0024Active(m_chainstate.m_chain.Tip());
     }
 
@@ -796,7 +790,7 @@ void CInstantSendManager::ProcessMessageInstantSendLock(const CNode& pfrom, cons
     }
 
     // Deterministic ISLocks are only produced by rotation quorums, if we don't see DIP24 as active, then we won't be able to validate it anyway
-    if (islock->IsDeterministic() && fDIP0024IsActive) {
+    if (fDIP0024IsActive) {
         const auto blockIndex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(islock->cycleHash));
         if (blockIndex == nullptr) {
             // Maybe we don't have the block yet or maybe some peer spams invalid values for cycleHash
@@ -854,19 +848,6 @@ bool CInstantSendLock::TriviallyValid() const
 }
 
 bool CInstantSendManager::ProcessPendingInstantSendLocks()
-{
-    const CBlockIndex* pBlockIndexTip = WITH_LOCK(cs_main, return m_chainstate.m_chain.Tip());
-    if (pBlockIndexTip && utils::GetInstantSendLLMQType(qman, pBlockIndexTip) == Params().GetConsensus().llmqTypeDIP0024InstantSend) {
-        // Don't short circuit. Try to process both deterministic and not deterministic islocks independable
-        bool deterministicRes = ProcessPendingInstantSendLocks(true);
-        bool nondeterministicRes = ProcessPendingInstantSendLocks(false);
-        return deterministicRes && nondeterministicRes;
-    } else {
-        return ProcessPendingInstantSendLocks(false);
-    }
-}
-
-bool CInstantSendManager::ProcessPendingInstantSendLocks(bool deterministic)
 {
     decltype(pendingInstantSendLocks) pend;
     bool fMoreWork{false};
@@ -885,16 +866,11 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks(bool deterministic)
         removed.reserve(maxCount);
 
         for (const auto& [islockHash, nodeid_islptr_pair] : pendingInstantSendLocks) {
-            const auto& [_, islock_ptr] = nodeid_islptr_pair;
             // Check if we've reached max count
             if (pend.size() >= maxCount) {
                 fMoreWork = true;
                 break;
             }
-            // Check if we care about this islock on this run
-            if (islock_ptr->IsDeterministic() != deterministic) {
-                continue;
-            }
             pend.emplace(islockHash, std::move(nodeid_islptr_pair));
             removed.emplace_back(islockHash);
         }
@@ -909,7 +885,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks(bool deterministic)
     }
 
     //TODO Investigate if leaving this is ok
-    auto llmqType = utils::GetInstantSendLLMQType(deterministic);
+    auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend;
     const auto& llmq_params_opt = GetLLMQParams(llmqType);
     assert(llmq_params_opt);
     const auto& llmq_params = llmq_params_opt.value();
@@ -965,7 +941,7 @@ std::unordered_set<uint256, StaticSaltedHasher> CInstantSendManager::ProcessPend
         }
 
         int nSignHeight{-1};
-        if (islock->IsDeterministic()) {
+        {
             LOCK(cs_main);
 
             const auto blockIndex = m_chainstate.m_blockman.LookupBlockIndex(islock->cycleHash);
@@ -1075,18 +1051,8 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has
 
     const auto sameTxIsLock = db.GetInstantSendLockByTxid(islock->txid);
     if (sameTxIsLock != nullptr) {
-        if (sameTxIsLock->IsDeterministic() == islock->IsDeterministic()) {
-            // shouldn't happen, investigate
-            LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: duplicate islock, other islock=%s, peer=%d\n", __func__,
-                      islock->txid.ToString(), hash.ToString(), ::SerializeHash(*sameTxIsLock).ToString(), from);
-        }
-        if (sameTxIsLock->IsDeterministic()) {
-            // can happen, nothing to do
-            return;
-        } else if (islock->IsDeterministic()) {
-            // can happen, remove and archive the non-deterministic sameTxIsLock
-            db.RemoveAndArchiveInstantSendLock(sameTxIsLock, WITH_LOCK(::cs_main, return m_chainstate.m_chain.Height()));
-        }
+        // can happen, nothing to do
+        return;
     } else {
         for (const auto& in : islock->inputs) {
             const auto sameOutpointIsLock = db.GetInstantSendLockByInput(in);
@@ -1114,14 +1080,13 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has
     // We only need the ISLOCK from now on to detect conflicts
     TruncateRecoveredSigsForInputs(*islock);
 
-    const auto is_det = islock->IsDeterministic();
-    CInv inv(is_det ? MSG_ISDLOCK : MSG_ISLOCK, hash);
+    CInv inv(MSG_ISDLOCK, hash);
     if (tx != nullptr) {
-        connman.RelayInvFiltered(inv, *tx, is_det ? ISDLOCK_PROTO_VERSION : MIN_PEER_PROTO_VERSION);
+        connman.RelayInvFiltered(inv, *tx, ISDLOCK_PROTO_VERSION);
     } else {
         // we don't have the TX yet, so we only filter based on txid. Later when that TX arrives, we will re-announce
         // with the TX taken into account.
-        connman.RelayInvFiltered(inv, islock->txid, is_det ? ISDLOCK_PROTO_VERSION : MIN_PEER_PROTO_VERSION);
+        connman.RelayInvFiltered(inv, islock->txid, ISDLOCK_PROTO_VERSION);
     }
 
     ResolveBlockConflicts(hash, *islock);
@@ -1309,7 +1274,7 @@ void CInstantSendManager::TruncateRecoveredSigsForInputs(const llmq::CInstantSen
     for (const auto& in : islock.inputs) {
         auto inputRequestId = ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, in));
         WITH_LOCK(cs_inputReqests, inputRequestIds.erase(inputRequestId));
-        sigman.TruncateRecoveredSig(utils::GetInstantSendLLMQType(islock.IsDeterministic()), inputRequestId);
+        sigman.TruncateRecoveredSig(utils::GetInstantSendLLMQType(true), inputRequestId);
     }
 }
 
@@ -1360,7 +1325,7 @@ void CInstantSendManager::HandleFullyConfirmedBlock(const CBlockIndex* pindex)
 
         // And we don't need the recovered sig for the ISLOCK anymore, as the block in which it got mined is considered
         // fully confirmed now
-        sigman.TruncateRecoveredSig(utils::GetInstantSendLLMQType(islock->IsDeterministic()), islock->GetRequestId());
+        sigman.TruncateRecoveredSig(utils::GetInstantSendLLMQType(true), islock->GetRequestId());
     }
 
     db.RemoveArchivedInstantSendLocks(pindex->nHeight - 100);

diff --git a/src/llmq/instantsend.h b/src/llmq/instantsend.h
@@ -46,24 +46,23 @@ struct CInstantSendLock
     uint256 cycleHash;
     CBLSLazySignature sig;
 
-    CInstantSendLock() : CInstantSendLock(islock_version) {}
+    CInstantSendLock() : CInstantSendLock(isdlock_version) {}
     explicit CInstantSendLock(const uint8_t desiredVersion) : nVersion(desiredVersion) {}
 
     SERIALIZE_METHODS(CInstantSendLock, obj)
     {
-        if (s.GetVersion() >= ISDLOCK_PROTO_VERSION && obj.IsDeterministic()) {
+        if (s.GetVersion() >= ISDLOCK_PROTO_VERSION) {
             READWRITE(obj.nVersion);
         }
         READWRITE(obj.inputs);
         READWRITE(obj.txid);
-        if (s.GetVersion() >= ISDLOCK_PROTO_VERSION && obj.IsDeterministic()) {
+        if (s.GetVersion() >= ISDLOCK_PROTO_VERSION) {
             READWRITE(obj.cycleHash);
         }
         READWRITE(obj.sig);
     }
 
     uint256 GetRequestId() const;
-    bool IsDeterministic() const { return nVersion != islock_version; }
     bool TriviallyValid() const;
 };
 
@@ -293,8 +292,7 @@ class CInstantSendManager : public CRecoveredSigsListener
     void TrySignInstantSendLock(const CTransaction& tx) LOCKS_EXCLUDED(cs_creating);
 
     void ProcessMessageInstantSendLock(const CNode& pfrom, const CInstantSendLockPtr& islock);
-    bool ProcessPendingInstantSendLocks();
-    bool ProcessPendingInstantSendLocks(bool deterministic) LOCKS_EXCLUDED(cs_pendingLocks);
+    bool ProcessPendingInstantSendLocks() LOCKS_EXCLUDED(cs_pendingLocks);
 
     std::unordered_set<uint256, StaticSaltedHasher> ProcessPendingInstantSendLocks(const Consensus::LLMQParams& llmq_params,
                                                                                    int signOffset,

diff --git a/src/llmq/snapshot.cpp b/src/llmq/snapshot.cpp
@@ -134,7 +134,7 @@ bool BuildQuorumRotationInfo(const CGetQuorumRotationInfo& request, CQuorumRotat
     }
 
     //Quorum rotation is enabled only for InstantSend atm.
-    Consensus::LLMQType llmqType = utils::GetInstantSendLLMQType(qman, blockIndex);
+    Consensus::LLMQType llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend;
 
     // Since the returned quorums are in reversed order, the most recent one is at index 0
     const auto& llmq_params_opt = GetLLMQParams(llmqType);

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -1053,7 +1053,6 @@ std::chrono::microseconds GetObjectInterval(int invType)
             return std::chrono::seconds{15};
         case MSG_CLSIG:
             return std::chrono::seconds{5};
-        case MSG_ISLOCK:
         case MSG_ISDLOCK:
             return std::chrono::seconds{10};
         default:
@@ -1879,7 +1878,6 @@ bool PeerManagerImpl::AlreadyHave(const CInv& inv)
         return m_llmq_ctx->sigman->AlreadyHave(inv);
     case MSG_CLSIG:
         return m_llmq_ctx->clhandler->AlreadyHave(inv);
-    case MSG_ISLOCK:
     case MSG_ISDLOCK:
         return m_llmq_ctx->isman->AlreadyHave(inv);
     }
@@ -2042,7 +2040,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, const CChainParams& chai
                     for (PairType &pair : merkleBlock.vMatchedTxn) {
                         auto islock = isman.GetInstantSendLockByTxid(pair.second);
                         if (islock != nullptr) {
-                            connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::ISLOCK, *islock));
+                            connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::ISDLOCK, *islock));
                         }
                     }
                 }
@@ -2258,10 +2256,10 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
                 }
             }
 
-            if (!push && (inv.type == MSG_ISLOCK || inv.type == MSG_ISDLOCK)) {
+            if (!push && inv.type == MSG_ISDLOCK) {
                 llmq::CInstantSendLock o;
                 if (m_llmq_ctx->isman->GetInstantSendLockByHash(inv.hash, o)) {
-                    const auto msg_type = inv.type == MSG_ISLOCK ? NetMsgType::ISLOCK : NetMsgType::ISDLOCK;
+                    const auto msg_type = NetMsgType::ISDLOCK;
                     m_connman.PushMessage(&pfrom, msgMaker.Make(msg_type, o));
                     push = true;
                 }
@@ -3281,7 +3279,7 @@ void PeerManagerImpl::ProcessMessage(
                     pfrom.fDisconnect = true;
                     return;
                 } else if (!fAlreadyHave) {
-                    if (fBlocksOnly && (inv.type == MSG_ISLOCK || inv.type == MSG_ISDLOCK)) {
+                    if (fBlocksOnly && inv.type == MSG_ISDLOCK) {
                         if (pfrom.GetRecvVersion() <= ADDRV2_PROTO_VERSION) {
                             // It's ok to receive these invs, we just ignore them
                             // and do not request corresponding objects.
@@ -4985,8 +4983,8 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
 
                         const auto islock = m_llmq_ctx->isman->GetInstantSendLockByTxid(hash);
                         if (islock == nullptr) continue;
-                        if (pto->nVersion < ISDLOCK_PROTO_VERSION && islock->IsDeterministic()) continue;
-                        queueAndMaybePushInv(CInv(islock->IsDeterministic() ? MSG_ISDLOCK : MSG_ISLOCK, ::SerializeHash(*islock)));
+                        if (pto->nVersion < ISDLOCK_PROTO_VERSION) continue;
+                        queueAndMaybePushInv(CInv(MSG_ISDLOCK, ::SerializeHash(*islock)));
                     }
 
                     // Send an inv for the best ChainLock we have
@@ -5068,7 +5066,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
                     if (pto->m_tx_relay->filterInventoryKnown.contains(inv.hash)) {
                         continue;
                     }
-                    if (!fSendIS && (inv.type == MSG_ISLOCK || inv.type == MSG_ISDLOCK)) {
+                    if (!fSendIS && inv.type == MSG_ISDLOCK) {
                         continue;
                     }
                     queueAndMaybePushInv(inv);

diff --git a/src/protocol.cpp b/src/protocol.cpp
@@ -298,7 +298,6 @@ const char* CInv::GetCommandInternal() const
         case MSG_QUORUM_PREMATURE_COMMITMENT:   return NetMsgType::QPCOMMITMENT;
         case MSG_QUORUM_RECOVERED_SIG:          return NetMsgType::QSIGREC;
         case MSG_CLSIG:                         return NetMsgType::CLSIG;
-        case MSG_ISLOCK:                        return NetMsgType::ISLOCK;
         case MSG_ISDLOCK:                       return NetMsgType::ISDLOCK;
         default:
             return nullptr;

diff --git a/src/protocol.h b/src/protocol.h
@@ -287,7 +287,6 @@ extern const char* QSIGSHARE;
 extern const char* QGETDATA;
 extern const char* QDATA;
 extern const char* CLSIG;
-extern const char* ISLOCK;
 extern const char* ISDLOCK;
 extern const char* MNAUTH;
 extern const char* GETHEADERS2;
@@ -525,7 +524,7 @@ enum GetDataMsg {
     /* MSG_QUORUM_DEBUG_STATUS = 27, */               // was shortly used on testnet/devnet/regtest
     MSG_QUORUM_RECOVERED_SIG = 28,
     MSG_CLSIG = 29,
-    MSG_ISLOCK = 30,
+    /* MSG_ISLOCK = 30, */ // Non-deterministic InstantSend and not used anymore
     MSG_ISDLOCK = 31,
 };
 

diff --git a/src/test/evo_instantsend_tests.cpp b/src/test/evo_instantsend_tests.cpp
diff --git a/test/functional/feature_llmq_chainlocks.py b/test/functional/feature_llmq_chainlocks.py
@@ -18,7 +18,7 @@
 
 class LLMQChainLocksTest(DashTestFramework):
     def set_test_params(self):
-        self.set_dash_test_params(4, 3, fast_dip3_enforcement=True)
+        self.set_dash_test_params(5, 4, fast_dip3_enforcement=True)
 
     def run_test(self):
 
@@ -46,9 +46,14 @@ def run_test(self):
         self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
         self.wait_for_sporks_same()
 
-        self.log.info("Mining 4 quorums")
-        for i in range(4):
-            self.mine_quorum()
+        self.move_to_next_cycle()
+        self.log.info("Cycle H height:" + str(self.nodes[0].getblockcount()))
+        self.move_to_next_cycle()
+        self.log.info("Cycle H+C height:" + str(self.nodes[0].getblockcount()))
+        self.move_to_next_cycle()
+        self.log.info("Cycle H+2C height:" + str(self.nodes[0].getblockcount()))
+        self.mine_cycle_quorum(llmq_type_name="llmq_test_dip0024", llmq_type=103)
+
 
         self.log.info("Mine single block, wait for chainlock")
         self.nodes[0].generate(1)