diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h index 382af36306..5a615a6ce9 100644 --- a/src/coinjoin/client.h +++ b/src/coinjoin/client.h @@ -8,7 +8,7 @@ #include #include -#include +#include #include #include diff --git a/src/coinjoin/server.h b/src/coinjoin/server.h index f5539cd783..5f528a2bfb 100644 --- a/src/coinjoin/server.h +++ b/src/coinjoin/server.h @@ -7,7 +7,7 @@ #include -#include +#include class CActiveMasternodeManager; class CChainState; diff --git a/src/evo/mnauth.h b/src/evo/mnauth.h index c109961d8e..514503c39f 100644 --- a/src/evo/mnauth.h +++ b/src/evo/mnauth.h @@ -6,7 +6,7 @@ #define BITCOIN_EVO_MNAUTH_H #include -#include +#include #include class CActiveMasternodeManager; diff --git a/src/governance/governance.h b/src/governance/governance.h index aa1dec1306..1417344551 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -10,7 +10,7 @@ #include #include -#include +#include #include #include diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index eec397ec51..b11ab32f81 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include @@ -44,14 +43,16 @@ static const std::string DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT_Q_INDEXED = "q_m static const std::string DB_BEST_BLOCK_UPGRADE = "q_bbu2"; -CQuorumBlockProcessor::CQuorumBlockProcessor(CChainState& chainstate, CDeterministicMNManager& dmnman, CEvoDB& evoDb, - const std::unique_ptr& peerman) : - m_chainstate(chainstate), m_dmnman(dmnman), m_evoDb(evoDb), m_peerman(peerman) +CQuorumBlockProcessor::CQuorumBlockProcessor(CChainState& chainstate, CDeterministicMNManager& dmnman, CEvoDB& evoDb) : + m_chainstate(chainstate), + m_dmnman(dmnman), + m_evoDb(evoDb) { utils::InitQuorumsCache(mapHasMinedCommitmentCache); } -PeerMsgRet CQuorumBlockProcessor::ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv) +MessageProcessingResult CQuorumBlockProcessor::ProcessMessage(const CNode& peer, std::string_view msg_type, + CDataStream& vRecv) { if (msg_type != NetMsgType::QFCOMMITMENT) { return {}; @@ -60,19 +61,21 @@ PeerMsgRet CQuorumBlockProcessor::ProcessMessage(const CNode& peer, std::string_ CFinalCommitment qc; vRecv >> qc; - WITH_LOCK(::cs_main, Assert(m_peerman)->EraseObjectRequest(peer.GetId(), - CInv(MSG_QUORUM_FINAL_COMMITMENT, ::SerializeHash(qc)))); + MessageProcessingResult ret; + ret.m_to_erase = CInv{MSG_QUORUM_FINAL_COMMITMENT, ::SerializeHash(qc)}; if (qc.IsNull()) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- null commitment from peer=%d\n", __func__, peer.GetId()); - return tl::unexpected{100}; + ret.m_error = MisbehavingError{100}; + return ret; } const auto& llmq_params_opt = Params().GetLLMQ(qc.llmqType); if (!llmq_params_opt.has_value()) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- invalid commitment type %d from peer=%d\n", __func__, ToUnderlying(qc.llmqType), peer.GetId()); - return tl::unexpected{100}; + ret.m_error = MisbehavingError{100}; + return ret; } auto type = qc.llmqType; @@ -86,32 +89,33 @@ PeerMsgRet CQuorumBlockProcessor::ProcessMessage(const CNode& peer, std::string_ qc.quorumHash.ToString(), peer.GetId()); // can't really punish the node here, as we might simply be the one that is on the wrong chain or not // fully synced - return {}; + return ret; } if (m_chainstate.m_chain.Tip()->GetAncestor(pQuorumBaseBlockIndex->nHeight) != pQuorumBaseBlockIndex) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- block %s not in active chain, peer=%d\n", __func__, qc.quorumHash.ToString(), peer.GetId()); // same, can't punish - return {}; + return ret; } if (int quorumHeight = pQuorumBaseBlockIndex->nHeight - (pQuorumBaseBlockIndex->nHeight % llmq_params_opt->dkgInterval) + int(qc.quorumIndex); quorumHeight != pQuorumBaseBlockIndex->nHeight) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- block %s is not the first block in the DKG interval, peer=%d\n", __func__, qc.quorumHash.ToString(), peer.GetId()); - return tl::unexpected{100}; + ret.m_error = MisbehavingError{100}; + return ret; } if (pQuorumBaseBlockIndex->nHeight < (m_chainstate.m_chain.Height() - llmq_params_opt->dkgInterval)) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- block %s is too old, peer=%d\n", __func__, qc.quorumHash.ToString(), peer.GetId()); // TODO: enable punishment in some future version when all/most nodes are running with this fix - // return tl::unexpected{100}; - return {}; + // ret.m_error = MisbehavingError{100}; + return ret; } if (HasMinedCommitment(type, qc.quorumHash)) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- commitment for quorum hash[%s], type[%d], quorumIndex[%d] is already mined, peer=%d\n", __func__, qc.quorumHash.ToString(), ToUnderlying(type), qc.quorumIndex, peer.GetId()); // NOTE: do not punish here - return {}; + return ret; } } @@ -124,7 +128,7 @@ PeerMsgRet CQuorumBlockProcessor::ProcessMessage(const CNode& peer, std::string_ if (it != minableCommitmentsByQuorum.end()) { auto jt = minableCommitments.find(it->second); if (jt != minableCommitments.end() && jt->second.CountSigners() <= qc.CountSigners()) { - return {}; + return ret; } } } @@ -133,14 +137,15 @@ PeerMsgRet CQuorumBlockProcessor::ProcessMessage(const CNode& peer, std::string_ LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- commitment for quorum %s:%d is not valid quorumIndex[%d] nversion[%d], peer=%d\n", __func__, qc.quorumHash.ToString(), ToUnderlying(qc.llmqType), qc.quorumIndex, qc.nVersion, peer.GetId()); - return tl::unexpected{100}; + ret.m_error = MisbehavingError{100}; + return ret; } LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- received commitment for quorum %s:%d, validMembers=%d, signers=%d, peer=%d\n", __func__, qc.quorumHash.ToString(), ToUnderlying(qc.llmqType), qc.CountValidMembers(), qc.CountSigners(), peer.GetId()); - AddMineableCommitmentAndRelay(qc); - return {}; + ret.m_inventory = AddMineableCommitment(qc); + return ret; } bool CQuorumBlockProcessor::ProcessBlock(const CBlock& block, gsl::not_null pindex, BlockValidationState& state, bool fJustCheck, bool fBLSChecks) @@ -637,7 +642,7 @@ bool CQuorumBlockProcessor::HasMineableCommitment(const uint256& hash) const return minableCommitments.count(hash) != 0; } -std::optional CQuorumBlockProcessor::AddMineableCommitment(const CFinalCommitment& fqc) +std::optional CQuorumBlockProcessor::AddMineableCommitment(const CFinalCommitment& fqc) { const uint256 commitmentHash = ::SerializeHash(fqc); @@ -663,17 +668,7 @@ std::optional CQuorumBlockProcessor::AddMineableCommitment(const CFinal return false; }(); - return relay ? std::make_optional(commitmentHash) : std::nullopt; -} - -void CQuorumBlockProcessor::AddMineableCommitmentAndRelay(const CFinalCommitment& fqc) -{ - const auto commitmentHashOpt = AddMineableCommitment(fqc); - // We only relay the new commitment if it's new or better then the old one - if (commitmentHashOpt) { - CInv inv(MSG_QUORUM_FINAL_COMMITMENT, *commitmentHashOpt); - Assert(m_peerman)->RelayInv(inv); - } + return relay ? std::make_optional(CInv{MSG_QUORUM_FINAL_COMMITMENT, commitmentHash}) : std::nullopt; } bool CQuorumBlockProcessor::GetMineableCommitmentByHash(const uint256& commitmentHash, llmq::CFinalCommitment& ret) const diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index 2deeebfd90..e6cb2740dd 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -9,8 +9,8 @@ #include #include -#include #include +#include #include #include @@ -26,7 +26,6 @@ class CDataStream; class CDeterministicMNManager; class CEvoDB; class CNode; -class PeerManager; extern RecursiveMutex cs_main; @@ -42,7 +41,6 @@ private: CChainState& m_chainstate; CDeterministicMNManager& m_dmnman; CEvoDB& m_evoDb; - const std::unique_ptr& m_peerman; mutable Mutex minableCommitmentsCs; std::map, uint256> minableCommitmentsByQuorum GUARDED_BY(minableCommitmentsCs); @@ -51,15 +49,15 @@ private: mutable std::map> mapHasMinedCommitmentCache GUARDED_BY(minableCommitmentsCs); public: - explicit CQuorumBlockProcessor(CChainState& chainstate, CDeterministicMNManager& dmnman, CEvoDB& evoDb, - const std::unique_ptr& peerman); + explicit CQuorumBlockProcessor(CChainState& chainstate, CDeterministicMNManager& dmnman, CEvoDB& evoDb); - PeerMsgRet ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv); + MessageProcessingResult ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv); bool ProcessBlock(const CBlock& block, gsl::not_null pindex, BlockValidationState& state, bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool UndoBlock(const CBlock& block, gsl::not_null pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void AddMineableCommitmentAndRelay(const CFinalCommitment& fqc); + //! it returns hash of commitment if it should be relay, otherwise nullopt + std::optional AddMineableCommitment(const CFinalCommitment& fqc); bool HasMineableCommitment(const uint256& hash) const; bool GetMineableCommitmentByHash(const uint256& commitmentHash, CFinalCommitment& ret) const; std::optional> GetMineableCommitments(const Consensus::LLMQParams& llmqParams, int nHeight) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -74,8 +72,6 @@ public: std::vector> GetLastMinedCommitmentsPerQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, size_t cycle) const; std::optional GetLastMinedCommitmentsByQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, int quorumIndex, size_t cycle) const; private: - //! it returns hash of commitment if it should be relay, otherwise nullopt - std::optional AddMineableCommitment(const CFinalCommitment& fqc); static bool GetCommitmentsFromBlock(const CBlock& block, gsl::not_null pindex, std::multimap& ret, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool ProcessCommitment(int nHeight, const uint256& blockHash, const CFinalCommitment& qc, BlockValidationState& state, bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main); static bool IsMiningPhase(const Consensus::LLMQParams& llmqParams, const CChain& active_chain, int nHeight) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index cfd2938935..1ef7d22b91 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -23,14 +22,18 @@ #include #include +static bool ChainLocksSigningEnabled(const CSporkManager& sporkman) +{ + return sporkman.GetSporkValue(SPORK_19_CHAINLOCKS_ENABLED) == 0; +} + namespace llmq { std::unique_ptr chainLocksHandler; CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman, CSigSharesManager& _shareman, CSporkManager& sporkman, CTxMemPool& _mempool, - const CMasternodeSync& mn_sync, const std::unique_ptr& peerman, - bool is_masternode) : + const CMasternodeSync& mn_sync, bool is_masternode) : m_chainstate(chainstate), qman(_qman), sigman(_sigman), @@ -38,10 +41,10 @@ CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& spork_manager(sporkman), mempool(_mempool), m_mn_sync(mn_sync), - m_peerman(peerman), m_is_masternode{is_masternode}, scheduler(std::make_unique()), - scheduler_thread(std::make_unique(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))) + scheduler_thread( + std::make_unique(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))) { } @@ -93,31 +96,11 @@ CChainLockSig CChainLocksHandler::GetBestChainLock() const return bestChainLock; } -PeerMsgRet CChainLocksHandler::ProcessMessage(const CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) -{ - if (!AreChainLocksEnabled(spork_manager)) { - return {}; - } - - if (msg_type == NetMsgType::CLSIG) { - CChainLockSig clsig; - vRecv >> clsig; - - return ProcessNewChainLock(pfrom.GetId(), clsig, ::SerializeHash(clsig)); - } - return {}; -} - -PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq::CChainLockSig& clsig, const uint256& hash) +MessageProcessingResult CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq::CChainLockSig& clsig, + const uint256& hash) { CheckActiveState(); - CInv clsigInv(MSG_CLSIG, hash); - - if (from != -1) { - WITH_LOCK(::cs_main, Assert(m_peerman)->EraseObjectRequest(from, clsigInv)); - } - { LOCK(cs); if (!seenChainLocks.emplace(hash, GetTimeMillis()).second) { @@ -133,7 +116,7 @@ PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq if (const auto ret = VerifyChainLock(clsig); ret != VerifyRecSigStatus::Valid) { LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- invalid CLSIG (%s), status=%d peer=%d\n", __func__, clsig.ToString(), ToUnderlying(ret), from); if (from != -1) { - return tl::unexpected{10}; + return MisbehavingError{10}; } return {}; } @@ -162,14 +145,12 @@ PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq // Note: make sure to still relay clsig further. } - // Note: do not hold cs while calling RelayInv - AssertLockNotHeld(cs); - Assert(m_peerman)->RelayInv(clsigInv); + CInv clsigInv(MSG_CLSIG, hash); if (pindex == nullptr) { // we don't know the block/header for this CLSIG yet, so bail out for now // when the block or the header later comes in, we will enforce the correct chain - return {}; + return clsigInv; } scheduler->scheduleFromNow([&]() { @@ -179,7 +160,7 @@ PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- processed new CLSIG (%s), peer=%d\n", __func__, clsig.ToString(), from); - return {}; + return clsigInv; } void CChainLocksHandler::AcceptedBlockHeader(gsl::not_null pindexNew) @@ -520,10 +501,10 @@ void CChainLocksHandler::EnforceBestChainLock() uiInterface.NotifyChainLock(clsig->getBlockHash().ToString(), clsig->getHeight()); } -void CChainLocksHandler::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) +MessageProcessingResult CChainLocksHandler::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) { if (!isEnabled) { - return; + return {}; } CChainLockSig clsig; @@ -532,17 +513,17 @@ void CChainLocksHandler::HandleNewRecoveredSig(const llmq::CRecoveredSig& recove if (recoveredSig.getId() != lastSignedRequestId || recoveredSig.getMsgHash() != lastSignedMsgHash) { // this is not what we signed, so lets not create a CLSIG for it - return; + return {}; } if (bestChainLock.getHeight() >= lastSignedHeight) { // already got the same or a better CLSIG through the CLSIG message - return; + return {}; } clsig = CChainLockSig(lastSignedHeight, lastSignedMsgHash, recoveredSig.sig.Get()); } - ProcessNewChainLock(-1, clsig, ::SerializeHash(clsig)); + return ProcessNewChainLock(-1, clsig, ::SerializeHash(clsig)); } bool CChainLocksHandler::HasChainLock(int nHeight, const uint256& blockHash) const @@ -678,9 +659,4 @@ bool AreChainLocksEnabled(const CSporkManager& sporkman) return sporkman.IsSporkActive(SPORK_19_CHAINLOCKS_ENABLED); } -bool ChainLocksSigningEnabled(const CSporkManager& sporkman) -{ - return sporkman.GetSporkValue(SPORK_19_CHAINLOCKS_ENABLED) == 0; -} - } // namespace llmq diff --git a/src/llmq/chainlocks.h b/src/llmq/chainlocks.h index 79f96ccb68..2c44fb1ec6 100644 --- a/src/llmq/chainlocks.h +++ b/src/llmq/chainlocks.h @@ -29,7 +29,6 @@ class CMasternodeSync; class CScheduler; class CSporkManager; class CTxMemPool; -class PeerManager; namespace llmq { @@ -53,7 +52,6 @@ private: CSporkManager& spork_manager; CTxMemPool& mempool; const CMasternodeSync& m_mn_sync; - const std::unique_ptr& m_peerman; const bool m_is_masternode; std::unique_ptr scheduler; @@ -89,8 +87,7 @@ private: public: explicit CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman, CSigSharesManager& _shareman, CSporkManager& sporkman, CTxMemPool& _mempool, - const CMasternodeSync& mn_sync, const std::unique_ptr& peerman, - bool is_masternode); + const CMasternodeSync& mn_sync, bool is_masternode); ~CChainLocksHandler(); void Start(); @@ -100,8 +97,8 @@ public: bool GetChainLockByHash(const uint256& hash, CChainLockSig& ret) const EXCLUSIVE_LOCKS_REQUIRED(!cs); CChainLockSig GetBestChainLock() const EXCLUSIVE_LOCKS_REQUIRED(!cs); - PeerMsgRet ProcessMessage(const CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs); - PeerMsgRet ProcessNewChainLock(NodeId from, const CChainLockSig& clsig, const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(!cs); + [[nodiscard]] MessageProcessingResult ProcessNewChainLock(NodeId from, const CChainLockSig& clsig, + const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(!cs); void AcceptedBlockHeader(gsl::not_null pindexNew) EXCLUSIVE_LOCKS_REQUIRED(!cs); void UpdatedBlockTip(); @@ -111,7 +108,8 @@ public: void CheckActiveState() EXCLUSIVE_LOCKS_REQUIRED(!cs); void TrySignChainTip() EXCLUSIVE_LOCKS_REQUIRED(!cs); void EnforceBestChainLock() EXCLUSIVE_LOCKS_REQUIRED(!cs); - void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override EXCLUSIVE_LOCKS_REQUIRED(!cs); + [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override + EXCLUSIVE_LOCKS_REQUIRED(!cs); bool HasChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -132,8 +130,6 @@ private: extern std::unique_ptr chainLocksHandler; bool AreChainLocksEnabled(const CSporkManager& sporkman); -bool ChainLocksSigningEnabled(const CSporkManager& sporkman); - } // namespace llmq #endif // BITCOIN_LLMQ_CHAINLOCKS_H diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index aee90f9f8b..2c6c11446e 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -25,7 +25,7 @@ LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterminis is_masternode{mn_activeman != nullptr}, bls_worker{std::make_shared()}, dkg_debugman{std::make_unique()}, - quorum_block_processor{std::make_unique(chainstate, dmnman, evo_db, peerman)}, + quorum_block_processor{std::make_unique(chainstate, dmnman, evo_db)}, qdkgsman{std::make_unique(*bls_worker, chainstate, connman, dmnman, *dkg_debugman, mn_metaman, *quorum_block_processor, mn_activeman, sporkman, peerman, unit_tests, wipe)}, @@ -36,7 +36,8 @@ LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterminis shareman{std::make_unique(connman, *sigman, mn_activeman, *qman, sporkman, peerman)}, clhandler{[&]() -> llmq::CChainLocksHandler* const { assert(llmq::chainLocksHandler == nullptr); - llmq::chainLocksHandler = std::make_unique(chainstate, *qman, *sigman, *shareman, sporkman, mempool, mn_sync, peerman, is_masternode); + llmq::chainLocksHandler = std::make_unique(chainstate, *qman, *sigman, *shareman, + sporkman, mempool, mn_sync, is_masternode); return llmq::chainLocksHandler.get(); }()}, isman{[&]() -> llmq::CInstantSendManager* const { @@ -45,7 +46,7 @@ LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterminis return llmq::quorumInstantSendManager.get(); }()}, ehfSignalsHandler{ - std::make_unique(chainstate, mnhfman, *sigman, *shareman, mempool, *qman, peerman)} + std::make_unique(chainstate, mnhfman, *sigman, *shareman, mempool, *qman)} { } diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp index 661cafdcad..09e24bf429 100644 --- a/src/llmq/dkgsessionhandler.cpp +++ b/src/llmq/dkgsessionhandler.cpp @@ -612,7 +612,9 @@ void CDKGSessionHandler::HandleDKGRound() auto finalCommitments = curSession->FinalizeCommitments(); for (const auto& fqc : finalCommitments) { - quorumBlockProcessor.AddMineableCommitmentAndRelay(fqc); + if (auto inv_opt = quorumBlockProcessor.AddMineableCommitment(fqc); inv_opt.has_value()) { + Assert(m_peerman.get())->RelayInv(inv_opt.value()); + } } } diff --git a/src/llmq/ehf_signals.cpp b/src/llmq/ehf_signals.cpp index b5ec7a1e33..3ea7bf8677 100644 --- a/src/llmq/ehf_signals.cpp +++ b/src/llmq/ehf_signals.cpp @@ -14,7 +14,6 @@ #include #include #include // g_txindex -#include #include #include #include @@ -23,15 +22,13 @@ namespace llmq { CEHFSignalsHandler::CEHFSignalsHandler(CChainState& chainstate, CMNHFManager& mnhfman, CSigningManager& sigman, - CSigSharesManager& shareman, CTxMemPool& mempool, const CQuorumManager& qman, - const std::unique_ptr& peerman) : + CSigSharesManager& shareman, CTxMemPool& mempool, const CQuorumManager& qman) : chainstate(chainstate), mnhfman(mnhfman), sigman(sigman), shareman(shareman), mempool(mempool), - qman(qman), - m_peerman(peerman) + qman(qman) { sigman.RegisterRecoveredSigsListener(this); } @@ -94,7 +91,7 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind sigman.AsyncSignIfMember(llmqType, shareman, requestId, msgHash, quorum->qc->quorumHash, false, true); } -void CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) +MessageProcessingResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) { if (g_txindex) { g_txindex->BlockUntilSyncedToCurrentChain(); @@ -102,9 +99,10 @@ void CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig if (WITH_LOCK(cs, return ids.find(recoveredSig.getId()) == ids.end())) { // Do nothing, it's not for this handler - return; + return {}; } + MessageProcessingResult ret; const auto ehfSignals = mnhfman.GetSignalsStage(WITH_LOCK(cs_main, return chainstate.m_chain.Tip())); MNHFTxPayload mnhfPayload; for (const auto& deployment : Params().GetConsensus().vDeployments) { @@ -130,12 +128,13 @@ void CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig LOCK(cs_main); const MempoolAcceptResult result = AcceptToMemoryPool(chainstate, mempool, tx_to_sent, /* bypass_limits */ false); if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) { - Assert(m_peerman)->RelayTransaction(tx_to_sent->GetHash()); + ret.m_transactions.push_back(tx_to_sent->GetHash()); } else { LogPrintf("CEHFSignalsHandler::HandleNewRecoveredSig -- AcceptToMemoryPool failed: %s\n", result.m_state.ToString()); } } break; } + return ret; } } // namespace llmq diff --git a/src/llmq/ehf_signals.h b/src/llmq/ehf_signals.h index 22a01cf3fe..41f888d48d 100644 --- a/src/llmq/ehf_signals.h +++ b/src/llmq/ehf_signals.h @@ -13,7 +13,6 @@ class CBlockIndex; class CChainState; class CMNHFManager; class CTxMemPool; -class PeerManager; namespace llmq { @@ -30,7 +29,6 @@ private: CSigSharesManager& shareman; CTxMemPool& mempool; const CQuorumManager& qman; - const std::unique_ptr& m_peerman; /** * keep freshly generated IDs for easier filter sigs in HandleNewRecoveredSig @@ -39,8 +37,8 @@ private: std::set ids GUARDED_BY(cs); public: explicit CEHFSignalsHandler(CChainState& chainstate, CMNHFManager& mnhfman, CSigningManager& sigman, - CSigSharesManager& shareman, CTxMemPool& mempool, const CQuorumManager& qman, - const std::unique_ptr& peerman); + CSigSharesManager& shareman, CTxMemPool& mempool, const CQuorumManager& qman); + ~CEHFSignalsHandler(); @@ -49,7 +47,8 @@ public: */ void UpdatedBlockTip(const CBlockIndex* const pindexNew, bool is_masternode) EXCLUSIVE_LOCKS_REQUIRED(!cs); - void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override EXCLUSIVE_LOCKS_REQUIRED(!cs); + [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override + EXCLUSIVE_LOCKS_REQUIRED(!cs); private: void trySignEHFSignal(int bit, const CBlockIndex* const pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs); diff --git a/src/llmq/instantsend.cpp b/src/llmq/instantsend.cpp index 2d3b988332..713e874be5 100644 --- a/src/llmq/instantsend.cpp +++ b/src/llmq/instantsend.cpp @@ -622,14 +622,14 @@ bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebu return true; } -void CInstantSendManager::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) +MessageProcessingResult CInstantSendManager::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) { if (!IsInstantSendEnabled()) { - return; + return {}; } if (Params().GetConsensus().llmqTypeDIP0024InstantSend == Consensus::LLMQType::LLMQ_NONE) { - return; + return {}; } uint256 txid; @@ -641,6 +641,7 @@ void CInstantSendManager::HandleNewRecoveredSig(const CRecoveredSig& recoveredSi } else if (/*isInstantSendLock=*/ WITH_LOCK(cs_creating, return creatingInstantSendLocks.count(recoveredSig.getId()))) { HandleNewInstantSendLockRecoveredSig(recoveredSig); } + return {}; } void CInstantSendManager::HandleNewInputLockRecoveredSig(const CRecoveredSig& recoveredSig, const uint256& txid) diff --git a/src/llmq/instantsend.h b/src/llmq/instantsend.h index 196d7ab25b..d08522436f 100644 --- a/src/llmq/instantsend.h +++ b/src/llmq/instantsend.h @@ -330,7 +330,7 @@ public: bool IsWaitingForTx(const uint256& txHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks); CInstantSendLockPtr GetConflictingLock(const CTransaction& tx) const; - void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override + [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests, !cs_pendingLocks); PeerMsgRet ProcessMessage(const CNode& pfrom, std::string_view msg_type, CDataStream& vRecv); diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index 4756011e14..ad6d22c304 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -13,8 +13,8 @@ #include #include +#include -#include #include #include diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index e04f924ba4..c31ef78039 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -829,7 +829,7 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptrHandleNewRecoveredSig(*recoveredSig); + Assert(m_peerman)->PostProcessMessage(l->HandleNewRecoveredSig(*recoveredSig)); } GetMainSignals().NotifyRecoveredSig(recoveredSig); diff --git a/src/llmq/signing.h b/src/llmq/signing.h index ae2fd48224..540157f347 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -8,7 +8,7 @@ #include #include #include -#include +#include #include #include #include @@ -154,7 +154,7 @@ class CRecoveredSigsListener public: virtual ~CRecoveredSigsListener() = default; - virtual void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) = 0; + [[nodiscard]] virtual MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) = 0; }; class CSigningManager diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index da63536c99..0b157f218a 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -1553,10 +1553,11 @@ void CSigSharesManager::ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus } } -void CSigSharesManager::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) +MessageProcessingResult CSigSharesManager::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) { LOCK(cs); RemoveSigSharesForSession(recoveredSig.buildSignHash()); + return {}; } } // namespace llmq diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index b1f0acbf31..9f6d8a2e6b 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -432,7 +432,7 @@ public: std::optional CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const; void ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash); - void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override; + [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override; static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256& id, int attempt); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index dd484f51e9..5fa577ce05 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -593,8 +593,9 @@ public: bool IsInvInFilter(NodeId nodeid, const uint256& hash) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); private: - /** Helper to process result of external handlers of message */ + /** Helpers to process result of external handlers of message */ void ProcessPeerMsgRet(const PeerMsgRet& ret, CNode& pfrom) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void PostProcessMessage(MessageProcessingResult&& ret, NodeId node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ void ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seconds time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_msgproc_mutex); @@ -3318,6 +3319,22 @@ void PeerManagerImpl::ProcessPeerMsgRet(const PeerMsgRet& ret, CNode& pfrom) if (!ret) Misbehaving(pfrom.GetId(), ret.error().score, ret.error().message); } +void PeerManagerImpl::PostProcessMessage(MessageProcessingResult&& result, NodeId node) +{ + if (result.m_error) { + Misbehaving(node, result.m_error->score, result.m_error->message); + } + if (result.m_to_erase) { + WITH_LOCK(cs_main, EraseObjectRequest(node, result.m_to_erase.value())); + } + for (const auto& tx : result.m_transactions) { + WITH_LOCK(cs_main, RelayTransaction(tx)); + } + if (result.m_inventory) { + RelayInv(result.m_inventory.value(), MIN_PEER_PROTO_VERSION); + } +} + void PeerManagerImpl::ProcessMessage( CNode& pfrom, const std::string& msg_type, @@ -4994,12 +5011,23 @@ void PeerManagerImpl::ProcessMessage( m_mn_sync.ProcessMessage(pfrom, msg_type, vRecv); ProcessPeerMsgRet(m_govman.ProcessMessage(pfrom, m_connman, *this, msg_type, vRecv), pfrom); ProcessPeerMsgRet(CMNAuth::ProcessMessage(pfrom, peer->m_their_services, m_connman, m_mn_metaman, m_mn_activeman, m_chainman.ActiveChain(), m_mn_sync, m_dmnman->GetListAtChainTip(), msg_type, vRecv), pfrom); - ProcessPeerMsgRet(m_llmq_ctx->quorum_block_processor->ProcessMessage(pfrom, msg_type, vRecv), pfrom); + PostProcessMessage(m_llmq_ctx->quorum_block_processor->ProcessMessage(pfrom, msg_type, vRecv), pfrom.GetId()); ProcessPeerMsgRet(m_llmq_ctx->qdkgsman->ProcessMessage(pfrom, this, is_masternode, msg_type, vRecv), pfrom); ProcessPeerMsgRet(m_llmq_ctx->qman->ProcessMessage(pfrom, msg_type, vRecv), pfrom); m_llmq_ctx->shareman->ProcessMessage(pfrom, m_sporkman, msg_type, vRecv); ProcessPeerMsgRet(m_llmq_ctx->sigman->ProcessMessage(pfrom, msg_type, vRecv), pfrom); - ProcessPeerMsgRet(m_llmq_ctx->clhandler->ProcessMessage(pfrom, msg_type, vRecv), pfrom); + + if (msg_type == NetMsgType::CLSIG) { + if (llmq::AreChainLocksEnabled(m_sporkman)) { + llmq::CChainLockSig clsig; + vRecv >> clsig; + const uint256& hash = ::SerializeHash(clsig); + WITH_LOCK(::cs_main, EraseObjectRequest(pfrom.GetId(), CInv{MSG_CLSIG, hash})); + PostProcessMessage(m_llmq_ctx->clhandler->ProcessNewChainLock(pfrom.GetId(), clsig, hash), pfrom.GetId()); + } + return; // CLSIG + } + ProcessPeerMsgRet(m_llmq_ctx->isman->ProcessMessage(pfrom, msg_type, vRecv), pfrom); return; } diff --git a/src/net_processing.h b/src/net_processing.h index 295fcc1d6e..a857201516 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -127,6 +127,9 @@ public: virtual void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; + /** Finish message processing. Used for some specific messages */ + virtual void PostProcessMessage(MessageProcessingResult&& ret, NodeId node = -1) = 0; + /** This function is used for testing the stale tip eviction logic, see denialofservice_tests.cpp */ virtual void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) = 0; diff --git a/src/net_types.h b/src/net_types.h index 927d261072..6a7e5f41bc 100644 --- a/src/net_types.h +++ b/src/net_types.h @@ -5,8 +5,6 @@ #ifndef BITCOIN_NET_TYPES_H #define BITCOIN_NET_TYPES_H -#include - #include #include #include @@ -60,21 +58,5 @@ UniValue BanMapToJson(const banmap_t& bans); */ void BanMapFromJson(const UniValue& bans_json, banmap_t& bans); -struct MisbehavingError -{ - int score; - std::string message; - - MisbehavingError(int s) : score{s} {} - - // Constructor does a perfect forwarding reference - template - MisbehavingError(int s, T&& msg) : - score{s}, - message{std::forward(msg)} - {} -}; - -using PeerMsgRet = tl::expected; #endif // BITCOIN_NET_TYPES_H diff --git a/src/protocol.h b/src/protocol.h index aa07b3caeb..a7913802ab 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -17,6 +17,9 @@ #include #include +#include + + #include #include #include @@ -561,5 +564,57 @@ public: uint256 hash; }; +struct MisbehavingError +{ + int score; + std::string message; + + MisbehavingError(int s) : score{s} {} + + // Constructor does a perfect forwarding reference + template + MisbehavingError(int s, T&& msg) : + score{s}, + message{std::forward(msg)} + {} +}; + +// TODO: replace usages of PeerMsgRet to MessageProcessingResult which is cover this one +using PeerMsgRet = tl::expected; + +/** + * This struct is a helper to return values from handlers that are processing + * network messages but implemented outside of net_processing.cpp, + * for example llmq's messages. + * + * These handlers do not supposed to know anything about PeerManager to avoid + * circular dependencies. + * + * See `PeerManagerImpl::PostProcessMessage` to see how each type of return code + * is processed. + */ +struct MessageProcessingResult +{ + //! @m_error triggers Misbehaving error with score and optional message if not nullopt + std::optional m_error; + + //! @m_inventory will relay this inventory to connected peers if not nullopt + std::optional m_inventory; + + //! @m_transactions will relay transactions to peers which is ready to accept it (some peers does not accept transactions) + std::vector m_transactions; + + //! @m_to_erase triggers EraseObjectRequest from PeerManager for this inventory if not nullopt + std::optional m_to_erase; + + MessageProcessingResult() = default; + MessageProcessingResult(MisbehavingError error) : + m_error(error) + {} + MessageProcessingResult(CInv inv) : + m_inventory(inv) + { + } +}; #endif // BITCOIN_PROTOCOL_H diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 0ccff814a9..763d89bbd4 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -1121,7 +1122,8 @@ static RPCHelpMan submitchainlock() throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature"); } - llmq_ctx.clhandler->ProcessNewChainLock(-1, clsig, ::SerializeHash(clsig)); + PeerManager& peerman = EnsurePeerman(node); + peerman.PostProcessMessage(llmq_ctx.clhandler->ProcessNewChainLock(-1, clsig, ::SerializeHash(clsig))); return llmq_ctx.clhandler->GetBestChainLock().getHeight(); }, }; diff --git a/src/validation.cpp b/src/validation.cpp index 6ed82760a1..ef8eeb920a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -35,7 +35,6 @@ #include