feat: Merge moderation portion of new groupchats codebase

[JFreegman]

Continuation of #2168 which was accidentally closed.

---

This change is 

[iphydf]

Reviewed 2 of 10 files at r1, 1 of 1 files at r6.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @JFreegman and @robinlinden)

---

toxcore/group_moderation.c, line 710 at r6 (raw file):

    }

    /** Operate on a copy of the list in case something goes wrong. */

This code looks very similar to the code in remove entry. Maybe we can extract a function?

[iphydf]

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

---

toxcore/util.h, line 43 at r6 (raw file):

non_null()
int public_key_cmp(const uint8_t *first_id, const uint8_t *second_id);

Do we ever need this function?

[iphydf]

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

---

toxcore/util.c, line 69 at r6 (raw file):

}

/* frees all pointers in a uint8_t pointer array, as well as the array itself. */

doxygen-style comment here and on jenkins.

[iphydf]

Reviewed 2 of 10 files at r1, 1 of 2 files at r3.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @JFreegman)

---

toxcore/crypto_core.h, line 202 at r6 (raw file):

 *   respectively. The Encryption keys are derived from the signature keys.
 *
 * @param pk The buffer where the public key will be stored. Must have room for EXT_PUBLIC_KEY_SIZE bytes.

Can we put these constants into crypto_core.h? I want to write a test for it without knowing anything and without having to look around for values in other header files. Tests should be written purely based on the documentation here.

[JFreegman]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @iphydf and @robinlinden)

---

toxcore/crypto_core.h, line 202 at r6 (raw file):

Previously, iphydf wrote…

Can we put these constants into crypto_core.h? I want to write a test for it without knowing anything and without having to look around for values in other header files. Tests should be written purely based on the documentation here.

Done.

---

toxcore/group_moderation.c, line 623 at r6 (raw file):

Previously, robinlinden (Robin Lindén) wrote…

Wouldn't passing both the current list and the new entry be the full sanctions list?

There is no new entry. In this case we just removed an entry and need to validate the resulting list against the supplied credentials. Either way, the validate function is used both when adding and removing an entry. So if we were to refactor the code to avoid the copy stuff, in one case we'd need to pass the validation function the list + new entry, and in the other, the list + index being removed. We'd end up with two fairly large/complex validation functions that behave a bit differently (or one monolith function that does both).

---

toxcore/group_moderation.c, line 710 at r6 (raw file):

Previously, iphydf wrote…

This code looks very similar to the code in remove entry. Maybe we can extract a function?

Done.

---

toxcore/util.h, line 43 at r6 (raw file):

Previously, iphydf wrote…

Do we ever need this function?

Done. It was a leftover from an unused function I removed.

---

toxcore/util.c, line 69 at r6 (raw file):

Previously, iphydf wrote…

doxygen-style comment here and on jenkins.

Done.

[iphydf]

Reviewed 4 of 5 files at r7, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @JFreegman and @robinlinden)

---

toxcore/group_moderation.h, line 136 at r7 (raw file):

/** @brief Adds a mod to the moderator list.
 *
 * @param mod_data must be MOD_LIST_ENTRY_SIZE bytes.

What's the format of mod_data? Is it any arbitrary byte array of that length?

---

toxcore/group_moderation.h, line 151 at r7 (raw file):

void mod_list_cleanup(Moderation *moderation);

/** @brief Packs num_sanctions sanctions into data of maxlength length.

How large should data be? Should we have a sizing function here too?

---

toxcore/group_moderation.h, line 175 at r7 (raw file):

/** @brief Packs sanction list credentials into data.
 *
 * @param data must have room for MOD_SANCTIONS_CREDS_SIZE bytes.

Since data needs an exact size, should we still pass the length? I guess it might be defence-in-depth, but we don't do this for any of the other fixed-size arrays like the keys.

---

toxcore/group_moderation.h, line 184 at r7 (raw file):

/** @brief Unpacks sanctions credentials into creds from data.
 *
 * @param data must have room for MOD_SANCTIONS_CREDS_SIZE bytes.

Same here.

---

toxcore/group_moderation.c, line 710 at r6 (raw file):

Previously, JFreegman wrote…

Done.

Where is it?

---

toxcore/util.c, line 69 at r6 (raw file):

Previously, JFreegman wrote…

Done.

Not committed?

[JFreegman]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @iphydf and @robinlinden)

---

toxcore/group_moderation.h, line 136 at r7 (raw file):

Previously, iphydf wrote…

What's the format of mod_data? Is it any arbitrary byte array of that length?

It's a list of public keys, but you should assume it's just an arbitrary byte array of length size.

---

toxcore/group_moderation.h, line 151 at r7 (raw file):

Previously, iphydf wrote…

How large should data be? Should we have a sizing function here too?

It's only ever called with either 1 entry or the entire list. Do you think a size function is necessary?

---

toxcore/group_moderation.h, line 175 at r7 (raw file):

Previously, iphydf wrote…

Since data needs an exact size, should we still pass the length? I guess it might be defence-in-depth, but we don't do this for any of the other fixed-size arrays like the keys.

Done. Length isn't necessary as long as we do bounds checking before calling it.

---

toxcore/group_moderation.h, line 184 at r7 (raw file):

Previously, iphydf wrote…

Same here.

Done.

---

toxcore/group_moderation.c, line 710 at r6 (raw file):

Previously, iphydf wrote…

Where is it?

sanctions_list_apply_new() and sanctions_list_copy()

---

toxcore/util.c, line 69 at r6 (raw file):

Previously, iphydf wrote…

Not committed?

Done. I fixed the one in the header and meant to remove that one.

[JFreegman]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @iphydf and @robinlinden)

---

toxcore/group_moderation.h, line 10 at r11 (raw file):

Previously, iphydf wrote…

C_TOXCORE_TOXCORE_GROUP_MODERATION_H

Done.

[codecov]

Codecov Report

Merging #2169 (015305a) into master (09575dc) will increase coverage by 0.22%.
The diff coverage is 86.21%.

@@            Coverage Diff             @@
##           master    #2169      +/-   ##
==========================================
+ Coverage   77.85%   78.08%   +0.22%     
==========================================
  Files         117      119       +2     
  Lines       21826    22414     +588     
==========================================
+ Hits        16992    17501     +509     
- Misses       4834     4913      +79     

Impacted Files	Coverage Δ
toxcore/util.c	57.89% <50.00%> (-6.40%)	⬇️
toxcore/group_moderation.c	82.98% <82.98%> (ø)
toxcore/DHT.c	82.92% <100.00%> (+0.01%)	⬆️
toxcore/Messenger.c	83.02% <100.00%> (-0.09%)	⬇️
toxcore/crypto_core.c	93.95% <100.00%> (+0.38%)	⬆️
toxcore/crypto_core_test.cc	100.00% <100.00%> (ø)
toxcore/group_moderation_test.cc	100.00% <100.00%> (ø)
toxav/bwcontroller.c	38.55% <0.00%> (-7.23%)	⬇️
toxcore/ping.c	85.71% <0.00%> (-1.25%)	⬇️
toxcore/group.c	84.40% <0.00%> (-0.07%)	⬇️
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09575dc...015305a. Read the comment docs.