From deccf1c4848620cfff2a9642c5a6b5acdfbc33bd Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 13 Jul 2023 13:01:54 -0600 Subject: [PATCH 1/9] Move IsPeerAddrLocalGood() declaration from header to implementation --- src/net.cpp | 2 +- src/net.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 1b1b540417..e8567348f6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -223,7 +223,7 @@ static int GetnScore(const CService& addr) } // Is our peer's addrLocal potentially useful as an external IP source? -bool IsPeerAddrLocalGood(CNode *pnode) +[[nodiscard]] static bool IsPeerAddrLocalGood(CNode *pnode) { CService addrLocal = pnode->GetAddrLocal(); return fDiscover && pnode->addr.IsRoutable() && addrLocal.IsRoutable() && diff --git a/src/net.h b/src/net.h index 7427d0f45b..e26bf523e8 100644 --- a/src/net.h +++ b/src/net.h @@ -145,7 +145,6 @@ enum LOCAL_MAX }; -bool IsPeerAddrLocalGood(CNode *pnode); /** Returns a local address that we should advertise to this peer. */ std::optional GetLocalAddrForPeer(CNode& node); From 11426f6557ac09489d5e13bf3b9d94fbd5073c8e Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 13 Jul 2023 14:30:30 -0600 Subject: [PATCH 2/9] Move CaptureMessageToFile() declaration from header to implementation --- src/net.cpp | 9 +++++---- src/net.h | 6 ------ 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index e8567348f6..ee4faeed9f 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2915,10 +2915,11 @@ uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& address) const return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup.data(), vchNetGroup.size()).Finalize(); } -void CaptureMessageToFile(const CAddress& addr, - const std::string& msg_type, - Span data, - bool is_incoming) +// Dump binary message to file, with timestamp. +static void CaptureMessageToFile(const CAddress& addr, + const std::string& msg_type, + Span data, + bool is_incoming) { // Note: This function captures the message at the time of processing, // not at socket receive/send time. diff --git a/src/net.h b/src/net.h index e26bf523e8..1c2b60a30f 100644 --- a/src/net.h +++ b/src/net.h @@ -1221,12 +1221,6 @@ private: friend struct ConnmanTestMsg; }; -/** Dump binary message to file, with timestamp */ -void CaptureMessageToFile(const CAddress& addr, - const std::string& msg_type, - Span data, - bool is_incoming); - /** Defaults to `CaptureMessageToFile()`, but can be overridden by unit tests. */ extern std::function Date: Thu, 13 Jul 2023 12:14:16 -0600 Subject: [PATCH 3/9] Move GetLocal() declaration from header to implementation --- src/net.cpp | 2 +- src/net.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index ee4faeed9f..654e0ac9fd 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -146,7 +146,7 @@ uint16_t GetListenPort() } // find 'best' local address for a particular peer -bool GetLocal(CService& addr, const CNode& peer) +[[nodiscard]] static bool GetLocal(CService& addr, const CNode& peer) { if (!fListen) return false; diff --git a/src/net.h b/src/net.h index 1c2b60a30f..54c3839fcc 100644 --- a/src/net.h +++ b/src/net.h @@ -163,7 +163,6 @@ bool AddLocal(const CNetAddr& addr, int nScore = LOCAL_NONE); void RemoveLocal(const CService& addr); bool SeenLocal(const CService& addr); bool IsLocal(const CService& addr); -bool GetLocal(CService& addr, const CNode& peer); CService GetLocalAddress(const CNode& peer); CService MaybeFlipIPv6toCJDNS(const CService& service); From fb4265747c9eb022d80c6f2988e574c689130348 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 14 Jul 2023 10:48:30 -0600 Subject: [PATCH 4/9] Add and use CNetAddr::HasCJDNSPrefix() helper --- src/net.cpp | 2 +- src/netaddress.cpp | 3 +-- src/netaddress.h | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 654e0ac9fd..d76874b233 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -275,7 +275,7 @@ std::optional GetLocalAddrForPeer(CNode& node) CService MaybeFlipIPv6toCJDNS(const CService& service) { CService ret{service}; - if (ret.m_net == NET_IPV6 && ret.m_addr[0] == 0xfc && IsReachable(NET_CJDNS)) { + if (ret.m_net == NET_IPV6 && ret.HasCJDNSPrefix() && IsReachable(NET_CJDNS)) { ret.m_net = NET_CJDNS; } return ret; diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 4758f24680..8196a6a48c 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -450,8 +450,7 @@ bool CNetAddr::IsValid() const return false; } - // CJDNS addresses always start with 0xfc - if (IsCJDNS() && (m_addr[0] != 0xFC)) { + if (IsCJDNS() && !HasCJDNSPrefix()) { return false; } diff --git a/src/netaddress.h b/src/netaddress.h index 36dc886406..5e67d843e2 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -182,6 +182,7 @@ public: bool IsTor() const; bool IsI2P() const; bool IsCJDNS() const; + [[nodiscard]] bool HasCJDNSPrefix() const { return m_addr[0] == 0xfc; } bool IsLocal() const; bool IsRoutable() const; bool IsInternal() const; From df488563b280c63f5d74d5ac0fcb1a2cad489d55 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Tue, 18 Jul 2023 10:07:41 -0600 Subject: [PATCH 5/9] GetLocal() type-safety, naming, const, and formatting cleanups Co-authored-by: Jon Atack --- src/net.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index d76874b233..83440c2497 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -148,30 +148,27 @@ uint16_t GetListenPort() // find 'best' local address for a particular peer [[nodiscard]] static bool GetLocal(CService& addr, const CNode& peer) { - if (!fListen) - return false; + if (!fListen) return false; int nBestScore = -1; int nBestReachability = -1; { LOCK(g_maplocalhost_mutex); - for (const auto& entry : mapLocalHost) - { + for (const auto& [local_addr, local_service_info] : mapLocalHost) { // For privacy reasons, don't advertise our privacy-network address // to other networks and don't advertise our other-network address // to privacy networks. - const Network our_net{entry.first.GetNetwork()}; + const Network our_net{local_addr.GetNetwork()}; const Network peers_net{peer.ConnectedThroughNetwork()}; if (our_net != peers_net && (our_net == NET_ONION || our_net == NET_I2P || peers_net == NET_ONION || peers_net == NET_I2P)) { continue; } - int nScore = entry.second.nScore; - int nReachability = entry.first.GetReachabilityFrom(peer.addr); - if (nReachability > nBestReachability || (nReachability == nBestReachability && nScore > nBestScore)) - { - addr = CService(entry.first, entry.second.nPort); + const int nScore{local_service_info.nScore}; + const int nReachability{local_addr.GetReachabilityFrom(peer.addr)}; + if (nReachability > nBestReachability || (nReachability == nBestReachability && nScore > nBestScore)) { + addr = CService{local_addr, local_service_info.nPort}; nBestReachability = nReachability; nBestScore = nScore; } From 07f589158835151a7613e4b2a508c0dd61a18fd7 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Wed, 19 Jul 2023 11:11:06 -0600 Subject: [PATCH 6/9] Add CNetAddr::IsPrivacyNet() and CNode::IsConnectedThroughPrivacyNet() Co-authored-by: Vasil Dimov --- src/net.cpp | 5 +++++ src/net.h | 3 +++ src/netaddress.h | 7 +++++++ 3 files changed, 15 insertions(+) diff --git a/src/net.cpp b/src/net.cpp index 83440c2497..e1f3040485 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -620,6 +620,11 @@ Network CNode::ConnectedThroughNetwork() const return m_inbound_onion ? NET_ONION : addr.GetNetClass(); } +bool CNode::IsConnectedThroughPrivacyNet() const +{ + return m_inbound_onion || addr.IsPrivacyNet(); +} + #undef X #define X(name) stats.name = name void CNode::CopyStats(CNodeStats& stats) diff --git a/src/net.h b/src/net.h index 54c3839fcc..8cf203b309 100644 --- a/src/net.h +++ b/src/net.h @@ -506,6 +506,9 @@ public: */ Network ConnectedThroughNetwork() const; + /** Whether this peer connected through a privacy network. */ + [[nodiscard]] bool IsConnectedThroughPrivacyNet() const; + // We selected peer as (compact blocks) high-bandwidth peer (BIP152) std::atomic m_bip152_highbandwidth_to{false}; // Peer selected us as (compact blocks) high-bandwidth peer (BIP152) diff --git a/src/netaddress.h b/src/netaddress.h index 5e67d843e2..428ab87423 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -188,6 +188,13 @@ public: bool IsInternal() const; bool IsValid() const; + /** + * Whether this object is a privacy network. + * TODO: consider adding IsCJDNS() here when more peers adopt CJDNS, see: + * https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1497176155 + */ + [[nodiscard]] bool IsPrivacyNet() const { return IsTor() || IsI2P(); } + /** * Check if the current object can be serialized in pre-ADDRv2/BIP155 format. */ From f1304db136bbeac70b52522b5a4522165bfb3fc0 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 14 Jul 2023 06:51:21 -0600 Subject: [PATCH 7/9] Use higher-level CNetAddr and CNode helpers in net.cpp rather than low-level comparisons with Network enum values. --- src/net.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index e1f3040485..a230b64a74 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -158,11 +158,8 @@ uint16_t GetListenPort() // For privacy reasons, don't advertise our privacy-network address // to other networks and don't advertise our other-network address // to privacy networks. - const Network our_net{local_addr.GetNetwork()}; - const Network peers_net{peer.ConnectedThroughNetwork()}; - if (our_net != peers_net && - (our_net == NET_ONION || our_net == NET_I2P || - peers_net == NET_ONION || peers_net == NET_I2P)) { + if (local_addr.GetNetwork() != peer.ConnectedThroughNetwork() + && (local_addr.IsPrivacyNet() || peer.IsConnectedThroughPrivacyNet())) { continue; } const int nScore{local_service_info.nScore}; @@ -272,7 +269,7 @@ std::optional GetLocalAddrForPeer(CNode& node) CService MaybeFlipIPv6toCJDNS(const CService& service) { CService ret{service}; - if (ret.m_net == NET_IPV6 && ret.HasCJDNSPrefix() && IsReachable(NET_CJDNS)) { + if (ret.IsIPv6() && ret.HasCJDNSPrefix() && IsReachable(NET_CJDNS)) { ret.m_net = NET_CJDNS; } return ret; @@ -488,7 +485,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo const bool use_proxy{GetProxy(addrConnect.GetNetwork(), proxy)}; bool proxyConnectionFailed = false; - if (addrConnect.GetNetwork() == NET_I2P && use_proxy) { + if (addrConnect.IsI2P() && use_proxy) { i2p::Connection conn; if (m_i2p_sam_session) { From 5316ae5dd8d90623f9bb883bb253fa6463ee4d34 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 13 Jul 2023 12:38:06 -0600 Subject: [PATCH 8/9] Convert GetLocal() to std::optional and remove out-param Co-authored-by: stickies-v --- src/net.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index a230b64a74..a12c7a2e1c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -145,11 +145,12 @@ uint16_t GetListenPort() return static_cast(gArgs.GetIntArg("-port", Params().GetDefaultPort())); } -// find 'best' local address for a particular peer -[[nodiscard]] static bool GetLocal(CService& addr, const CNode& peer) +// Determine the "best" local address for a particular peer. +[[nodiscard]] static std::optional GetLocal(const CNode& peer) { - if (!fListen) return false; + if (!fListen) return std::nullopt; + std::optional addr; int nBestScore = -1; int nBestReachability = -1; { @@ -165,13 +166,13 @@ uint16_t GetListenPort() const int nScore{local_service_info.nScore}; const int nReachability{local_addr.GetReachabilityFrom(peer.addr)}; if (nReachability > nBestReachability || (nReachability == nBestReachability && nScore > nBestScore)) { - addr = CService{local_addr, local_service_info.nPort}; + addr.emplace(CService{local_addr, local_service_info.nPort}); nBestReachability = nReachability; nBestScore = nScore; } } } - return nBestScore >= 0; + return addr; } //! Convert the serialized seeds into usable address objects. @@ -196,17 +197,13 @@ static std::vector ConvertSeeds(const std::vector &vSeedsIn) return vSeedsOut; } -// get best local address for a particular peer as a CAddress -// Otherwise, return the unroutable 0.0.0.0 but filled in with +// Determine the "best" local address for a particular peer. +// If none, return the unroutable 0.0.0.0 but filled in with // the normal parameters, since the IP may be changed to a useful // one by discovery. CService GetLocalAddress(const CNode& peer) { - CService addr; - if (GetLocal(addr, peer)) { - return addr; - } - return CService{CNetAddr(), GetListenPort()}; + return GetLocal(peer).value_or(CService{CNetAddr(), GetListenPort()}); } static int GetnScore(const CService& addr) From 4ecfd3eaf434d868455466e001adae4b68515ab8 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 14 Jul 2023 07:08:23 -0600 Subject: [PATCH 9/9] Inline short, often-called, rarely-changed basic CNetAddr getters and make them nodiscard. Member functions containing a few lines of code are usually inlined, either implicitly by defining them in the declaration as done here, or declared inline. References https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-inline https://google.github.io/styleguide/cppguide#Inline_Functions https://www.ibm.com/docs/en/i/7.1?topic=only-inline-member-functions-c --- src/netaddress.cpp | 20 -------------------- src/netaddress.h | 10 +++++----- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 8196a6a48c..7530334db1 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -309,10 +309,6 @@ bool CNetAddr::IsBindAny() const return std::all_of(m_addr.begin(), m_addr.end(), [](uint8_t b) { return b == 0; }); } -bool CNetAddr::IsIPv4() const { return m_net == NET_IPV4; } - -bool CNetAddr::IsIPv6() const { return m_net == NET_IPV6; } - bool CNetAddr::IsRFC1918() const { return IsIPv4() && ( @@ -400,22 +396,6 @@ bool CNetAddr::IsHeNet() const return IsIPv6() && HasPrefix(m_addr, std::array{0x20, 0x01, 0x04, 0x70}); } -/** - * Check whether this object represents a TOR address. - * @see CNetAddr::SetSpecial(const std::string &) - */ -bool CNetAddr::IsTor() const { return m_net == NET_ONION; } - -/** - * Check whether this object represents an I2P address. - */ -bool CNetAddr::IsI2P() const { return m_net == NET_I2P; } - -/** - * Check whether this object represents a CJDNS address. - */ -bool CNetAddr::IsCJDNS() const { return m_net == NET_CJDNS; } - bool CNetAddr::IsLocal() const { // IPv4 loopback (127.0.0.0/8 or 0.0.0.0/8) diff --git a/src/netaddress.h b/src/netaddress.h index 428ab87423..867042b387 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -162,8 +162,8 @@ public: bool SetSpecial(const std::string& addr); bool IsBindAny() const; // INADDR_ANY equivalent - bool IsIPv4() const; // IPv4 mapped address (::FFFF:0:0/96, 0.0.0.0/0) - bool IsIPv6() const; // IPv6 address (not mapped IPv4, not Tor) + [[nodiscard]] bool IsIPv4() const { return m_net == NET_IPV4; } // IPv4 mapped address (::FFFF:0:0/96, 0.0.0.0/0) + [[nodiscard]] bool IsIPv6() const { return m_net == NET_IPV6; } // IPv6 address (not mapped IPv4, not Tor) bool IsRFC1918() const; // IPv4 private networks (10.0.0.0/8, 192.168.0.0/16, 172.16.0.0/12) bool IsRFC2544() const; // IPv4 inter-network communications (198.18.0.0/15) bool IsRFC6598() const; // IPv4 ISP-level NAT (100.64.0.0/10) @@ -179,9 +179,9 @@ public: bool IsRFC6052() const; // IPv6 well-known prefix for IPv4-embedded address (64:FF9B::/96) bool IsRFC6145() const; // IPv6 IPv4-translated address (::FFFF:0:0:0/96) (actually defined in RFC2765) bool IsHeNet() const; // IPv6 Hurricane Electric - https://he.net (2001:0470::/36) - bool IsTor() const; - bool IsI2P() const; - bool IsCJDNS() const; + [[nodiscard]] bool IsTor() const { return m_net == NET_ONION; } + [[nodiscard]] bool IsI2P() const { return m_net == NET_I2P; } + [[nodiscard]] bool IsCJDNS() const { return m_net == NET_CJDNS; } [[nodiscard]] bool HasCJDNSPrefix() const { return m_addr[0] == 0xfc; } bool IsLocal() const; bool IsRoutable() const;