From b4d001a602e5d2869205324f2f1b9dfe5ec7b465 Mon Sep 17 00:00:00 2001 From: gabriel-bjg Date: Tue, 5 Oct 2021 19:42:34 +0200 Subject: [PATCH] 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 | 73 +++++++++++++++---- src/llmq/quorums_instantsend.h | 25 ++++++- src/net_processing.cpp | 15 ++-- src/protocol.cpp | 3 + src/protocol.h | 2 + src/version.h | 5 +- .../feature_llmq_is_cl_conflicts.py | 13 ++-- test/functional/interface_zmq_dash.py | 4 +- test/functional/rpc_verifyislock.py | 2 +- test/functional/test_framework/messages.py | 34 ++++++++- test/functional/test_framework/mininode.py | 3 + .../test_framework/test_framework.py | 12 ++- 12 files changed, 156 insertions(+), 35 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index f188e95810..ac17d810e3 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/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(); + ret = std::make_shared(CInstantSendLock::isdlock_version); bool exists = db->Read(std::make_tuple(DB_ISLOCK_BY_HASH, hash), *ret); if (!exists) { - ret = nullptr; + ret = std::make_shared(); + 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(); + 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(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 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()); diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index e0be2724e1..c9907c2f0c 100644 --- a/src/llmq/quorums_instantsend.h +++ b/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 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 CInstantSendLockPtr; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 08153e19c7..a97a6ea5ad 100644 --- a/src/net_processing.cpp +++ b/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 diff --git a/src/protocol.cpp b/src/protocol.cpp index 8406d2f79c..fc03795959 100644 --- a/src/protocol.cpp +++ b/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 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; } diff --git a/src/protocol.h b/src/protocol.h index 55d6f30d26..7bf1e5011d 100644 --- a/src/protocol.h +++ b/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 */ diff --git a/src/version.h b/src/version.h index e84ec8c746..0fd5440302 100644 --- a/src/version.h +++ b/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 diff --git a/test/functional/feature_llmq_is_cl_conflicts.py b/test/functional/feature_llmq_is_cl_conflicts.py index ee46e13d99..6dab6e4d24 100755 --- a/test/functional/feature_llmq_is_cl_conflicts.py +++ b/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 diff --git a/test/functional/interface_zmq_dash.py b/test/functional/interface_zmq_dash.py index 1d32bb7209..40964d0600 100755 --- a/test/functional/interface_zmq_dash.py +++ b/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 diff --git a/test/functional/rpc_verifyislock.py b/test/functional/rpc_verifyislock.py index 2a84090202..bec78afb59 100755 --- a/test/functional/rpc_verifyislock.py +++ b/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 diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 4cf6fbcbb0..b89a8258ef 100755 --- a/test/functional/test_framework/messages.py +++ b/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("