perf: Slightly reduce bandwidth usage when there are few nodes.

This mainly saves spam in test logs, but may save some packets here and
there, if nodes are randomly selected twice for GET_NODES and onion
routing packets.

CMakeLists.txt

@@ -284,6 +284,8 @@ set(toxcore_SOURCES
   toxcore/ping_array.h
   toxcore/ping.c
   toxcore/ping.h
+  toxcore/pk_set.c
+  toxcore/pk_set.h
   toxcore/shared_key_cache.c
   toxcore/shared_key_cache.h
   toxcore/state.c

auto_tests/onion_test.c

@@ -53,7 +53,7 @@ static int handle_test_1(void *object, const IP_Port *source, const uint8_t *pac
     res_packet[0] = NET_PACKET_ANNOUNCE_RESPONSE;
     memcpy(res_packet + 1, res_message, sizeof(res_message));
 
-    if (send_onion_response(onion->net, source, res_packet, sizeof(res_packet),
+    if (send_onion_response(onion->log, onion->net, source, res_packet, sizeof(res_packet),
                             packet + sizeof(res_packet)) == -1) {
         return 1;
     }
@@ -293,7 +293,7 @@ static void test_basic(void)
     uint64_t s;
     memcpy(&s, sb_data, sizeof(uint64_t));
     memcpy(test_3_pub_key, nodes[3].public_key, CRYPTO_PUBLIC_KEY_SIZE);
-    int ret = send_announce_request(onion1->net, rng, &path, &nodes[3],
+    int ret = send_announce_request(log1, onion1->net, rng, &path, &nodes[3],
                                     dht_get_self_public_key(onion1->dht),
                                     dht_get_self_secret_key(onion1->dht),
                                     zeroes,
@@ -315,7 +315,7 @@ static void test_basic(void)
     memcpy(onion_announce_entry_public_key(onion2_a, 1), dht_get_self_public_key(onion2->dht), CRYPTO_PUBLIC_KEY_SIZE);
     onion_announce_entry_set_time(onion2_a, 1, mono_time_get(mono_time2));
     networking_registerhandler(onion1->net, NET_PACKET_ONION_DATA_RESPONSE, &handle_test_4, onion1);
-    send_announce_request(onion1->net, rng, &path, &nodes[3],
+    send_announce_request(log1, onion1->net, rng, &path, &nodes[3],
                           dht_get_self_public_key(onion1->dht),
                           dht_get_self_secret_key(onion1->dht),
                           test_3_ping_id,
@@ -340,7 +340,7 @@ static void test_basic(void)
     ck_assert_msg((onion3 != nullptr), "Onion failed initializing.");
 
     random_nonce(rng, nonce);
-    ret = send_data_request(onion3->net, rng, &path, &nodes[3].ip_port,
+    ret = send_data_request(log3, onion3->net, rng, &path, &nodes[3].ip_port,
                             dht_get_self_public_key(onion1->dht),
                             dht_get_self_public_key(onion1->dht),
                             nonce, (const uint8_t *)"Install gentoo", sizeof("Install gentoo"));

other/bootstrap_daemon/docker/tox-bootstrapd.sha256

@@ -1 +1 @@
-1608abb52cd16775216d01d06569e6437d86ed11e9fc8dc6dc9c1c28668ccd8b  /usr/local/bin/tox-bootstrapd
+b6e269802eec92d0b9605310de1f450c012288af866b4356b59ef0b87a487b45  /usr/local/bin/tox-bootstrapd

toxav/msi.c

@@ -821,6 +821,7 @@ static void handle_pop(MSICall *call, const MSIMessage *msg)
         switch (call->state) {
             case MSI_CALL_INACTIVE: {
                 LOGGER_FATAL(call->session->messenger->log, "Handling what should be impossible case");
+                break;
             }
 
             case MSI_CALL_ACTIVE: {

toxcore/BUILD.bazel

@@ -135,6 +135,18 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "pk_set",
+    srcs = ["pk_set.c"],
+    hdrs = ["pk_set.h"],
+    deps = [
+        ":attributes",
+        ":ccompat",
+        ":crypto_core",
+        ":mem",
+    ],
+)
+
 cc_library(
     name = "list",
     srcs = ["list.c"],
@@ -197,6 +209,7 @@ cc_library(
         ":attributes",
         ":ccompat",
         ":mem",
+        ":util",
         "@pthread",
     ],
 )
@@ -226,6 +239,7 @@ cc_library(
     deps = [
         ":ccompat",
         ":crypto_core",
+        ":logger",
         ":mem",
         ":mono_time",
     ],
@@ -344,6 +358,7 @@ cc_library(
         ":mono_time",
         ":network",
         ":ping_array",
+        ":pk_set",
         ":shared_key_cache",
         ":state",
         ":util",

toxcore/DHT.c

@@ -18,6 +18,7 @@
 #include "mono_time.h"
 #include "network.h"
 #include "ping.h"
+#include "pk_set.h"
 #include "shared_key_cache.h"
 #include "state.h"
 #include "util.h"
@@ -1792,7 +1793,16 @@ static uint8_t do_ping_and_sendnode_requests(DHT *dht, uint64_t *lastgetnode, co
     unsigned int sort = 0;
     bool sort_ok = false;
 
-    if (client_list == nullptr || assoc_list == nullptr) {
+    // Keep track of which public keys we've sent GET_NODES to in this iteration,
+    // so we don't send GET_NODES to the same node twice. We may send it again next
+    // iteration, but this avoids a bit of wasted bandwidth.
+    //
+    // We can't share this across all calls within an iteration, because we're
+    // potentially requesting different public_keys in each GET_NODES packet.
+    Pk_Set *sent = pk_set_new(dht->mem, list_count * 2);
+
+    if (client_list == nullptr || assoc_list == nullptr || sent == nullptr) {
+        pk_set_free(sent);
         mem_delete(dht->mem, assoc_list);
         mem_delete(dht->mem, client_list);
         return 0;
@@ -1812,7 +1822,16 @@ static uint8_t do_ping_and_sendnode_requests(DHT *dht, uint64_t *lastgetnode, co
                 ++not_kill;
 
                 if (mono_time_is_timeout(dht->mono_time, assoc->last_pinged, PING_INTERVAL)) {
-                    dht_getnodes(dht, &assoc->ip_port, client->public_key, public_key);
+                    const IP_Port *target = &assoc->ip_port;
+                    const uint8_t *target_key = client->public_key;
+                    if (!pk_set_contains(sent, target_key)) {
+                        dht_getnodes(dht, target, target_key, public_key);
+                        pk_set_add(sent, target_key);
+                    } else {
+                        Ip_Ntoa ip_str;
+                        LOGGER_TRACE(dht->log, "not sending repeated GET_NODES to %s:%d",
+                                net_ip_ntoa(&target->ip, &ip_str), net_ntohs(target->port));
+                    }
                     assoc->last_pinged = temp_time;
                 }
 
@@ -1845,12 +1864,22 @@ static uint8_t do_ping_and_sendnode_requests(DHT *dht, uint64_t *lastgetnode, co
             rand_node += random_range_u32(dht->rng, num_nodes - (rand_node + 1));
         }
 
-        dht_getnodes(dht, &assoc_list[rand_node]->ip_port, client_list[rand_node]->public_key, public_key);
+        const IP_Port *target = &assoc_list[rand_node]->ip_port;
+        const uint8_t *target_key = client_list[rand_node]->public_key;
+        if (!pk_set_contains(sent, target_key)) {
+            dht_getnodes(dht, target, target_key, public_key);
+            pk_set_add(sent, target_key);
+        } else {
+            Ip_Ntoa ip_str;
+            LOGGER_TRACE(dht->log, "not sending repeated GET_NODES to %s:%d",
+                    net_ip_ntoa(&target->ip, &ip_str), net_ntohs(target->port));
+        }
 
         *lastgetnode = temp_time;
         ++*bootstrap_times;
     }
 
+    pk_set_free(sent);
     mem_delete(dht->mem, assoc_list);
     mem_delete(dht->mem, client_list);
     return not_kill;
@@ -1874,8 +1903,7 @@ static void do_dht_friends(DHT *dht)
         dht_friend->num_to_bootstrap = 0;
 
         do_ping_and_sendnode_requests(dht, &dht_friend->lastgetnode, dht_friend->public_key, dht_friend->client_list,
-                                      MAX_FRIEND_CLIENTS,
-                                      &dht_friend->bootstrap_times, true);
+                                      MAX_FRIEND_CLIENTS, &dht_friend->bootstrap_times, true);
     }
 }
 
@@ -2618,6 +2646,7 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
     DHT *const dht = (DHT *)mem_alloc(mem, sizeof(DHT));
 
     if (dht == nullptr) {
+        LOGGER_ERROR(log, "failed to allocate DHT struct (%ld bytes)", (unsigned long)sizeof(DHT));
         return nullptr;
     }
 
@@ -2635,6 +2664,7 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
     dht->ping = ping_new(mem, mono_time, rng, dht);
 
     if (dht->ping == nullptr) {
+        LOGGER_ERROR(log, "failed to initialise ping");
         kill_dht(dht);
         return nullptr;
     }
@@ -2651,10 +2681,11 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
 
     crypto_new_keypair(rng, dht->self_public_key, dht->self_secret_key);
 
-    dht->shared_keys_recv = shared_key_cache_new(mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
-    dht->shared_keys_sent = shared_key_cache_new(mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
+    dht->shared_keys_recv = shared_key_cache_new(log, mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
+    dht->shared_keys_sent = shared_key_cache_new(log, mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
 
     if (dht->shared_keys_recv == nullptr || dht->shared_keys_sent == nullptr) {
+        LOGGER_ERROR(log, "failed to initialise shared key cache");
         kill_dht(dht);
         return nullptr;
     }
@@ -2663,6 +2694,7 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
     dht->dht_ping_array = ping_array_new(mem, DHT_PING_ARRAY_SIZE, PING_TIMEOUT);
 
     if (dht->dht_ping_array == nullptr) {
+        LOGGER_ERROR(log, "failed to initialise ping array");
         kill_dht(dht);
         return nullptr;
     }
@@ -2675,6 +2707,7 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
 
         uint32_t token; // We don't intend to delete these ever, but need to pass the token
         if (dht_addfriend(dht, random_public_key_bytes, nullptr, nullptr, 0, &token) != 0) {
+            LOGGER_ERROR(log, "failed to add initial random seed DHT friends");
             kill_dht(dht);
             return nullptr;
         }

toxcore/Makefile.inc

@@ -51,6 +51,8 @@ libtoxcore_la_SOURCES = ../third_party/cmp/cmp.c \
                         ../toxcore/timed_auth.c \
                         ../toxcore/ping_array.h \
                         ../toxcore/ping_array.c \
+                        ../toxcore/pk_set.h \
+                        ../toxcore/pk_set.c \
                         ../toxcore/net_crypto.h \
                         ../toxcore/net_crypto.c \
                         ../toxcore/friend_requests.h \

toxcore/Messenger.c

@@ -3569,6 +3569,7 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
         mem_delete(mem, m);
 
         if (error != nullptr && net_err == 1) {
+            LOGGER_WARNING(m->log, "network initialisation failed (no ports available)");
             *error = MESSENGER_ERROR_PORT;
         }
 
@@ -3588,6 +3589,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
     m->net_crypto = new_net_crypto(m->log, m->mem, m->rng, m->ns, m->mono_time, m->dht, &options->proxy_info);
 
     if (m->net_crypto == nullptr) {
+        LOGGER_WARNING(m->log, "net_crypto initialisation failed");
+
         kill_dht(m->dht);
         kill_networking(m->net);
         friendreq_kill(m->fr);
@@ -3600,6 +3603,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
     m->group_announce = new_gca_list();
 
     if (m->group_announce == nullptr) {
+        LOGGER_WARNING(m->log, "DHT group chats initialisation failed");
+
         kill_net_crypto(m->net_crypto);
         kill_dht(m->dht);
         kill_networking(m->net);
@@ -3626,6 +3631,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
 
     if ((options->dht_announcements_enabled && (m->forwarding == nullptr || m->announce == nullptr)) ||
             m->onion == nullptr || m->onion_a == nullptr || m->onion_c == nullptr || m->fr_c == nullptr) {
+        LOGGER_WARNING(m->log, "onion initialisation failed");
+
         kill_onion(m->onion);
         kill_onion_announce(m->onion_a);
         kill_onion_client(m->onion_c);
@@ -3650,6 +3657,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
     m->group_handler = new_dht_groupchats(m);
 
     if (m->group_handler == nullptr) {
+        LOGGER_WARNING(m->log, "conferences initialisation failed");
+
         kill_onion(m->onion);
         kill_onion_announce(m->onion_a);
         kill_onion_client(m->onion_c);
@@ -3674,6 +3683,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
                                        m->onion, m->forwarding);
 
         if (m->tcp_server == nullptr) {
+            LOGGER_WARNING(m->log, "TCP server initialisation failed");
+
             kill_onion(m->onion);
             kill_onion_announce(m->onion_a);
 #ifndef VANILLA_NACL

toxcore/TCP_common.c

@@ -201,7 +201,11 @@ int read_tcp_packet(
     const uint16_t count = net_socket_data_recv_buffer(ns, sock);
 
     if (count < length) {
-        LOGGER_TRACE(logger, "recv buffer has %d bytes, but requested %d bytes", count, length);
+        if (count != 0) {
+            // Only log when there are some bytes available, as empty buffer
+            // is a very common case and this spams our logs.
+            LOGGER_TRACE(logger, "recv buffer has %d bytes, but requested %d bytes", count, length);
+        }
         return -1;
     }
 

toxcore/TCP_connection.h

@@ -120,7 +120,7 @@ bool tcp_get_random_conn_ip_port(const TCP_Connections *tcp_c, IP_Port *ip_port)
  * return -1 on failure.
  */
 non_null()
-int tcp_send_onion_request(TCP_Connections *tcp_c, unsigned int tcp_connections_number, const uint8_t *data,
+int tcp_send_onion_request(TCP_Connections *tcp_c, uint32_t tcp_connections_number, const uint8_t *data,
                            uint16_t length);
 
 /** @brief Set if we want TCP_connection to allocate some connection for onion use.

toxcore/announce.c

@@ -660,7 +660,7 @@ Announcements *new_announcements(const Logger *log, const Memory *mem, const Ran
     announce->public_key = dht_get_self_public_key(announce->dht);
     announce->secret_key = dht_get_self_secret_key(announce->dht);
     new_hmac_key(announce->rng, announce->hmac_key);
-    announce->shared_keys = shared_key_cache_new(mono_time, mem, announce->secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
+    announce->shared_keys = shared_key_cache_new(log, mono_time, mem, announce->secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
     if (announce->shared_keys == nullptr) {
         free(announce);
         return nullptr;

toxcore/logger.c

@@ -117,3 +117,8 @@ void logger_write(const Logger *log, Logger_Level level, const char *file, int l
 
     log->callback(log->context, level, file, line, func, msg, log->userdata);
 }
+
+void logger_abort(void)
+{
+    abort();
+}

toxcore/logger.h

@@ -67,6 +67,9 @@ void logger_write(
     const Logger *log, Logger_Level level, const char *file, int line, const char *func,
     const char *format, ...);
 
+/* @brief Terminate the program with a signal. */
+void logger_abort(void);
+
 
 #define LOGGER_WRITE(log, level, ...)                                            \
     do {                                                                         \
@@ -85,7 +88,7 @@ void logger_write(
 #define LOGGER_FATAL(log, ...)          \
     do {                                \
         LOGGER_ERROR(log, __VA_ARGS__); \
-        abort();                        \
+        logger_abort();                 \
     } while (0)
 
 #define LOGGER_ASSERT(log, cond, ...)              \

toxcore/mono_time.c

@@ -32,6 +32,7 @@
 #include <time.h>
 
 #include "ccompat.h"
+#include "util.h"
 
 /** don't call into system billions of times for no reason */
 struct Mono_Time {
@@ -165,7 +166,7 @@ Mono_Time *mono_time_new(const Memory *mem, mono_time_current_time_cb *current_t
     // Maximum reproducibility. Never return time = 0.
     mono_time->base_time = 1;
 #else
-    mono_time->base_time = (uint64_t)time(nullptr) * 1000ULL - current_time_monotonic(mono_time);
+    mono_time->base_time = max_u64(1, (uint64_t)time(nullptr)) * 1000ULL - current_time_monotonic(mono_time);
 #endif
 
     mono_time_update(mono_time);

toxcore/network.c

@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: GPL-3.0-or-later
- * Copyright © 2016-2018 The TokTok team.
+ * Copyright © 2016-2023 The TokTok team.
  * Copyright © 2013 Tox project.
  */
 
@@ -778,22 +778,22 @@ static void loglogdata(const Logger *log, const char *message, const uint8_t *bu
         Ip_Ntoa ip_str;
         const int error = net_error();
         char *strerror = net_new_strerror(error);
-        LOGGER_TRACE(log, "[%02x = %-20s] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
+        LOGGER_TRACE(log, "[%02x = %-21s] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
                      buffer[0], net_packet_type_name((Net_Packet_Type)buffer[0]), message,
                      min_u16(buflen, 999), 'E',
                      net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), error,
                      strerror, data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]);
         net_kill_strerror(strerror);
     } else if ((res > 0) && ((size_t)res <= buflen)) {
         Ip_Ntoa ip_str;
-        LOGGER_TRACE(log, "[%02x = %-20s] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
+        LOGGER_TRACE(log, "[%02x = %-21s] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
                      buffer[0], net_packet_type_name((Net_Packet_Type)buffer[0]), message,
                      min_u16(res, 999), (size_t)res < buflen ? '<' : '=',
                      net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), 0, "OK",
                      data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]);
     } else { /* empty or overwrite */
         Ip_Ntoa ip_str;
-        LOGGER_TRACE(log, "[%02x = %-20s] %s %lu%c%u %s:%u (%u: %s) | %08x%08x...%02x",
+        LOGGER_TRACE(log, "[%02x = %-21s] %s %lu%c%u %s:%u (%u: %s) | %08x%08x...%02x",
                      buffer[0], net_packet_type_name((Net_Packet_Type)buffer[0]), message,
                      res, res == 0 ? '!' : '>', buflen,
                      net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), 0, "OK",