Merge #6630: backport: merge bitcoin#25286, bitcoin#25989, bitcoin#27719, bitcoin#28078, bitcoin#27071, bitcoin#28077, bitcoin#28692, bitcoin#29813, bitcoin#30085, bitcoin#29833, partial bitcoin#24356 (networking backports: part 11)

5699158 merge bitcoin#29833: fix and improve logs (Kittywhiskers Van Gogh)
95a1853 merge bitcoin#30085: detect addnode cjdns peers in GetAddedNodeInfo() (Kittywhiskers Van Gogh)
9d959d7 merge bitcoin#29813: improve `-i2pacceptincoming` mention (Kittywhiskers Van Gogh)
5045fa3 merge bitcoin#28692: Delete i2p fuzz test (Kittywhiskers Van Gogh)
7b01e8b merge bitcoin#28077: also sleep after errors in Accept() & destroy the session if we get an unexpected error (Kittywhiskers Van Gogh)
0c42410 merge bitcoin#27071: Handle CJDNS from LookupSubNet() (Kittywhiskers Van Gogh)
821c11f merge bitcoin#28078: remove unneeded exports, use helpers over low-level code, use optional (Kittywhiskers Van Gogh)
d051929 refactor: replace external `::GetLocal()` usage (Kittywhiskers Van Gogh)
481175f merge bitcoin#27719: remove Tor link & generalize onion getnodeaddresses RPC (Kittywhiskers Van Gogh)
378ad01 merge bitcoin#25989: abort if i2p/cjdns are chosen via -onlynet but are unreachable (Kittywhiskers Van Gogh)
8b23bfb partial bitcoin#24356: replace CConnman::SocketEvents() with mockable Sock::WaitMany() (Kittywhiskers Van Gogh)
1a64e8d merge bitcoin#25286: remove duplicate categories from LogPrint output (Kittywhiskers Van Gogh)

Pull request description:

  ## Breaking Changes

  None expected.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 5699158
  PastaPastaPasta:
    utACK 5699158

Tree-SHA512: b1ffc310574de5ec40d18aa4a453ef328d8f935c42d43a204c14fa264f099a106ddea793a018f56dcf75d9548fb06d1580c6aa1d90f66afa95089435e0b9606d

Author: PastaPastaPasta

doc/cjdns.md

@@ -91,5 +91,4 @@ There are several ways to see your CJDNS address in Dash Core:
 To see which CJDNS peers your node is connected to, use `dash-cli -netinfo 4`
 or the `getpeerinfo` RPC (i.e. `dash-cli getpeerinfo`).
 
-To see which CJDNS addresses your node knows, use the `getnodeaddresses 0 cjdns`
-RPC.
+You can use the `getnodeaddresses` RPC to fetch a number of CJDNS peers known to your node; run `dash-cli help getnodeaddresses` for details.

doc/i2p.md

@@ -79,8 +79,8 @@ one of the networks has issues.
 ## Persistent vs transient I2P addresses
 
 The first time Dash Core connects to the I2P router, it automatically
-generates a persistent I2P address and its corresponding private key by default
-or if `-i2pacceptincoming=1` is set.  The private key is saved in a file named
+generates a persistent I2P address and its corresponding private key by default,
+unless `-i2pacceptincoming=0` is set.  The private key is saved in a file named
 `i2p_private_key` in the Dash Core data directory.  The persistent I2P
 address is used for making outbound connections and accepting inbound
 connections.
@@ -109,8 +109,7 @@ incoming I2P connections (`-i2pacceptincoming`):
 To see which I2P peers your node is connected to, use `dash-cli -netinfo 4`
 or the `getpeerinfo` RPC (e.g. `dash-cli getpeerinfo`).
 
-To see which I2P addresses your node knows, use the `getnodeaddresses 0 i2p`
-RPC.
+You can use the `getnodeaddresses` RPC to fetch a number of I2P peers known to your node; run `dash-cli help getnodeaddresses` for details.
 
 ## Compatibility
 

doc/tor.md

@@ -2,11 +2,7 @@
 
 It is possible to run Dash Core as a Tor onion service, and connect to such services.
 
-The following directions assume you have a Tor proxy running on port 9050. Many
-distributions default to having a SOCKS proxy listening on port 9050, but others
-may not. In particular, the Tor Browser Bundle defaults to listening on port 9150.
-See [Tor Project FAQ:TBBSocksPort](https://www.torproject.org/docs/faq.html.en#TBBSocksPort)
-for how to properly configure Tor.
+The following directions assume you have a Tor proxy running on port 9050. Many distributions default to having a SOCKS proxy listening on port 9050, but others may not. In particular, the Tor Browser Bundle defaults to listening on port 9150.
 
 ## Compatibility
 
@@ -30,8 +26,7 @@ CLI `-addrinfo` returns the number of addresses known to your node per
 network. This can be useful to see how many onion peers your node knows,
 e.g. for `-onlynet=onion`.
 
-To fetch a number of onion addresses that your node knows, for example seven
-addresses, use the `getnodeaddresses 7 onion` RPC.
+You can use the `getnodeaddresses` RPC to fetch a number of onion peers known to your node; run `dash-cli help getnodeaddresses` for details.
 
 ## 1. Run Dash Core behind a Tor proxy
 

src/Makefile.test.include

@@ -289,7 +289,6 @@ test_fuzz_fuzz_SOURCES = \
  test/fuzz/golomb_rice.cpp \
  test/fuzz/hex.cpp \
  test/fuzz/http_request.cpp \
- test/fuzz/i2p.cpp \
  test/fuzz/integer.cpp \
  test/fuzz/key.cpp \
  test/fuzz/key_io.cpp \

src/httpserver.cpp

@@ -653,7 +653,7 @@ CService HTTPRequest::GetPeer() const
         evhttp_connection_get_peer(con, (char**)&address, &port);
 #endif // HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR
 
-        peer = LookupNumeric(address, port);
+        peer = MaybeFlipIPv6toCJDNS(LookupNumeric(address, port));
     }
     return peer;
 }

src/i2p.cpp

@@ -12,6 +12,7 @@
 #include <netaddress.h>
 #include <netbase.h>
 #include <random.h>
+#include <sync.h>
 #include <tinyformat.h>
 #include <util/readwritefile.h>
 #include <util/sock.h>
@@ -146,35 +147,71 @@ bool Session::Listen(Connection& conn)
         conn.sock = StreamAccept();
         return true;
     } catch (const std::runtime_error& e) {
-        Log("Error listening: %s", e.what());
+        LogPrintLevel(BCLog::I2P, BCLog::Level::Error, "Couldn't listen: %s\n", e.what());
         CheckControlSock();
     }
     return false;
 }
 
 bool Session::Accept(Connection& conn)
 {
-    try {
-        while (!*m_interrupt) {
-            Sock::Event occurred;
-            if (!conn.sock->Wait(MAX_WAIT_FOR_IO, Sock::RECV, &occurred)) {
-                throw std::runtime_error("wait on socket failed");
-            }
+    AssertLockNotHeld(m_mutex);
 
-            if ((occurred & Sock::RECV) == 0) {
-                // Timeout, no incoming connections within MAX_WAIT_FOR_IO.
-                continue;
-            }
+    std::string errmsg;
+    bool disconnect{false};
+
+    while (!*m_interrupt) {
+        Sock::Event occurred;
+        if (!conn.sock->Wait(MAX_WAIT_FOR_IO, Sock::RECV, &occurred)) {
+            errmsg = "wait on socket failed";
+            break;
+        }
 
-            const std::string& peer_dest =
-                conn.sock->RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt, MAX_MSG_SIZE);
+        if (occurred == 0) {
+            // Timeout, no incoming connections or errors within MAX_WAIT_FOR_IO.
+            continue;
+        }
 
-            conn.peer = CService(DestB64ToAddr(peer_dest), I2P_SAM31_PORT);
+        std::string peer_dest;
+        try {
+            peer_dest = conn.sock->RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt, MAX_MSG_SIZE);
+        } catch (const std::runtime_error& e) {
+            errmsg = e.what();
+            break;
+        }
 
-            return true;
+        CNetAddr peer_addr;
+        try {
+            peer_addr = DestB64ToAddr(peer_dest);
+        } catch (const std::runtime_error& e) {
+            // The I2P router is expected to send the Base64 of the connecting peer,
+            // but it may happen that something like this is sent instead:
+            // STREAM STATUS RESULT=I2P_ERROR MESSAGE="Session was closed"
+            // In that case consider the session damaged and close it right away,
+            // even if the control socket is alive.
+            if (peer_dest.find("RESULT=I2P_ERROR") != std::string::npos) {
+                errmsg = strprintf("unexpected reply that hints the session is unusable: %s", peer_dest);
+                disconnect = true;
+            } else {
+                errmsg = e.what();
+            }
+            break;
         }
-    } catch (const std::runtime_error& e) {
-        Log("Error accepting: %s", e.what());
+
+        conn.peer = CService(peer_addr, I2P_SAM31_PORT);
+
+        return true;
+    }
+
+    if (*m_interrupt) {
+        LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Accept was interrupted\n");
+    } else {
+        LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error accepting%s: %s\n", disconnect ? " (will close the session)" : "", errmsg);
+    }
+    if (disconnect) {
+        LOCK(m_mutex);
+        Disconnect();
+    } else {
         CheckControlSock();
     }
     return false;
@@ -185,7 +222,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
     // Refuse connecting to arbitrary ports. We don't specify any destination port to the SAM proxy
     // when connecting (SAM 3.1 does not use ports) and it forces/defaults it to I2P_SAM31_PORT.
     if (to.GetPort() != I2P_SAM31_PORT) {
-        Log("Error connecting to %s, connection refused due to arbitrary port %s", to.ToStringAddrPort(), to.GetPort());
+        LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error connecting to %s, connection refused due to arbitrary port %s\n", to.ToStringAddrPort(), to.GetPort());
         proxy_error = false;
         return false;
     }
@@ -233,7 +270,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
 
         throw std::runtime_error(strprintf("\"%s\"", connect_reply.full));
     } catch (const std::runtime_error& e) {
-        Log("Error connecting to %s: %s", to.ToStringAddrPort(), e.what());
+        LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error connecting to %s: %s\n", to.ToStringAddrPort(), e.what());
         CheckControlSock();
         return false;
     }
@@ -251,12 +288,6 @@ std::string Session::Reply::Get(const std::string& key) const
     return pos->second.value();
 }
 
-template <typename... Args>
-void Session::Log(const std::string& fmt, const Args&... args) const
-{
-    LogPrint(BCLog::I2P, "I2P: %s\n", tfm::format(fmt, args...));
-}
-
 Session::Reply Session::SendRequestAndGetReply(const Sock& sock,
                                                const std::string& request,
                                                bool check_result_ok) const
@@ -316,7 +347,7 @@ void Session::CheckControlSock()
 
     std::string errmsg;
     if (!m_control_sock->IsConnected(errmsg)) {
-        Log("Control socket error: %s", errmsg);
+        LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Control socket error: %s\n", errmsg);
         Disconnect();
     }
 }
@@ -371,7 +402,7 @@ void Session::CreateIfNotCreatedAlready()
     const auto session_type = m_transient ? "transient" : "persistent";
     const auto session_id = GetRandHash().GetHex().substr(0, 10); // full is overkill, too verbose in the logs
 
-    Log("Creating %s SAM session %s with %s", session_type, session_id, m_control_host.ToStringAddrPort());
+    LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Creating %s SAM session %s with %s\n", session_type, session_id, m_control_host.ToStringAddrPort());
 
     auto sock = Hello();
 
@@ -408,7 +439,7 @@ void Session::CreateIfNotCreatedAlready()
     m_session_id = session_id;
     m_control_sock = std::move(sock);
 
-    Log("%s SAM session %s created, my address=%s",
+    LogPrintLevel(BCLog::I2P, BCLog::Level::Info, "%s SAM session %s created, my address=%s\n",
         Capitalize(session_type),
         m_session_id,
         m_my_addr.ToStringAddrPort());
@@ -439,9 +470,9 @@ void Session::Disconnect()
 {
     if (m_control_sock->Get() != INVALID_SOCKET) {
         if (m_session_id.empty()) {
-            Log("Destroying incomplete SAM session");
+            LogPrintLevel(BCLog::I2P, BCLog::Level::Info, "Destroying incomplete SAM session\n");
         } else {
-            Log("Destroying SAM session %s", m_session_id);
+            LogPrintLevel(BCLog::I2P, BCLog::Level::Info, "Destroying SAM session %s\n", m_session_id);
         }
     }
     m_control_sock = std::make_unique<Sock>(INVALID_SOCKET);

src/i2p.h

@@ -105,7 +105,7 @@ class Session
      * completion the `peer` member will be set to the address of the incoming peer.
      * @return true on success
      */
-    bool Accept(Connection& conn);
+    bool Accept(Connection& conn) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /**
      * Connect to an I2P peer.
@@ -155,14 +155,6 @@ class Session
         std::string Get(const std::string& key) const;
     };
 
-    /**
-     * Log a message in the `BCLog::I2P` category.
-     * @param[in] fmt printf(3)-like format string.
-     * @param[in] args printf(3)-like arguments that correspond to `fmt`.
-     */
-    template <typename... Args>
-    void Log(const std::string& fmt, const Args&... args) const;
-
     /**
      * Send request and get a reply from the SAM proxy.
      * @param[in] sock A socket that is connected to the SAM proxy.

src/init.cpp

@@ -1694,33 +1694,32 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     }
 
     if (args.IsArgSet("-onlynet")) {
-        std::set<enum Network> nets;
+        g_reachable_nets.RemoveAll();
         for (const std::string& snet : args.GetArgs("-onlynet")) {
             enum Network net = ParseNetwork(snet);
             if (net == NET_UNROUTABLE)
                 return InitError(strprintf(_("Unknown network specified in -onlynet: '%s'"), snet));
-            nets.insert(net);
-        }
-        for (int n = 0; n < NET_MAX; n++) {
-            enum Network net = (enum Network)n;
-            assert(IsReachable(net));
-            if (!nets.count(net))
-                SetReachable(net, false);
+            g_reachable_nets.Add(net);
         }
     }
 
     if (!args.IsArgSet("-cjdnsreachable")) {
-        SetReachable(NET_CJDNS, false);
+        if (args.IsArgSet("-onlynet") && g_reachable_nets.Contains(NET_CJDNS)) {
+            return InitError(
+                _("Outbound connections restricted to CJDNS (-onlynet=cjdns) but "
+                  "-cjdnsreachable is not provided"));
+        }
+        g_reachable_nets.Remove(NET_CJDNS);
     }
-    // Now IsReachable(NET_CJDNS) is true if:
+    // Now g_reachable_nets.Contains(NET_CJDNS) is true if:
     // 1. -cjdnsreachable is given and
     // 2.1. -onlynet is not given or
     // 2.2. -onlynet=cjdns is given
 
     // Requesting DNS seeds entails connecting to IPv4/IPv6, which -onlynet options may prohibit:
     // If -dnsseed=1 is explicitly specified, abort. If it's left unspecified by the user, we skip
     // the DNS seeds by adjusting -dnsseed in InitParameterInteraction.
-    if (args.GetBoolArg("-dnsseed", DEFAULT_DNSSEED) == true && !IsReachable(NET_IPV4) && !IsReachable(NET_IPV6)) {
+    if (args.GetBoolArg("-dnsseed", DEFAULT_DNSSEED) == true && !g_reachable_nets.Contains(NET_IPV4) && !g_reachable_nets.Contains(NET_IPV6)) {
         return InitError(strprintf(_("Incompatible options: -dnsseed=1 was explicitly specified, but -onlynet forbids connections to IPv4/IPv6")));
     };
 
@@ -1750,7 +1749,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         onion_proxy = addrProxy;
     }
 
-    const bool onlynet_used_with_onion{args.IsArgSet("-onlynet") && IsReachable(NET_ONION)};
+    const bool onlynet_used_with_onion{args.IsArgSet("-onlynet") && g_reachable_nets.Contains(NET_ONION)};
 
     // -onion can be used to set only a proxy for .onion, or override normal proxy for .onion addresses
     // -noonion (or -onion=0) disables connecting to .onion entirely
@@ -1785,7 +1784,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                   "reaching the Tor network is not provided: none of -proxy, -onion or "
                   "-listenonion is given"));
         }
-        SetReachable(NET_ONION, false);
+        g_reachable_nets.Remove(NET_ONION);
     }
 
     for (const std::string& strAddr : args.GetArgs("-externalip")) {
@@ -2412,7 +2411,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         }
         SetProxy(NET_I2P, Proxy{addr.value()});
     } else {
-        SetReachable(NET_I2P, false);
+        if (args.IsArgSet("-onlynet") && g_reachable_nets.Contains(NET_I2P)) {
+            return InitError(
+                _("Outbound connections restricted to i2p (-onlynet=i2p) but "
+                  "-i2psam is not provided"));
+        }
+        g_reachable_nets.Remove(NET_I2P);
     }
 
     connOptions.m_i2p_accept_incoming = args.GetBoolArg("-i2pacceptincoming", DEFAULT_I2P_ACCEPT_INCOMING);

src/masternode/node.cpp

@@ -239,10 +239,14 @@ bool CActiveMasternodeManager::GetLocalAddress(CService& addrRet)
     if (!fFoundLocal) {
         bool empty = true;
         // If we have some peers, let's try to find our local address from one of them
-        auto service = m_info.service;
         m_connman.ForEachNodeContinueIf(CConnman::AllNodes, [&](CNode* pnode) {
             empty = false;
-            if (pnode->addr.IsIPv4()) fFoundLocal = GetLocal(service, *pnode) && IsValidNetAddr(service);
+            if (pnode->addr.IsIPv4()) {
+                if (auto addr = ::GetLocalAddress(*pnode); IsValidNetAddr(addr)) {
+                    addrRet = addr;
+                    fFoundLocal = true;
+                }
+            }
             return !fFoundLocal;
         });
         // nothing and no live connections, can't do anything for now
@@ -257,10 +261,10 @@ bool CActiveMasternodeManager::GetLocalAddress(CService& addrRet)
 
 bool CActiveMasternodeManager::IsValidNetAddr(const CService& addrIn)
 {
+    if (!addrIn.IsValid() || !addrIn.IsIPv4()) return false;
     // TODO: regtest is fine with any addresses for now,
     // should probably be a bit smarter if one day we start to implement tests for this
-    return !Params().RequireRoutableExternalIP() ||
-           (addrIn.IsIPv4() && IsReachable(addrIn) && addrIn.IsRoutable());
+    return !Params().RequireRoutableExternalIP() || (g_reachable_nets.Contains(addrIn) && addrIn.IsRoutable());
 }
 
 template <template <typename> class EncryptedObj, typename Obj>