Merge branch 'remove-sort_list' of https://github.com/pyruvate/toxcore

14 files changed, 451 insertions, 269 deletions

diff --git a/auto_tests/Makefile.inc b/auto_tests/Makefile.inc
index ecf3f31f..c68f313b 100644
--- a/auto_tests/Makefile.inc
+++ b/auto_tests/Makefile.inc
@@ -1,7 +1,7 @@
 if BUILD_TESTS
 
-TESTS = messenger_autotest crypto_test network_test assoc_test onion_test TCP_test tox_test
-check_PROGRAMS = messenger_autotest crypto_test network_test assoc_test onion_test TCP_test tox_test
+TESTS = messenger_autotest crypto_test network_test assoc_test onion_test TCP_test tox_test dht_autotest
+check_PROGRAMS = messenger_autotest crypto_test network_test assoc_test onion_test TCP_test tox_test dht_autotest
 
 AUTOTEST_CFLAGS = \
                          $(LIBSODIUM_CFLAGS) \
@@ -74,6 +74,12 @@ tox_test_CFLAGS = $(AUTOTEST_CFLAGS)
 tox_test_LDADD = $(AUTOTEST_LDADD)
 
 
+dht_autotest_SOURCES = ../auto_tests/dht_test.c
+
+dht_autotest_CFLAGS = $(AUTOTEST_CFLAGS)
+
+dht_autotest_LDADD = $(AUTOTEST_LDADD)
+
 
 if BUILD_AV
 toxav_basic_test_SOURCES = ../auto_tests/toxav_basic_test.c
diff --git a/auto_tests/TCP_test.c b/auto_tests/TCP_test.c
index 8e75fae0..25827df3 100644
--- a/auto_tests/TCP_test.c
+++ b/auto_tests/TCP_test.c
@@ -14,6 +14,8 @@
 
 #include "../toxcore/util.h"
 
+#include "helpers.h"
+
 #if defined(_WIN32) || defined(__WIN32__) || defined (WIN32)
 #define c_sleep(x) Sleep(1*x)
 #else
@@ -489,14 +491,6 @@ START_TEST(test_client_invalid)
 }
 END_TEST
 
-#define DEFTESTCASE(NAME) \
-    TCase *tc_##NAME = tcase_create(#NAME); \
-    tcase_add_test(tc_##NAME, test_##NAME); \
-    suite_add_tcase(s, tc_##NAME);
-
-#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \
-    DEFTESTCASE(NAME) \
-    tcase_set_timeout(tc_##NAME, TIMEOUT);
 Suite *TCP_suite(void)
 {
     Suite *s = suite_create("TCP");
diff --git a/auto_tests/assoc_test.c b/auto_tests/assoc_test.c
index 5f496ece..b377cadf 100644
--- a/auto_tests/assoc_test.c
+++ b/auto_tests/assoc_test.c
@@ -1,4 +1,3 @@
-
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
@@ -14,6 +13,8 @@
 
 #include <check.h>
 
+#include "helpers.h"
+
 START_TEST(test_basics)
 {
     /* TODO: real test */
@@ -132,15 +133,6 @@ START_TEST(test_fillup)
 }
 END_TEST
 
-#define DEFTESTCASE(NAME) \
-    TCase *tc_##NAME = tcase_create(#NAME); \
-    tcase_add_test(tc_##NAME, test_##NAME); \
-    suite_add_tcase(s, tc_##NAME);
-
-#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \
-    DEFTESTCASE(NAME) \
-    tcase_set_timeout(tc_##NAME, TIMEOUT);
-
 Suite *Assoc_suite(void)
 {
     Suite *s = suite_create("Assoc");
diff --git a/auto_tests/crypto_test.c b/auto_tests/crypto_test.c
index 252f7275..2fb08b8b 100644
--- a/auto_tests/crypto_test.c
+++ b/auto_tests/crypto_test.c
@@ -10,6 +10,8 @@
 #include <stdlib.h>
 #include <time.h>
 
+#include "helpers.h"
+
 void rand_bytes(uint8_t *b, size_t blen)
 {
     size_t i;
@@ -270,16 +272,6 @@ START_TEST(test_large_data_symmetric)
 }
 END_TEST
 
-
-#define DEFTESTCASE(NAME) \
-    TCase *NAME = tcase_create(#NAME); \
-    tcase_add_test(NAME, test_##NAME); \
-    suite_add_tcase(s, NAME);
-
-#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \
-    DEFTESTCASE(NAME) \
-    tcase_set_timeout(NAME, TIMEOUT);
-
 Suite *crypto_suite(void)
 {
     Suite *s = suite_create("Crypto");
diff --git a/auto_tests/dht_test.c b/auto_tests/dht_test.c
new file mode 100644
index 00000000..6d7e70ad
--- /dev/null
+++ b/auto_tests/dht_test.c
@@ -0,0 +1,362 @@
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <sys/param.h>
+#include <time.h>
+
+#include "../toxcore/tox.h"
+#include "../toxcore/DHT.c"
+
+#include "helpers.h"
+
+#define swap(x,y) do \
+   { unsigned char swap_temp[sizeof(x) == sizeof(y) ? (signed)sizeof(x) : -1]; \
+     memcpy(swap_temp,&y,sizeof(x)); \
+     memcpy(&y,&x,       sizeof(x)); \
+     memcpy(&x,swap_temp,sizeof(x)); \
+    } while(0)
+
+
+void mark_bad(IPPTsPng *ipptp)
+{
+    ipptp->timestamp = unix_time() - 2 * BAD_NODE_TIMEOUT;
+    ipptp->hardening.routes_requests_ok = 0;
+    ipptp->hardening.send_nodes_ok = 0;
+    ipptp->hardening.testing_requests = 0;
+}
+
+void mark_possible_bad(IPPTsPng *ipptp)
+{
+    ipptp->timestamp = unix_time();
+    ipptp->hardening.routes_requests_ok = 0;
+    ipptp->hardening.send_nodes_ok = 0;
+    ipptp->hardening.testing_requests = 0;
+}
+
+void mark_good(IPPTsPng *ipptp)
+{
+    ipptp->timestamp = unix_time();
+    ipptp->hardening.routes_requests_ok = (HARDENING_ALL_OK >> 0) & 1;
+    ipptp->hardening.send_nodes_ok = (HARDENING_ALL_OK >> 1) & 1;
+    ipptp->hardening.testing_requests = (HARDENING_ALL_OK >> 2) & 1;
+}
+
+void mark_all_good(Client_data *list, uint32_t length, uint8_t ipv6)
+{
+    uint32_t i;
+    for (i = 0; i < length; ++i) {
+        if (ipv6)
+            mark_good(&list[i].assoc6);
+        else
+            mark_good(&list[i].assoc4);
+    }
+}
+
+/* Returns 1 if client_id has a furthest distance to comp_client_id
+   than all client_id's  in the list */
+uint8_t is_furthest(const uint8_t *comp_client_id, Client_data *list, uint32_t length, const uint8_t *client_id)
+{
+    uint32_t i;
+    for (i = 0; i < length; ++i)
+        if (id_closest(comp_client_id, client_id, list[i].client_id) == 1)
+            return 0;
+    return 1;
+}
+
+int client_in_list(Client_data *list, uint32_t length, const uint8_t *client_id)
+{
+    int i;
+    for (i = 0; i < (int)length; ++i)
+        if (id_equal(client_id, list[i].client_id))
+            return i;
+    return -1;
+}
+
+void test_addto_lists_update(DHT            *dht,
+                             Client_data    *list,
+                             uint32_t        length,
+                             IP_Port        *ip_port)
+{
+    int used, test, test1, test2, found;
+    IP_Port test_ipp;
+    uint8_t test_id[CLIENT_ID_SIZE];
+    uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0;
+
+    // check id update for existing ip_port
+    test = rand() % length;
+    ipport_copy(&test_ipp, ipv6 ? &list[test].assoc6.ip_port : &list[test].assoc4.ip_port);
+
+    randombytes(test_id, sizeof(test_id));
+    used = addto_lists(dht, test_ipp, test_id);
+    ck_assert_msg(used >= 1, "Wrong number of added clients");
+    // it is possible to have ip_port duplicates in the list, so ip_port @ found not always equal to ip_port @ test
+    found = client_in_list(list, length, test_id);
+    ck_assert_msg(found >= 0, "Client id is not in the list");
+    ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[found].assoc6.ip_port : &list[found].assoc4.ip_port), "Client IP_Port is incorrect");
+
+    // check ip_port update for existing id
+    test = rand() % length;
+    test_ipp.port = rand() % TOX_PORT_DEFAULT;
+    id_copy(test_id, list[test].client_id);
+
+    used = addto_lists(dht, test_ipp, test_id);
+    ck_assert_msg(used >= 1, "Wrong number of added clients");
+    // it is not possible to have id duplicates in the list, so id @ found must be equal id @ test
+    ck_assert_msg(client_in_list(list, length, test_id) == test, "Client id is not in the list");
+    ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test].assoc6.ip_port : &list[test].assoc4.ip_port), "Client IP_Port is incorrect");
+
+    // check ip_port update for existing id and ip_port (... port ... id ...)
+    test1 = rand() % (length / 2);
+    test2 = rand() % (length / 2) + length / 2;
+
+    ipport_copy(&test_ipp, ipv6 ? &list[test1].assoc6.ip_port : &list[test1].assoc4.ip_port);
+    id_copy(test_id, list[test2].client_id);
+    if (ipv6) list[test2].assoc6.ip_port.port = -1; else list[test2].assoc4.ip_port.port = -1;
+    used = addto_lists(dht, test_ipp, test_id);
+    ck_assert_msg(used >= 1, "Wrong number of added clients");
+    ck_assert_msg(client_in_list(list, length, test_id) == test2, "Client id is not in the list");
+    ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test2].assoc6.ip_port : &list[test2].assoc4.ip_port), "Client IP_Port is incorrect");
+
+    // check ip_port update for existing id and ip_port (... id ... port ...)
+    test1 = rand() % (length / 2);
+    test2 = rand() % (length / 2) + length / 2;
+
+    ipport_copy(&test_ipp, ipv6 ? &list[test2].assoc6.ip_port : &list[test2].assoc4.ip_port);
+    id_copy(test_id, list[test1].client_id);
+    if (ipv6) list[test1].assoc6.ip_port.port = -1; else list[test1].assoc4.ip_port.port = -1;
+    used = addto_lists(dht, test_ipp, test_id);
+    ck_assert_msg(used >= 1, "Wrong number of added clients");
+    ck_assert_msg(client_in_list(list, length, test_id) == test1, "Client id is not in the list");
+    ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test1].assoc6.ip_port : &list[test1].assoc4.ip_port), "Client IP_Port is incorrect");
+}
+
+void test_addto_lists_bad(DHT            *dht,
+                          Client_data    *list,
+                          uint32_t        length,
+                          IP_Port        *ip_port)
+{
+    // check "bad" clients replacement
+    int used, test1, test2, test3;
+    uint8_t client_id[CLIENT_ID_SIZE], test_id1[CLIENT_ID_SIZE], test_id2[CLIENT_ID_SIZE], test_id3[CLIENT_ID_SIZE];
+    uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0;
+
+    randombytes(client_id, sizeof(client_id));
+    mark_all_good(list, length, ipv6);
+
+    test1 = rand() % (length / 3);
+    test2 = rand() % (length / 3) + length / 3;
+    test3 = rand() % (length / 3) + 2 * length / 3;
+    ck_assert_msg(!(test1 == test2 || test1 == test3 || test2 == test3), "Wrong test indices are chosen");
+
+    id_copy((uint8_t*)&test_id1, list[test1].client_id);
+    id_copy((uint8_t*)&test_id2, list[test2].client_id);
+    id_copy((uint8_t*)&test_id3, list[test3].client_id);
+
+    // mark nodes as "bad"
+    if (ipv6) {
+        mark_bad(&list[test1].assoc6);
+        mark_bad(&list[test2].assoc6);
+        mark_bad(&list[test3].assoc6);
+    } else {
+        mark_bad(&list[test1].assoc4);
+        mark_bad(&list[test2].assoc4);
+        mark_bad(&list[test3].assoc4);
+    }
+
+    ip_port->port += 1;
+    used = addto_lists(dht, *ip_port, client_id);
+    ck_assert_msg(used >= 1, "Wrong number of added clients");
+
+    ck_assert_msg(client_in_list(list, length, client_id) >= 0, "Client id is not in the list");
+    ck_assert_msg(id_equal(client_id, list[test1].client_id), "Wrong bad client removed");
+    ck_assert_msg(id_equal(test_id2, list[test2].client_id), "Wrong bad client removed");
+    ck_assert_msg(id_equal(test_id3, list[test3].client_id), "Wrong bad client removed");
+}
+
+void test_addto_lists_possible_bad(DHT            *dht,
+                                   Client_data    *list,
+                                   uint32_t        length,
+                                   IP_Port        *ip_port,
+                                   const uint8_t  *comp_client_id)
+{
+    // check "possibly bad" clients replacement
+    int used, test1, test2, test3;
+    uint8_t client_id[CLIENT_ID_SIZE], test_id1[CLIENT_ID_SIZE], test_id2[CLIENT_ID_SIZE], test_id3[CLIENT_ID_SIZE];
+    uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0;
+
+    randombytes(client_id, sizeof(client_id));
+    mark_all_good(list, length, ipv6);
+
+    test1 = rand() % (length / 3);
+    test2 = rand() % (length / 3) + length / 3;
+    test3 = rand() % (length / 3) + 2 * length / 3;
+    ck_assert_msg(!(test1 == test2 || test1 == test3 || test2 == test3), "Wrong test indices are chosen");
+
+    id_copy((uint8_t*)&test_id1, list[test1].client_id);
+    id_copy((uint8_t*)&test_id2, list[test2].client_id);
+    id_copy((uint8_t*)&test_id3, list[test3].client_id);
+
+    // mark nodes as "possibly bad"
+    if (ipv6) {
+        mark_possible_bad(&list[test1].assoc6);
+        mark_possible_bad(&list[test2].assoc6);
+        mark_possible_bad(&list[test3].assoc6);
+    } else {
+        mark_possible_bad(&list[test1].assoc4);
+        mark_possible_bad(&list[test2].assoc4);
+        mark_possible_bad(&list[test3].assoc4);
+    }
+
+    ip_port->port += 1;
+    used = addto_lists(dht, *ip_port, client_id);
+    ck_assert_msg(used >= 1, "Wrong number of added clients");
+
+    ck_assert_msg(client_in_list(list, length, client_id) >= 0, "Client id is not in the list");
+
+    int inlist_id1 = client_in_list(list, length, test_id1) >= 0;
+    int inlist_id2 = client_in_list(list, length, test_id2) >= 0;
+    int inlist_id3 = client_in_list(list, length, test_id3) >= 0;
+
+    ck_assert_msg(inlist_id1 + inlist_id2 + inlist_id3 == 2, "Wrong client removed");
+
+    if (!inlist_id1) {
+        ck_assert_msg(id_closest(comp_client_id, test_id2, test_id1) == 1, "Id has been removed but is closer to than another one");
+        ck_assert_msg(id_closest(comp_client_id, test_id3, test_id1) == 1, "Id has been removed but is closer to than another one");
+    } else if (!inlist_id2) {
+        ck_assert_msg(id_closest(comp_client_id, test_id1, test_id2) == 1, "Id has been removed but is closer to than another one");
+        ck_assert_msg(id_closest(comp_client_id, test_id3, test_id2) == 1, "Id has been removed but is closer to than another one");
+    } else if (!inlist_id3) {
+        ck_assert_msg(id_closest(comp_client_id, test_id1, test_id3) == 1, "Id has been removed but is closer to than another one");
+        ck_assert_msg(id_closest(comp_client_id, test_id2, test_id3) == 1, "Id has been removed but is closer to than another one");
+    }
+}
+
+void test_addto_lists_good(DHT            *dht,
+                           Client_data    *list,
+                           uint32_t        length,
+                           IP_Port        *ip_port,
+                           const uint8_t  *comp_client_id)
+{
+    uint8_t client_id[CLIENT_ID_SIZE];
+    uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0;
+
+    mark_all_good(list, length, ipv6);
+
+    // check "good" client id replacement
+    do {
+        randombytes(client_id, sizeof(client_id));
+    } while (is_furthest(comp_client_id, list, length, client_id));
+    ip_port->port += 1;
+    addto_lists(dht, *ip_port, client_id);
+    ck_assert_msg(client_in_list(list, length, client_id) >= 0, "Good client id is not in the list");
+
+    // check "good" client id skip
+    do {
+        randombytes(client_id, sizeof(client_id));
+    } while (!is_furthest(comp_client_id, list, length, client_id));
+
+    ip_port->port += 1;
+    addto_lists(dht, *ip_port, client_id);
+    ck_assert_msg(client_in_list(list, length, client_id) == -1, "Good client id is in the list");
+}
+
+void test_addto_lists(IP ip)
+{
+    Networking_Core* net = new_networking(ip, TOX_PORT_DEFAULT);
+    ck_assert_msg(net != 0, "Failed to create Networking_Core");
+
+    DHT* dht = new_DHT(net);
+    ck_assert_msg(dht != 0, "Failed to create DHT");
+
+    IP_Port ip_port = { .ip = ip, .port = TOX_PORT_DEFAULT };
+    uint8_t client_id[CLIENT_ID_SIZE];
+    int i, used;
+
+    // check lists filling
+    for (i = 0; i < MAX(LCLIENT_LIST, MAX_FRIEND_CLIENTS); ++i) {
+        randombytes(client_id, sizeof(client_id));
+        used = addto_lists(dht, ip_port, client_id);
+        ck_assert_msg(used == dht->num_friends + 1, "Wrong number of added clients with existing ip_port");
+    }
+
+    for (i = 0; i < MAX(LCLIENT_LIST, MAX_FRIEND_CLIENTS); ++i) {
+        ip_port.port += 1;
+        used = addto_lists(dht, ip_port, client_id);
+        ck_assert_msg(used == dht->num_friends + 1, "Wrong number of added clients with existing client_id");
+    }
+
+    for (i = 0; i < MAX(LCLIENT_LIST, MAX_FRIEND_CLIENTS); ++i) {
+        ip_port.port += 1;
+        randombytes(client_id, sizeof(client_id));
+        used = addto_lists(dht, ip_port, client_id);
+        ck_assert_msg(used >= 1, "Wrong number of added clients");
+    }
+
+    /*check: Current behavior if there are two clients with the same id is
+     * to replace the first ip by the second. */
+    test_addto_lists_update(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port);
+    for (i = 0; i < dht->num_friends; ++i)
+        test_addto_lists_update(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port);
+
+    // check "bad" entries
+    test_addto_lists_bad(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port);
+    for (i = 0; i < dht->num_friends; ++i)
+        test_addto_lists_bad(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port);
+
+    // check "possibly bad" entries
+    test_addto_lists_possible_bad(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port, dht->self_public_key);
+    for (i = 0; i < dht->num_friends; ++i)
+        test_addto_lists_possible_bad(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port, dht->friends_list[i].client_id);
+
+    // check "good" entries
+    test_addto_lists_good(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port, dht->self_public_key);
+    for (i = 0; i < dht->num_friends; ++i)
+        test_addto_lists_good(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port, dht->friends_list[i].client_id);
+
+    kill_DHT(dht);
+    kill_networking(net);
+}
+
+START_TEST(test_addto_lists_ipv4)
+{
+    IP ip;
+    ip_init(&ip, 0);
+    test_addto_lists(ip);
+
+}
+END_TEST
+
+START_TEST(test_addto_lists_ipv6)
+{
+    IP ip;
+    ip_init(&ip, 1);
+    test_addto_lists(ip);
+
+}
+END_TEST
+
+Suite *dht_suite(void)
+{
+    Suite *s = suite_create("DHT");
+
+    DEFTESTCASE(addto_lists_ipv4);
+    DEFTESTCASE(addto_lists_ipv6);
+    return s;
+}
+
+int main(int argc, char *argv[])
+{
+    srand((unsigned int) time(NULL));
+
+    Suite *dht = dht_suite();
+    SRunner *test_runner = srunner_create(dht);
+
+    int number_failed = 0;
+    srunner_run_all(test_runner, CK_NORMAL);
+    number_failed = srunner_ntests_failed(test_runner);
+
+    srunner_free(test_runner);
+
+    return number_failed;
+}
diff --git a/auto_tests/helpers.h b/auto_tests/helpers.h
new file mode 100644
index 00000000..9e8043db
--- /dev/null
+++ b/auto_tests/helpers.h
@@ -0,0 +1,15 @@
+#ifndef TOXCORE_TEST_HELPERS_H
+#define TOXCORE_TEST_HELPERS_H
+
+#include <check.h>
+
+#define DEFTESTCASE(NAME)                   \
+    TCase *tc_##NAME = tcase_create(#NAME); \
+    tcase_add_test(tc_##NAME, test_##NAME); \
+    suite_add_tcase(s, tc_##NAME);
+
+#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \
+    DEFTESTCASE(NAME) \
+    tcase_set_timeout(tc_##NAME, TIMEOUT);
+
+#endif // TOXCORE_TEST_HELPERS_H
diff --git a/auto_tests/messenger_test.c b/auto_tests/messenger_test.c
index 7ab7d674..3024ed1a 100644
--- a/auto_tests/messenger_test.c
+++ b/auto_tests/messenger_test.c
@@ -21,6 +21,8 @@
 #include <string.h>
 #include <check.h>
 
+#include "helpers.h"
+
 #define REALLY_BIG_NUMBER ((1) << (sizeof(uint16_t) * 7))
 #define STRINGS_EQUAL(X, Y) (strcmp(X, Y) == 0)
 
@@ -298,11 +300,6 @@ START_TEST(test_messenger_state_saveloadsave)
 }
 END_TEST
 
-#define DEFTESTCASE(NAME) \
-    TCase *tc_##NAME = tcase_create(#NAME); \
-    tcase_add_test(tc_##NAME, test_##NAME); \
-    suite_add_tcase(s, tc_##NAME);
-
 Suite *messenger_suite(void)
 {
     Suite *s = suite_create("Messenger");
diff --git a/auto_tests/network_test.c b/auto_tests/network_test.c
index afd90e7a..9d07fbb4 100644
--- a/auto_tests/network_test.c
+++ b/auto_tests/network_test.c
@@ -11,6 +11,8 @@
 
 #include "../toxcore/network.h"
 
+#include "helpers.h"
+
 START_TEST(test_addr_resolv_localhost)
 {
 #ifdef __CYGWIN__
@@ -141,11 +143,6 @@ START_TEST(test_struct_sizes)
 }
 END_TEST
 
-#define DEFTESTCASE(NAME) \
-    TCase *tc_##NAME = tcase_create(#NAME); \
-    tcase_add_test(tc_##NAME, test_##NAME); \
-    suite_add_tcase(s, tc_##NAME);
-
 Suite *network_suite(void)
 {
     Suite *s = suite_create("Network");
diff --git a/auto_tests/onion_test.c b/auto_tests/onion_test.c
index 2394e03d..ed297022 100644
--- a/auto_tests/onion_test.c
+++ b/auto_tests/onion_test.c
@@ -14,6 +14,8 @@
 #include "../toxcore/onion_client.h"
 #include "../toxcore/util.h"
 
+#include "helpers.h"
+
 #if defined(_WIN32) || defined(__WIN32__) || defined (WIN32)
 #define c_sleep(x) Sleep(1*x)
 #else
@@ -335,14 +337,6 @@ START_TEST(test_announce)
 }
 END_TEST
 
-#define DEFTESTCASE(NAME) \
-    TCase *tc_##NAME = tcase_create(#NAME); \
-    tcase_add_test(tc_##NAME, test_##NAME); \
-    suite_add_tcase(s, tc_##NAME);
-
-#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \
-    DEFTESTCASE(NAME) \
-    tcase_set_timeout(tc_##NAME, TIMEOUT);
 Suite *onion_suite(void)
 {
     Suite *s = suite_create("Onion");
diff --git a/auto_tests/skeleton_test.c b/auto_tests/skeleton_test.c
index 89ef1b8b..27c9123a 100644
--- a/auto_tests/skeleton_test.c
+++ b/auto_tests/skeleton_test.c
@@ -9,6 +9,8 @@
 #include <stdlib.h>
 #include <time.h>
 
+#include "helpers.h"
+
 /*
 #include "../<stuff to test>"
 */
@@ -20,12 +22,6 @@ START_TEST(test_creativetestnamegoeshere)
 }
 END_TEST
 
-
-#define DEFTESTCASE(NAME) \
-    TCase *tc_##NAME = tcase_create(#NAME); \
-    tcase_add_test(tc_##NAME, test_##NAME); \
-    suite_add_tcase(s, tc_##NAME);
-
 Suite *creativesuitenamegoeshere_suite(void)
 {
     Suite *s = suite_create("creativesuitedescritptiongoeshere");
diff --git a/auto_tests/tox_test.c b/auto_tests/tox_test.c
index 43fb7a1c..f765fcd3 100644
--- a/auto_tests/tox_test.c
+++ b/auto_tests/tox_test.c
@@ -12,6 +12,8 @@
 
 #include "../toxcore/tox.h"
 
+#include "helpers.h"
+
 #if defined(_WIN32) || defined(__WIN32__) || defined (WIN32)
 #define c_sleep(x) Sleep(1*x)
 #else
@@ -360,14 +362,6 @@ loop_top:
 }
 END_TEST
 
-#define DEFTESTCASE(NAME) \
-    TCase *tc_##NAME = tcase_create(#NAME); \
-    tcase_add_test(tc_##NAME, test_##NAME); \
-    suite_add_tcase(s, tc_##NAME);
-
-#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \
-    DEFTESTCASE(NAME) \
-    tcase_set_timeout(tc_##NAME, TIMEOUT);
 Suite *tox_suite(void)
 {
     Suite *s = suite_create("Tox");
diff --git a/auto_tests/toxav_basic_test.c b/auto_tests/toxav_basic_test.c
index 57685bfc..c6481366 100644
--- a/auto_tests/toxav_basic_test.c
+++ b/auto_tests/toxav_basic_test.c
@@ -16,6 +16,8 @@
 #include "../toxcore/crypto_core.h"
 #include "../toxav/toxav.h"
 
+#include "helpers.h"
+
 #if defined(_WIN32) || defined(__WIN32__) || defined (WIN32)
 #define c_sleep(x) Sleep(1*x)
 #else
@@ -214,7 +216,6 @@ case 3: /* Wait for Both to have status ended */\
 if (status_control.Alice.status == Ended && status_control.Bob.status == Ended) running = 0; break; } c_sleep(20); } } printf("\n");
 
 START_TEST(test_AV_flows)
-// int test_AV_flows()
 {
     long long unsigned int cur_time = time(NULL);
     Tox *bootstrap_node = tox_new(0);
@@ -574,10 +575,7 @@ Suite *tox_suite(void)
 {
     Suite *s = suite_create("ToxAV");
 
-    TCase *tc_av_flows = tcase_create("AV_flows");
-    tcase_add_test(tc_av_flows, test_AV_flows);
-    tcase_set_timeout(tc_av_flows, 200);
-    suite_add_tcase(s, tc_av_flows);
+    DEFTESTCASE_SLOW(AV_flows, 200);
 
     return s;
 }
diff --git a/toxcore/DHT.c b/toxcore/DHT.c
index 77d126c1..5658d757 100644
--- a/toxcore/DHT.c
+++ b/toxcore/DHT.c
@@ -65,17 +65,6 @@
 /* Number of get node requests to send to quickly find close nodes. */
 #define MAX_BOOTSTRAP_TIMES 10
 
-/* Used in the comparison function for sorting lists of Client_data. */
-typedef struct {
-    Client_data c1;
-    Client_data c2;
-} ClientPair;
-
-/* Create the declaration for a quick sort for ClientPair structures. */
-declare_quick_sort(ClientPair);
-/* Create the quicksort function. See misc_tools.h for the definition. */
-make_quick_sort(ClientPair);
-
 Client_data *DHT_get_close_list(DHT *dht)
 {
     return dht->close_clientlist;
@@ -107,19 +96,6 @@ int id_closest(const uint8_t *id, const uint8_t *id1, const uint8_t *id2)
     return 0;
 }
 
-/* Turns the result of id_closest into something quick_sort can use.
- * Assumes p1->c1 == p2->c1.
- */
-static int client_id_cmp(const ClientPair p1, const ClientPair p2)
-{
-    int c = id_closest(p1.c1.client_id, p1.c2.client_id, p2.c2.client_id);
-
-    if (c == 2)
-        return -1;
-
-    return c;
-}
-
 /* Shared key generations are costly, it is therefor smart to store commonly used
  * ones so that they can re used later without being computed again.
  *
@@ -629,172 +605,61 @@ int get_close_nodes(const DHT *dht, const uint8_t *client_id, Node_format *nodes
 #endif
 }
 
-/* Replace first bad (or empty) node with this one.
+/* Replace a first bad (or empty) node with this one
+ *  or replace a possibly bad node (tests failed or not done yet)
+ *  that is further than any other in the list
+ *  from the comp_client_id
+ *  or replace a good node that is further
+ *  than any other in the list from the comp_client_id
+ *  and further than client_id.
  *
- *  return 0 if successful.
- *  return 1 if not (list contains no bad nodes).
- */
-static int replace_bad(    Client_data    *list,
-                           uint32_t        length,
-                           const uint8_t  *client_id,
-                           IP_Port         ip_port )
+ * Do not replace any node if the list has no bad or possibly bad nodes
+ *  and all nodes in the list are closer to comp_client_id
+ *  than client_id.
+ *
+ *  returns True(1) when the item was stored, False(0) otherwise */
+static int replace_all(   Client_data    *list,
+                          uint32_t        length,
+                          const uint8_t  *client_id,
+                          IP_Port         ip_port,
+                          const uint8_t  *comp_client_id )
 {
     if ((ip_port.ip.family != AF_INET) && (ip_port.ip.family != AF_INET6))
         return 1;
 
-    uint32_t i;
+    uint32_t i, replace = ~0, bad = ~0, possibly_bad = ~0, good = ~0;
 
     for (i = 0; i < length; ++i) {
-        /* If node is bad */
+
         Client_data *client = &list[i];
 
         if (is_timeout(client->assoc4.timestamp, BAD_NODE_TIMEOUT) &&
                 is_timeout(client->assoc6.timestamp, BAD_NODE_TIMEOUT)) {
-
-            IPPTsPng *ipptp_write = NULL;
-            IPPTsPng *ipptp_clear = NULL;
-
-            if (ip_port.ip.family == AF_INET) {
-                ipptp_write = &client->assoc4;
-                ipptp_clear = &client->assoc6;
-            } else {
-                ipptp_write = &client->assoc6;
-                ipptp_clear = &client->assoc4;
-            }
-
-            memcpy(client->client_id, client_id, CLIENT_ID_SIZE);
-            ipptp_write->ip_port = ip_port;
-            ipptp_write->timestamp = unix_time();
-
-            ip_reset(&ipptp_write->ret_ip_port.ip);
-            ipptp_write->ret_ip_port.port = 0;
-            ipptp_write->ret_timestamp = 0;
-
-            /* zero out other address */
-            memset(ipptp_clear, 0, sizeof(*ipptp_clear));
-
-            return 0;
-        }
-    }
-
-    return 1;
-}
-
-
-/* Sort the list. It will be sorted from furthest to closest.
- *  Turns list into data that quick sort can use and reverts it back.
- */
-static void sort_list(Client_data *list, uint32_t length, const uint8_t *comp_client_id)
-{
-    Client_data cd;
-    ClientPair pairs[length];
-    uint32_t i;
-
-    memcpy(cd.client_id, comp_client_id, CLIENT_ID_SIZE);
-
-    for (i = 0; i < length; ++i) {
-        pairs[i].c1 = cd;
-        pairs[i].c2 = list[i];
-    }
-
-    ClientPair_quick_sort(pairs, length, client_id_cmp);
-
-    for (i = 0; i < length; ++i)
-        list[i] = pairs[i].c2;
-}
-
-/* Replace first node that is possibly bad (tests failed or not done yet.) with this one.
- *
- *  return 0 if successful.
- *  return 1 if not (list contains no bad nodes).
- */
-static int replace_possible_bad(    Client_data    *list,
-                                    uint32_t        length,
-                                    const uint8_t  *client_id,
-                                    IP_Port         ip_port,
-                                    const uint8_t  *comp_client_id )
-{
-    if ((ip_port.ip.family != AF_INET) && (ip_port.ip.family != AF_INET6))
-        return 1;
-
-    sort_list(list, length, comp_client_id);
-
-    /* TODO: decide if the following lines should stay commented or not.
-    if (id_closest(comp_client_id, list[0].client_id, client_id) == 1)
-        return 0;*/
-
-    uint32_t i;
-
-    for (i = 0; i < length; ++i) {
-        /* If node is bad */
-        Client_data *client = &list[i];
-
-        if (hardening_correct(&client->assoc4.hardening) != HARDENING_ALL_OK &&
-                hardening_correct(&client->assoc6.hardening) != HARDENING_ALL_OK) {
-
-            IPPTsPng *ipptp_write = NULL;
-            IPPTsPng *ipptp_clear = NULL;
-
-            if (ip_port.ip.family == AF_INET) {
-                ipptp_write = &client->assoc4;
-                ipptp_clear = &client->assoc6;
-            } else {
-                ipptp_write = &client->assoc6;
-                ipptp_clear = &client->assoc4;
-            }
-
-            memcpy(client->client_id, client_id, CLIENT_ID_SIZE);
-            ipptp_write->ip_port = ip_port;
-            ipptp_write->timestamp = unix_time();
-
-            ip_reset(&ipptp_write->ret_ip_port.ip);
-            ipptp_write->ret_ip_port.port = 0;
-            ipptp_write->ret_timestamp = 0;
-
-            /* zero out other address */
-            memset(ipptp_clear, 0, sizeof(*ipptp_clear));
-
-            return 0;
+            // "bad" node
+            bad = i;
+            break;
+        } else if (hardening_correct(&client->assoc4.hardening) != HARDENING_ALL_OK &&
+                   hardening_correct(&client->assoc6.hardening) != HARDENING_ALL_OK) {
+            // "possibly bad" node
+            if (possibly_bad == ~0 ||
+                    id_closest(comp_client_id, list[possibly_bad].client_id, list[i].client_id) == 1)
+                possibly_bad = i;
+        } else {
+            // "good" node
+            if (good == ~0 ||
+                    id_closest(comp_client_id, list[good].client_id, list[i].client_id) == 1)
+                good = i;
         }
     }
 
-    return 1;
-}
+    if (bad != ~0)
+        replace = bad;
+    else if (possibly_bad != ~0)
+        replace = possibly_bad;
+    else if (good != ~0 && id_closest(comp_client_id, list[good].client_id, client_id) == 2)
+        replace = good;
 
-/* Replace the first good node that is further to the comp_client_id than that of the client_id in the list
- *
- *  returns 0 when the item was stored, 1 otherwise */
-static int replace_good(   Client_data    *list,
-                           uint32_t        length,
-                           const uint8_t  *client_id,
-                           IP_Port         ip_port,
-                           const uint8_t  *comp_client_id )
-{
-    if ((ip_port.ip.family != AF_INET) && (ip_port.ip.family != AF_INET6))
-        return 1;
-
-    /* TODO: eventually remove this.*/
-    if (length != LCLIENT_LIST)
-        sort_list(list, length, comp_client_id);
-
-    int8_t replace = -1;
-
-    /* Because the list is sorted, we can simply check the client_id at the
-     * border, either it is closer, then every other one is as well, or it is
-     * further, then it gets pushed out in favor of the new address, which
-     * will with the next sort() move to its "rightful" position
-     *
-     * CAVEAT: weirdly enough, the list is sorted DESCENDING in distance
-     * so the furthest element is the first, NOT the last (at least that's
-     * what the comment above sort_list() claims)
-     */
-    if (id_closest(comp_client_id, list[0].client_id, client_id) == 2)
-        replace = 0;
-
-    if (replace != -1) {
-#ifdef DEBUG
-        assert(replace >= 0 && replace < length);
-#endif
+    if (replace != ~0) {
         Client_data *client = &list[replace];
         IPPTsPng *ipptp_write = NULL;
         IPPTsPng *ipptp_clear = NULL;
@@ -807,7 +672,7 @@ static int replace_good(   Client_data    *list,
             ipptp_clear = &client->assoc4;
         }
 
-        memcpy(client->client_id, client_id, CLIENT_ID_SIZE);
+        id_copy(client->client_id, client_id);
         ipptp_write->ip_port = ip_port;
         ipptp_write->timestamp = unix_time();
 
@@ -818,10 +683,10 @@ static int replace_good(   Client_data    *list,
         /* zero out other address */
         memset(ipptp_clear, 0, sizeof(*ipptp_clear));
 
-        return 0;
+        return 1;
     }
 
-    return 1;
+    return 0;
 }
 
 /* Attempt to add client with ip_port and client_id to the friends client list
@@ -843,16 +708,7 @@ int addto_lists(DHT *dht, IP_Port ip_port, const uint8_t *client_id)
      * to replace the first ip by the second.
      */
     if (!client_or_ip_port_in_list(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port)) {
-        if (replace_bad(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port)) {
-            if (replace_possible_bad(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port,
-                                     dht->self_public_key)) {
-                /* If we can't replace bad nodes we try replacing good ones. */
-                if (!replace_good(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port,
-                                  dht->self_public_key))
-                    used++;
-            } else
-                used++;
-        } else
+        if (replace_all(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port, dht->self_public_key))
             used++;
     } else
         used++;
@@ -860,19 +716,8 @@ int addto_lists(DHT *dht, IP_Port ip_port, const uint8_t *client_id)
     for (i = 0; i < dht->num_friends; ++i) {
         if (!client_or_ip_port_in_list(dht->friends_list[i].client_list,
                                        MAX_FRIEND_CLIENTS, client_id, ip_port)) {
-
-            if (replace_bad(dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS,
-                            client_id, ip_port)) {
-                /*if (replace_possible_bad(dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS,
-                                client_id, ip_port, dht->friends_list[i].client_id)) {*/
-                /* If we can't replace bad nodes we try replacing good ones. */
-                if (!replace_good(dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS,
-                                  client_id, ip_port, dht->friends_list[i].client_id))
-                    used++;
-
-                /*} else
-                    used++;*/
-            } else
+            if (replace_all(dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS,
+                            client_id, ip_port, dht->friends_list[i].client_id))
                 used++;
         } else
             used++;
diff --git a/toxcore/onion_client.c b/toxcore/onion_client.c
index fbabea6d..f296426f 100644
--- a/toxcore/onion_client.c
+++ b/toxcore/onion_client.c
@@ -67,7 +67,7 @@ static uint16_t random_nodes_path_onion(const Onion_Client *onion_c, Node_format
     if (onion_c->path_nodes_index < 3)
         return random_nodes_path(onion_c->dht, nodes, max_num);
 
-    unsigned int i, num_nodes = onion_c->path_nodes_index < MAX_PATH_NODES ? onion_c->path_nodes_index : MAX_PATH_NODES;
+    unsigned int i, num_nodes = (onion_c->path_nodes_index < MAX_PATH_NODES) ? onion_c->path_nodes_index : MAX_PATH_NODES;
 
     for (i = 0; i < max_num; ++i) {
         nodes[i] = onion_c->path_nodes[rand() % num_nodes];