Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#28078: net, refactor: remove unneeded exports, …
Browse files Browse the repository at this point in the history 
…use helpers over low-level code, use optional

4ecfd3e Inline short, often-called, rarely-changed basic CNetAddr getters (Jon Atack)
5316ae5 Convert GetLocal() to std::optional and remove out-param (Jon Atack)
f1304db Use higher-level CNetAddr and CNode helpers in net.cpp (Jon Atack)
07f5891 Add CNetAddr::IsPrivacyNet() and CNode::IsConnectedThroughPrivacyNet() (Jon Atack)
df48856 GetLocal() type-safety, naming, const, and formatting cleanups (stickies-v)
fb42657 Add and use CNetAddr::HasCJDNSPrefix() helper (Jon Atack)
5ba73cd Move GetLocal() declaration from header to implementation (Jon Atack)
11426f6 Move CaptureMessageToFile() declaration from header to implementation (Jon Atack)
deccf1c Move IsPeerAddrLocalGood() declaration from header to implementation (Jon Atack)

Pull request description:

  and other improvements noticed while reviewing #27411.

  Addresses bitcoin/bitcoin#27411 (comment) and bitcoin/bitcoin#27411 (comment).

  See commit messages for details.

ACKs for top commit:
  achow101:
    ACK 4ecfd3e
  vasild:
    ACK 4ecfd3e
  stickies-v:
    ACK 4ecfd3e

Tree-SHA512: d792bb2cb24690aeae9bedf97e92b64fb6fd080c39385a4bdb8ed05f37f3134d85bda99da025490829c03017fd56382afe7951cdd039aede1c08ba98fb1aa7f9
  • Loading branch information
@achow101
achow101 committed Sep 21, 2023
2 parents 5027d41 + 4ecfd3e commit 41cb17f
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 66 deletions.
59 changes: 28 additions & 31 deletions src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,39 +158,34 @@ uint16_t GetListenPort()
return static_cast<uint16_t>(gArgs.GetIntArg("-port", Params().GetDefaultPort()));
}

// find 'best' local address for a particular peer
bool GetLocal(CService& addr, const CNode& peer)
// Determine the "best" local address for a particular peer.
[[nodiscard]] static std::optional<CService> GetLocal(const CNode& peer)
{
if (!fListen)
return false;
if (!fListen) return std::nullopt;

std::optional<CService> addr;
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 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;
}
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.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.
Expand All @@ -216,17 +211,13 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &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)
Expand All @@ -237,7 +228,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() &&
Expand Down Expand Up @@ -289,7 +280,7 @@ std::optional<CService> 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.IsIPv6() && ret.HasCJDNSPrefix() && IsReachable(NET_CJDNS)) {
ret.m_net = NET_CJDNS;
}
return ret;
Expand Down Expand Up @@ -505,7 +496,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) {
Expand Down Expand Up @@ -637,6 +628,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)
Expand Down Expand Up @@ -3753,10 +3749,11 @@ uint64_t CConnman::CalculateKeyedNetGroup(const CAddress& address) const
return GetDeterministicRandomizer(RANDOMIZER_ID_NETGROUP).Write(vchNetGroup).Finalize();
}

void CaptureMessageToFile(const CAddress& addr,
const std::string& msg_type,
Span<const unsigned char> data,
bool is_incoming)
// Dump binary message to file, with timestamp.
static void CaptureMessageToFile(const CAddress& addr,
const std::string& msg_type,
Span<const unsigned char> data,
bool is_incoming)
{
// Note: This function captures the message at the time of processing,
// not at socket receive/send time.
Expand Down
11 changes: 3 additions & 8 deletions src/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ enum
LOCAL_MAX
};

bool IsPeerAddrLocalGood(CNode *pnode);
/** Returns a local address that we should advertise to this peer. */
std::optional<CService> GetLocalAddrForPeer(CNode& node);

Expand All @@ -170,7 +169,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);

Expand Down Expand Up @@ -834,6 +832,9 @@ class CNode
*/
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<bool> m_bip152_highbandwidth_to{false};
// Peer selected us as (compact blocks) high-bandwidth peer (BIP152)
Expand Down Expand Up @@ -1575,12 +1576,6 @@ class CConnman
friend struct ConnmanTestMsg;
};

/** Dump binary message to file, with timestamp */
void CaptureMessageToFile(const CAddress& addr,
const std::string& msg_type,
Span<const unsigned char> data,
bool is_incoming);

/** Defaults to `CaptureMessageToFile()`, but can be overridden by unit tests. */
extern std::function<void(const CAddress& addr,
const std::string& msg_type,
Expand Down
23 changes: 1 addition & 22 deletions src/netaddress.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() && (
Expand Down Expand Up @@ -400,22 +396,6 @@ bool CNetAddr::IsHeNet() const
return IsIPv6() && HasPrefix(m_addr, std::array<uint8_t, 4>{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)
Expand Down Expand Up @@ -450,8 +430,7 @@ bool CNetAddr::IsValid() const
return false;
}

// CJDNS addresses always start with 0xfc
if (IsCJDNS() && (m_addr[0] != 0xFC)) {
if (IsCJDNS() && !HasCJDNSPrefix()) {
return false;
}

Expand Down
18 changes: 13 additions & 5 deletions src/netaddress.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ class CNetAddr
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)
Expand All @@ -171,14 +171,22 @@ class CNetAddr
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;
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.
*/
Expand Down

0 comments on commit 41cb17f

Please sign in to comment.