From f1c164de8a8f2b20c98cb55342893f4ba0fa5acb Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 10 Feb 2026 08:36:05 -0600 Subject: [PATCH 1/4] refactor: cache BLS sig and unify bad-source/bad-message handling in ProcessPendingInstantSendLocks Materialize CBLSLazySignature::Get() into a local CBLSSignature once instead of calling it twice (for IsValid and PushMessage). Merge the separate badSources penalization loop and badMessages check into a single pass over pending locks, using a `penalized` set to deduplicate PeerMisbehaving calls. --- src/instantsend/net_instantsend.cpp | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/instantsend/net_instantsend.cpp b/src/instantsend/net_instantsend.cpp index 3b1a05e6425d..70849f8ca7cc 100644 --- a/src/instantsend/net_instantsend.cpp +++ b/src/instantsend/net_instantsend.cpp @@ -104,7 +104,8 @@ Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( continue; } - if (!islock->sig.Get().IsValid()) { + CBLSSignature sig = islock->sig.Get(); + if (!sig.IsValid()) { batchVerifier.badSources.emplace(nodeId); continue; } @@ -140,7 +141,7 @@ Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( return {}; } uint256 signHash = llmq::SignHash{llmq_params.type, quorum->qc->quorumHash, id, islock->txid}.Get(); - batchVerifier.PushMessage(nodeId, hash, signHash, islock->sig.Get(), quorum->qc->quorumPublicKey); + batchVerifier.PushMessage(nodeId, hash, signHash, sig, quorum->qc->quorumPublicKey); verifyCount++; // We can reconstruct the CRecoveredSig objects from the islock and pass it to the signing manager, which @@ -160,23 +161,27 @@ Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( __func__, verifyCount, alreadyVerified, verifyTimer.count(), batchVerifier.GetUniqueSourceCount()); Uint256HashSet badISLocks; + std::set penalized; - if (ban && !batchVerifier.badSources.empty()) { - for (const auto& nodeId : batchVerifier.badSources) { - // Let's not be too harsh, as the peer might simply be unlucky and might have sent us an old lock which - // does not validate anymore due to changed quorums - m_peer_manager->PeerMisbehaving(nodeId, 20); - } - } for (const auto& p : pend) { const auto& hash = p.first; auto nodeId = p.second.node_id; const auto& islock = p.second.islock; - if (batchVerifier.badMessages.count(hash)) { - LogPrint(BCLog::INSTANTSEND, "NetInstantSend::%s -- txid=%s, islock=%s: invalid sig in islock, peer=%d\n", + const bool source_bad = batchVerifier.badSources.count(nodeId); + const bool message_bad = batchVerifier.badMessages.count(hash); + + if (source_bad || message_bad) { + LogPrint(BCLog::INSTANTSEND, "NetInstantSend::%s -- txid=%s, islock=%s: verification failed, peer=%d\n", __func__, islock->txid.ToString(), hash.ToString(), nodeId); - badISLocks.emplace(hash); + if (ban && source_bad && penalized.emplace(nodeId).second) { + // Let's not be too harsh, as the peer might simply be unlucky and might have sent us + // an old lock which does not validate anymore due to changed quorums + m_peer_manager->PeerMisbehaving(nodeId, 20); + } + if (message_bad) { + badISLocks.emplace(hash); + } continue; } From ece1b84826a72452fed9c0f010bf86fc81ffe926 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 12 Feb 2026 15:07:47 -0600 Subject: [PATCH 2/4] refactor: split IS lock verification flow and reduce header coupling --- src/instantsend/instantsend.cpp | 4 +- src/instantsend/instantsend.h | 6 +- src/instantsend/net_instantsend.cpp | 250 +++++++++++++++------------- src/instantsend/net_instantsend.h | 26 ++- 4 files changed, 166 insertions(+), 120 deletions(-) diff --git a/src/instantsend/instantsend.cpp b/src/instantsend/instantsend.cpp index 636cfbbef3f3..449ceeaf5458 100644 --- a/src/instantsend/instantsend.cpp +++ b/src/instantsend/instantsend.cpp @@ -101,13 +101,13 @@ instantsend::PendingState CInstantSendManager::FetchPendingLocks() std::vector removed; removed.reserve(std::min(maxCount, pendingInstantSendLocks.size())); - for (auto& [islockHash, nodeid_islptr_pair] : pendingInstantSendLocks) { + for (auto& [islockHash, pending] : pendingInstantSendLocks) { // Check if we've reached max count if (ret.m_pending_is.size() >= maxCount) { ret.m_pending_work = true; break; } - ret.m_pending_is.emplace_back(islockHash, std::move(nodeid_islptr_pair)); + ret.m_pending_is.push_back(instantsend::PendingISLockEntry{std::move(pending), islockHash}); removed.emplace_back(islockHash); } diff --git a/src/instantsend/instantsend.h b/src/instantsend/instantsend.h index af326ce1949e..8c3ec763f05d 100644 --- a/src/instantsend/instantsend.h +++ b/src/instantsend/instantsend.h @@ -49,9 +49,13 @@ struct PendingISLockFromPeer { InstantSendLockPtr islock; }; +struct PendingISLockEntry : PendingISLockFromPeer { + uint256 islock_hash; +}; + struct PendingState { bool m_pending_work{false}; - std::vector> m_pending_is; + std::vector m_pending_is; }; } // namespace instantsend diff --git a/src/instantsend/net_instantsend.cpp b/src/instantsend/net_instantsend.cpp index 70849f8ca7cc..effe962417a4 100644 --- a/src/instantsend/net_instantsend.cpp +++ b/src/instantsend/net_instantsend.cpp @@ -14,99 +14,34 @@ #include #include -void NetInstantSend::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) -{ - if (msg_type != NetMsgType::ISDLOCK) { - return; - } - - if (!m_is_manager.IsInstantSendEnabled()) return; - - auto islock = std::make_shared(); - vRecv >> *islock; - - uint256 hash = ::SerializeHash(*islock); - - WITH_LOCK(::cs_main, m_peer_manager->PeerEraseObjectRequest(pfrom.GetId(), CInv{MSG_ISDLOCK, hash})); - - if (!islock->TriviallyValid()) { - m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100); - return; - } - - auto cycleHeightOpt = m_is_manager.GetBlockHeight(islock->cycleHash); - if (!cycleHeightOpt) { - 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 - m_peer_manager->PeerMisbehaving(pfrom.GetId(), 1); - return; - } - m_is_manager.CacheBlockHeight(blockIndex); - cycleHeightOpt = blockIndex->nHeight; - } - const int block_height = *cycleHeightOpt; - - // Deterministic islocks MUST use rotation based llmq - auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; - const auto& llmq_params_opt = Params().GetLLMQ(llmqType); - assert(llmq_params_opt); - if (block_height % llmq_params_opt->dkgInterval != 0) { - m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100); - return; - } +#include - if (!m_is_manager.AlreadyHave(CInv{MSG_ISDLOCK, hash})) { - LogPrint(BCLog::INSTANTSEND, "NetInstantSend -- ISDLOCK txid=%s, islock=%s: received islock, peer=%d\n", - islock->txid.ToString(), hash.ToString(), pfrom.GetId()); - - m_is_manager.EnqueueInstantSendLock(pfrom.GetId(), hash, std::move(islock)); - } -} - -void NetInstantSend::Start() -{ - // can't start new thread if we have one running already - if (workThread.joinable()) { - assert(false); - } - - workThread = std::thread(&util::TraceThread, "isman", [this] { WorkThreadMain(); }); -} - -void NetInstantSend::Stop() -{ - // make sure to call Interrupt() first - if (!workInterrupt) { - assert(false); - } - - if (workThread.joinable()) { - workThread.join(); - } -} - -Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( - const Consensus::LLMQParams& llmq_params, int signOffset, bool ban, - const std::vector>& pend) -{ - CBLSBatchVerifier batchVerifier(false, true, 8); +struct NetInstantSend::BatchVerificationData { + CBLSBatchVerifier batchVerifier{false, true, 8}; Uint256HashMap recSigs; + size_t verifyCount{0}; + size_t alreadyVerified{0}; +}; + +std::unique_ptr NetInstantSend::BuildVerificationBatch( + const Consensus::LLMQParams& llmq_params, + int signOffset, + const std::vector& pend) +{ + auto data = std::make_unique(); - size_t verifyCount = 0; - size_t alreadyVerified = 0; - for (const auto& p : pend) { - const auto& hash = p.first; - auto nodeId = p.second.node_id; - const auto& islock = p.second.islock; + for (const auto& pending : pend) { + const auto& hash = pending.islock_hash; + auto nodeId = pending.node_id; + const auto& islock = pending.islock; - if (batchVerifier.badSources.count(nodeId)) { + if (data->batchVerifier.badSources.count(nodeId)) { continue; } CBLSSignature sig = islock->sig.Get(); if (!sig.IsValid()) { - batchVerifier.badSources.emplace(nodeId); + data->batchVerifier.badSources.emplace(nodeId); continue; } @@ -114,13 +49,13 @@ Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( // no need to verify an ISLOCK if we already have verified the recovered sig that belongs to it if (m_is_manager.Sigman().HasRecoveredSig(llmq_params.type, id, islock->txid)) { - alreadyVerified++; + data->alreadyVerified++; continue; } auto cycleHeightOpt = m_is_manager.GetBlockHeight(islock->cycleHash); if (!cycleHeightOpt) { - batchVerifier.badSources.emplace(nodeId); + data->batchVerifier.badSources.emplace(nodeId); continue; } @@ -138,38 +73,40 @@ Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( if (!quorum) { // should not happen, but if one fails to select, all others will also fail to select - return {}; + return nullptr; } uint256 signHash = llmq::SignHash{llmq_params.type, quorum->qc->quorumHash, id, islock->txid}.Get(); - batchVerifier.PushMessage(nodeId, hash, signHash, sig, quorum->qc->quorumPublicKey); - verifyCount++; + data->batchVerifier.PushMessage(nodeId, hash, signHash, sig, quorum->qc->quorumPublicKey); + data->verifyCount++; // We can reconstruct the CRecoveredSig objects from the islock and pass it to the signing manager, which // avoids unnecessary double-verification of the signature. We however only do this when verification here // turns out to be good (which is checked further down) if (!m_is_manager.Sigman().HasRecoveredSigForId(llmq_params.type, id)) { - recSigs.try_emplace(hash, llmq::CRecoveredSig(llmq_params.type, quorum->qc->quorumHash, id, islock->txid, - islock->sig)); + data->recSigs.try_emplace(hash, llmq::CRecoveredSig(llmq_params.type, quorum->qc->quorumHash, id, islock->txid, + islock->sig)); } } - cxxtimer::Timer verifyTimer(true); - batchVerifier.Verify(); - verifyTimer.stop(); - - LogPrint(BCLog::INSTANTSEND, "NetInstantSend::%s -- verified locks. count=%d, alreadyVerified=%d, vt=%d, nodes=%d\n", - __func__, verifyCount, alreadyVerified, verifyTimer.count(), batchVerifier.GetUniqueSourceCount()); + return data; +} +Uint256HashSet NetInstantSend::ApplyVerificationResults( + const Consensus::LLMQParams& llmq_params, + bool ban, + BatchVerificationData& data, + const std::vector& pend) +{ Uint256HashSet badISLocks; std::set penalized; - for (const auto& p : pend) { - const auto& hash = p.first; - auto nodeId = p.second.node_id; - const auto& islock = p.second.islock; + for (const auto& pending : pend) { + const auto& hash = pending.islock_hash; + auto nodeId = pending.node_id; + const auto& islock = pending.islock; - const bool source_bad = batchVerifier.badSources.count(nodeId); - const bool message_bad = batchVerifier.badMessages.count(hash); + const bool source_bad = data.batchVerifier.badSources.count(nodeId); + const bool message_bad = data.batchVerifier.badMessages.count(hash); if (source_bad || message_bad) { LogPrint(BCLog::INSTANTSEND, "NetInstantSend::%s -- txid=%s, islock=%s: verification failed, peer=%d\n", @@ -196,10 +133,9 @@ Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( assert(std::holds_alternative(ret)); } - // See comment further on top. We pass a reconstructed recovered sig to the signing manager to avoid - // double-verification of the sig. - auto it = recSigs.find(hash); - if (it != recSigs.end()) { + // Pass a reconstructed recovered sig to the signing manager to avoid double-verification of the sig. + auto it = data.recSigs.find(hash); + if (it != data.recSigs.end()) { auto recSig = std::make_shared(std::move(it->second)); if (!m_is_manager.Sigman().HasRecoveredSigForId(llmq_params.type, recSig->getId())) { LogPrint(BCLog::INSTANTSEND, /* Continued */ @@ -214,8 +150,98 @@ Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( return badISLocks; } +void NetInstantSend::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) +{ + if (msg_type != NetMsgType::ISDLOCK) { + return; + } + + if (!m_is_manager.IsInstantSendEnabled()) return; + + auto islock = std::make_shared(); + vRecv >> *islock; + + uint256 hash = ::SerializeHash(*islock); + + WITH_LOCK(::cs_main, m_peer_manager->PeerEraseObjectRequest(pfrom.GetId(), CInv{MSG_ISDLOCK, hash})); + + if (!islock->TriviallyValid()) { + m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100); + return; + } + + auto cycleHeightOpt = m_is_manager.GetBlockHeight(islock->cycleHash); + if (!cycleHeightOpt) { + 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 + m_peer_manager->PeerMisbehaving(pfrom.GetId(), 1); + return; + } + m_is_manager.CacheBlockHeight(blockIndex); + cycleHeightOpt = blockIndex->nHeight; + } + const int block_height = *cycleHeightOpt; + + // Deterministic islocks MUST use rotation based llmq + auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; + const auto& llmq_params_opt = Params().GetLLMQ(llmqType); + assert(llmq_params_opt); + if (block_height % llmq_params_opt->dkgInterval != 0) { + m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100); + return; + } + + if (!m_is_manager.AlreadyHave(CInv{MSG_ISDLOCK, hash})) { + LogPrint(BCLog::INSTANTSEND, "NetInstantSend -- ISDLOCK txid=%s, islock=%s: received islock, peer=%d\n", + islock->txid.ToString(), hash.ToString(), pfrom.GetId()); + + m_is_manager.EnqueueInstantSendLock(pfrom.GetId(), hash, std::move(islock)); + } +} + +void NetInstantSend::Start() +{ + // can't start new thread if we have one running already + if (workThread.joinable()) { + assert(false); + } + + workThread = std::thread(&util::TraceThread, "isman", [this] { WorkThreadMain(); }); +} + +void NetInstantSend::Stop() +{ + // make sure to call Interrupt() first + if (!workInterrupt) { + assert(false); + } + + if (workThread.joinable()) { + workThread.join(); + } +} + +Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( + const Consensus::LLMQParams& llmq_params, int signOffset, bool ban, + const std::vector& pend) +{ + auto batch = BuildVerificationBatch(llmq_params, signOffset, pend); + if (!batch) return {}; + + cxxtimer::Timer verifyTimer(true); + batch->batchVerifier.Verify(); + verifyTimer.stop(); + + LogPrint(BCLog::INSTANTSEND, "NetInstantSend::%s -- verified locks. count=%d, alreadyVerified=%d, vt=%d, nodes=%d\n", + __func__, batch->verifyCount, batch->alreadyVerified, + verifyTimer.count(), batch->batchVerifier.GetUniqueSourceCount()); + + return ApplyVerificationResults(llmq_params, ban, *batch, pend); +} + -void NetInstantSend::ProcessPendingISLocks(std::vector>&& locks_to_process) +void NetInstantSend::ProcessPendingISLocks(std::vector&& locks_to_process) { // TODO Investigate if leaving this is ok auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; @@ -230,11 +256,11 @@ void NetInstantSend::ProcessPendingISLocks(std::vector> still_pending; + std::vector still_pending; still_pending.reserve(bad_is_locks.size()); - for (auto& p : locks_to_process) { - if (bad_is_locks.contains(p.first)) { - still_pending.emplace_back(std::move(p)); + for (auto& pending : locks_to_process) { + if (bad_is_locks.contains(pending.islock_hash)) { + still_pending.emplace_back(std::move(pending)); } } // Now check against the previous active set and perform banning if this fails diff --git a/src/instantsend/net_instantsend.h b/src/instantsend/net_instantsend.h index d21ffb734ef1..b1681a85ad17 100644 --- a/src/instantsend/net_instantsend.h +++ b/src/instantsend/net_instantsend.h @@ -6,13 +6,19 @@ #define BITCOIN_INSTANTSEND_NET_INSTANTSEND_H #include +#include #include +#include +#include +#include + +namespace Consensus { +struct LLMQParams; +} // namespace Consensus namespace instantsend { -struct InstantSendLock; -struct PendingISLockFromPeer; -using InstantSendLockPtr = std::shared_ptr; +struct PendingISLockEntry; } // namespace instantsend namespace llmq { class CInstantSendManager; @@ -40,11 +46,21 @@ class NetInstantSend final : public NetHandler void WorkThreadMain(); private: - void ProcessPendingISLocks(std::vector>&& locks_to_process); + struct BatchVerificationData; + + std::unique_ptr BuildVerificationBatch( + const Consensus::LLMQParams& llmq_params, int signOffset, + const std::vector& pend); + Uint256HashSet ApplyVerificationResults( + const Consensus::LLMQParams& llmq_params, bool ban, + BatchVerificationData& data, + const std::vector& pend); + + void ProcessPendingISLocks(std::vector&& locks_to_process); Uint256HashSet ProcessPendingInstantSendLocks( const Consensus::LLMQParams& llmq_params, int signOffset, bool ban, - const std::vector>& pend); + const std::vector& pend); llmq::CInstantSendManager& m_is_manager; llmq::CQuorumManager& m_qman; const CChainState& m_chainstate; From 6e07c83f4ec3388c4945ede903b5061586bbf7cb Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 12 Feb 2026 15:16:29 -0600 Subject: [PATCH 3/4] refactor: replace net instantsend magic numbers with named constants --- src/instantsend/net_instantsend.cpp | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/instantsend/net_instantsend.cpp b/src/instantsend/net_instantsend.cpp index effe962417a4..a089114284bc 100644 --- a/src/instantsend/net_instantsend.cpp +++ b/src/instantsend/net_instantsend.cpp @@ -14,10 +14,19 @@ #include #include +#include #include +namespace { +constexpr int BATCH_VERIFIER_SOURCE_THRESHOLD{8}; +constexpr int INVALID_ISLOCK_MISBEHAVIOR_SCORE{100}; +constexpr int UNKNOWN_CYCLE_HASH_MISBEHAVIOR_SCORE{1}; +constexpr int OLD_ACTIVE_SET_FAILURE_MISBEHAVIOR_SCORE{20}; +constexpr auto WORK_THREAD_SLEEP_INTERVAL{std::chrono::milliseconds{100}}; +} // namespace + struct NetInstantSend::BatchVerificationData { - CBLSBatchVerifier batchVerifier{false, true, 8}; + CBLSBatchVerifier batchVerifier{false, true, BATCH_VERIFIER_SOURCE_THRESHOLD}; Uint256HashMap recSigs; size_t verifyCount{0}; size_t alreadyVerified{0}; @@ -114,7 +123,7 @@ Uint256HashSet NetInstantSend::ApplyVerificationResults( if (ban && source_bad && penalized.emplace(nodeId).second) { // Let's not be too harsh, as the peer might simply be unlucky and might have sent us // an old lock which does not validate anymore due to changed quorums - m_peer_manager->PeerMisbehaving(nodeId, 20); + m_peer_manager->PeerMisbehaving(nodeId, OLD_ACTIVE_SET_FAILURE_MISBEHAVIOR_SCORE); } if (message_bad) { badISLocks.emplace(hash); @@ -166,7 +175,7 @@ void NetInstantSend::ProcessMessage(CNode& pfrom, const std::string& msg_type, C WITH_LOCK(::cs_main, m_peer_manager->PeerEraseObjectRequest(pfrom.GetId(), CInv{MSG_ISDLOCK, hash})); if (!islock->TriviallyValid()) { - m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100); + m_peer_manager->PeerMisbehaving(pfrom.GetId(), INVALID_ISLOCK_MISBEHAVIOR_SCORE); return; } @@ -175,7 +184,7 @@ void NetInstantSend::ProcessMessage(CNode& pfrom, const std::string& msg_type, C 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 - m_peer_manager->PeerMisbehaving(pfrom.GetId(), 1); + m_peer_manager->PeerMisbehaving(pfrom.GetId(), UNKNOWN_CYCLE_HASH_MISBEHAVIOR_SCORE); return; } m_is_manager.CacheBlockHeight(blockIndex); @@ -188,7 +197,7 @@ void NetInstantSend::ProcessMessage(CNode& pfrom, const std::string& msg_type, C const auto& llmq_params_opt = Params().GetLLMQ(llmqType); assert(llmq_params_opt); if (block_height % llmq_params_opt->dkgInterval != 0) { - m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100); + m_peer_manager->PeerMisbehaving(pfrom.GetId(), INVALID_ISLOCK_MISBEHAVIOR_SCORE); return; } @@ -284,7 +293,7 @@ void NetInstantSend::WorkThreadMain() return more_work; }(); - if (!fMoreWork && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { + if (!fMoreWork && !workInterrupt.sleep_for(WORK_THREAD_SLEEP_INTERVAL)) { return; } } From 418dcda2b26befcc5c40ec798f9bdd764dab95d7 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 12 Feb 2026 15:16:38 -0600 Subject: [PATCH 4/4] refactor: extract ISLOCK message validation helpers --- src/instantsend/net_instantsend.cpp | 70 +++++++++++++++++++++-------- src/instantsend/net_instantsend.h | 6 +++ 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/instantsend/net_instantsend.cpp b/src/instantsend/net_instantsend.cpp index a089114284bc..b5676bdc9388 100644 --- a/src/instantsend/net_instantsend.cpp +++ b/src/instantsend/net_instantsend.cpp @@ -32,6 +32,46 @@ struct NetInstantSend::BatchVerificationData { size_t alreadyVerified{0}; }; +bool NetInstantSend::ValidateIncomingISLock(const instantsend::InstantSendLock& islock, NodeId node_id) +{ + if (!islock.TriviallyValid()) { + m_peer_manager->PeerMisbehaving(node_id, INVALID_ISLOCK_MISBEHAVIOR_SCORE); + return false; + } + + return true; +} + +std::optional NetInstantSend::ResolveCycleHeight(const uint256& cycle_hash) +{ + auto cycle_height = m_is_manager.GetBlockHeight(cycle_hash); + if (cycle_height) { + return cycle_height; + } + + const auto block_index = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(cycle_hash)); + if (block_index == nullptr) { + return std::nullopt; + } + + m_is_manager.CacheBlockHeight(block_index); + return block_index->nHeight; +} + +bool NetInstantSend::ValidateDeterministicCycleHeight( + int cycle_height, + const Consensus::LLMQParams& llmq_params, + NodeId node_id) +{ + // Deterministic islocks MUST use rotation based llmq + if (cycle_height % llmq_params.dkgInterval == 0) { + return true; + } + + m_peer_manager->PeerMisbehaving(node_id, INVALID_ISLOCK_MISBEHAVIOR_SCORE); + return false; +} + std::unique_ptr NetInstantSend::BuildVerificationBatch( const Consensus::LLMQParams& llmq_params, int signOffset, @@ -170,42 +210,34 @@ void NetInstantSend::ProcessMessage(CNode& pfrom, const std::string& msg_type, C auto islock = std::make_shared(); vRecv >> *islock; + const NodeId from = pfrom.GetId(); uint256 hash = ::SerializeHash(*islock); - WITH_LOCK(::cs_main, m_peer_manager->PeerEraseObjectRequest(pfrom.GetId(), CInv{MSG_ISDLOCK, hash})); + WITH_LOCK(::cs_main, m_peer_manager->PeerEraseObjectRequest(from, CInv{MSG_ISDLOCK, hash})); - if (!islock->TriviallyValid()) { - m_peer_manager->PeerMisbehaving(pfrom.GetId(), INVALID_ISLOCK_MISBEHAVIOR_SCORE); + if (!ValidateIncomingISLock(*islock, from)) { return; } - auto cycleHeightOpt = m_is_manager.GetBlockHeight(islock->cycleHash); - if (!cycleHeightOpt) { - 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 - m_peer_manager->PeerMisbehaving(pfrom.GetId(), UNKNOWN_CYCLE_HASH_MISBEHAVIOR_SCORE); - return; - } - m_is_manager.CacheBlockHeight(blockIndex); - cycleHeightOpt = blockIndex->nHeight; + auto cycle_height = ResolveCycleHeight(islock->cycleHash); + if (!cycle_height) { + // Maybe we don't have the block yet or maybe some peer spams invalid values for cycleHash + m_peer_manager->PeerMisbehaving(from, UNKNOWN_CYCLE_HASH_MISBEHAVIOR_SCORE); + return; } - const int block_height = *cycleHeightOpt; - // Deterministic islocks MUST use rotation based llmq auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; const auto& llmq_params_opt = Params().GetLLMQ(llmqType); assert(llmq_params_opt); - if (block_height % llmq_params_opt->dkgInterval != 0) { - m_peer_manager->PeerMisbehaving(pfrom.GetId(), INVALID_ISLOCK_MISBEHAVIOR_SCORE); + if (!ValidateDeterministicCycleHeight(*cycle_height, *llmq_params_opt, from)) { return; } if (!m_is_manager.AlreadyHave(CInv{MSG_ISDLOCK, hash})) { LogPrint(BCLog::INSTANTSEND, "NetInstantSend -- ISDLOCK txid=%s, islock=%s: received islock, peer=%d\n", - islock->txid.ToString(), hash.ToString(), pfrom.GetId()); + islock->txid.ToString(), hash.ToString(), from); - m_is_manager.EnqueueInstantSendLock(pfrom.GetId(), hash, std::move(islock)); + m_is_manager.EnqueueInstantSendLock(from, hash, std::move(islock)); } } diff --git a/src/instantsend/net_instantsend.h b/src/instantsend/net_instantsend.h index b1681a85ad17..5601cbb5f1c2 100644 --- a/src/instantsend/net_instantsend.h +++ b/src/instantsend/net_instantsend.h @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -18,6 +19,7 @@ namespace Consensus { struct LLMQParams; } // namespace Consensus namespace instantsend { +struct InstantSendLock; struct PendingISLockEntry; } // namespace instantsend namespace llmq { @@ -48,6 +50,10 @@ class NetInstantSend final : public NetHandler private: struct BatchVerificationData; + bool ValidateIncomingISLock(const instantsend::InstantSendLock& islock, NodeId node_id); + std::optional ResolveCycleHeight(const uint256& cycle_hash); + bool ValidateDeterministicCycleHeight(int cycle_height, const Consensus::LLMQParams& llmq_params, NodeId node_id); + std::unique_ptr BuildVerificationBatch( const Consensus::LLMQParams& llmq_params, int signOffset, const std::vector& pend);