Merge pull request #3 from Bilb/feature/local_members_sending_state

use single letter for keys dump() & set pending_ send_state

Author: mpretty-cyro

include/session/config/groups/keys.hpp

@@ -30,7 +30,7 @@ using namespace std::literals;
 /// - it isn't compressed (since most of the data fields are encrypted or random, compression
 ///   reduction would be minimal).
 ///
-/// Fields used (in ascii order):
+/// Fields used for generating key messages (in ascii order):
 /// # -- 24-byte nonce used for all the encrypted values in this message; required.
 ///
 /// For non-supplemental messages:
@@ -58,6 +58,19 @@ using namespace std::literals;
 ///      `~` since it is the largest 7-bit ascii character value).  Note that this signature
 ///      mechanism works exactly the same as the signature on regular config messages.
 ///
+/// Fields used for dumping the config (in ascii order):
+/// A -- active config messages list. A list of lists, where each list has:
+///      - as first argument the generation number
+///      - the rest are the hashes valid for that generation number
+/// L -- a list of dict representing all the keys. Each dict has:
+///      - k -- same as "For supplemental messages"
+///      - g -- same as "For supplemental messages"
+///      - t -- same as "For supplemental messages"
+/// P -- pending key as a dict (if present).
+///      - c -- the pending config message to push
+///      - g -- same as "For supplemental messages"
+///      - k -- same as "For supplemental messages"
+///
 /// Some extra details:
 ///
 /// - each copy of the encryption key uses xchacha20_poly1305 using the `#` nonce

include/session/config/groups/members.hpp

@@ -330,6 +330,28 @@ class Members : public ConfigBase {
     /// - `member` - Returns a filled out member struct
     member get_or_construct(std::string_view pubkey_hex) const;
 
+    /// API: groups/Members::has_pending_send
+    ///
+    /// This function can be used to check if a member is pending send locally.
+    ///
+    /// Inputs:
+    /// - `pubkey_hex` -- hex string of the session id
+    ///
+    /// Outputs:
+    /// - `bool` - true if that sessionid is marked as pending send locally
+    bool has_pending_send(std::string pubkey_hex) const;
+
+    /// API: groups/Members::set_pending_send
+    ///
+    /// This function can be used to set the pending send state of a member.
+    /// If that effectively made a change, it will set _needs_dump to true.
+    ///
+    /// Inputs:
+    /// - `pubkey_hex` -- hex string of the session id
+    /// - `pending`  -- pending send state to set for that member
+    ///
+    void set_pending_send(std::string pubkey_hex, bool pending);
+
     /// API: groups/Members::get_status
     ///
     /// This function goes through the various status values and returns a single consolidated
@@ -351,8 +373,7 @@ class Members : public ConfigBase {
 
         // If the member is promoted then we return the relevant promoted status
         if (member.admin) {
-            if (member.promotion_status == STATUS_NOT_SENT &&
-                pending_send_ids.find(member.session_id) != pending_send_ids.end())
+            if (member.promotion_status == STATUS_NOT_SENT && has_pending_send(member.session_id))
                 return member::Status::promotion_sending;
             else if (member.promotion_status == STATUS_NOT_SENT)
                 return member::Status::promotion_not_sent;
@@ -367,8 +388,7 @@ class Members : public ConfigBase {
         }
 
         // Otherwise the member is a standard member
-        if (member.invite_status == STATUS_NOT_SENT &&
-            pending_send_ids.find(member.session_id) != pending_send_ids.end())
+        if (member.invite_status == STATUS_NOT_SENT && has_pending_send(member.session_id))
             return member::Status::invite_sending;
         else if (member.invite_status == STATUS_NOT_SENT)
             return member::Status::invite_not_sent;

src/config/groups/keys.cpp

@@ -76,7 +76,7 @@ ustring Keys::dump() {
 ustring Keys::make_dump() const {
     oxenc::bt_dict_producer d;
     {
-        auto active = d.append_list("active");
+        auto active = d.append_list("A");
         for (const auto& [gen, hashes] : active_msgs_) {
             auto lst = active.append_list();
             lst.append(gen);
@@ -86,7 +86,7 @@ ustring Keys::make_dump() const {
     }
 
     {
-        auto keys = d.append_list("keys");
+        auto keys = d.append_list("L");
         for (auto& k : keys_) {
             auto ki = keys.append_dict();
             // NB: Keys must be in sorted order
@@ -101,7 +101,7 @@ ustring Keys::make_dump() const {
     }
 
     if (!pending_key_config_.empty()) {
-        auto pending = d.append_dict("pending");
+        auto pending = d.append_dict("P");
         // NB: Keys must be in sorted order
         pending.append("c", from_unsigned_sv(pending_key_config_));
         pending.append("g", pending_gen_);
@@ -114,7 +114,7 @@ ustring Keys::make_dump() const {
 void Keys::load_dump(ustring_view dump) {
     oxenc::bt_dict_consumer d{from_unsigned_sv(dump)};
 
-    if (d.skip_until("active")) {
+    if (d.skip_until("A")) {
         auto active = d.consume_list_consumer();
         while (!active.is_finished()) {
             auto lst = active.consume_list_consumer();
@@ -126,7 +126,7 @@ void Keys::load_dump(ustring_view dump) {
         throw config_value_error{"Invalid Keys dump: `active` not found"};
     }
 
-    if (d.skip_until("keys")) {
+    if (d.skip_until("L")) {
         auto keys = d.consume_list_consumer();
         while (!keys.is_finished()) {
             auto kd = keys.consume_dict_consumer();
@@ -156,7 +156,7 @@ void Keys::load_dump(ustring_view dump) {
         throw config_value_error{"Invalid Keys dump: `keys` not found"};
     }
 
-    if (d.skip_until("pending")) {
+    if (d.skip_until("P")) {
         auto pending = d.consume_dict_consumer();
 
         if (!pending.skip_until("c"))

src/config/groups/members.cpp

@@ -76,11 +76,11 @@ void Members::set(const member& mem) {
     // add them to the `pending_send_ids` until they are given a new status
     if ((!mem.admin && mem.invite_status == STATUS_NOT_SENT) ||
         (mem.admin && mem.promotion_status == STATUS_NOT_SENT))
-        pending_send_ids.emplace(mem.session_id);
+        set_pending_send(mem.session_id, true);
     else if (
             (!mem.admin && mem.invite_status != STATUS_NOT_SENT) ||
             (mem.admin && mem.promotion_status != STATUS_NOT_SENT))
-        pending_send_ids.erase(mem.session_id);
+        set_pending_send(mem.session_id, false);
 }
 
 void member::load(const dict& info_dict) {
@@ -150,8 +150,7 @@ bool Members::erase(std::string_view session_id) {
     bool ret = info.exists();
     info.erase();
 
-    if (pending_send_ids.erase(std::string(session_id)) > 0)
-        _needs_dump = true;
+    set_pending_send(std::string(session_id), false);
 
     return ret;
 }
@@ -162,6 +161,20 @@ size_t Members::size() const {
     return 0;
 }
 
+bool Members::has_pending_send(std::string pubkey_hex) const {
+    return pending_send_ids.count(pubkey_hex);
+}
+
+void Members::set_pending_send(std::string pubkey_hex, bool pending) {
+    bool changed = false;
+    if (pending)
+        changed = pending_send_ids.insert(pubkey_hex).second;
+    else
+        changed = pending_send_ids.erase(pubkey_hex);
+    if(changed)
+        _needs_dump = true;
+}
+
 member::member(std::string sid) : session_id{std::move(sid)} {
     check_session_id(session_id);
 }

tests/test_config_userprofile.cpp

@@ -21,6 +21,8 @@ void log_msg(config_log_level lvl, const char* msg, void*) {
          << ": " << msg);
 }
 
+auto empty_extra_data = "1:+de";
+
 TEST_CASE("UserProfile", "[config][user_profile]") {
 
     const auto seed = "0123456789abcdef0123456789abcdef00000000000000000000000000000000"_hexbytes;
@@ -221,7 +223,7 @@ TEST_CASE("user profile C API", "[config][user_profile][c]") {
           "1:!" "i2e"
           "1:$" + std::to_string(exp_push1_decrypted.size()) + ":" + std::string{to_sv(exp_push1_decrypted)} + ""
           "1:(" "0:"
-          "1:)" "le"
+          "1:)" "le" + empty_extra_data +
         "e"));
     // clang-format on
     free(dump1);  // done with the dump; don't leak!
@@ -240,7 +242,7 @@ TEST_CASE("user profile C API", "[config][user_profile][c]") {
           "1:!" "i0e"
           "1:$" + std::to_string(exp_push1_decrypted.size()) + ":" + std::string{to_sv(exp_push1_decrypted)} + ""
           "1:(" "9:fakehash1"
-          "1:)" "le"
+          "1:)" "le" + empty_extra_data +
         "e"));
     // clang-format on
     free(dump1);

tests/test_group_members.cpp

@@ -138,6 +138,10 @@ TEST_CASE("Group Members", "[config][groups][members]") {
                 CHECK(gmem2.get_status(m) ==
                       session::config::groups::member::Status::promotion_accepted);
             } else {
+                // on gmem1, our local extra data marks m as invite_sending
+                CHECK(gmem1.get_status(m) ==
+                      session::config::groups::member::Status::invite_sending);
+                // that extra data is not pushed, so gmem2 doesn't know about it
                 CHECK(gmem2.get_status(m) ==
                       session::config::groups::member::Status::invite_not_sent);
                 CHECK_FALSE(m.admin);
@@ -330,3 +334,43 @@ TEST_CASE("Group Members", "[config][groups][members]") {
           "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678"
           "901234567890");
 }
+
+TEST_CASE("Group Members restores extra data", "[config][groups][members]") {
+
+    const auto seed = "0123456789abcdef0123456789abcdeffedcba9876543210fedcba9876543210"_hexbytes;
+    std::array<unsigned char, 32> ed_pk;
+    std::array<unsigned char, 64> ed_sk;
+    crypto_sign_ed25519_seed_keypair(
+            ed_pk.data(), ed_sk.data(), reinterpret_cast<const unsigned char*>(seed.data()));
+
+    REQUIRE(oxenc::to_hex(ed_pk.begin(), ed_pk.end()) ==
+            "cbd569f56fb13ea95a3f0c05c331cc24139c0090feb412069dc49fab34406ece");
+    CHECK(oxenc::to_hex(seed.begin(), seed.end()) ==
+          oxenc::to_hex(ed_sk.begin(), ed_sk.begin() + 32));
+
+    groups::Members gmem1{to_usv(ed_pk), to_usv(ed_sk), std::nullopt};
+
+    auto memberId1 = "050000000000000000000000000000000000000000000000000000000000000000";
+    auto memberId2 = "051111111111111111111111111111111111111111111111111111111111111111";
+
+    auto member1 = gmem1.get_or_construct(memberId1);
+    auto member2 = gmem1.get_or_construct(memberId2);
+
+    member2.set_promoted();
+    gmem1.set(member1);  // should be marked as "invite sending" right away
+    gmem1.set(member2);  // should be marked as "promotion sending" right away
+
+    CHECK(gmem1.get_status(gmem1.get_or_construct(memberId1)) ==
+          groups::member::Status::invite_sending);
+    CHECK(gmem1.get_status(gmem1.get_or_construct(memberId2)) ==
+          groups::member::Status::promotion_sending);
+
+    auto dumped = gmem1.dump();
+
+    groups::Members gmem2{to_usv(ed_pk), to_usv(ed_sk), dumped};
+
+    CHECK(gmem2.get_status(gmem1.get_or_construct(memberId1)) ==
+          groups::member::Status::invite_sending);
+    CHECK(gmem2.get_status(gmem1.get_or_construct(memberId2)) ==
+          groups::member::Status::promotion_sending);
+}