Sync nuclear subtree from NUClear@e689ffdb

A peer whose data socket lands on the same ephemeral port as ours is no
longer mistaken for our own announce coming back, which had made that one
peer invisible while every other peer worked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: Trent Houliston

README.md

@@ -8,7 +8,7 @@ Node.js module for interacting with the [NUClear](https://github.com/Fastcode/NU
 
 Version 2 uses the redesigned **NUClearNet** library from [NUClear PR #190](https://github.com/Fastcode/NUClear/pull/190) (wire protocol **0x03**). It is **not** compatible with 1.x clients or NUClear builds that still use the old `NUClearNetwork` stack (protocol 0x02). Upgrade Node clients and NUClear robots together.
 
-The vendored NUClear tree is updated via `git subtree` from the `houliston/nuclearnet-v2` branch (currently [NUClear@2053a375](https://github.com/Fastcode/NUClear/commit/2053a375)).
+The vendored NUClear tree is updated via `git subtree` from the `houliston/nuclearnet-v2` branch (currently [NUClear@e689ffdb](https://github.com/Fastcode/NUClear/commit/e689ffdb)).
 
 Peer join events may arrive slightly later than in 1.x because connection requires both multicast announce and a unicast CONNECT handshake.
 

src/nuclear/src/nuclearnet/Discovery.cpp

@@ -113,6 +113,27 @@ namespace network {
         return packet;
     }
 
+    bool Discovery::peek_announce_name(const uint8_t* data, std::size_t length, std::string& name) {
+        // Minimum size: header(5) + name_length(2) + num_subscriptions(2) = 9
+        if (length < sizeof(PacketHeader) + sizeof(uint16_t) + sizeof(uint16_t)) {
+            return false;
+        }
+
+        const uint8_t* ptr = data + sizeof(PacketHeader);
+
+        uint16_t name_len = 0;
+        std::memcpy(&name_len, ptr, sizeof(uint16_t));
+        ptr += sizeof(uint16_t);
+
+        if (length - sizeof(PacketHeader) - sizeof(uint16_t) < name_len + sizeof(uint16_t)) {
+            return false;
+        }
+
+        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+        name.assign(reinterpret_cast<const char*>(ptr), name_len);
+        return !name.empty();
+    }
+
     Discovery::AnnounceResult Discovery::process_announce(const sock_t& source,
                                                           const uint8_t* data,
                                                           std::size_t length,

src/nuclear/src/nuclearnet/Discovery.hpp

@@ -120,6 +120,17 @@ namespace network {
         static std::vector<uint8_t> build_announce_packet(const std::string& name,
                                                           const std::vector<uint64_t>& subscriptions);
 
+        /**
+         * Read the node name out of an announce packet without otherwise processing it.
+         *
+         * @param data   Pointer to the announce packet
+         * @param length Length of the packet in bytes
+         * @param name   Filled with the name if the packet is well formed
+         *
+         * @return true if a name could be read
+         */
+        static bool peek_announce_name(const uint8_t* data, std::size_t length, std::string& name);
+
         /**
          * Build a leave packet.
          *

src/nuclear/src/nuclearnet/NUClearNet.cpp

@@ -618,6 +618,14 @@ namespace {
         return fds;
     }
 
+    in_port_t NUClearNet::own_data_port() const {
+        switch (own_data_address.sock.sa_family) {
+            case AF_INET: return own_data_address.ipv4.sin_port;
+            case AF_INET6: return own_data_address.ipv6.sin6_port;
+            default: return 0;
+        }
+    }
+
     bool NUClearNet::is_own_data_endpoint(const sock_t& source) const {
         if (own_data_address.sock.sa_family == AF_UNSPEC) {
             return false;
@@ -634,6 +642,23 @@ namespace {
         return false;
     }
 
+    bool NUClearNet::is_own_announce(const sock_t& source, const uint8_t* data, std::size_t length) const {
+        // The data socket usually binds INADDR_ANY, so getsockname gives us a port but no usable address to
+        // compare against. The port alone is not enough: a remote peer's data socket can land on the same
+        // ephemeral port as ours, and treating it as us would silently discard every announce it ever sends.
+        // Require the name to match as well, so a port collision alone cannot hide a peer.
+        if (!is_own_data_endpoint(source)) {
+            return false;
+        }
+
+        std::string name;
+        if (!Discovery::peek_announce_name(data, length, name)) {
+            return false;
+        }
+
+        return name == node_name;
+    }
+
     void NUClearNet::announce() {
         if (!data_fd.valid()) {
             return;
@@ -711,7 +736,7 @@ namespace {
 
     void NUClearNet::process_announce_packet(const sock_t& source, const uint8_t* data, std::size_t length) {
         // Ignore our own announces (multicast/broadcast loopback) without blocking other nodes on 127.0.0.1
-        if (is_own_data_endpoint(source)) {
+        if (is_own_announce(source, data, length)) {
             if (should_log(LogLevel::Debug)) {
                 log(LogLevel::Debug, "net", "ignoring self announce from " + sock_str(source));
             }

src/nuclear/src/nuclearnet/NUClearNet.hpp

@@ -193,6 +193,9 @@ namespace network {
          */
         std::vector<fd_t> listen_fds() const;
 
+        /// The ephemeral port our data socket is bound to, in network byte order
+        in_port_t own_data_port() const;
+
         /// Process a single received packet (dispatches to per-type handlers)
         void process_packet(const sock_t& source, const uint8_t* data, std::size_t length);
 
@@ -243,6 +246,9 @@ namespace network {
         /// Returns true if the UDP source is this node's data socket (same ephemeral port).
         bool is_own_data_endpoint(const sock_t& source) const;
 
+        /// Whether an announce packet is one of ours looped back, rather than a peer that shares our port
+        bool is_own_announce(const sock_t& source, const uint8_t* data, std::size_t length) const;
+
         // Configuration
         NetworkConfig config;
         std::string node_name;

src/nuclear/tests/tests/nuclearnet/ProcessPacket.cpp

@@ -568,3 +568,60 @@ SCENARIO("A peer that never acknowledges a reliable send is disconnected",
         REQUIRE(left);
     }
 }
+
+SCENARIO("A peer sharing our ephemeral port is not mistaken for ourselves", "[nuclearnet][process_packet]") {
+    if (!test_util::has_ipv4_multicast()) {
+        SKIP("No multicast support");
+    }
+
+    auto net = make_test_net("Us");
+
+    bool joined = false;
+    net->set_join_callback([&](const NUClear::network::PeerInfo& p) { joined = (p.name == "Them"); });
+
+    // A remote peer whose data socket happens to have landed on the same ephemeral port as ours. Only the
+    // port is available to compare against, because our own socket binds INADDR_ANY.
+    sock_t peer{};
+    peer.ipv4.sin_family      = AF_INET;
+    peer.ipv4.sin_addr.s_addr = htonl(0x0A000001);
+    peer.ipv4.sin_port        = net->own_data_port();
+
+    auto announce_pkt = build_announce("Them", {});
+    net->process_packet(peer, announce_pkt.data(), announce_pkt.size());
+
+    // Complete the handshake so the join fires
+    auto syn = build_connect(SYN);
+    net->process_connect_packet(peer, syn.data(), syn.size());
+    auto syn_ack = build_connect(SYN | CON_ACK);
+    net->process_connect_packet(peer, syn_ack.data(), syn_ack.size());
+    auto ack = build_connect(CON_ACK);
+    net->process_connect_packet(peer, ack.data(), ack.size());
+
+    THEN("their announce is processed rather than discarded as our own") {
+        REQUIRE(joined);
+    }
+}
+
+SCENARIO("Our own announce looped back is still ignored", "[nuclearnet][process_packet]") {
+    if (!test_util::has_ipv4_multicast()) {
+        SKIP("No multicast support");
+    }
+
+    auto net = make_test_net("Us");
+
+    bool joined = false;
+    net->set_join_callback([&](const NUClear::network::PeerInfo&) { joined = true; });
+
+    // Our own announce, coming back to us with our own name and port
+    sock_t self{};
+    self.ipv4.sin_family      = AF_INET;
+    self.ipv4.sin_addr.s_addr = htonl(0x0A000001);
+    self.ipv4.sin_port        = net->own_data_port();
+
+    auto announce_pkt = build_announce("Us", {});
+    net->process_packet(self, announce_pkt.data(), announce_pkt.size());
+
+    THEN("it is ignored") {
+        REQUIRE_FALSE(joined);
+    }
+}