Merge bitcoin#20210: net: assert CNode::m_inbound_onion is inbound in ctor, add getter, unit tests

86c4952 net: add CNode::IsInboundOnion() public getter and unit tests (Jon Atack)
6609eb8 net: assert CNode::m_inbound_onion is inbound in ctor (Jon Atack)
993d1ec test, fuzz: fix constructing CNode with invalid inbound_onion (Jon Atack)

Pull request description:

  The goal of this PR is to be able to depend on `m_inbound_onion` in AttemptToEvictConnection in bitcoin#20197:

  - asserts `CNode::m_inbound_onion` is inbound in the CNode ctor to have a validity check at the class boundary
  - fixes a unit test and a fuzz utility that were passing invalid inbound onion values to the CNode ctor
  - drops an unneeded check in `CNode::ConnectedThroughNetwork()` for its inbound status
  - adds a public getter `IsInboundOnion()` that also allows unit testing it
  - adds unit test coverage

ACKs for top commit:
  sipa:
    utACK 86c4952
  LarryRuane:
    ACK 86c4952
  vasild:
    ACK 86c4952
  MarcoFalke:
    review ACK 86c4952 🐍

Tree-SHA512: 21109105bc4e5e03076fadd489204be00eac710c9de0127708ca2d0a10a048ff81f640f589a7429967ac3eb51d35fe24bb2b12e53e7aa3efbc47aaff6396d204

Author: MarcoFalke

src/net.cpp

@@ -555,7 +555,7 @@ void CNode::SetAddrLocal(const CService& addrLocalIn) {
 
 Network CNode::ConnectedThroughNetwork() const
 {
-    return IsInboundConn() && m_inbound_onion ? NET_ONION : addr.GetNetClass();
+    return m_inbound_onion ? NET_ONION : addr.GetNetClass();
 }
 
 #undef X
@@ -2947,6 +2947,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
       nMyStartingHeight(nMyStartingHeightIn),
       m_inbound_onion(inbound_onion)
 {
+    if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);
     hSocket = hSocketIn;
     addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn;
     if (conn_type_in != ConnectionType::BLOCK_RELAY) {

src/net.h

@@ -1095,7 +1095,7 @@ class CNode
     CService addrLocal GUARDED_BY(cs_addrLocal);
     mutable RecursiveMutex cs_addrLocal;
 
-    //! Whether this peer connected via our Tor onion service.
+    //! Whether this peer is an inbound onion, e.g. connected via our Tor onion service.
     const bool m_inbound_onion{false};
 
 public:
@@ -1219,6 +1219,9 @@ class CNode
     void MaybeSetAddrName(const std::string& addrNameIn);
 
     std::string ConnectionTypeAsString() const;
+
+    /** Whether this peer is an inbound onion, e.g. connected via our Tor onion service. */
+    bool IsInboundOnion() const { return m_inbound_onion; }
 };
 
 /** Return a timestamp in the future (in microseconds) for exponentially distributed events. */

src/test/fuzz/util.h

@@ -298,7 +298,7 @@ inline CNode ConsumeNode(FuzzedDataProvider& fuzzed_data_provider) noexcept
     const CAddress addr_bind = ConsumeAddress(fuzzed_data_provider);
     const std::string addr_name = fuzzed_data_provider.ConsumeRandomLengthString(64);
     const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH});
-    const bool inbound_onion = fuzzed_data_provider.ConsumeBool();
+    const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false};
     return {node_id, local_services, my_starting_height, socket, address, keyed_net_group, local_host_nonce, addr_bind, addr_name, conn_type, inbound_onion};
 }
 

src/test/net_tests.cpp

@@ -200,6 +200,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     BOOST_CHECK(pnode1->IsFeelerConn() == false);
     BOOST_CHECK(pnode1->IsAddrFetchConn() == false);
     BOOST_CHECK(pnode1->IsInboundConn() == false);
+    BOOST_CHECK(pnode1->IsInboundOnion() == false);
     BOOST_CHECK_EQUAL(pnode1->ConnectedThroughNetwork(), Network::NET_IPV4);
 
     std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>(
@@ -214,20 +215,22 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     BOOST_CHECK(pnode2->IsFeelerConn() == false);
     BOOST_CHECK(pnode2->IsAddrFetchConn() == false);
     BOOST_CHECK(pnode2->IsInboundConn() == true);
+    BOOST_CHECK(pnode2->IsInboundOnion() == false);
     BOOST_CHECK_EQUAL(pnode2->ConnectedThroughNetwork(), Network::NET_IPV4);
 
     std::unique_ptr<CNode> pnode3 = MakeUnique<CNode>(
         id++, NODE_NETWORK, height, hSocket, addr,
         /* nKeyedNetGroupIn = */ 0,
         /* nLocalHostNonceIn = */ 0,
         CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY,
-        /* inbound_onion = */ true);
+        /* inbound_onion = */ false);
     BOOST_CHECK(pnode3->IsFullOutboundConn() == true);
     BOOST_CHECK(pnode3->IsManualConn() == false);
     BOOST_CHECK(pnode3->IsBlockOnlyConn() == false);
     BOOST_CHECK(pnode3->IsFeelerConn() == false);
     BOOST_CHECK(pnode3->IsAddrFetchConn() == false);
     BOOST_CHECK(pnode3->IsInboundConn() == false);
+    BOOST_CHECK(pnode3->IsInboundOnion() == false);
     BOOST_CHECK_EQUAL(pnode3->ConnectedThroughNetwork(), Network::NET_IPV4);
 
     std::unique_ptr<CNode> pnode4 = MakeUnique<CNode>(
@@ -242,6 +245,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     BOOST_CHECK(pnode4->IsFeelerConn() == false);
     BOOST_CHECK(pnode4->IsAddrFetchConn() == false);
     BOOST_CHECK(pnode4->IsInboundConn() == true);
+    BOOST_CHECK(pnode4->IsInboundOnion() == true);
     BOOST_CHECK_EQUAL(pnode4->ConnectedThroughNetwork(), Network::NET_ONION);
 }