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.

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) {
+    if (strCommand == NetMsgType::ISLOCK || strCommand == NetMsgType::ISDLOCK) {
-        auto islock = std::make_shared<CInstantSendLock>();
+        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,7 +790,7 @@ 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)) {
@@ -783,6 +798,21 @@ void CInstantSendManager::ProcessMessageInstantSendLock(const CNode* pfrom, cons
         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));
+        CInv inv(islock->IsDeterministic() ? MSG_ISDLOCK : MSG_ISLOCK, ::SerializeHash(*islock));
-        g_connman->RelayInvFiltered(inv, *tx, LLMQS_PROTO_VERSION);
+        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;
+                    const auto islock = llmq::quorumInstantSendManager->GetInstantSendLockByTxid(hash);
-                    if (!llmq::quorumInstantSendManager->GetInstantSendLockHashByTxid(hash, islockHash)) continue;
+                    if (islock == nullptr) continue;
-                    queueAndMaybePushInv(CInv(MSG_ISLOCK, islockHash));
+                    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 @@ class TestP2PConn(P2PInterface):
         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 @@ class LLMQ_IS_CL_Conflicts(DashTestFramework):
         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 @@ class LLMQ_IS_CL_Conflicts(DashTestFramework):
                 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 @@ class LLMQ_IS_CL_Conflicts(DashTestFramework):
         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 @@ class LLMQ_IS_CL_Conflicts(DashTestFramework):
 
         # 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 @@ from test_framework.messages import (
     hash256,
     msg_clsig,
     msg_inv,
-    msg_islock,
+    msg_isdlock,
     msg_tx,
     ser_string,
     uint256_from_str,
@@ -266,7 +266,7 @@ class DashZMQTest (DashTestFramework):
         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 @@ class RPCVerifyISLockTest(DashTestFramework):
                 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

test/functional/test_framework/messages.py

@@ -32,7 +32,7 @@ from test_framework.util import hex_str_to_bytes
 import dash_hash
 
 MIN_VERSION_SUPPORTED = 60001
-MY_VERSION = 70219  # LLMQ_DATA_MESSAGES_VERSION
+MY_VERSION = 70220  # ISDLOCK_PROTO_VERSION
 MY_SUBVERSION = b"/python-mininode-tester:0.0.3%s/"
 MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37)
 
@@ -1773,6 +1773,38 @@ class msg_islock:
         return "msg_islock(inputs=%s, txid=%064x)" % (repr(self.inputs), self.txid)
 
 
+class msg_isdlock:
+    __slots__ = ("nVersion", "inputs", "txid", "cycleHash", "sig")
+    command = b"isdlock"
+
+    def __init__(self, nVersion=1, inputs=[], txid=0, cycleHash=0, sig=b'\\x0' * 96):
+        self.nVersion = nVersion
+        self.inputs = inputs
+        self.txid = txid
+        self.cycleHash = cycleHash
+        self.sig = sig
+
+    def deserialize(self, f):
+        self.nVersion = struct.unpack("<B", f.read(1))[0]
+        self.inputs = deser_vector(f, COutPoint)
+        self.txid = deser_uint256(f)
+        self.cycleHash = deser_uint256(f)
+        self.sig = f.read(96)
+
+    def serialize(self):
+        r = b""
+        r += struct.pack("<B", self.nVersion)
+        r += ser_vector(self.inputs)
+        r += ser_uint256(self.txid)
+        r += ser_uint256(self.cycleHash)
+        r += self.sig
+        return r
+
+    def __repr__(self):
+        return "msg_isdlock(nVersion=%d, inputs=%s, txid=%064x, cycleHash=%064x)" % \
+               (self.nVersion, repr(self.inputs), self.txid, self.cycleHash)
+
+
 class msg_qsigshare:
     __slots__ = ("sig_shares",)
     command = b"qsigshare"

test/functional/test_framework/mininode.py

@@ -44,6 +44,7 @@ from test_framework.messages import (
     msg_headers,
     msg_inv,
     msg_islock,
+    msg_isdlock,
     msg_mempool,
     msg_mnlistdiff,
     msg_notfound,
@@ -102,6 +103,7 @@ MESSAGEMAP = {
     b"getsporks": None,
     b"govsync": None,
     b"islock": msg_islock,
+    b"isdlock": msg_isdlock,
     b"mnlistdiff": msg_mnlistdiff,
     b"notfound": msg_notfound,
     b"qfcommit": None,
@@ -408,6 +410,7 @@ class P2PInterface(P2PConnection):
     def on_mnlistdiff(self, message): pass
     def on_clsig(self, message): pass
     def on_islock(self, message): pass
+    def on_isdlock(self, message): pass
 
     def on_qgetdata(self, message): pass
     def on_qdata(self, message): pass

test/functional/test_framework/test_framework.py

@@ -26,6 +26,7 @@ from .messages import (
     FromHex,
     hash256,
     msg_islock,
+    msg_isdlock,
     ser_compact_size,
     ser_string,
 )
@@ -950,7 +951,7 @@ class DashTestFramework(BitcoinTestFramework):
         if wait_until(check_tx, timeout=timeout, sleep=0.5, do_assert=expected) and not expected:
             raise AssertionError("waiting unexpectedly succeeded")
 
-    def create_islock(self, hextx):
+    def create_islock(self, hextx, deterministic=False):
         tx = FromHex(CTransaction(), hextx)
         tx.rehash()
 
@@ -969,7 +970,14 @@ class DashTestFramework(BitcoinTestFramework):
                 quorum_member = mn
 
         rec_sig = self.get_recovered_sig(request_id, message_hash, node=quorum_member.node)
-        islock = msg_islock(inputs, tx.sha256, hex_str_to_bytes(rec_sig['sig']))
+
+        if deterministic:
+            block_count = quorum_member.node.getblockcount()
+            cycle_hash = int(quorum_member.node.getblockhash(block_count - (block_count % 24)), 16)
+            islock = msg_isdlock(1, inputs, tx.sha256, cycle_hash, hex_str_to_bytes(rec_sig['sig']))
+        else:
+            islock = msg_islock(inputs, tx.sha256, hex_str_to_bytes(rec_sig['sig']))
+
         return islock
 
     def wait_for_instantlock(self, txid, node, expected=True, timeout=15):