Make some improvements to how often/when we announce a group

Instead of announcing a group whenever our connection status changes,
including when we gain or lose a TCP relay connection, we now only
when our UDP status changes, or if our previously announced
relay has gone offline. We also refresh our announcement
20 minutes regardless of any connection changes.

change should vastly reduce the amount of unnecessary DHT
traffic related to group announcements.

Author: JFreegman

other/bootstrap_daemon/docker/tox-bootstrapd.sha256

@@ -1 +1 @@
-cbd8bea9d23a961f27aacd35c4509fc1c22d72356f61d1ec74cc469b6b14490d  /usr/local/bin/tox-bootstrapd
+5cf49ad258527f6a2734a9d1f863084f66189338f47ca9d0a49482b4217577fb  /usr/local/bin/tox-bootstrapd

toxcore/Messenger.c

@@ -2573,6 +2573,13 @@ static bool self_announce_group(const Messenger *m, GC_Chat *chat, Onion_Friend
 
     onion_friend_set_gc_data(onion_friend, gc_data, (uint16_t)length);
     chat->update_self_announces = false;
+    chat->last_time_self_announce = mono_time_get(chat->mono_time);
+
+    if (tcp_num > 0) {
+        pk_copy(chat->announced_tcp_relay_pk, announce.base_announce.tcp_relays[0].public_key);
+    } else {
+        memset(chat->announced_tcp_relay_pk, 0, sizeof(chat->announced_tcp_relay_pk));
+    }
 
     LOGGER_DEBUG(chat->log, "Published group announce. TCP relays: %d, UDP status: %d", tcp_num,
                  chat->self_udp_status);

toxcore/TCP_connection.c

@@ -277,7 +277,6 @@ static TCP_con *get_tcp_connection(const TCP_Connections *tcp_c, int tcp_connect
     return &tcp_c->tcp_connections[tcp_connections_number];
 }
 
-/** Returns the number of connected TCP relays */
 uint32_t tcp_connected_relays_count(const TCP_Connections *tcp_c)
 {
     uint32_t count = 0;
@@ -635,6 +634,11 @@ static int find_tcp_connection_relay(const TCP_Connections *tcp_c, const uint8_t
     return -1;
 }
 
+bool tcp_relay_is_valid(const TCP_Connections *tcp_c, const uint8_t *relay_pk)
+{
+    return find_tcp_connection_relay(tcp_c, relay_pk) != -1;
+}
+
 /** @brief Create a new TCP connection to public_key.
  *
  * public_key must be the counterpart to the secret key that the other peer used with `new_tcp_connections()`.

toxcore/TCP_connection.h

@@ -78,10 +78,14 @@ const uint8_t *tcp_connections_public_key(const TCP_Connections *tcp_c);
 non_null()
 uint32_t tcp_connections_count(const TCP_Connections *tcp_c);
 
-/** Returns the number of connected TCP relays */
+/** @brief Returns the number of connected TCP relays. */
 non_null()
 uint32_t tcp_connected_relays_count(const TCP_Connections *tcp_c);
 
+/** @brief Returns true if we know of a valid TCP relay with the passed public key. */
+non_null()
+bool tcp_relay_is_valid(const TCP_Connections *tcp_c, const uint8_t *relay_pk);
+
 /** @brief Send a packet to the TCP connection.
  *
  * return -1 on failure.

toxcore/group_chats.c

@@ -2016,7 +2016,7 @@ static bool send_gc_tcp_relays(const GC_Chat *chat, GC_Connection *gconn)
 
     for (uint32_t i = 0; i < n; ++i) {
         add_tcp_relay_connection(chat->tcp_conn, gconn->tcp_connection_num, &tcp_relays[i].ip_port,
-                                     tcp_relays[i].public_key);
+                                 tcp_relays[i].public_key);
     }
 
     const int nodes_len = pack_nodes(chat->log, data, sizeof(data), tcp_relays, n);
@@ -7050,9 +7050,10 @@ static void do_gc_tcp(const GC_Session *c, GC_Chat *chat, void *userdata)
         set_tcp_connection_to_status(chat->tcp_conn, gconn->tcp_connection_num, tcp_set);
     }
 
-    if (mono_time_is_timeout(chat->mono_time, chat->last_checked_tcp_relays, TCP_RELAYS_CHECK_INTERVAL) &&
-            tcp_connected_relays_count(chat->tcp_conn) != chat->tcp_connections) {
+    if (mono_time_is_timeout(chat->mono_time, chat->last_checked_tcp_relays, TCP_RELAYS_CHECK_INTERVAL)
+            && tcp_connected_relays_count(chat->tcp_conn) != chat->connected_tcp_relays) {
         add_tcp_relays_to_chat(c, chat);
+        chat->connected_tcp_relays = tcp_connected_relays_count(chat->tcp_conn);
         chat->last_checked_tcp_relays = mono_time_get(chat->mono_time);
     }
 }
@@ -7061,27 +7062,27 @@ static void do_gc_tcp(const GC_Session *c, GC_Chat *chat, void *userdata)
  * Updates our TCP and UDP connection status and flags a new announcement if our connection has
  * changed and we have either a UDP or TCP connection.
  */
-#define GC_SELF_CONNECTION_CHECK_INTERVAL 2
+#define GC_SELF_CONNECTION_CHECK_INTERVAL 5  // how often in seconds we should run this function
+#define GC_SELF_REFRESH_ANNOUNCE_INTERVAL (60 * 20)  // how often in seconds we force refresh our group announcement
 non_null()
 static void do_self_connection(const GC_Session *c, GC_Chat *chat)
 {
     if (!mono_time_is_timeout(chat->mono_time, chat->last_self_announce_check, GC_SELF_CONNECTION_CHECK_INTERVAL)) {
         return;
     }
 
-    const uint32_t tcp_connections = tcp_connected_relays_count(chat->tcp_conn);
-
-    const Messenger *m = c->messenger;
-    const unsigned int self_udp_status = ipport_self_copy(m->dht, &chat->self_ip_port);
-
+    const unsigned int self_udp_status = ipport_self_copy(c->messenger->dht, &chat->self_ip_port);
     const bool udp_change = (chat->self_udp_status != self_udp_status) && (self_udp_status != SELF_UDP_STATUS_NONE);
-    const bool tcp_change = tcp_connections != chat->tcp_connections;
 
-    if (is_public_chat(chat) && (udp_change || tcp_change)) {
+    // We flag a group announce if our UDP status has changed since last run, or if our last announced TCP
+    // relay is no longer valid. Additionally, we will always flag an announce in the specified interval
+    // regardless of the prior conditions. Private groups are never announced.
+    if (is_public_chat(chat) &&
+            ((udp_change || !tcp_relay_is_valid(chat->tcp_conn, chat->announced_tcp_relay_pk))
+             || mono_time_is_timeout(chat->mono_time, chat->last_time_self_announce, GC_SELF_REFRESH_ANNOUNCE_INTERVAL))) {
         chat->update_self_announces = true;
     }
 
-    chat->tcp_connections = tcp_connections;
     chat->self_udp_status = (Self_UDP_Status) self_udp_status;
     chat->last_self_announce_check = mono_time_get(chat->mono_time);
 }

toxcore/group_chats.h

@@ -242,8 +242,7 @@ void gc_get_topic(const GC_Chat *chat, uint8_t *topic);
 
 /** @brief Returns the topic length.
  *
- * The return value is equal to the `length` agument received by the last topic
- * callback.
+ * The return value is equal to the `length` agument received by the last topic callback.
  */
 non_null()
 uint16_t gc_get_topic_size(const GC_Chat *chat);

toxcore/group_common.h

@@ -249,13 +249,13 @@ typedef struct GC_Chat {
     const Logger    *log;
     const Random    *rng;
 
+    uint32_t        connected_tcp_relays;
     Self_UDP_Status self_udp_status;
     IP_Port         self_ip_port;
 
     Networking_Core *net;
     TCP_Connections *tcp_conn;
 
-    uint32_t        tcp_connections; // the number of global TCP relays we're connected to
     uint64_t        last_checked_tcp_relays;
     Group_Handshake_Join_Type join_type;
 
@@ -312,6 +312,8 @@ typedef struct GC_Chat {
 
     bool        update_self_announces;  // true if we should try to update our announcements
     uint64_t    last_self_announce_check;  // the last time we checked if we should update our announcements
+    uint64_t    last_time_self_announce;  // the last time we announced the group
+    uint8_t     announced_tcp_relay_pk[CRYPTO_PUBLIC_KEY_SIZE];  // The pk of the last TCP relay we announced
 
     uint8_t     m_group_public_key[CRYPTO_PUBLIC_KEY_SIZE];  // public key for group's messenger friend connection
     int         friend_connection_id;  // identifier for group's messenger friend connection