diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 292e672f0a3f..d237a39618f8 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -767,3 +768,43 @@ bool CoinJoin::ValidateFinalTxComposition(size_t nTotalInputs, size_t nTotalOutp return nSessionInputs >= nLargerOutputs * size_t(PROMOTION_RATIO) && nSessionOutputs >= nLargerInputs * size_t(PROMOTION_RATIO); } + +CoinJoin::DSQueueValidation CoinJoin::ValidateDSQueue(CCoinJoinQueue& dsq, const CDeterministicMNList& mn_list, + const std::function& is_duplicate) +{ + if (!IsValidDenomination(dsq.nDenom)) { + LogPrint(BCLog::COINJOIN, "DSQUEUE -- invalid denomination %d\n", dsq.nDenom); + return {.misbehavior = 10}; + } + + if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) { + return {.misbehavior = 100}; + } + + if (dsq.masternodeOutpoint.IsNull()) { + if (auto dmn = mn_list.GetValidMN(dsq.m_protxHash)) { + dsq.masternodeOutpoint = dmn->collateralOutpoint; + } else { + return {.misbehavior = 10}; + } + } + + if (is_duplicate(dsq)) return {}; + + LogPrint(BCLog::COINJOIN, "DSQUEUE -- %s new\n", dsq.ToString()); + + if (dsq.IsTimeOutOfBounds()) return {}; + + auto dmn = mn_list.GetValidMNByCollateral(dsq.masternodeOutpoint); + if (!dmn) return {}; + + if (dsq.m_protxHash.IsNull()) { + dsq.m_protxHash = dmn->proTxHash; + } + + if (!dsq.CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) { + return {.misbehavior = 10}; + } + + return {.dmn = std::move(dmn)}; +} diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index df89dbddaf5a..b07ac6eb17f6 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -8,6 +8,7 @@ #include #include +#include #include // IWYU pragma: export #include #include @@ -20,6 +21,7 @@ #include #include +#include #include #include #include @@ -29,6 +31,7 @@ class Chainstate; class CBLSPublicKey; class CBlockIndex; +class CDeterministicMNList; class ChainstateManager; class CTxMemPool; @@ -531,6 +534,24 @@ namespace CoinJoin */ bool ValidateFinalTxComposition(size_t nTotalInputs, size_t nTotalOutputs, size_t nLargerInputs, size_t nLargerOutputs); + + struct DSQueueValidation { + //! The masternode that signed the queue, null if the queue was rejected + CDeterministicMNCPtr dmn{}; + //! Score to penalize the sending peer with, 0 if the queue was accepted or the rejection + //! is not the peer's fault + int misbehavior{0}; + }; + + /** + * Runs the DSQUEUE checks common to mixing clients and servers and fills in the queue's + * masternodeOutpoint from the masternode list. + * @param is_duplicate Called once the outpoint is known and before the signature is verified, + * so duplicates are dropped without a BLS check. Returning true rejects + * the queue without a penalty. + */ + DSQueueValidation ValidateDSQueue(CCoinJoinQueue& dsq, const CDeterministicMNList& mn_list, + const std::function& is_duplicate); } class CDSTXManager diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index b06652025431..ad218be6facc 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -134,52 +134,20 @@ void CCoinJoinServer::ProcessDSQUEUE(NodeId from, CDataStream& vRecv) WITH_LOCK(cs_main, m_peer_manager->PeerEraseObjectRequest(from, CInv{MSG_DSQ, dsq.GetHash()})); - // Validate denomination first - if (!CoinJoin::IsValidDenomination(dsq.nDenom)) { - LogPrint(BCLog::COINJOIN, "DSQUEUE -- invalid denomination %d from peer %d\n", dsq.nDenom, from); - m_peer_manager->PeerMisbehaving(from, 10); - return; - } - - if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) { - m_peer_manager->PeerMisbehaving(from, 100); - return; - } - const auto tip_mn_list = m_dmnman.GetListAtChainTip(); - if (dsq.masternodeOutpoint.IsNull()) { - if (auto dmn = tip_mn_list.GetValidMN(dsq.m_protxHash)) { - dsq.masternodeOutpoint = dmn->collateralOutpoint; - } else { - m_peer_manager->PeerMisbehaving(from, 10); - return; - } - } - - { - const auto isDup = m_queueman.TryCheckDuplicate(dsq); - if (!isDup.has_value()) return; + const auto [dmn, misbehavior] = CoinJoin::ValidateDSQueue(dsq, tip_mn_list, [this, from](const CCoinJoinQueue& q) { + const auto isDup = m_queueman.TryCheckDuplicate(q); + if (!isDup.has_value()) return true; if (*isDup) { - LogPrint(BCLog::COINJOIN, "DSQUEUE -- Peer %d is sending WAY too many dsq messages for a masternode with collateral %s\n", from, dsq.masternodeOutpoint.ToStringShort()); - return; + LogPrint(BCLog::COINJOIN, "DSQUEUE -- Peer %d is sending WAY too many dsq messages for a masternode with collateral %s\n", from, q.masternodeOutpoint.ToStringShort()); } - } - - LogPrint(BCLog::COINJOIN, "DSQUEUE -- %s new\n", dsq.ToString()); - - if (dsq.IsTimeOutOfBounds()) return; - - auto dmn = tip_mn_list.GetValidMNByCollateral(dsq.masternodeOutpoint); - if (!dmn) return; - - if (dsq.m_protxHash.IsNull()) { - dsq.m_protxHash = dmn->proTxHash; - } - - if (!dsq.CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) { - m_peer_manager->PeerMisbehaving(from, 10); + return *isDup; + }); + if (misbehavior > 0) { + m_peer_manager->PeerMisbehaving(from, misbehavior); return; } + if (!dmn) return; if (!dsq.fReady) { //don't allow a few nodes to dominate the queuing process diff --git a/src/coinjoin/walletman.cpp b/src/coinjoin/walletman.cpp index dc774c18bf87..88bbe419d08a 100644 --- a/src/coinjoin/walletman.cpp +++ b/src/coinjoin/walletman.cpp @@ -258,50 +258,29 @@ MessageProcessingResult CJWalletManagerImpl::ProcessDSQueue(NodeId from, CConnma MessageProcessingResult ret{}; ret.m_to_erase = CInv{MSG_DSQ, dsq.GetHash()}; - if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) { - ret.m_error = MisbehavingError{100}; - return ret; - } - const auto tip_mn_list = m_dmnman.GetListAtChainTip(); - if (dsq.masternodeOutpoint.IsNull()) { - if (auto dmn = tip_mn_list.GetValidMN(dsq.m_protxHash)) { - dsq.masternodeOutpoint = dmn->collateralOutpoint; - } else { - ret.m_error = MisbehavingError{10}; - return ret; - } - } { LOCK(cs_ProcessDSQueue); - if (m_queueman->HasQueue(dsq.GetHash())) return ret; - - if (m_queueman->HasQueueFromMasternode(dsq.masternodeOutpoint, dsq.fReady)) { - // no way the same mn can send another dsq with the same readiness this soon - LogPrint(BCLog::COINJOIN, /* Continued */ - "DSQUEUE -- Peer %d is sending WAY too many dsq messages for a masternode with collateral %s\n", - from, dsq.masternodeOutpoint.ToStringShort()); + const auto validation = CoinJoin::ValidateDSQueue(dsq, tip_mn_list, [this, from](const CCoinJoinQueue& q) { + if (m_queueman->HasQueue(q.GetHash())) return true; + if (m_queueman->HasQueueFromMasternode(q.masternodeOutpoint, q.fReady)) { + // no way the same mn can send another dsq with the same readiness this soon + LogPrint(BCLog::COINJOIN, /* Continued */ + "DSQUEUE -- Peer %d is sending WAY too many dsq messages for a masternode with collateral %s\n", + from, q.masternodeOutpoint.ToStringShort()); + return true; + } + return false; + }); + if (validation.misbehavior > 0) { + ret.m_error = MisbehavingError{validation.misbehavior}; return ret; } - - LogPrint(BCLog::COINJOIN, "DSQUEUE -- %s new\n", dsq.ToString()); - - if (dsq.IsTimeOutOfBounds()) return ret; - - auto dmn = tip_mn_list.GetValidMNByCollateral(dsq.masternodeOutpoint); + const auto& dmn = validation.dmn; if (!dmn) return ret; - if (dsq.m_protxHash.IsNull()) { - dsq.m_protxHash = dmn->proTxHash; - } - - if (!dsq.CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) { - ret.m_error = MisbehavingError{10}; - return ret; - } - // if the queue is ready, submit if we can if (dsq.fReady && ForAnyCJClientMan([&connman, &dmn](CCoinJoinClientManager& clientman) { return clientman.TrySubmitDenominate(dmn->proTxHash, connman); diff --git a/src/test/coinjoin_queue_tests.cpp b/src/test/coinjoin_queue_tests.cpp index 3440c8a1fd36..471a5e7d21ca 100644 --- a/src/test/coinjoin_queue_tests.cpp +++ b/src/test/coinjoin_queue_tests.cpp @@ -2,21 +2,107 @@ // Distributed under the MIT/X11 software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include #include #include +#include #include +#include +#include +#include +#include +#include +#include