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

[iphydf]

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

More importantly (and somewhat concerningly), toxcore on esp32 never connects to the onion without this.

---

This change is 

[codecov]

Codecov Report

Attention: 42 lines in your changes are missing coverage. Please review.

Comparison is base (90f7496) 71.55% compared to head (7dba867) 71.56%.

❗ Current head 7dba867 differs from pull request most recent head 028b017. Consider uploading reports for the commit 028b017 to get more accurate results

Files	Patch %	Lines
toxcore/onion_client.c	71.66%	17 Missing ⚠️
toxcore/Messenger.c	0.00%	6 Missing ⚠️
toxcore/onion.c	83.78%	6 Missing ⚠️
toxcore/DHT.c	64.28%	5 Missing ⚠️
toxcore/logger.c	0.00%	2 Missing ⚠️
toxcore/util.c	0.00%	2 Missing ⚠️
toxav/msi.c	0.00%	1 Missing ⚠️
toxcore/network.c	66.66%	1 Missing ⚠️
toxcore/shared_key_cache.c	90.00%	1 Missing ⚠️
toxcore/tox.c	50.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2442   +/-   ##
=======================================
  Coverage   71.55%   71.56%           
=======================================
  Files          75       75           
  Lines       25200    25291   +91     
=======================================
+ Hits        18033    18100   +67     
- Misses       7167     7191   +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JFreegman]

Reviewed 18 of 25 files at r1, 1 of 4 files at r2, 3 of 7 files at r3, 5 of 5 files at r4, all commit messages.
Reviewable status: 0 of 1 approvals obtained (waiting on @iphydf)

---

toxcore/mono_time.c line 169 at r4 (raw file):

    mono_time->base_time = 1;
#else
    mono_time->base_time = max_u64(1, (uint64_t)time(nullptr)) * 1000ULL - current_time_monotonic(mono_time);

What does this do?

---

toxcore/onion_client.c line 2006 at r4 (raw file):

            const Node_format *target = &path_nodes[num];

            if (!node_list_contains(targets, targets_count, path_nodes[num].public_key)) {

This saves some bandwidth but at the expense of using more CPU, doing ~15 additional calls to pk_equal each tox_iterate(). Is the bandwidth saved worth the trade off, considering that pk_equal uses an expensive secure memory comparison function?

---

toxcore/pk_set.c line 66 at r4 (raw file):

    if (pks->size == UINT8_MAX) {
        return false;

Why not use integer error codes to distinguish between types of failure?

---

toxcore/pk_set.c line 94 at r4 (raw file):

}

bool pk_set_contains(Pk_Set *pks, const uint8_t *pk)

If this function were moved somewhere else this entire module could be generalized for any data type with just some name changes. Is there a reason we don't want that?

[iphydf]

Reviewable status: 0 of 1 approvals obtained (waiting on @JFreegman)

---

toxcore/mono_time.c line 169 at r4 (raw file):

Previously, JFreegman wrote…

What does this do?

Added comment.

---

toxcore/onion_client.c line 2006 at r4 (raw file):

Previously, JFreegman wrote…

This saves some bandwidth but at the expense of using more CPU, doing ~15 additional calls to pk_equal each tox_iterate(). Is the bandwidth saved worth the trade off, considering that pk_equal uses an expensive secure memory comparison function?

Changed it to memeq. There's no need for this to be secure comparison.

---

toxcore/pk_set.c line 66 at r4 (raw file):

Previously, JFreegman wrote…

Why not use integer error codes to distinguish between types of failure?

Deleted.

---

toxcore/pk_set.c line 94 at r4 (raw file):

Previously, JFreegman wrote…

If this function were moved somewhere else this entire module could be generalized for any data type with just some name changes. Is there a reason we don't want that?

Deleted.

[JFreegman]

Reviewed 11 of 11 files at r5, 6 of 6 files at r6, all commit messages.
Reviewable status: complete! 1 of 1 approvals obtained