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(-) (limited to 'src/net.h') 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.h | 64 +++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 36 insertions(+), 28 deletions(-) (limited to 'src/net.h') 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); } } -- 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.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src/net.h') 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); -- 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.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/net.h') 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); -- 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(+) (limited to 'src/net.h') 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/net.h | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) (limited to 'src/net.h') 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; -- 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.h | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/net.h') 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); -- cgit v1.2.3