From 26a93bce29fd813e1402b013f402869c25b656d1 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Fri, 8 Mar 2019 13:30:45 -0500 Subject: Remove unused variable --- src/net.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/net.h b/src/net.h index 75c05c9cb5..4e8a96497c 100644 --- a/src/net.h +++ b/src/net.h @@ -703,7 +703,6 @@ public: std::vector vAddrToSend; CRollingBloomFilter addrKnown; bool fGetAddr{false}; - std::set setKnown; int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing){0}; int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing){0}; -- cgit v1.2.3 From 4de0dbac9b286c42a9b10132b7c2d76712f1a319 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Fri, 8 Mar 2019 14:26:36 -0500 Subject: [refactor] Move tx relay state to separate structure --- src/net.cpp | 15 +++---- src/net.h | 64 +++++++++++++++++------------- src/net_processing.cpp | 105 +++++++++++++++++++++++++------------------------ 3 files changed, 95 insertions(+), 89 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 0464a6e9ea..527c001308 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -500,8 +500,8 @@ void CNode::copyStats(CNodeStats &stats) X(addr); X(addrBind); { - LOCK(cs_filter); - X(fRelayTxes); + LOCK(m_tx_relay.cs_filter); + stats.fRelayTxes = m_tx_relay.fRelayTxes; } X(nLastSend); X(nLastRecv); @@ -529,8 +529,8 @@ void CNode::copyStats(CNodeStats &stats) X(m_legacyWhitelisted); X(m_permissionFlags); { - LOCK(cs_feeFilter); - X(minFeeFilter); + LOCK(m_tx_relay.cs_feeFilter); + stats.minFeeFilter = m_tx_relay.minFeeFilter; } // It is common for nodes with good ping times to suddenly become lagged, @@ -818,11 +818,11 @@ bool CConnman::AttemptToEvictConnection() continue; if (node->fDisconnect) continue; - LOCK(node->cs_filter); + LOCK(node->m_tx_relay.cs_filter); NodeEvictionCandidate candidate = {node->GetId(), node->nTimeConnected, node->nMinPingUsecTime, node->nLastBlockTime, node->nLastTXTime, HasAllDesirableServiceFlags(node->nServices), - node->fRelayTxes, node->pfilter != nullptr, node->addr, node->nKeyedNetGroup, + node->m_tx_relay.fRelayTxes, node->m_tx_relay.pfilter != nullptr, node->addr, node->nKeyedNetGroup, node->m_prefer_evict}; vEvictionCandidates.push_back(candidate); } @@ -2625,7 +2625,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn fInbound(fInboundIn), nKeyedNetGroup(nKeyedNetGroupIn), addrKnown(5000, 0.001), - filterInventoryKnown(50000, 0.000001), id(idIn), nLocalHostNonce(nLocalHostNonceIn), nLocalServices(nLocalServicesIn), @@ -2634,8 +2633,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn hSocket = hSocketIn; addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn; hashContinue = uint256(); - filterInventoryKnown.reset(); - pfilter = MakeUnique(); for (const std::string &msg : getAllNetMessageTypes()) mapRecvBytesPerMsgCmd[msg] = 0; diff --git a/src/net.h b/src/net.h index 4e8a96497c..bd46d995ec 100644 --- a/src/net.h +++ b/src/net.h @@ -676,15 +676,8 @@ public: // Setting fDisconnect to true will cause the node to be disconnected the // next time DisconnectNodes() runs std::atomic_bool fDisconnect{false}; - // We use fRelayTxes for two purposes - - // a) it allows us to not relay tx invs before receiving the peer's version message - // b) the peer may tell us in its version message that we should not relay tx invs - // unless it loads a bloom filter. - bool fRelayTxes GUARDED_BY(cs_filter){false}; bool fSentAddr{false}; CSemaphoreGrant grantOutbound; - mutable CCriticalSection cs_filter; - std::unique_ptr pfilter PT_GUARDED_BY(cs_filter); std::atomic nRefCount{0}; const uint64_t nKeyedNetGroup; @@ -706,24 +699,43 @@ public: int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing){0}; int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing){0}; - // inventory based relay - CRollingBloomFilter filterInventoryKnown GUARDED_BY(cs_inventory); - // Set of transaction ids we still have to announce. - // They are sorted by the mempool before relay, so the order is not important. - std::set setInventoryTxToSend; // List of block ids we still have announce. // There is no final sorting before sending, as they are always sent immediately // and in the order requested. std::vector vInventoryBlockToSend GUARDED_BY(cs_inventory); CCriticalSection cs_inventory; - int64_t nNextInvSend{0}; + + struct TxRelay { + TxRelay() { pfilter = MakeUnique(); } + mutable CCriticalSection cs_filter; + // We use fRelayTxes for two purposes - + // a) it allows us to not relay tx invs before receiving the peer's version message + // b) the peer may tell us in its version message that we should not relay tx invs + // unless it loads a bloom filter. + bool fRelayTxes GUARDED_BY(cs_filter){false}; + std::unique_ptr pfilter PT_GUARDED_BY(cs_filter) GUARDED_BY(cs_filter); + + mutable CCriticalSection cs_tx_inventory; + CRollingBloomFilter filterInventoryKnown GUARDED_BY(cs_tx_inventory){50000, 0.000001}; + // Set of transaction ids we still have to announce. + // They are sorted by the mempool before relay, so the order is not important. + std::set setInventoryTxToSend; + // Used for BIP35 mempool sending + bool fSendMempool GUARDED_BY(cs_tx_inventory){false}; + // Last time a "MEMPOOL" request was serviced. + std::atomic timeLastMempoolReq{0}; + int64_t nNextInvSend{0}; + + CCriticalSection cs_feeFilter; + // Minimum fee rate with which to filter inv's to this node + CAmount minFeeFilter GUARDED_BY(cs_feeFilter){0}; + CAmount lastSentFeeFilter{0}; + int64_t nextSendTimeFeeFilter{0}; + }; + + TxRelay m_tx_relay; // Used for headers announcements - unfiltered blocks to relay std::vector vBlockHashesToAnnounce GUARDED_BY(cs_inventory); - // Used for BIP35 mempool sending - bool fSendMempool GUARDED_BY(cs_inventory){false}; - - // Last time a "MEMPOOL" request was serviced. - std::atomic timeLastMempoolReq{0}; // Block and TXN accept times std::atomic nLastBlockTime{0}; @@ -740,11 +752,6 @@ public: std::atomic nMinPingUsecTime{std::numeric_limits::max()}; // Whether a ping is requested. std::atomic fPingQueued{false}; - // Minimum fee rate with which to filter inv's to this node - CAmount minFeeFilter GUARDED_BY(cs_feeFilter){0}; - CCriticalSection cs_feeFilter; - CAmount lastSentFeeFilter{0}; - int64_t nextSendTimeFeeFilter{0}; std::set orphan_work_set; @@ -842,19 +849,20 @@ public: void AddInventoryKnown(const CInv& inv) { { - LOCK(cs_inventory); - filterInventoryKnown.insert(inv.hash); + LOCK(m_tx_relay.cs_tx_inventory); + m_tx_relay.filterInventoryKnown.insert(inv.hash); } } void PushInventory(const CInv& inv) { - LOCK(cs_inventory); if (inv.type == MSG_TX) { - if (!filterInventoryKnown.contains(inv.hash)) { - setInventoryTxToSend.insert(inv.hash); + LOCK(m_tx_relay.cs_tx_inventory); + if (!m_tx_relay.filterInventoryKnown.contains(inv.hash)) { + m_tx_relay.setInventoryTxToSend.insert(inv.hash); } } else if (inv.type == MSG_BLOCK) { + LOCK(cs_inventory); vInventoryBlockToSend.push_back(inv.hash); } } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3db460d444..c88d17b6ee 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1449,10 +1449,10 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c bool sendMerkleBlock = false; CMerkleBlock merkleBlock; { - LOCK(pfrom->cs_filter); - if (pfrom->pfilter) { + LOCK(pfrom->m_tx_relay.cs_filter); + if (pfrom->m_tx_relay.pfilter) { sendMerkleBlock = true; - merkleBlock = CMerkleBlock(*pblock, *pfrom->pfilter); + merkleBlock = CMerkleBlock(*pblock, *pfrom->m_tx_relay.pfilter); } } if (sendMerkleBlock) { @@ -1532,11 +1532,11 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm if (mi != mapRelay.end()) { connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *mi->second)); push = true; - } else if (pfrom->timeLastMempoolReq) { + } else if (pfrom->m_tx_relay.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) { + if (txinfo.tx && txinfo.nTime <= pfrom->m_tx_relay.timeLastMempoolReq) { connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *txinfo.tx)); push = true; } @@ -1996,8 +1996,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr pfrom->m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED)); { - LOCK(pfrom->cs_filter); - pfrom->fRelayTxes = fRelay; // set to true after we get the first filter* message + LOCK(pfrom->m_tx_relay.cs_filter); + pfrom->m_tx_relay.fRelayTxes = fRelay; // set to true after we get the first filter* message } // Change version @@ -3030,8 +3030,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; } - LOCK(pfrom->cs_inventory); - pfrom->fSendMempool = true; + LOCK(pfrom->m_tx_relay.cs_tx_inventory); + pfrom->m_tx_relay.fSendMempool = true; return true; } @@ -3124,10 +3124,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } else { - LOCK(pfrom->cs_filter); - pfrom->pfilter.reset(new CBloomFilter(filter)); - pfrom->pfilter->UpdateEmptyFull(); - pfrom->fRelayTxes = true; + LOCK(pfrom->m_tx_relay.cs_filter); + pfrom->m_tx_relay.pfilter.reset(new CBloomFilter(filter)); + pfrom->m_tx_relay.pfilter->UpdateEmptyFull(); + pfrom->m_tx_relay.fRelayTxes = true; } return true; } @@ -3142,9 +3142,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) { bad = true; } else { - LOCK(pfrom->cs_filter); - if (pfrom->pfilter) { - pfrom->pfilter->insert(vData); + LOCK(pfrom->m_tx_relay.cs_filter); + if (pfrom->m_tx_relay.pfilter) { + pfrom->m_tx_relay.pfilter->insert(vData); } else { bad = true; } @@ -3157,11 +3157,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } if (strCommand == NetMsgType::FILTERCLEAR) { - LOCK(pfrom->cs_filter); + LOCK(pfrom->m_tx_relay.cs_filter); if (pfrom->GetLocalServices() & NODE_BLOOM) { - pfrom->pfilter.reset(new CBloomFilter()); + pfrom->m_tx_relay.pfilter.reset(new CBloomFilter()); } - pfrom->fRelayTxes = true; + pfrom->m_tx_relay.fRelayTxes = true; return true; } @@ -3170,8 +3170,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr vRecv >> newFeeFilter; if (MoneyRange(newFeeFilter)) { { - LOCK(pfrom->cs_feeFilter); - pfrom->minFeeFilter = newFeeFilter; + LOCK(pfrom->m_tx_relay.cs_feeFilter); + pfrom->m_tx_relay.minFeeFilter = newFeeFilter; } LogPrint(BCLog::NET, "received: feefilter of %s from peer=%d\n", CFeeRate(newFeeFilter).ToString(), pfrom->GetId()); } @@ -3791,69 +3791,70 @@ bool PeerLogicValidation::SendMessages(CNode* pto) } pto->vInventoryBlockToSend.clear(); + LOCK(pto->m_tx_relay.cs_tx_inventory); // Check whether periodic sends should happen bool fSendTrickle = pto->HasPermission(PF_NOBAN); - if (pto->nNextInvSend < nNow) { + if (pto->m_tx_relay.nNextInvSend < nNow) { fSendTrickle = true; if (pto->fInbound) { - pto->nNextInvSend = connman->PoissonNextSendInbound(nNow, INVENTORY_BROADCAST_INTERVAL); + pto->m_tx_relay.nNextInvSend = connman->PoissonNextSendInbound(nNow, INVENTORY_BROADCAST_INTERVAL); } else { // Use half the delay for outbound peers, as there is less privacy concern for them. - pto->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> 1); + pto->m_tx_relay.nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> 1); } } // Time to send but the peer has requested we not relay transactions. if (fSendTrickle) { - LOCK(pto->cs_filter); - if (!pto->fRelayTxes) pto->setInventoryTxToSend.clear(); + LOCK(pto->m_tx_relay.cs_filter); + if (!pto->m_tx_relay.fRelayTxes) pto->m_tx_relay.setInventoryTxToSend.clear(); } // Respond to BIP35 mempool requests - if (fSendTrickle && pto->fSendMempool) { + if (fSendTrickle && pto->m_tx_relay.fSendMempool) { auto vtxinfo = mempool.infoAll(); - pto->fSendMempool = false; + pto->m_tx_relay.fSendMempool = false; CAmount filterrate = 0; { - LOCK(pto->cs_feeFilter); - filterrate = pto->minFeeFilter; + LOCK(pto->m_tx_relay.cs_feeFilter); + filterrate = pto->m_tx_relay.minFeeFilter; } - LOCK(pto->cs_filter); + LOCK(pto->m_tx_relay.cs_filter); for (const auto& txinfo : vtxinfo) { const uint256& hash = txinfo.tx->GetHash(); CInv inv(MSG_TX, hash); - pto->setInventoryTxToSend.erase(hash); + pto->m_tx_relay.setInventoryTxToSend.erase(hash); if (filterrate) { if (txinfo.feeRate.GetFeePerK() < filterrate) continue; } - if (pto->pfilter) { - if (!pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (pto->m_tx_relay.pfilter) { + if (!pto->m_tx_relay.pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; } - pto->filterInventoryKnown.insert(hash); + pto->m_tx_relay.filterInventoryKnown.insert(hash); vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } } - pto->timeLastMempoolReq = GetTime(); + pto->m_tx_relay.timeLastMempoolReq = GetTime(); } // Determine transactions to relay if (fSendTrickle) { // Produce a vector with all candidates for sending std::vector::iterator> vInvTx; - vInvTx.reserve(pto->setInventoryTxToSend.size()); - for (std::set::iterator it = pto->setInventoryTxToSend.begin(); it != pto->setInventoryTxToSend.end(); it++) { + vInvTx.reserve(pto->m_tx_relay.setInventoryTxToSend.size()); + for (std::set::iterator it = pto->m_tx_relay.setInventoryTxToSend.begin(); it != pto->m_tx_relay.setInventoryTxToSend.end(); it++) { vInvTx.push_back(it); } CAmount filterrate = 0; { - LOCK(pto->cs_feeFilter); - filterrate = pto->minFeeFilter; + LOCK(pto->m_tx_relay.cs_feeFilter); + filterrate = pto->m_tx_relay.minFeeFilter; } // Topologically and fee-rate sort the inventory we send for privacy and priority reasons. // A heap is used so that not all items need sorting if only a few are being sent. @@ -3862,7 +3863,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // No reason to drain out at many times the network's capacity, // especially since we have many peers and some will draw much shorter delays. unsigned int nRelayedTransactions = 0; - LOCK(pto->cs_filter); + LOCK(pto->m_tx_relay.cs_filter); while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) { // Fetch the top element from the heap std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); @@ -3870,9 +3871,9 @@ bool PeerLogicValidation::SendMessages(CNode* pto) vInvTx.pop_back(); uint256 hash = *it; // Remove it from the to-be-sent set - pto->setInventoryTxToSend.erase(it); + pto->m_tx_relay.setInventoryTxToSend.erase(it); // Check if not in the filter already - if (pto->filterInventoryKnown.contains(hash)) { + if (pto->m_tx_relay.filterInventoryKnown.contains(hash)) { continue; } // Not in the mempool anymore? don't bother sending it. @@ -3883,7 +3884,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) if (filterrate && txinfo.feeRate.GetFeePerK() < filterrate) { continue; } - if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (pto->m_tx_relay.pfilter && !pto->m_tx_relay.pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send vInv.push_back(CInv(MSG_TX, hash)); nRelayedTransactions++; @@ -3904,7 +3905,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } - pto->filterInventoryKnown.insert(hash); + pto->m_tx_relay.filterInventoryKnown.insert(hash); } } } @@ -4069,23 +4070,23 @@ bool PeerLogicValidation::SendMessages(CNode* pto) !pto->HasPermission(PF_FORCERELAY)) { CAmount currentFilter = mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); int64_t timeNow = GetTimeMicros(); - if (timeNow > pto->nextSendTimeFeeFilter) { + if (timeNow > pto->m_tx_relay.nextSendTimeFeeFilter) { static CFeeRate default_feerate(DEFAULT_MIN_RELAY_TX_FEE); static FeeFilterRounder filterRounder(default_feerate); CAmount filterToSend = filterRounder.round(currentFilter); // We always have a fee filter of at least minRelayTxFee filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK()); - if (filterToSend != pto->lastSentFeeFilter) { + if (filterToSend != pto->m_tx_relay.lastSentFeeFilter) { connman->PushMessage(pto, msgMaker.Make(NetMsgType::FEEFILTER, filterToSend)); - pto->lastSentFeeFilter = filterToSend; + pto->m_tx_relay.lastSentFeeFilter = filterToSend; } - pto->nextSendTimeFeeFilter = PoissonNextSend(timeNow, AVG_FEEFILTER_BROADCAST_INTERVAL); + pto->m_tx_relay.nextSendTimeFeeFilter = PoissonNextSend(timeNow, AVG_FEEFILTER_BROADCAST_INTERVAL); } // If the fee filter has changed substantially and it's still more than MAX_FEEFILTER_CHANGE_DELAY // until scheduled broadcast, then move the broadcast to within MAX_FEEFILTER_CHANGE_DELAY. - else if (timeNow + MAX_FEEFILTER_CHANGE_DELAY * 1000000 < pto->nextSendTimeFeeFilter && - (currentFilter < 3 * pto->lastSentFeeFilter / 4 || currentFilter > 4 * pto->lastSentFeeFilter / 3)) { - pto->nextSendTimeFeeFilter = timeNow + GetRandInt(MAX_FEEFILTER_CHANGE_DELAY) * 1000000; + else if (timeNow + MAX_FEEFILTER_CHANGE_DELAY * 1000000 < pto->m_tx_relay.nextSendTimeFeeFilter && + (currentFilter < 3 * pto->m_tx_relay.lastSentFeeFilter / 4 || currentFilter > 4 * pto->m_tx_relay.lastSentFeeFilter / 3)) { + pto->m_tx_relay.nextSendTimeFeeFilter = timeNow + GetRandInt(MAX_FEEFILTER_CHANGE_DELAY) * 1000000; } } } -- cgit v1.2.3 From c4aa2ba82211ea5988ed7fe21e1b08bc3367e6d4 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Fri, 8 Mar 2019 15:30:36 -0500 Subject: [refactor] Change tx_relay structure to be unique_ptr --- src/net.cpp | 13 +++--- src/net.h | 12 +++--- src/net_processing.cpp | 106 ++++++++++++++++++++++++------------------------- 3 files changed, 66 insertions(+), 65 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 527c001308..78b33954d6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -500,8 +500,8 @@ void CNode::copyStats(CNodeStats &stats) X(addr); X(addrBind); { - LOCK(m_tx_relay.cs_filter); - stats.fRelayTxes = m_tx_relay.fRelayTxes; + LOCK(m_tx_relay->cs_filter); + stats.fRelayTxes = m_tx_relay->fRelayTxes; } X(nLastSend); X(nLastRecv); @@ -529,8 +529,8 @@ void CNode::copyStats(CNodeStats &stats) X(m_legacyWhitelisted); X(m_permissionFlags); { - LOCK(m_tx_relay.cs_feeFilter); - stats.minFeeFilter = m_tx_relay.minFeeFilter; + LOCK(m_tx_relay->cs_feeFilter); + stats.minFeeFilter = m_tx_relay->minFeeFilter; } // It is common for nodes with good ping times to suddenly become lagged, @@ -818,11 +818,11 @@ bool CConnman::AttemptToEvictConnection() continue; if (node->fDisconnect) continue; - LOCK(node->m_tx_relay.cs_filter); + LOCK(node->m_tx_relay->cs_filter); NodeEvictionCandidate candidate = {node->GetId(), node->nTimeConnected, node->nMinPingUsecTime, node->nLastBlockTime, node->nLastTXTime, HasAllDesirableServiceFlags(node->nServices), - node->m_tx_relay.fRelayTxes, node->m_tx_relay.pfilter != nullptr, node->addr, node->nKeyedNetGroup, + node->m_tx_relay->fRelayTxes, node->m_tx_relay->pfilter != nullptr, node->addr, node->nKeyedNetGroup, node->m_prefer_evict}; vEvictionCandidates.push_back(candidate); } @@ -2633,6 +2633,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn hSocket = hSocketIn; addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn; hashContinue = uint256(); + m_tx_relay = MakeUnique(); for (const std::string &msg : getAllNetMessageTypes()) mapRecvBytesPerMsgCmd[msg] = 0; diff --git a/src/net.h b/src/net.h index bd46d995ec..d12fc3ae9a 100644 --- a/src/net.h +++ b/src/net.h @@ -733,7 +733,7 @@ public: int64_t nextSendTimeFeeFilter{0}; }; - TxRelay m_tx_relay; + std::unique_ptr m_tx_relay; // Used for headers announcements - unfiltered blocks to relay std::vector vBlockHashesToAnnounce GUARDED_BY(cs_inventory); @@ -849,17 +849,17 @@ public: void AddInventoryKnown(const CInv& inv) { { - LOCK(m_tx_relay.cs_tx_inventory); - m_tx_relay.filterInventoryKnown.insert(inv.hash); + LOCK(m_tx_relay->cs_tx_inventory); + m_tx_relay->filterInventoryKnown.insert(inv.hash); } } void PushInventory(const CInv& inv) { if (inv.type == MSG_TX) { - LOCK(m_tx_relay.cs_tx_inventory); - if (!m_tx_relay.filterInventoryKnown.contains(inv.hash)) { - m_tx_relay.setInventoryTxToSend.insert(inv.hash); + LOCK(m_tx_relay->cs_tx_inventory); + if (!m_tx_relay->filterInventoryKnown.contains(inv.hash)) { + m_tx_relay->setInventoryTxToSend.insert(inv.hash); } } else if (inv.type == MSG_BLOCK) { LOCK(cs_inventory); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c88d17b6ee..303a060a99 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1449,10 +1449,10 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c bool sendMerkleBlock = false; CMerkleBlock merkleBlock; { - LOCK(pfrom->m_tx_relay.cs_filter); - if (pfrom->m_tx_relay.pfilter) { + LOCK(pfrom->m_tx_relay->cs_filter); + if (pfrom->m_tx_relay->pfilter) { sendMerkleBlock = true; - merkleBlock = CMerkleBlock(*pblock, *pfrom->m_tx_relay.pfilter); + merkleBlock = CMerkleBlock(*pblock, *pfrom->m_tx_relay->pfilter); } } if (sendMerkleBlock) { @@ -1532,11 +1532,11 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm if (mi != mapRelay.end()) { connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *mi->second)); push = true; - } else if (pfrom->m_tx_relay.timeLastMempoolReq) { + } else if (pfrom->m_tx_relay->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->m_tx_relay.timeLastMempoolReq) { + if (txinfo.tx && txinfo.nTime <= pfrom->m_tx_relay->timeLastMempoolReq) { connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *txinfo.tx)); push = true; } @@ -1996,8 +1996,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr pfrom->m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED)); { - LOCK(pfrom->m_tx_relay.cs_filter); - pfrom->m_tx_relay.fRelayTxes = fRelay; // set to true after we get the first filter* message + LOCK(pfrom->m_tx_relay->cs_filter); + pfrom->m_tx_relay->fRelayTxes = fRelay; // set to true after we get the first filter* message } // Change version @@ -3030,8 +3030,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; } - LOCK(pfrom->m_tx_relay.cs_tx_inventory); - pfrom->m_tx_relay.fSendMempool = true; + LOCK(pfrom->m_tx_relay->cs_tx_inventory); + pfrom->m_tx_relay->fSendMempool = true; return true; } @@ -3124,10 +3124,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } else { - LOCK(pfrom->m_tx_relay.cs_filter); - pfrom->m_tx_relay.pfilter.reset(new CBloomFilter(filter)); - pfrom->m_tx_relay.pfilter->UpdateEmptyFull(); - pfrom->m_tx_relay.fRelayTxes = true; + LOCK(pfrom->m_tx_relay->cs_filter); + pfrom->m_tx_relay->pfilter.reset(new CBloomFilter(filter)); + pfrom->m_tx_relay->pfilter->UpdateEmptyFull(); + pfrom->m_tx_relay->fRelayTxes = true; } return true; } @@ -3142,9 +3142,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) { bad = true; } else { - LOCK(pfrom->m_tx_relay.cs_filter); - if (pfrom->m_tx_relay.pfilter) { - pfrom->m_tx_relay.pfilter->insert(vData); + LOCK(pfrom->m_tx_relay->cs_filter); + if (pfrom->m_tx_relay->pfilter) { + pfrom->m_tx_relay->pfilter->insert(vData); } else { bad = true; } @@ -3157,11 +3157,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } if (strCommand == NetMsgType::FILTERCLEAR) { - LOCK(pfrom->m_tx_relay.cs_filter); + LOCK(pfrom->m_tx_relay->cs_filter); if (pfrom->GetLocalServices() & NODE_BLOOM) { - pfrom->m_tx_relay.pfilter.reset(new CBloomFilter()); + pfrom->m_tx_relay->pfilter.reset(new CBloomFilter()); } - pfrom->m_tx_relay.fRelayTxes = true; + pfrom->m_tx_relay->fRelayTxes = true; return true; } @@ -3170,8 +3170,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr vRecv >> newFeeFilter; if (MoneyRange(newFeeFilter)) { { - LOCK(pfrom->m_tx_relay.cs_feeFilter); - pfrom->m_tx_relay.minFeeFilter = newFeeFilter; + LOCK(pfrom->m_tx_relay->cs_feeFilter); + pfrom->m_tx_relay->minFeeFilter = newFeeFilter; } LogPrint(BCLog::NET, "received: feefilter of %s from peer=%d\n", CFeeRate(newFeeFilter).ToString(), pfrom->GetId()); } @@ -3791,70 +3791,70 @@ bool PeerLogicValidation::SendMessages(CNode* pto) } pto->vInventoryBlockToSend.clear(); - LOCK(pto->m_tx_relay.cs_tx_inventory); + LOCK(pto->m_tx_relay->cs_tx_inventory); // Check whether periodic sends should happen bool fSendTrickle = pto->HasPermission(PF_NOBAN); - if (pto->m_tx_relay.nNextInvSend < nNow) { + if (pto->m_tx_relay->nNextInvSend < nNow) { fSendTrickle = true; if (pto->fInbound) { - pto->m_tx_relay.nNextInvSend = connman->PoissonNextSendInbound(nNow, INVENTORY_BROADCAST_INTERVAL); + pto->m_tx_relay->nNextInvSend = connman->PoissonNextSendInbound(nNow, INVENTORY_BROADCAST_INTERVAL); } else { // Use half the delay for outbound peers, as there is less privacy concern for them. - pto->m_tx_relay.nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> 1); + pto->m_tx_relay->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> 1); } } // Time to send but the peer has requested we not relay transactions. if (fSendTrickle) { - LOCK(pto->m_tx_relay.cs_filter); - if (!pto->m_tx_relay.fRelayTxes) pto->m_tx_relay.setInventoryTxToSend.clear(); + LOCK(pto->m_tx_relay->cs_filter); + if (!pto->m_tx_relay->fRelayTxes) pto->m_tx_relay->setInventoryTxToSend.clear(); } // Respond to BIP35 mempool requests - if (fSendTrickle && pto->m_tx_relay.fSendMempool) { + if (fSendTrickle && pto->m_tx_relay->fSendMempool) { auto vtxinfo = mempool.infoAll(); - pto->m_tx_relay.fSendMempool = false; + pto->m_tx_relay->fSendMempool = false; CAmount filterrate = 0; { - LOCK(pto->m_tx_relay.cs_feeFilter); - filterrate = pto->m_tx_relay.minFeeFilter; + LOCK(pto->m_tx_relay->cs_feeFilter); + filterrate = pto->m_tx_relay->minFeeFilter; } - LOCK(pto->m_tx_relay.cs_filter); + LOCK(pto->m_tx_relay->cs_filter); for (const auto& txinfo : vtxinfo) { const uint256& hash = txinfo.tx->GetHash(); CInv inv(MSG_TX, hash); - pto->m_tx_relay.setInventoryTxToSend.erase(hash); + pto->m_tx_relay->setInventoryTxToSend.erase(hash); if (filterrate) { if (txinfo.feeRate.GetFeePerK() < filterrate) continue; } - if (pto->m_tx_relay.pfilter) { - if (!pto->m_tx_relay.pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (pto->m_tx_relay->pfilter) { + if (!pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; } - pto->m_tx_relay.filterInventoryKnown.insert(hash); + pto->m_tx_relay->filterInventoryKnown.insert(hash); vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } } - pto->m_tx_relay.timeLastMempoolReq = GetTime(); + pto->m_tx_relay->timeLastMempoolReq = GetTime(); } // Determine transactions to relay if (fSendTrickle) { // Produce a vector with all candidates for sending std::vector::iterator> vInvTx; - vInvTx.reserve(pto->m_tx_relay.setInventoryTxToSend.size()); - for (std::set::iterator it = pto->m_tx_relay.setInventoryTxToSend.begin(); it != pto->m_tx_relay.setInventoryTxToSend.end(); it++) { + vInvTx.reserve(pto->m_tx_relay->setInventoryTxToSend.size()); + for (std::set::iterator it = pto->m_tx_relay->setInventoryTxToSend.begin(); it != pto->m_tx_relay->setInventoryTxToSend.end(); it++) { vInvTx.push_back(it); } CAmount filterrate = 0; { - LOCK(pto->m_tx_relay.cs_feeFilter); - filterrate = pto->m_tx_relay.minFeeFilter; + LOCK(pto->m_tx_relay->cs_feeFilter); + filterrate = pto->m_tx_relay->minFeeFilter; } // Topologically and fee-rate sort the inventory we send for privacy and priority reasons. // A heap is used so that not all items need sorting if only a few are being sent. @@ -3863,7 +3863,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // No reason to drain out at many times the network's capacity, // especially since we have many peers and some will draw much shorter delays. unsigned int nRelayedTransactions = 0; - LOCK(pto->m_tx_relay.cs_filter); + LOCK(pto->m_tx_relay->cs_filter); while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) { // Fetch the top element from the heap std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); @@ -3871,9 +3871,9 @@ bool PeerLogicValidation::SendMessages(CNode* pto) vInvTx.pop_back(); uint256 hash = *it; // Remove it from the to-be-sent set - pto->m_tx_relay.setInventoryTxToSend.erase(it); + pto->m_tx_relay->setInventoryTxToSend.erase(it); // Check if not in the filter already - if (pto->m_tx_relay.filterInventoryKnown.contains(hash)) { + if (pto->m_tx_relay->filterInventoryKnown.contains(hash)) { continue; } // Not in the mempool anymore? don't bother sending it. @@ -3884,7 +3884,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) if (filterrate && txinfo.feeRate.GetFeePerK() < filterrate) { continue; } - if (pto->m_tx_relay.pfilter && !pto->m_tx_relay.pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (pto->m_tx_relay->pfilter && !pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send vInv.push_back(CInv(MSG_TX, hash)); nRelayedTransactions++; @@ -3905,7 +3905,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); vInv.clear(); } - pto->m_tx_relay.filterInventoryKnown.insert(hash); + pto->m_tx_relay->filterInventoryKnown.insert(hash); } } } @@ -4070,23 +4070,23 @@ bool PeerLogicValidation::SendMessages(CNode* pto) !pto->HasPermission(PF_FORCERELAY)) { CAmount currentFilter = mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); int64_t timeNow = GetTimeMicros(); - if (timeNow > pto->m_tx_relay.nextSendTimeFeeFilter) { + if (timeNow > pto->m_tx_relay->nextSendTimeFeeFilter) { static CFeeRate default_feerate(DEFAULT_MIN_RELAY_TX_FEE); static FeeFilterRounder filterRounder(default_feerate); CAmount filterToSend = filterRounder.round(currentFilter); // We always have a fee filter of at least minRelayTxFee filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK()); - if (filterToSend != pto->m_tx_relay.lastSentFeeFilter) { + if (filterToSend != pto->m_tx_relay->lastSentFeeFilter) { connman->PushMessage(pto, msgMaker.Make(NetMsgType::FEEFILTER, filterToSend)); - pto->m_tx_relay.lastSentFeeFilter = filterToSend; + pto->m_tx_relay->lastSentFeeFilter = filterToSend; } - pto->m_tx_relay.nextSendTimeFeeFilter = PoissonNextSend(timeNow, AVG_FEEFILTER_BROADCAST_INTERVAL); + pto->m_tx_relay->nextSendTimeFeeFilter = PoissonNextSend(timeNow, AVG_FEEFILTER_BROADCAST_INTERVAL); } // If the fee filter has changed substantially and it's still more than MAX_FEEFILTER_CHANGE_DELAY // until scheduled broadcast, then move the broadcast to within MAX_FEEFILTER_CHANGE_DELAY. - else if (timeNow + MAX_FEEFILTER_CHANGE_DELAY * 1000000 < pto->m_tx_relay.nextSendTimeFeeFilter && - (currentFilter < 3 * pto->m_tx_relay.lastSentFeeFilter / 4 || currentFilter > 4 * pto->m_tx_relay.lastSentFeeFilter / 3)) { - pto->m_tx_relay.nextSendTimeFeeFilter = timeNow + GetRandInt(MAX_FEEFILTER_CHANGE_DELAY) * 1000000; + else if (timeNow + MAX_FEEFILTER_CHANGE_DELAY * 1000000 < pto->m_tx_relay->nextSendTimeFeeFilter && + (currentFilter < 3 * pto->m_tx_relay->lastSentFeeFilter / 4 || currentFilter > 4 * pto->m_tx_relay->lastSentFeeFilter / 3)) { + pto->m_tx_relay->nextSendTimeFeeFilter = timeNow + GetRandInt(MAX_FEEFILTER_CHANGE_DELAY) * 1000000; } } } -- cgit v1.2.3 From e75c39cd425f8c4e5b6bbb2beecb9c80034fefe1 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Fri, 8 Mar 2019 15:48:41 -0500 Subject: Check that tx_relay is initialized before access --- src/net.cpp | 18 +++- src/net.h | 4 +- src/net_processing.cpp | 227 +++++++++++++++++++++++++------------------------ 3 files changed, 133 insertions(+), 116 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 78b33954d6..9514da8809 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -499,9 +499,11 @@ void CNode::copyStats(CNodeStats &stats) X(nServices); X(addr); X(addrBind); - { + if (m_tx_relay != nullptr) { LOCK(m_tx_relay->cs_filter); stats.fRelayTxes = m_tx_relay->fRelayTxes; + } else { + stats.fRelayTxes = false; } X(nLastSend); X(nLastRecv); @@ -528,9 +530,11 @@ void CNode::copyStats(CNodeStats &stats) } X(m_legacyWhitelisted); X(m_permissionFlags); - { + if (m_tx_relay != nullptr) { LOCK(m_tx_relay->cs_feeFilter); stats.minFeeFilter = m_tx_relay->minFeeFilter; + } else { + stats.minFeeFilter = 0; } // It is common for nodes with good ping times to suddenly become lagged, @@ -818,11 +822,17 @@ bool CConnman::AttemptToEvictConnection() continue; if (node->fDisconnect) continue; - LOCK(node->m_tx_relay->cs_filter); + bool peer_relay_txes = false; + bool peer_filter_not_null = false; + if (node->m_tx_relay != nullptr) { + LOCK(node->m_tx_relay->cs_filter); + peer_relay_txes = node->m_tx_relay->fRelayTxes; + peer_filter_not_null = node->m_tx_relay->pfilter != nullptr; + } NodeEvictionCandidate candidate = {node->GetId(), node->nTimeConnected, node->nMinPingUsecTime, node->nLastBlockTime, node->nLastTXTime, HasAllDesirableServiceFlags(node->nServices), - node->m_tx_relay->fRelayTxes, node->m_tx_relay->pfilter != nullptr, node->addr, node->nKeyedNetGroup, + peer_relay_txes, peer_filter_not_null, node->addr, node->nKeyedNetGroup, node->m_prefer_evict}; vEvictionCandidates.push_back(candidate); } diff --git a/src/net.h b/src/net.h index d12fc3ae9a..218dd67ba6 100644 --- a/src/net.h +++ b/src/net.h @@ -848,7 +848,7 @@ public: void AddInventoryKnown(const CInv& inv) { - { + if (m_tx_relay != nullptr) { LOCK(m_tx_relay->cs_tx_inventory); m_tx_relay->filterInventoryKnown.insert(inv.hash); } @@ -856,7 +856,7 @@ public: void PushInventory(const CInv& inv) { - if (inv.type == MSG_TX) { + if (inv.type == MSG_TX && m_tx_relay != nullptr) { LOCK(m_tx_relay->cs_tx_inventory); if (!m_tx_relay->filterInventoryKnown.contains(inv.hash)) { m_tx_relay->setInventoryTxToSend.insert(inv.hash); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 303a060a99..0802cf424d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1448,7 +1448,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c { bool sendMerkleBlock = false; CMerkleBlock merkleBlock; - { + if (pfrom->m_tx_relay != nullptr) { LOCK(pfrom->m_tx_relay->cs_filter); if (pfrom->m_tx_relay->pfilter) { sendMerkleBlock = true; @@ -1512,7 +1512,7 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm std::deque::iterator it = pfrom->vRecvGetData.begin(); std::vector vNotFound; const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); - { + if (pfrom->m_tx_relay != nullptr) { LOCK(cs_main); while (it != pfrom->vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX)) { @@ -1995,7 +1995,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // set nodes not capable of serving the complete blockchain history as "limited nodes" pfrom->m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED)); - { + if (pfrom->m_tx_relay != nullptr) { LOCK(pfrom->m_tx_relay->cs_filter); pfrom->m_tx_relay->fRelayTxes = fRelay; // set to true after we get the first filter* message } @@ -3030,8 +3030,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return true; } - LOCK(pfrom->m_tx_relay->cs_tx_inventory); - pfrom->m_tx_relay->fSendMempool = true; + if (pfrom->m_tx_relay != nullptr) { + LOCK(pfrom->m_tx_relay->cs_tx_inventory); + pfrom->m_tx_relay->fSendMempool = true; + } return true; } @@ -3122,7 +3124,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LOCK(cs_main); Misbehaving(pfrom->GetId(), 100); } - else + else if (pfrom->m_tx_relay != nullptr) { LOCK(pfrom->m_tx_relay->cs_filter); pfrom->m_tx_relay->pfilter.reset(new CBloomFilter(filter)); @@ -3141,7 +3143,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr bool bad = false; if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) { bad = true; - } else { + } else if (pfrom->m_tx_relay != nullptr) { LOCK(pfrom->m_tx_relay->cs_filter); if (pfrom->m_tx_relay->pfilter) { pfrom->m_tx_relay->pfilter->insert(vData); @@ -3157,6 +3159,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } if (strCommand == NetMsgType::FILTERCLEAR) { + if (pfrom->m_tx_relay == nullptr) { + return true; + } LOCK(pfrom->m_tx_relay->cs_filter); if (pfrom->GetLocalServices() & NODE_BLOOM) { pfrom->m_tx_relay->pfilter.reset(new CBloomFilter()); @@ -3169,7 +3174,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr CAmount newFeeFilter = 0; vRecv >> newFeeFilter; if (MoneyRange(newFeeFilter)) { - { + if (pfrom->m_tx_relay != nullptr) { LOCK(pfrom->m_tx_relay->cs_feeFilter); pfrom->m_tx_relay->minFeeFilter = newFeeFilter; } @@ -3791,121 +3796,123 @@ bool PeerLogicValidation::SendMessages(CNode* pto) } pto->vInventoryBlockToSend.clear(); - LOCK(pto->m_tx_relay->cs_tx_inventory); - // Check whether periodic sends should happen - bool fSendTrickle = pto->HasPermission(PF_NOBAN); - if (pto->m_tx_relay->nNextInvSend < nNow) { - fSendTrickle = true; - if (pto->fInbound) { - pto->m_tx_relay->nNextInvSend = connman->PoissonNextSendInbound(nNow, INVENTORY_BROADCAST_INTERVAL); - } else { - // Use half the delay for outbound peers, as there is less privacy concern for them. - pto->m_tx_relay->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> 1); + if (pto->m_tx_relay != nullptr) { + LOCK(pto->m_tx_relay->cs_tx_inventory); + // Check whether periodic sends should happen + bool fSendTrickle = pto->HasPermission(PF_NOBAN); + if (pto->m_tx_relay->nNextInvSend < nNow) { + fSendTrickle = true; + if (pto->fInbound) { + pto->m_tx_relay->nNextInvSend = connman->PoissonNextSendInbound(nNow, INVENTORY_BROADCAST_INTERVAL); + } else { + // Use half the delay for outbound peers, as there is less privacy concern for them. + pto->m_tx_relay->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> 1); + } } - } - - // Time to send but the peer has requested we not relay transactions. - if (fSendTrickle) { - LOCK(pto->m_tx_relay->cs_filter); - if (!pto->m_tx_relay->fRelayTxes) pto->m_tx_relay->setInventoryTxToSend.clear(); - } - // Respond to BIP35 mempool requests - if (fSendTrickle && pto->m_tx_relay->fSendMempool) { - auto vtxinfo = mempool.infoAll(); - pto->m_tx_relay->fSendMempool = false; - CAmount filterrate = 0; - { - LOCK(pto->m_tx_relay->cs_feeFilter); - filterrate = pto->m_tx_relay->minFeeFilter; + // Time to send but the peer has requested we not relay transactions. + if (fSendTrickle) { + LOCK(pto->m_tx_relay->cs_filter); + if (!pto->m_tx_relay->fRelayTxes) pto->m_tx_relay->setInventoryTxToSend.clear(); } - LOCK(pto->m_tx_relay->cs_filter); - - for (const auto& txinfo : vtxinfo) { - const uint256& hash = txinfo.tx->GetHash(); - CInv inv(MSG_TX, hash); - pto->m_tx_relay->setInventoryTxToSend.erase(hash); - if (filterrate) { - if (txinfo.feeRate.GetFeePerK() < filterrate) - continue; - } - if (pto->m_tx_relay->pfilter) { - if (!pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + // Respond to BIP35 mempool requests + if (fSendTrickle && pto->m_tx_relay->fSendMempool) { + auto vtxinfo = mempool.infoAll(); + pto->m_tx_relay->fSendMempool = false; + CAmount filterrate = 0; + { + LOCK(pto->m_tx_relay->cs_feeFilter); + filterrate = pto->m_tx_relay->minFeeFilter; } - pto->m_tx_relay->filterInventoryKnown.insert(hash); - vInv.push_back(inv); - if (vInv.size() == MAX_INV_SZ) { - connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); - vInv.clear(); + + LOCK(pto->m_tx_relay->cs_filter); + + for (const auto& txinfo : vtxinfo) { + const uint256& hash = txinfo.tx->GetHash(); + CInv inv(MSG_TX, hash); + pto->m_tx_relay->setInventoryTxToSend.erase(hash); + if (filterrate) { + if (txinfo.feeRate.GetFeePerK() < filterrate) + continue; + } + if (pto->m_tx_relay->pfilter) { + if (!pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + } + pto->m_tx_relay->filterInventoryKnown.insert(hash); + vInv.push_back(inv); + if (vInv.size() == MAX_INV_SZ) { + connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + vInv.clear(); + } } + pto->m_tx_relay->timeLastMempoolReq = GetTime(); } - pto->m_tx_relay->timeLastMempoolReq = GetTime(); - } - // Determine transactions to relay - if (fSendTrickle) { - // Produce a vector with all candidates for sending - std::vector::iterator> vInvTx; - vInvTx.reserve(pto->m_tx_relay->setInventoryTxToSend.size()); - for (std::set::iterator it = pto->m_tx_relay->setInventoryTxToSend.begin(); it != pto->m_tx_relay->setInventoryTxToSend.end(); it++) { - vInvTx.push_back(it); - } - CAmount filterrate = 0; - { - LOCK(pto->m_tx_relay->cs_feeFilter); - filterrate = pto->m_tx_relay->minFeeFilter; - } - // Topologically and fee-rate sort the inventory we send for privacy and priority reasons. - // A heap is used so that not all items need sorting if only a few are being sent. - CompareInvMempoolOrder compareInvMempoolOrder(&mempool); - std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); - // No reason to drain out at many times the network's capacity, - // especially since we have many peers and some will draw much shorter delays. - unsigned int nRelayedTransactions = 0; - LOCK(pto->m_tx_relay->cs_filter); - while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) { - // Fetch the top element from the heap - std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); - std::set::iterator it = vInvTx.back(); - vInvTx.pop_back(); - uint256 hash = *it; - // Remove it from the to-be-sent set - pto->m_tx_relay->setInventoryTxToSend.erase(it); - // Check if not in the filter already - if (pto->m_tx_relay->filterInventoryKnown.contains(hash)) { - continue; - } - // Not in the mempool anymore? don't bother sending it. - auto txinfo = mempool.info(hash); - if (!txinfo.tx) { - continue; + // Determine transactions to relay + if (fSendTrickle) { + // Produce a vector with all candidates for sending + std::vector::iterator> vInvTx; + vInvTx.reserve(pto->m_tx_relay->setInventoryTxToSend.size()); + for (std::set::iterator it = pto->m_tx_relay->setInventoryTxToSend.begin(); it != pto->m_tx_relay->setInventoryTxToSend.end(); it++) { + vInvTx.push_back(it); } - if (filterrate && txinfo.feeRate.GetFeePerK() < filterrate) { - continue; - } - if (pto->m_tx_relay->pfilter && !pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; - // Send - vInv.push_back(CInv(MSG_TX, hash)); - nRelayedTransactions++; + CAmount filterrate = 0; { - // Expire old relay messages - while (!vRelayExpiration.empty() && vRelayExpiration.front().first < nNow) - { - mapRelay.erase(vRelayExpiration.front().second); - vRelayExpiration.pop_front(); + LOCK(pto->m_tx_relay->cs_feeFilter); + filterrate = pto->m_tx_relay->minFeeFilter; + } + // Topologically and fee-rate sort the inventory we send for privacy and priority reasons. + // A heap is used so that not all items need sorting if only a few are being sent. + CompareInvMempoolOrder compareInvMempoolOrder(&mempool); + std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); + // No reason to drain out at many times the network's capacity, + // especially since we have many peers and some will draw much shorter delays. + unsigned int nRelayedTransactions = 0; + LOCK(pto->m_tx_relay->cs_filter); + while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) { + // Fetch the top element from the heap + std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); + std::set::iterator it = vInvTx.back(); + vInvTx.pop_back(); + uint256 hash = *it; + // Remove it from the to-be-sent set + pto->m_tx_relay->setInventoryTxToSend.erase(it); + // Check if not in the filter already + if (pto->m_tx_relay->filterInventoryKnown.contains(hash)) { + continue; + } + // Not in the mempool anymore? don't bother sending it. + auto txinfo = mempool.info(hash); + if (!txinfo.tx) { + continue; + } + if (filterrate && txinfo.feeRate.GetFeePerK() < filterrate) { + continue; } + if (pto->m_tx_relay->pfilter && !pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + // Send + vInv.push_back(CInv(MSG_TX, hash)); + nRelayedTransactions++; + { + // Expire old relay messages + while (!vRelayExpiration.empty() && vRelayExpiration.front().first < nNow) + { + mapRelay.erase(vRelayExpiration.front().second); + vRelayExpiration.pop_front(); + } - auto ret = mapRelay.insert(std::make_pair(hash, std::move(txinfo.tx))); - if (ret.second) { - vRelayExpiration.push_back(std::make_pair(nNow + 15 * 60 * 1000000, ret.first)); + auto ret = mapRelay.insert(std::make_pair(hash, std::move(txinfo.tx))); + if (ret.second) { + vRelayExpiration.push_back(std::make_pair(nNow + 15 * 60 * 1000000, ret.first)); + } } + if (vInv.size() == MAX_INV_SZ) { + connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + vInv.clear(); + } + pto->m_tx_relay->filterInventoryKnown.insert(hash); } - if (vInv.size() == MAX_INV_SZ) { - connman->PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); - vInv.clear(); - } - pto->m_tx_relay->filterInventoryKnown.insert(hash); } } } @@ -4066,7 +4073,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // Message: feefilter // // We don't want white listed peers to filter txs to us if we have -whitelistforcerelay - if (pto->nVersion >= FEEFILTER_VERSION && gArgs.GetBoolArg("-feefilter", DEFAULT_FEEFILTER) && + if (pto->m_tx_relay != nullptr && pto->nVersion >= FEEFILTER_VERSION && gArgs.GetBoolArg("-feefilter", DEFAULT_FEEFILTER) && !pto->HasPermission(PF_FORCERELAY)) { CAmount currentFilter = mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); int64_t timeNow = GetTimeMicros(); -- cgit v1.2.3 From b83f51a4bbe29bf130a2b0c0e85e5bffea107f75 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Fri, 8 Mar 2019 16:04:55 -0500 Subject: Add comment explaining intended use of m_tx_relay --- src/net.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/net.h b/src/net.h index 218dd67ba6..31f446cde9 100644 --- a/src/net.h +++ b/src/net.h @@ -733,6 +733,7 @@ public: int64_t nextSendTimeFeeFilter{0}; }; + // m_tx_relay == nullptr if we're not relaying transactions with this peer std::unique_ptr m_tx_relay; // Used for headers announcements - unfiltered blocks to relay std::vector vBlockHashesToAnnounce GUARDED_BY(cs_inventory); -- cgit v1.2.3 From 3a5e885306ea954d7eccdc11502e91a51dab8ec6 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Sat, 9 Mar 2019 12:55:06 -0500 Subject: Add 2 outbound block-relay-only connections Transaction relay is primarily optimized for balancing redundancy/robustness with bandwidth minimization -- as a result transaction relay leaks information that adversaries can use to infer the network topology. Network topology is better kept private for (at least) two reasons: (a) Knowledge of the network graph can make it easier to find the source IP of a given transaction. (b) Knowledge of the network graph could be used to split a target node or nodes from the honest network (eg by knowing which peers to attack in order to achieve a network split). We can eliminate the risks of (b) by separating block relay from transaction relay; inferring network connectivity from the relay of blocks/block headers is much more expensive for an adversary. After this commit, bitcoind will make 2 additional outbound connections that are only used for block relay. (In the future, we might consider rotating our transaction-relay peers to help limit the effects of (a).) --- src/init.cpp | 3 ++- src/net.cpp | 42 +++++++++++++++++++++++++------------- src/net.h | 33 +++++++++++++++++++++--------- src/net_processing.cpp | 25 +++++++++++++++-------- src/test/denialofservice_tests.cpp | 12 +++++------ 5 files changed, 75 insertions(+), 40 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 25c964205a..1bc5eb3f7b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1753,7 +1753,8 @@ bool AppInitMain(InitInterfaces& interfaces) CConnman::Options connOptions; connOptions.nLocalServices = nLocalServices; connOptions.nMaxConnections = nMaxConnections; - connOptions.nMaxOutbound = std::min(MAX_OUTBOUND_CONNECTIONS, connOptions.nMaxConnections); + connOptions.m_max_outbound_full_relay = std::min(MAX_OUTBOUND_FULL_RELAY_CONNECTIONS, connOptions.nMaxConnections); + connOptions.m_max_outbound_block_relay = std::min(MAX_BLOCKS_ONLY_CONNECTIONS, connOptions.nMaxConnections-connOptions.m_max_outbound_full_relay); connOptions.nMaxAddnode = MAX_ADDNODE_CONNECTIONS; connOptions.nMaxFeeler = 1; connOptions.nBestHeight = chain_active_height; diff --git a/src/net.cpp b/src/net.cpp index 9514da8809..e589b940ad 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -352,7 +352,7 @@ static CAddress GetBindAddress(SOCKET sock) return addr_bind; } -CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection) +CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection, bool block_relay_only) { if (pszDest == nullptr) { if (IsLocal(addrConnect)) @@ -442,7 +442,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo NodeId id = GetNewNodeId(); uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); CAddress addr_bind = GetBindAddress(hSocket); - CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false); + CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false, block_relay_only); pnode->AddRef(); return pnode; @@ -905,7 +905,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { SOCKET hSocket = accept(hListenSocket.socket, (struct sockaddr*)&sockaddr, &len); CAddress addr; int nInbound = 0; - int nMaxInbound = nMaxConnections - (nMaxOutbound + nMaxFeeler); + int nMaxInbound = nMaxConnections - m_max_outbound; if (hSocket != INVALID_SOCKET) { if (!addr.SetSockAddr((const struct sockaddr*)&sockaddr)) { @@ -1666,7 +1666,7 @@ int CConnman::GetExtraOutboundCount() } } } - return std::max(nOutbound - nMaxOutbound, 0); + return std::max(nOutbound - m_max_outbound_full_relay - m_max_outbound_block_relay, 0); } void CConnman::ThreadOpenConnections(const std::vector connect) @@ -1726,7 +1726,8 @@ void CConnman::ThreadOpenConnections(const std::vector connect) CAddress addrConnect; // Only connect out to one peer per network group (/16 for IPv4). - int nOutbound = 0; + int nOutboundFullRelay = 0; + int nOutboundBlockRelay = 0; std::set > setConnected; { LOCK(cs_vNodes); @@ -1738,7 +1739,11 @@ void CConnman::ThreadOpenConnections(const std::vector connect) // also have the added issue that they're attacker controlled and could be used // to prevent us from connecting to particular hosts if we used them here. setConnected.insert(pnode->addr.GetGroup()); - nOutbound++; + if (pnode->m_tx_relay == nullptr) { + nOutboundBlockRelay++; + } else if (!pnode->fFeeler) { + nOutboundFullRelay++; + } } } } @@ -1757,7 +1762,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) // bool fFeeler = false; - if (nOutbound >= nMaxOutbound && !GetTryNewOutboundPeer()) { + if (nOutboundFullRelay >= m_max_outbound_full_relay && nOutboundBlockRelay >= m_max_outbound_block_relay && !GetTryNewOutboundPeer()) { int64_t nTime = GetTimeMicros(); // The current time right now (in microseconds). if (nTime > nNextFeeler) { nNextFeeler = PoissonNextSend(nTime, FEELER_INTERVAL); @@ -1831,7 +1836,14 @@ void CConnman::ThreadOpenConnections(const std::vector connect) LogPrint(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToString()); } - OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, false, fFeeler); + // Open this connection as block-relay-only if we're already at our + // full-relay capacity, but not yet at our block-relay peer limit. + // (It should not be possible for fFeeler to be set if we're not + // also at our block-relay peer limit, but check against that as + // well for sanity.) + bool block_relay_only = nOutboundBlockRelay < m_max_outbound_block_relay && !fFeeler && nOutboundFullRelay >= m_max_outbound_full_relay; + + OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, false, fFeeler, false, block_relay_only); } } } @@ -1918,7 +1930,7 @@ void CConnman::ThreadOpenAddedConnections() } // if successful, this moves the passed grant to the constructed node -void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, bool fFeeler, bool manual_connection) +void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, bool fFeeler, bool manual_connection, bool block_relay_only) { // // Initiate outbound network connection @@ -1937,7 +1949,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai } else if (FindNode(std::string(pszDest))) return; - CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, manual_connection); + CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, manual_connection, block_relay_only); if (!pnode) return; @@ -2240,7 +2252,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) if (semOutbound == nullptr) { // initialize semaphore - semOutbound = MakeUnique(std::min((nMaxOutbound + nMaxFeeler), nMaxConnections)); + semOutbound = MakeUnique(std::min(m_max_outbound, nMaxConnections)); } if (semAddnode == nullptr) { // initialize semaphore @@ -2318,7 +2330,7 @@ void CConnman::Interrupt() InterruptSocks5(true); if (semOutbound) { - for (int i=0; i<(nMaxOutbound + nMaxFeeler); i++) { + for (int i=0; ipost(); } } @@ -2628,7 +2640,7 @@ int CConnman::GetBestHeight() const unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; } -CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, bool fInboundIn) +CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, bool fInboundIn, bool block_relay_only) : nTimeConnected(GetSystemTimeInSeconds()), addr(addrIn), addrBind(addrBindIn), @@ -2643,7 +2655,9 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn hSocket = hSocketIn; addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn; hashContinue = uint256(); - m_tx_relay = MakeUnique(); + if (!block_relay_only) { + m_tx_relay = MakeUnique(); + } for (const std::string &msg : getAllNetMessageTypes()) mapRecvBytesPerMsgCmd[msg] = 0; diff --git a/src/net.h b/src/net.h index 31f446cde9..20b7932037 100644 --- a/src/net.h +++ b/src/net.h @@ -56,10 +56,12 @@ static const unsigned int MAX_ADDR_TO_SEND = 1000; static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 4 * 1000 * 1000; /** Maximum length of the user agent string in `version` message */ static const unsigned int MAX_SUBVERSION_LENGTH = 256; -/** Maximum number of automatic outgoing nodes */ -static const int MAX_OUTBOUND_CONNECTIONS = 8; +/** Maximum number of automatic outgoing nodes over which we'll relay everything (blocks, tx, addrs, etc) */ +static const int MAX_OUTBOUND_FULL_RELAY_CONNECTIONS = 8; /** Maximum number of addnode outgoing nodes */ static const int MAX_ADDNODE_CONNECTIONS = 8; +/** Maximum number of block-relay-only outgoing connections */ +static const int MAX_BLOCKS_ONLY_CONNECTIONS = 2; /** -listen default */ static const bool DEFAULT_LISTEN = true; /** -upnp default */ @@ -126,7 +128,8 @@ public: { ServiceFlags nLocalServices = NODE_NONE; int nMaxConnections = 0; - int nMaxOutbound = 0; + int m_max_outbound_full_relay = 0; + int m_max_outbound_block_relay = 0; int nMaxAddnode = 0; int nMaxFeeler = 0; int nBestHeight = 0; @@ -150,10 +153,12 @@ public: void Init(const Options& connOptions) { nLocalServices = connOptions.nLocalServices; nMaxConnections = connOptions.nMaxConnections; - nMaxOutbound = std::min(connOptions.nMaxOutbound, connOptions.nMaxConnections); + m_max_outbound_full_relay = std::min(connOptions.m_max_outbound_full_relay, connOptions.nMaxConnections); + m_max_outbound_block_relay = connOptions.m_max_outbound_block_relay; m_use_addrman_outgoing = connOptions.m_use_addrman_outgoing; nMaxAddnode = connOptions.nMaxAddnode; nMaxFeeler = connOptions.nMaxFeeler; + m_max_outbound = m_max_outbound_full_relay + m_max_outbound_block_relay + nMaxFeeler; nBestHeight = connOptions.nBestHeight; clientInterface = connOptions.uiInterface; m_banman = connOptions.m_banman; @@ -192,7 +197,7 @@ public: bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active); - void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, bool fOneShot = false, bool fFeeler = false, bool manual_connection = false); + void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, bool fOneShot = false, bool fFeeler = false, bool manual_connection = false, bool block_relay_only = false); bool CheckIncomingNonce(uint64_t nonce); bool ForNode(NodeId id, std::function func); @@ -248,7 +253,7 @@ public: void AddNewAddresses(const std::vector& vAddr, const CAddress& addrFrom, int64_t nTimePenalty = 0); std::vector GetAddresses(); - // This allows temporarily exceeding nMaxOutbound, with the goal of finding + // This allows temporarily exceeding m_max_outbound_full_relay, with the goal of finding // a peer that is better than all our current peers. void SetTryNewOutboundPeer(bool flag); bool GetTryNewOutboundPeer(); @@ -350,7 +355,7 @@ private: CNode* FindNode(const CService& addr); bool AttemptToEvictConnection(); - CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection); + CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection, bool block_relay_only); void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const; void DeleteNode(CNode* pnode); @@ -409,9 +414,17 @@ private: std::unique_ptr semOutbound; std::unique_ptr semAddnode; int nMaxConnections; - int nMaxOutbound; + + // How many full-relay (tx, block, addr) outbound peers we want + int m_max_outbound_full_relay; + + // How many block-relay only outbound peers we want + // We do not relay tx or addr messages with these peers + int m_max_outbound_block_relay; + int nMaxAddnode; int nMaxFeeler; + int m_max_outbound; bool m_use_addrman_outgoing; std::atomic nBestHeight; CClientUIInterface* clientInterface; @@ -437,7 +450,7 @@ private: std::thread threadMessageHandler; /** flag for deciding to connect to an extra outbound peer, - * in excess of nMaxOutbound + * in excess of m_max_outbound_full_relay * This takes the place of a feeler connection */ std::atomic_bool m_try_another_outbound_peer; @@ -756,7 +769,7 @@ public: std::set orphan_work_set; - CNode(NodeId id, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress &addrBindIn, const std::string &addrNameIn = "", bool fInboundIn = false); + CNode(NodeId id, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress &addrBindIn, const std::string &addrNameIn = "", bool fInboundIn = false, bool block_relay_only = false); ~CNode(); CNode(const CNode&) = delete; CNode& operator=(const CNode&) = delete; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0802cf424d..48a7b91dcb 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -262,7 +262,7 @@ struct CNodeState { bool fSupportsDesiredCmpctVersion; /** State used to enforce CHAIN_SYNC_TIMEOUT - * Only in effect for outbound, non-manual connections, with + * Only in effect for outbound, non-manual, full-relay connections, with * m_protect == false * Algorithm: if a peer's best known block has less work than our tip, * set a timeout CHAIN_SYNC_TIMEOUT seconds in the future: @@ -425,7 +425,7 @@ static void PushNodeVersion(CNode *pnode, CConnman* connman, int64_t nTime) CAddress addrMe = CAddress(CService(), nLocalNodeServices); connman->PushMessage(pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, (uint64_t)nLocalNodeServices, nTime, addrYou, addrMe, - nonce, strSubVersion, nNodeStartingHeight, ::g_relay_txes)); + nonce, strSubVersion, nNodeStartingHeight, ::g_relay_txes && pnode->m_tx_relay != nullptr)); if (fLogIPs) { LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, us=%s, them=%s, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, addrMe.ToString(), addrYou.ToString(), nodeid); @@ -757,7 +757,7 @@ void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) } // Returns true for outbound peers, excluding manual connections, feelers, and -// one-shots +// one-shots. static bool IsOutboundDisconnectionCandidate(const CNode *node) { return !(node->fInbound || node->m_manual_connection || node->fFeeler || node->fOneShot); @@ -1772,9 +1772,11 @@ bool static ProcessHeadersMessage(CNode *pfrom, CConnman *connman, const std::ve } } - if (!pfrom->fDisconnect && IsOutboundDisconnectionCandidate(pfrom) && nodestate->pindexBestKnownBlock != nullptr) { - // If this is an outbound peer, check to see if we should protect + if (!pfrom->fDisconnect && IsOutboundDisconnectionCandidate(pfrom) && nodestate->pindexBestKnownBlock != nullptr && pfrom->m_tx_relay != nullptr) { + // If this is an outbound full-relay peer, check to see if we should protect // it from the bad/lagging chain logic. + // Note that block-relay-only peers are already implicitly protected, so we + // only consider setting m_protect for the full-relay peers. if (g_outbound_peers_with_protect_from_disconnect < MAX_OUTBOUND_PEERS_TO_PROTECT_FROM_DISCONNECT && nodestate->pindexBestKnownBlock->nChainWork >= ::ChainActive().Tip()->nChainWork && !nodestate->m_chain_sync.m_protect) { LogPrint(BCLog::NET, "Protecting outbound peer=%d from eviction\n", pfrom->GetId()); nodestate->m_chain_sync.m_protect = true; @@ -2088,9 +2090,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // Mark this node as currently connected, so we update its timestamp later. LOCK(cs_main); State(pfrom->GetId())->fCurrentlyConnected = true; - LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s\n", - pfrom->nVersion.load(), pfrom->nStartingHeight, pfrom->GetId(), - (fLogIPs ? strprintf(", peeraddr=%s", pfrom->addr.ToString()) : "")); + LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s (%s)\n", + pfrom->nVersion.load(), pfrom->nStartingHeight, + pfrom->GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom->addr.ToString()) : ""), + pfrom->m_tx_relay == nullptr ? "block-relay" : "full-relay"); } if (pfrom->nVersion >= SENDHEADERS_VERSION) { @@ -2214,7 +2217,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr return false; } - bool fBlocksOnly = !g_relay_txes; + // We won't accept tx inv's if we're in blocks-only mode, or this is a + // block-relay-only peer + bool fBlocksOnly = !g_relay_txes || (pfrom->m_tx_relay == nullptr); // Allow whitelisted peers to send data other than blocks in blocks only mode if whitelistrelay is true if (pfrom->HasPermission(PF_RELAY)) @@ -3453,6 +3458,8 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) if (state == nullptr) return; // shouldn't be possible, but just in case // Don't evict our protected peers if (state->m_chain_sync.m_protect) return; + // Don't evict our block-relay-only peers. + if (pnode->m_tx_relay == nullptr) return; if (state->m_last_block_announcement < oldest_block_announcement || (state->m_last_block_announcement == oldest_block_announcement && pnode->GetId() > worst_peer)) { worst_peer = pnode->GetId(); oldest_block_announcement = state->m_last_block_announcement; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index a50d6854f8..b0a613372f 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -151,17 +151,17 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) auto peerLogic = MakeUnique(connman.get(), nullptr, scheduler, false); const Consensus::Params& consensusParams = Params().GetConsensus(); - constexpr int nMaxOutbound = 8; + constexpr int max_outbound_full_relay = 8; CConnman::Options options; options.nMaxConnections = 125; - options.nMaxOutbound = nMaxOutbound; + options.m_max_outbound_full_relay = max_outbound_full_relay; options.nMaxFeeler = 1; connman->Init(options); std::vector vNodes; // Mock some outbound peers - for (int i=0; iCheckForStaleTipAndEvictPeers(consensusParams); - for (int i=0; ifDisconnect == false); } // Last added node should get marked for eviction @@ -203,10 +203,10 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) UpdateLastBlockAnnounceTime(vNodes.back()->GetId(), GetTime()); peerLogic->CheckForStaleTipAndEvictPeers(consensusParams); - for (int i=0; ifDisconnect == false); } - BOOST_CHECK(vNodes[nMaxOutbound-1]->fDisconnect == true); + BOOST_CHECK(vNodes[max_outbound_full_relay-1]->fDisconnect == true); BOOST_CHECK(vNodes.back()->fDisconnect == false); bool dummy; -- cgit v1.2.3 From 430f489027f15c1e4948ea4378954df24e3fee88 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Fri, 5 Apr 2019 13:35:15 -0400 Subject: Don't relay addr messages to block-relay-only peers We don't want relay of addr messages to leak information about these network links. --- src/net.cpp | 4 ++++ src/net.h | 4 ++++ src/net_processing.cpp | 15 +++++++++++---- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index e589b940ad..b44048cbce 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2647,6 +2647,10 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn fInbound(fInboundIn), nKeyedNetGroup(nKeyedNetGroupIn), addrKnown(5000, 0.001), + // Don't relay addr messages to peers that we connect to as block-relay-only + // peers (to prevent adversaries from inferring these links from addr + // traffic). + m_addr_relay_peer(!block_relay_only), id(idIn), nLocalHostNonce(nLocalHostNonceIn), nLocalServices(nLocalServicesIn), diff --git a/src/net.h b/src/net.h index 20b7932037..605d7a8252 100644 --- a/src/net.h +++ b/src/net.h @@ -712,6 +712,9 @@ public: int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing){0}; int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing){0}; + const bool m_addr_relay_peer; + bool IsAddrRelayPeer() const { return m_addr_relay_peer; } + // List of block ids we still have announce. // There is no final sorting before sending, as they are always sent immediately // and in the order requested. @@ -748,6 +751,7 @@ public: // m_tx_relay == nullptr if we're not relaying transactions with this peer std::unique_ptr m_tx_relay; + // Used for headers announcements - unfiltered blocks to relay std::vector vBlockHashesToAnnounce GUARDED_BY(cs_inventory); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 48a7b91dcb..657109ba52 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1329,7 +1329,7 @@ static void RelayAddress(const CAddress& addr, bool fReachable, CConnman* connma assert(nRelayNodes <= best.size()); auto sortfunc = [&best, &hasher, nRelayNodes](CNode* pnode) { - if (pnode->nVersion >= CADDR_TIME_VERSION) { + if (pnode->nVersion >= CADDR_TIME_VERSION && pnode->IsAddrRelayPeer()) { uint64_t hashKey = CSipHasher(hasher).Write(pnode->GetId()).Finalize(); for (unsigned int i = 0; i < nRelayNodes; i++) { if (hashKey > best[i].first) { @@ -2018,7 +2018,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr UpdatePreferredDownload(pfrom, State(pfrom->GetId())); } - if (!pfrom->fInbound) + if (!pfrom->fInbound && pfrom->IsAddrRelayPeer()) { // Advertise our address if (fListen && !::ChainstateActive().IsInitialBlockDownload()) @@ -2134,6 +2134,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // Don't want addr from older versions unless seeding if (pfrom->nVersion < CADDR_TIME_VERSION && connman->GetAddressCount() > 1000) return true; + if (!pfrom->IsAddrRelayPeer()) { + return true; + } if (vAddr.size() > 1000) { LOCK(cs_main); @@ -2994,6 +2997,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrint(BCLog::NET, "Ignoring \"getaddr\" from outbound connection. peer=%d\n", pfrom->GetId()); return true; } + if (!pfrom->IsAddrRelayPeer()) { + LogPrint(BCLog::NET, "Ignoring \"getaddr\" from block-relay-only connection. peer=%d\n", pfrom->GetId()); + return true; + } // Only send one GetAddr response per connection to reduce resource waste // and discourage addr stamping of INV announcements. @@ -3587,7 +3594,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // Address refresh broadcast int64_t nNow = GetTimeMicros(); - if (!::ChainstateActive().IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) { + if (pto->IsAddrRelayPeer() && !::ChainstateActive().IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) { AdvertiseLocal(pto); pto->nNextLocalAddrSend = PoissonNextSend(nNow, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } @@ -3595,7 +3602,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // // Message: addr // - if (pto->nNextAddrSend < nNow) { + if (pto->IsAddrRelayPeer() && pto->nNextAddrSend < nNow) { pto->nNextAddrSend = PoissonNextSend(nNow, AVG_ADDRESS_BROADCAST_INTERVAL); std::vector vAddr; vAddr.reserve(pto->vAddrToSend.size()); -- cgit v1.2.3 From 937eba91e1550bc3038dc541c236ac83e0a0e6d5 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Tue, 13 Aug 2019 10:08:48 -0400 Subject: doc: improve comments relating to block-relay-only peers --- src/net_processing.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 657109ba52..baf3bc7448 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1512,6 +1512,11 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm std::deque::iterator it = pfrom->vRecvGetData.begin(); std::vector vNotFound; const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); + + // Note that if we receive a getdata for a MSG_TX or MSG_WITNESS_TX from a + // block-relay-only outbound peer, we will stop processing further getdata + // messages from this peer (likely resulting in our peer eventually + // disconnecting us). if (pfrom->m_tx_relay != nullptr) { LOCK(cs_main); -- cgit v1.2.3 From 0ba08020c9791f7caf5986ad6490c16a2b66cd83 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Wed, 28 Aug 2019 17:23:45 -0400 Subject: Disconnect peers violating blocks-only mode If we set fRelay=false in our VERSION message, and a peer sends an INV or TX message anyway, disconnect. Since we use fRelay=false to minimize bandwidth, we should not tolerate remaining connected to a peer violating the protocol. --- src/net_processing.cpp | 8 ++++++-- test/functional/p2p_blocksonly.py | 10 +++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index baf3bc7448..4418174f95 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2266,7 +2266,9 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr { pfrom->AddInventoryKnown(inv); if (fBlocksOnly) { - LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol peer=%d\n", inv.hash.ToString(), pfrom->GetId()); + LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom->GetId()); + pfrom->fDisconnect = true; + return true; } else if (!fAlreadyHave && !fImporting && !fReindex && !::ChainstateActive().IsInitialBlockDownload()) { RequestTx(State(pfrom->GetId()), inv.hash, current_time); } @@ -2483,9 +2485,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (strCommand == NetMsgType::TX) { // Stop processing the transaction early if // We are in blocks only mode and peer is either not whitelisted or whitelistrelay is off - if (!g_relay_txes && !pfrom->HasPermission(PF_RELAY)) + // or if this peer is supposed to be a block-relay-only peer + if ((!g_relay_txes && !pfrom->HasPermission(PF_RELAY)) || (pfrom->m_tx_relay == nullptr)) { LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom->GetId()); + pfrom->fDisconnect = true; return true; } diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py index 12cb06a407..3258a38e3c 100755 --- a/test/functional/p2p_blocksonly.py +++ b/test/functional/p2p_blocksonly.py @@ -19,7 +19,7 @@ class P2PBlocksOnly(BitcoinTestFramework): def run_test(self): self.nodes[0].add_p2p_connection(P2PInterface()) - self.log.info('Check that txs from p2p are rejected') + self.log.info('Check that txs from p2p are rejected and result in disconnect') prevtx = self.nodes[0].getblock(self.nodes[0].getblockhash(1), 2)['tx'][0] rawtx = self.nodes[0].createrawtransaction( inputs=[{ @@ -42,13 +42,17 @@ class P2PBlocksOnly(BitcoinTestFramework): assert_equal(self.nodes[0].getnetworkinfo()['localrelay'], False) with self.nodes[0].assert_debug_log(['transaction sent in violation of protocol peer=0']): self.nodes[0].p2p.send_message(msg_tx(FromHex(CTransaction(), sigtx))) - self.nodes[0].p2p.sync_with_ping() + self.nodes[0].p2p.wait_for_disconnect() assert_equal(self.nodes[0].getmempoolinfo()['size'], 0) + # Remove the disconnected peer and add a new one. + del self.nodes[0].p2ps[0] + self.nodes[0].add_p2p_connection(P2PInterface()) + self.log.info('Check that txs from rpc are not rejected and relayed to other peers') assert_equal(self.nodes[0].getpeerinfo()[0]['relaytxes'], True) txid = self.nodes[0].testmempoolaccept([sigtx])[0]['txid'] - with self.nodes[0].assert_debug_log(['received getdata for: tx {} peer=0'.format(txid)]): + with self.nodes[0].assert_debug_log(['received getdata for: tx {} peer=1'.format(txid)]): self.nodes[0].sendrawtransaction(sigtx) self.nodes[0].p2p.wait_for_tx(txid) assert_equal(self.nodes[0].getmempoolinfo()['size'], 1) -- cgit v1.2.3