From a8213cadb963633ac561afd3f9ba4a1818dcb948 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 22 Jan 2020 13:35:31 +0300 Subject: [PATCH] Various fixes for DSTX-es (#3295) * Check MNs up to 24 blocks deep when verifying `dstx` * Handle DSTX-es more like regular txes and not like "other" invs * Try asking for a DSTX too when trying to find missing tx parents * Check DSTX-es when chainlock arrives `HasChainLock` was always `false` in `IsExpired` because tip is updated before the corresponding chainlock is received * Apply `Handle DSTX-es more like regular txes` idea to `AlreadyHave()` * Alternative handling of DSTX+recentRejects Co-authored-by: Alexander Block --- src/dsnotificationinterface.cpp | 1 + src/net.h | 2 +- src/net_processing.cpp | 73 ++++++++++++++++++++++++--------- src/privatesend/privatesend.cpp | 7 ++++ src/privatesend/privatesend.h | 1 + 5 files changed, 64 insertions(+), 20 deletions(-) diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index 919c4ca4d4..2fa6e77b5a 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -107,4 +107,5 @@ void CDSNotificationInterface::NotifyMasternodeListChanged(bool undo, const CDet void CDSNotificationInterface::NotifyChainLock(const CBlockIndex* pindex, const llmq::CChainLockSig& clsig) { llmq::quorumInstantSendManager->NotifyChainLock(pindex); + CPrivateSend::NotifyChainLock(pindex); } diff --git a/src/net.h b/src/net.h index f4d6879034..e6177cea5a 100644 --- a/src/net.h +++ b/src/net.h @@ -999,7 +999,7 @@ public: void PushInventory(const CInv& inv) { LOCK(cs_inventory); - if (inv.type == MSG_TX) { + if (inv.type == MSG_TX || inv.type == MSG_DSTX) { if (!filterInventoryKnown.contains(inv.hash)) { LogPrint(BCLog::NET, "PushInventory -- inv: %s peer=%d\n", inv.ToString(), id); setInventoryTxToSend.insert(inv.hash); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9f01e5e13e..dfc9fd8b23 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1011,6 +1011,7 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) switch (inv.type) { case MSG_TX: + case MSG_DSTX: case MSG_LEGACY_TXLOCK_REQUEST: // we treat legacy IX messages as TX messages { assert(recentRejects); @@ -1034,7 +1035,17 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) // and re-request the locked transaction (which did not make it into the mempool // previously due to txn-mempool-conflict rule). This means that we must ignore // recentRejects filter for such locked txes here. - return (recentRejects->contains(inv.hash) && !llmq::quorumInstantSendManager->IsLocked(inv.hash)) || + // We also ignore recentRejects filter for DSTX-es because a malicious peer might + // relay a valid DSTX as a regular TX first which would skip all the specific checks + // but would cause such tx to be rejected by ATMP due to 0 fee. Ignoring it here + // should let DSTX to be propagated by honest peer later. Note, that a malicious + // masternode would not be able to exploit this to spam the network with specially + // crafted invalid DSTX-es and potentially cause high load cheaply, because + // corresponding checks in ProcessMessage won't let it to send DSTX-es too often. + bool fIgnoreRecentRejects = llmq::quorumInstantSendManager->IsLocked(inv.hash) || inv.type == MSG_DSTX; + + return (!fIgnoreRecentRejects && recentRejects->contains(inv.hash)) || + (inv.type == MSG_DSTX && static_cast(CPrivateSend::GetDSTX(inv.hash))) || mempool.exists(inv.hash) || pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 0)) || // Best effort: only try output 0 and 1 pcoinsTip->HaveCoinInCache(COutPoint(inv.hash, 1)) || @@ -1060,10 +1071,6 @@ bool static AlreadyHave(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) return sporkManager.GetSporkByHash(inv.hash, spork); } - case MSG_DSTX: { - return static_cast(CPrivateSend::GetDSTX(inv.hash)); - } - case MSG_GOVERNANCE_OBJECT: case MSG_GOVERNANCE_OBJECT_VOTE: return ! governance.ConfirmInventoryRequest(inv); @@ -1274,17 +1281,29 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam // Send stream from relay memory bool push = false; - if (inv.type == MSG_TX) { + if (inv.type == MSG_TX || inv.type == MSG_DSTX) { + CPrivateSendBroadcastTx dstx; + if (inv.type == MSG_DSTX) { + dstx = CPrivateSend::GetDSTX(inv.hash); + } auto mi = mapRelay.find(inv.hash); if (mi != mapRelay.end()) { - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *mi->second)); + if (dstx) { + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::DSTX, dstx)); + } else { + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *mi->second)); + } push = true; } else if (pfrom->timeLastMempoolReq) { auto txinfo = mempool.info(inv.hash); // To protect privacy, do not answer getdata using the mempool when // that TX couldn't have been INVed in reply to a MEMPOOL request. if (txinfo.tx && txinfo.nTime <= pfrom->timeLastMempoolReq) { - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *txinfo.tx)); + if (dstx) { + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::DSTX, dstx)); + } else { + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, *txinfo.tx)); + } push = true; } } @@ -1298,14 +1317,6 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam } } - if (!push && inv.type == MSG_DSTX) { - CPrivateSendBroadcastTx dstx = CPrivateSend::GetDSTX(inv.hash); - if(dstx) { - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::DSTX, dstx)); - push = true; - } - } - if (!push && inv.type == MSG_GOVERNANCE_OBJECT) { LogPrint(BCLog::NET, "ProcessGetData -- MSG_GOVERNANCE_OBJECT: inv = %s\n", inv.ToString()); CDataStream ss(SER_NETWORK, pfrom->GetSendVersion()); @@ -2452,7 +2463,19 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; // not an error } - auto dmn = deterministicMNManager->GetListAtChainTip().GetMNByCollateral(dstx.masternodeOutpoint); + const CBlockIndex* pindex{nullptr}; + CDeterministicMNCPtr dmn{nullptr}; + { + LOCK(cs_main); + pindex = chainActive.Tip(); + } + // It could be that a MN is no longer in the list but its DSTX is not yet mined. + // Try to find a MN up to 24 blocks deep to make sure such dstx-es are relayed and processed correctly. + for (int i = 0; i < 24 && pindex; ++i) { + dmn = deterministicMNManager->GetListForBlock(pindex).GetMNByCollateral(dstx.masternodeOutpoint); + if (dmn) break; + pindex = pindex->pprev; + } if(!dmn) { LogPrint(BCLog::PRIVATESEND, "DSTX -- Can't find masternode %s to verify %s\n", dstx.masternodeOutpoint.ToStringShort(), hashTx.ToString()); return false; @@ -2523,6 +2546,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr CInv _inv(MSG_TX, txin.prevout.hash); pfrom->AddInventoryKnown(_inv); if (!AlreadyHave(_inv)) pfrom->AskFor(_inv); + // We don't know if the previous tx was a regular or a mixing one, try both + CInv _inv2(MSG_DSTX, txin.prevout.hash); + pfrom->AddInventoryKnown(_inv2); + if (!AlreadyHave(_inv2)) pfrom->AskFor(_inv2); } AddOrphanTx(ptx, pfrom->GetId()); @@ -3785,7 +3812,11 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic& interruptM for (const auto& txinfo : vtxinfo) { const uint256& hash = txinfo.tx->GetHash(); - CInv inv(MSG_TX, hash); + int nInvType = MSG_TX; + if (CPrivateSend::GetDSTX(hash)) { + nInvType = MSG_DSTX; + } + CInv inv(nInvType, hash); pto->setInventoryTxToSend.erase(hash); if (pto->pfilter) { if (!pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; @@ -3851,7 +3882,11 @@ bool PeerLogicValidation::SendMessages(CNode* pto, std::atomic& interruptM } if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send - vInv.push_back(CInv(MSG_TX, hash)); + int nInvType = MSG_TX; + if (CPrivateSend::GetDSTX(hash)) { + nInvType = MSG_DSTX; + } + vInv.push_back(CInv(nInvType, hash)); nRelayedTransactions++; { // Expire old relay messages diff --git a/src/privatesend/privatesend.cpp b/src/privatesend/privatesend.cpp index 1225efe322..71d09aae37 100644 --- a/src/privatesend/privatesend.cpp +++ b/src/privatesend/privatesend.cpp @@ -603,6 +603,13 @@ void CPrivateSend::UpdatedBlockTip(const CBlockIndex* pindex) } } +void CPrivateSend::NotifyChainLock(const CBlockIndex* pindex) +{ + if (pindex && masternodeSync.IsBlockchainSynced()) { + CheckDSTXes(pindex); + } +} + void CPrivateSend::UpdateDSTXConfirmedHeight(const CTransactionRef& tx, int nHeight) { AssertLockHeld(cs_mapdstx); diff --git a/src/privatesend/privatesend.h b/src/privatesend/privatesend.h index 3f7687964d..8abd4e13ed 100644 --- a/src/privatesend/privatesend.h +++ b/src/privatesend/privatesend.h @@ -465,6 +465,7 @@ public: static CPrivateSendBroadcastTx GetDSTX(const uint256& hash); static void UpdatedBlockTip(const CBlockIndex* pindex); + static void NotifyChainLock(const CBlockIndex* pindex); static void UpdateDSTXConfirmedHeight(const CTransactionRef& tx, int nHeight); static void TransactionAddedToMempool(const CTransactionRef& tx);