net: tweak EventNewConnectionAccepted()

Move `MaybeFlipIPv6toCJDNS()`, which is Bitcoin P2P specific from the
callers of `CConnman::EventNewConnectionAccepted()` to inside that
method.

Move the IsSelectable check, the `TCP_NODELAY` option set and the
generation of new node id out of `CConnman::EventNewConnectionAccepted()`
because those are protocol agnostic. Move those to a new method
`SockMan::NewSockAccepted()` which is called instead of
`CConnman::EventNewConnectionAccepted()`.

src/net.cpp

@@ -1680,10 +1680,14 @@ bool CConnman::AttemptToEvictConnection()
     return false;
 }
 
-void CConnman::EventNewConnectionAccepted(std::unique_ptr<Sock>&& sock,
-                                          const CService& addr_bind,
-                                          const CService& addr)
+void CConnman::EventNewConnectionAccepted(NodeId node_id,
+                                          std::unique_ptr<Sock>&& sock,
+                                          const CService& addr_bind_,
+                                          const CService& addr_)
 {
+    const CService addr_bind{MaybeFlipIPv6toCJDNS(addr_bind_)};
+    const CService addr{MaybeFlipIPv6toCJDNS(addr_)};
+
     int nInbound = 0;
 
     NetPermissionFlags permission_flags = NetPermissionFlags::None;
@@ -1706,19 +1710,6 @@ void CConnman::EventNewConnectionAccepted(std::unique_ptr<Sock>&& sock,
         return;
     }
 
-    if (!sock->IsSelectable()) {
-        LogPrintf("connection from %s dropped: non-selectable socket\n", addr.ToStringAddrPort());
-        return;
-    }
-
-    // According to the internet TCP_NODELAY is not carried into accepted sockets
-    // on all platforms.  Set it again here just to be sure.
-    const int on{1};
-    if (sock->SetSockOpt(IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) == SOCKET_ERROR) {
-        LogDebug(BCLog::NET, "connection from %s: unable to set TCP_NODELAY, continuing anyway\n",
-                 addr.ToStringAddrPort());
-    }
-
     // Don't accept connections from banned peers.
     bool banned = m_banman && m_banman->IsBanned(addr);
     if (!NetPermissions::HasFlag(permission_flags, NetPermissionFlags::NoBan) && banned)
@@ -1744,16 +1735,15 @@ void CConnman::EventNewConnectionAccepted(std::unique_ptr<Sock>&& sock,
         }
     }
 
-    NodeId id = GetNewNodeId();
-    uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
+    const uint64_t nonce{GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(node_id).Finalize()};
 
     const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end();
     // The V2Transport transparently falls back to V1 behavior when an incoming V1 connection is
     // detected, so use it whenever we signal NODE_P2P_V2.
     ServiceFlags local_services = GetLocalServices();
     const bool use_v2transport(local_services & NODE_P2P_V2);
 
-    CNode* pnode = new CNode(id,
+    CNode* pnode = new CNode(node_id,
                              std::move(sock),
                              CAddress{addr, NODE_NONE},
                              CalculateKeyedNetGroup(addr),
@@ -1772,12 +1762,12 @@ void CConnman::EventNewConnectionAccepted(std::unique_ptr<Sock>&& sock,
     m_msgproc->InitializeNode(*pnode, local_services);
     {
         LOCK(m_nodes_mutex);
-        m_nodes.emplace(id, pnode);
+        m_nodes.emplace(node_id, pnode);
     }
     LogDebug(BCLog::NET, "connection from %s accepted\n", addr.ToStringAddrPort());
 
     // We received a new connection, harvest entropy from the time (and our peer count)
-    RandAddEvent((uint32_t)id);
+    RandAddEvent(static_cast<uint32_t>(node_id));
 }
 
 bool CConnman::AddConnection(const std::string& address, ConnectionType conn_type, bool use_v2transport = false)
@@ -2219,10 +2209,7 @@ void CConnman::SocketHandlerListening(const Sock::EventsPerSock& events_per_sock
             auto sock_accepted{AcceptConnection(*sock, addr_accepted)};
 
             if (sock_accepted) {
-                addr_accepted = MaybeFlipIPv6toCJDNS(addr_accepted);
-                const CService addr_bind{MaybeFlipIPv6toCJDNS(GetBindAddress(*sock))};
-
-                EventNewConnectionAccepted(std::move(sock_accepted), addr_bind, addr_accepted);
+                NewSockAccepted(std::move(sock_accepted), GetBindAddress(*sock), addr_accepted);
             }
         }
     }

src/net.h

@@ -1273,11 +1273,13 @@ class CConnman : private SockMan
 
     /**
      * Create a `CNode` object and add it to the `m_nodes` member.
+     * @param[in] node_id Id of the newly accepted connection.
      * @param[in] sock Connected socket to communicate with the peer.
      * @param[in] me The address and port at our side of the connection.
      * @param[in] them The address and port at the peer's side of the connection.
      */
-    virtual void EventNewConnectionAccepted(std::unique_ptr<Sock>&& sock,
+    virtual void EventNewConnectionAccepted(NodeId node_id,
+                                            std::unique_ptr<Sock>&& sock,
                                             const CService& me,
                                             const CService& them) override;
 

src/sockman.cpp

@@ -222,6 +222,26 @@ std::unique_ptr<Sock> SockMan::AcceptConnection(const Sock& listen_sock, CServic
     return sock;
 }
 
+void SockMan::NewSockAccepted(std::unique_ptr<Sock>&& sock, const CService& me, const CService& them)
+{
+    if (!sock->IsSelectable()) {
+        LogPrintf("connection from %s dropped: non-selectable socket\n", them.ToStringAddrPort());
+        return;
+    }
+
+    // According to the internet TCP_NODELAY is not carried into accepted sockets
+    // on all platforms.  Set it again here just to be sure.
+    const int on{1};
+    if (sock->SetSockOpt(IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) == SOCKET_ERROR) {
+        LogDebug(BCLog::NET, "connection from %s: unable to set TCP_NODELAY, continuing anyway\n",
+                 them.ToStringAddrPort());
+    }
+
+    const NodeId node_id{GetNewNodeId()};
+
+    EventNewConnectionAccepted(node_id, std::move(sock), me, them);
+}
+
 NodeId SockMan::GetNewNodeId()
 {
     return m_next_node_id.fetch_add(1, std::memory_order_relaxed);
@@ -272,7 +292,7 @@ void SockMan::ThreadI2PAccept()
             continue;
         }
 
-        EventNewConnectionAccepted(std::move(conn.sock), conn.me, conn.peer);
+        NewSockAccepted(std::move(conn.sock), conn.me, conn.peer);
 
         err_wait = err_wait_begin;
     }

src/sockman.h

@@ -118,6 +118,15 @@ class SockMan
      */
     std::unique_ptr<Sock> AcceptConnection(const Sock& listen_sock, CService& addr);
 
+    /**
+     * After a new socket with a peer has been created, configure its flags,
+     * make a new node id and call `EventNewConnectionAccepted()`.
+     * @param[in] sock The newly created socket.
+     * @param[in] me Address at our end of the connection.
+     * @param[in] them Address of the new peer.
+     */
+    void NewSockAccepted(std::unique_ptr<Sock>&& sock, const CService& me, const CService& them);
+
     /**
      * Generate an id for a newly created node.
      */
@@ -134,11 +143,13 @@ class SockMan
 
     /**
      * Be notified when a new connection has been accepted.
+     * @param[in] node_id Id of the newly accepted connection.
      * @param[in] sock Connected socket to communicate with the peer.
      * @param[in] me The address and port at our side of the connection.
      * @param[in] them The address and port at the peer's side of the connection.
      */
-    virtual void EventNewConnectionAccepted(std::unique_ptr<Sock>&& sock,
+    virtual void EventNewConnectionAccepted(NodeId node_id,
+                                            std::unique_ptr<Sock>&& sock,
                                             const CService& me,
                                             const CService& them) = 0;