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 3b1a05e6425d..b5676bdc9388 100644 --- a/src/instantsend/net_instantsend.cpp +++ b/src/instantsend/net_instantsend.cpp @@ -14,98 +14,83 @@ #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; +#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, BATCH_VERIFIER_SOURCE_THRESHOLD}; + Uint256HashMap recSigs; + size_t verifyCount{0}; + size_t alreadyVerified{0}; +}; - // 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; +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; } - 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)); - } + return true; } -void NetInstantSend::Start() +std::optional NetInstantSend::ResolveCycleHeight(const uint256& cycle_hash) { - // can't start new thread if we have one running already - if (workThread.joinable()) { - assert(false); + auto cycle_height = m_is_manager.GetBlockHeight(cycle_hash); + if (cycle_height) { + return cycle_height; } - workThread = std::thread(&util::TraceThread, "isman", [this] { WorkThreadMain(); }); + 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; } -void NetInstantSend::Stop() +bool NetInstantSend::ValidateDeterministicCycleHeight( + int cycle_height, + const Consensus::LLMQParams& llmq_params, + NodeId node_id) { - // make sure to call Interrupt() first - if (!workInterrupt) { - assert(false); + // Deterministic islocks MUST use rotation based llmq + if (cycle_height % llmq_params.dkgInterval == 0) { + return true; } - if (workThread.joinable()) { - workThread.join(); - } + m_peer_manager->PeerMisbehaving(node_id, INVALID_ISLOCK_MISBEHAVIOR_SCORE); + return false; } -Uint256HashSet NetInstantSend::ProcessPendingInstantSendLocks( - const Consensus::LLMQParams& llmq_params, int signOffset, bool ban, - const std::vector>& pend) +std::unique_ptr NetInstantSend::BuildVerificationBatch( + const Consensus::LLMQParams& llmq_params, + int signOffset, + const std::vector& pend) { - CBLSBatchVerifier batchVerifier(false, true, 8); - Uint256HashMap recSigs; + 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; } - if (!islock->sig.Get().IsValid()) { - batchVerifier.badSources.emplace(nodeId); + CBLSSignature sig = islock->sig.Get(); + if (!sig.IsValid()) { + data->batchVerifier.badSources.emplace(nodeId); continue; } @@ -113,13 +98,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; } @@ -137,46 +122,52 @@ 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, islock->sig.Get(), 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; - 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; + 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 = data.batchVerifier.badSources.count(nodeId); + const bool message_bad = data.batchVerifier.badMessages.count(hash); - if (batchVerifier.badMessages.count(hash)) { - LogPrint(BCLog::INSTANTSEND, "NetInstantSend::%s -- txid=%s, islock=%s: invalid sig in islock, peer=%d\n", + 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, OLD_ACTIVE_SET_FAILURE_MISBEHAVIOR_SCORE); + } + if (message_bad) { + badISLocks.emplace(hash); + } continue; } @@ -191,10 +182,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 */ @@ -209,8 +199,90 @@ 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; + + const NodeId from = pfrom.GetId(); + uint256 hash = ::SerializeHash(*islock); + + WITH_LOCK(::cs_main, m_peer_manager->PeerEraseObjectRequest(from, CInv{MSG_ISDLOCK, hash})); + + if (!ValidateIncomingISLock(*islock, from)) { + return; + } + + 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; + } + + auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; + const auto& llmq_params_opt = Params().GetLLMQ(llmqType); + assert(llmq_params_opt); + 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(), from); + + m_is_manager.EnqueueInstantSendLock(from, 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; @@ -225,11 +297,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 @@ -253,7 +325,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; } } diff --git a/src/instantsend/net_instantsend.h b/src/instantsend/net_instantsend.h index d21ffb734ef1..5601cbb5f1c2 100644 --- a/src/instantsend/net_instantsend.h +++ b/src/instantsend/net_instantsend.h @@ -6,13 +6,21 @@ #define BITCOIN_INSTANTSEND_NET_INSTANTSEND_H #include +#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 +48,25 @@ class NetInstantSend final : public NetHandler void WorkThreadMain(); private: - void ProcessPendingISLocks(std::vector>&& locks_to_process); + 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); + 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;