instantsend: deterministic lock using the same msg hash as islock (#4381)

When receiving an islock, propagate it as islock.
When creating/receiving and isdlock, propagate it as isdlock to peers which support it and as islock to peers which don't.
Functional tests to cover both islock and isdlock scenarios.

Author: gabriel-bjg

src/llmq/quorums_instantsend.cpp

@@ -39,6 +39,8 @@ static const std::string DB_ARCHIVED_BY_HASH = "is_a2";
 static const std::string DB_VERSION = "is_v";
 
 const int CInstantSendDb::CURRENT_VERSION;
+const uint8_t CInstantSendLock::islock_version;
+const uint8_t CInstantSendLock::isdlock_version;
 
 CInstantSendManager* quorumInstantSendManager;
 
@@ -325,10 +327,14 @@ CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByHash(const uint256& hash
         return ret;
     }
 
-    ret = std::make_shared<CInstantSendLock>();
+    ret = std::make_shared<CInstantSendLock>(CInstantSendLock::isdlock_version);
     bool exists = db->Read(std::make_tuple(DB_ISLOCK_BY_HASH, hash), *ret);
     if (!exists) {
-        ret = nullptr;
+        ret = std::make_shared<CInstantSendLock>();
+        exists = db->Read(std::make_tuple(DB_ISLOCK_BY_HASH, hash), *ret);
+        if (!exists) {
+            ret = nullptr;
+        }
     }
     islockCache.insert(hash, ret);
     return ret;
@@ -686,7 +692,7 @@ void CInstantSendManager::HandleNewInputLockRecoveredSig(const CRecoveredSig& re
 
 void CInstantSendManager::TrySignInstantSendLock(const CTransaction& tx)
 {
-    auto llmqType = Params().GetConsensus().llmqTypeInstantSend;
+    const auto llmqType = Params().GetConsensus().llmqTypeInstantSend;
 
     for (auto& in : tx.vin) {
         auto id = ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, in.prevout));
@@ -698,12 +704,20 @@ 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;
+    CInstantSendLock islock(CInstantSendLock::isdlock_version);
     islock.txid = tx.GetHash();
     for (auto& in : tx.vin) {
         islock.inputs.emplace_back(in.prevout);
     }
 
+    // compute cycle hash
+    {
+        LOCK(cs_main);
+        const auto dkgInterval = GetLLMQParams(llmqType).dkgInterval;
+        const auto quorumHeight = chainActive.Height() - (chainActive.Height() % dkgInterval);
+        islock.cycleHash = chainActive[quorumHeight]->GetBlockHash();
+    }
+
     auto id = islock.GetRequestId();
 
     if (quorumSigningManager->HasRecoveredSigForId(llmqType, id)) {
@@ -760,8 +774,9 @@ void CInstantSendManager::ProcessMessage(CNode* pfrom, const std::string& strCom
         return;
     }
 
-    if (strCommand == NetMsgType::ISLOCK) {
-        auto islock = std::make_shared<CInstantSendLock>();
+    if (strCommand == NetMsgType::ISLOCK || strCommand == NetMsgType::ISDLOCK) {
+        const auto islock_version = strCommand == NetMsgType::ISLOCK ? CInstantSendLock::islock_version : CInstantSendLock::isdlock_version;
+        const auto islock = std::make_shared<CInstantSendLock>(islock_version);
         vRecv >> *islock;
         ProcessMessageInstantSendLock(pfrom, islock);
     }
@@ -775,14 +790,29 @@ void CInstantSendManager::ProcessMessageInstantSendLock(const CNode* pfrom, cons
 
     {
         LOCK(cs_main);
-        EraseObjectRequest(pfrom->GetId(), CInv(MSG_ISLOCK, hash));
+        EraseObjectRequest(pfrom->GetId(), CInv(islock->IsDeterministic() ? MSG_ISDLOCK : MSG_ISLOCK, hash));
     }
 
     if (!PreVerifyInstantSendLock(*islock)) {
         LOCK(cs_main);
         Misbehaving(pfrom->GetId(), 100);
         return;
     }
+    if (islock->IsDeterministic()) {
+        const auto blockIndex = WITH_LOCK(cs_main, return LookupBlockIndex(islock->cycleHash));
+        if (blockIndex == nullptr) {
+            // Maybe we don't have the block yet or maybe some peer spams invalid values for cycleHash
+            WITH_LOCK(cs_main, Misbehaving(pfrom->GetId(), 1));
+            return;
+        }
+
+        const auto llmqType = Params().GetConsensus().llmqTypeInstantSend;
+        const auto dkgInterval = GetLLMQParams(llmqType).dkgInterval;
+        if (blockIndex->nHeight % dkgInterval != 0) {
+            WITH_LOCK(cs_main, Misbehaving(pfrom->GetId(), 100));
+            return;
+        }
+    }
 
     LOCK(cs);
     if (pendingInstantSendLocks.count(hash) || db.KnownInstantSendLock(hash)) {
@@ -901,7 +931,23 @@ std::unordered_set<uint256> CInstantSendManager::ProcessPendingInstantSendLocks(
             continue;
         }
 
-        auto quorum = llmq::CSigningManager::SelectQuorumForSigning(llmqType, id, -1, signOffset);
+        int nSignHeight{-1};
+        if (islock->IsDeterministic()) {
+            LOCK(cs_main);
+
+            const auto blockIndex = LookupBlockIndex(islock->cycleHash);
+            if (blockIndex == nullptr) {
+                batchVerifier.badSources.emplace(nodeId);
+                continue;
+            }
+
+            const auto dkgInterval = GetLLMQParams(Params().GetConsensus().llmqTypeInstantSend).dkgInterval;
+            if (blockIndex->nHeight + dkgInterval < chainActive.Height()) {
+                nSignHeight = blockIndex->nHeight + dkgInterval - 1;
+            }
+        }
+
+        auto quorum = llmq::CSigningManager::SelectQuorumForSigning(llmqType, id, nSignHeight, signOffset);
         if (!quorum) {
             // should not happen, but if one fails to select, all others will also fail to select
             return {};
@@ -1028,13 +1074,14 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has
         TruncateRecoveredSigsForInputs(*islock);
     }
 
-    CInv inv(MSG_ISLOCK, hash);
+    const auto is_det = islock->IsDeterministic();
+    CInv inv(is_det ? MSG_ISDLOCK : MSG_ISLOCK, hash);
     if (tx != nullptr) {
-        g_connman->RelayInvFiltered(inv, *tx, LLMQS_PROTO_VERSION);
+        g_connman->RelayInvFiltered(inv, *tx, is_det ? ISDLOCK_PROTO_VERSION : LLMQS_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.
-        g_connman->RelayInvFiltered(inv, islock->txid, LLMQS_PROTO_VERSION);
+        g_connman->RelayInvFiltered(inv, islock->txid, is_det ? ISDLOCK_PROTO_VERSION : LLMQS_PROTO_VERSION);
     }
 
     ResolveBlockConflicts(hash, *islock);
@@ -1068,8 +1115,8 @@ void CInstantSendManager::TransactionAddedToMempool(const CTransactionRef& tx)
         }
         // In case the islock was received before the TX, filtered announcement might have missed this islock because
         // we were unable to check for filter matches deep inside the TX. Now we have the TX, so we should retry.
-        CInv inv(MSG_ISLOCK, ::SerializeHash(*islock));
-        g_connman->RelayInvFiltered(inv, *tx, LLMQS_PROTO_VERSION);
+        CInv inv(islock->IsDeterministic() ? MSG_ISDLOCK : MSG_ISLOCK, ::SerializeHash(*islock));
+        g_connman->RelayInvFiltered(inv, *tx, islock->IsDeterministic() ? ISDLOCK_PROTO_VERSION : LLMQS_PROTO_VERSION);
         // If the islock was received before the TX, we know we were not able to send
         // the notification at that time, we need to do it now.
         LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- notify about an earlier received lock for tx %s\n", __func__, tx->GetHash().ToString());

src/llmq/quorums_instantsend.h

@@ -20,20 +20,37 @@
 namespace llmq
 {
 
-class CInstantSendLock
+struct CInstantSendLock
 {
-public:
+    // This is the old format of instant send lock, it must be 0
+    static const uint8_t islock_version = 0;
+    // This is the new format of instant send deterministic lock, this should be incremented for new isdlock versions
+    static const uint8_t isdlock_version = 1;
+
+    uint8_t nVersion;
     std::vector<COutPoint> inputs;
     uint256 txid;
+    uint256 cycleHash;
     CBLSLazySignature sig;
 
-public:
+    CInstantSendLock() : CInstantSendLock(islock_version) {}
+    explicit CInstantSendLock(const uint8_t desiredVersion) : nVersion(desiredVersion) {}
+
     SERIALIZE_METHODS(CInstantSendLock, obj)
     {
-        READWRITE(obj.inputs, obj.txid, obj.sig);
+        if (s.GetVersion() >= ISDLOCK_PROTO_VERSION && obj.IsDeterministic()) {
+            READWRITE(obj.nVersion);
+        }
+        READWRITE(obj.inputs);
+        READWRITE(obj.txid);
+        if (s.GetVersion() >= ISDLOCK_PROTO_VERSION && obj.IsDeterministic()) {
+            READWRITE(obj.cycleHash);
+        }
+        READWRITE(obj.sig);
     }
 
     uint256 GetRequestId() const;
+    bool IsDeterministic() const { return nVersion != islock_version; }
 };
 
 typedef std::shared_ptr<CInstantSendLock> CInstantSendLockPtr;

src/net_processing.cpp

@@ -792,6 +792,7 @@ std::chrono::microseconds GetObjectInterval(int invType)
         case MSG_CLSIG:
             return std::chrono::seconds{5};
         case MSG_ISLOCK:
+        case MSG_ISDLOCK:
             return std::chrono::seconds{10};
         default:
             return GETDATA_TX_INTERVAL;
@@ -1438,6 +1439,7 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
     case MSG_CLSIG:
         return llmq::chainLocksHandler->AlreadyHave(inv);
     case MSG_ISLOCK:
+    case MSG_ISDLOCK:
         return llmq::quorumInstantSendManager->AlreadyHave(inv);
     }
 
@@ -1772,10 +1774,11 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
                 }
             }
 
-            if (!push && (inv.type == MSG_ISLOCK)) {
+            if (!push && (inv.type == MSG_ISLOCK || inv.type == MSG_ISDLOCK)) {
                 llmq::CInstantSendLock o;
                 if (llmq::quorumInstantSendManager->GetInstantSendLockByHash(inv.hash, o)) {
-                    connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::ISLOCK, o));
+                    const auto msg_type = inv.type == MSG_ISLOCK ? NetMsgType::ISLOCK : NetMsgType::ISDLOCK;
+                    connman->PushMessage(pfrom, msgMaker.Make(msg_type, o));
                     push = true;
                 }
             }
@@ -4604,9 +4607,11 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
                     int nInvType = CCoinJoin::GetDSTX(hash) ? MSG_DSTX : MSG_TX;
                     queueAndMaybePushInv(CInv(nInvType, hash));
 
-                    uint256 islockHash;
-                    if (!llmq::quorumInstantSendManager->GetInstantSendLockHashByTxid(hash, islockHash)) continue;
-                    queueAndMaybePushInv(CInv(MSG_ISLOCK, islockHash));
+                    const auto islock = llmq::quorumInstantSendManager->GetInstantSendLockByTxid(hash);
+                    if (islock == nullptr) continue;
+                    if (pto->nVersion < LLMQS_PROTO_VERSION) continue;
+                    if (pto->nVersion < ISDLOCK_PROTO_VERSION && islock->IsDeterministic()) continue;
+                    queueAndMaybePushInv(CInv(islock->IsDeterministic() ? MSG_ISDLOCK : MSG_ISLOCK, ::SerializeHash(*islock)));
                 }
 
                 // Send an inv for the best ChainLock we have

src/protocol.cpp

@@ -80,6 +80,7 @@ const char* QGETDATA = "qgetdata";
 const char* QDATA = "qdata";
 const char *CLSIG="clsig";
 const char *ISLOCK="islock";
+const char *ISDLOCK="isdlock";
 const char *MNAUTH="mnauth";
 }; // namespace NetMsgType
 
@@ -157,6 +158,7 @@ const static std::string allNetMessageTypes[] = {
     NetMsgType::QDATA,
     NetMsgType::CLSIG,
     NetMsgType::ISLOCK,
+    NetMsgType::ISDLOCK,
     NetMsgType::MNAUTH,
 };
 const static std::vector<std::string> allNetMessageTypesVec(allNetMessageTypes, allNetMessageTypes+ARRAYLEN(allNetMessageTypes));
@@ -289,6 +291,7 @@ const char* CInv::GetCommandInternal() const
         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;
     }

src/protocol.h

@@ -291,6 +291,7 @@ extern const char* QGETDATA;
 extern const char* QDATA;
 extern const char *CLSIG;
 extern const char *ISLOCK;
+extern const char *ISDLOCK;
 extern const char *MNAUTH;
 };
 
@@ -459,6 +460,7 @@ enum GetDataMsg {
     MSG_QUORUM_RECOVERED_SIG = 28,
     MSG_CLSIG = 29,
     MSG_ISLOCK = 30,
+    MSG_ISDLOCK = 31,
 };
 
 /** inv message data */

src/version.h

@@ -11,7 +11,7 @@
  */
 
 
-static const int PROTOCOL_VERSION = 70219;
+static const int PROTOCOL_VERSION = 70220;
 
 //! initial proto version, to be increased after version/verack negotiation
 static const int INIT_PROTO_VERSION = 209;
@@ -48,6 +48,9 @@ static const int MNAUTH_NODE_VER_VERSION = 70218;
 //! introduction of QGETDATA/QDATA messages
 static const int LLMQ_DATA_MESSAGES_VERSION = 70219;
 
+//! introduction of instant send deterministic lock (ISDLOCK)
+static const int ISDLOCK_PROTO_VERSION = 70220;
+
 // Make sure that none of the values above collide with `ADDRV2_FORMAT`.
 
 #endif // BITCOIN_VERSION_H

test/functional/feature_llmq_is_cl_conflicts.py

@@ -35,11 +35,11 @@ def send_clsig(self, clsig):
         inv = msg_inv([CInv(29, hash)])
         self.send_message(inv)
 
-    def send_islock(self, islock):
+    def send_islock(self, islock, deterministic=False):
         hash = uint256_from_str(hash256(islock.serialize()))
         self.islocks[hash] = islock
 
-        inv = msg_inv([CInv(30, hash)])
+        inv = msg_inv([CInv(31 if deterministic else 30, hash)])
         self.send_message(inv)
 
     def on_getdata(self, message):
@@ -72,7 +72,8 @@ def run_test(self):
         self.test_chainlock_overrides_islock(False)
         self.test_chainlock_overrides_islock(True, False)
         self.test_chainlock_overrides_islock(True, True)
-        self.test_chainlock_overrides_islock_overrides_nonchainlock()
+        self.test_chainlock_overrides_islock_overrides_nonchainlock(False)
+        self.test_chainlock_overrides_islock_overrides_nonchainlock(True)
 
     def test_chainlock_overrides_islock(self, test_block_conflict, mine_confllicting=False):
         if not test_block_conflict:
@@ -191,7 +192,7 @@ def test_chainlock_overrides_islock(self, test_block_conflict, mine_confllicting
                 assert rawtx['instantlock']
                 assert not rawtx['instantlock_internal']
 
-    def test_chainlock_overrides_islock_overrides_nonchainlock(self):
+    def test_chainlock_overrides_islock_overrides_nonchainlock(self, deterministic):
         # create two raw TXs, they will conflict with each other
         rawtx1 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex']
         rawtx2 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex']
@@ -200,7 +201,7 @@ def test_chainlock_overrides_islock_overrides_nonchainlock(self):
         rawtx2_txid = encode(hash256(hex_str_to_bytes(rawtx2))[::-1], 'hex_codec').decode('ascii')
 
         # Create an ISLOCK but don't broadcast it yet
-        islock = self.create_islock(rawtx2)
+        islock = self.create_islock(rawtx2, deterministic)
 
         # Disable ChainLocks to avoid accidental locking
         self.nodes[0].spork("SPORK_19_CHAINLOCKS_ENABLED", 4070908800)
@@ -229,7 +230,7 @@ def test_chainlock_overrides_islock_overrides_nonchainlock(self):
 
         # Send the ISLOCK, which should result in the last 2 blocks to be invalidated, even though the nodes don't know
         # the locked transaction yet
-        self.test_node.send_islock(islock)
+        self.test_node.send_islock(islock, deterministic)
         time.sleep(5)
 
         assert self.nodes[0].getbestblockhash() == good_tip

test/functional/interface_zmq_dash.py

@@ -31,7 +31,7 @@
     hash256,
     msg_clsig,
     msg_inv,
-    msg_islock,
+    msg_isdlock,
     msg_tx,
     ser_string,
     uint256_from_str,
@@ -266,7 +266,7 @@ def test_instantsend_publishers(self):
         zmq_tx_lock_tx.deserialize(zmq_tx_lock_sig_stream)
         assert zmq_tx_lock_tx.is_valid()
         assert_equal(zmq_tx_lock_tx.hash, rpc_raw_tx_1['txid'])
-        zmq_tx_lock = msg_islock()
+        zmq_tx_lock = msg_isdlock()
         zmq_tx_lock.deserialize(zmq_tx_lock_sig_stream)
         assert_equal(uint256_to_string(zmq_tx_lock.txid), rpc_raw_tx_1['txid'])
         # Try to send the second transaction. This must throw an RPC error because it conflicts with rpc_raw_tx_1

test/functional/rpc_verifyislock.py

@@ -82,7 +82,7 @@ def run_test(self):
                 break
         assert selected_hash == oldest_quorum_hash
         # Create the ISLOCK, then mine a quorum to move the signing quorum out of the active set
-        islock = self.create_islock(rawtx)
+        islock = self.create_islock(rawtx, True)
         # Mine one block to trigger the "signHeight + dkgInterval" verification for the ISLOCK
         self.mine_quorum()
         # Verify the ISLOCK for a transaction that is not yet known by the node