From 34c43180da41f2464f8c2f84bd774f73dd7897bc Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 13 Aug 2025 10:51:57 +1000 Subject: [PATCH 1/4] Started working on trying to add custom conflict resolution handling --- include/session/config.hpp | 4 +++ include/session/config/base.hpp | 17 ++++++++++ include/session/config/user_profile.hpp | 42 +++++++++++++++++++++++++ src/config.cpp | 9 +++++- src/config/base.cpp | 3 ++ src/config/user_profile.cpp | 26 +++++++++++++++ 6 files changed, 100 insertions(+), 1 deletion(-) diff --git a/include/session/config.hpp b/include/session/config.hpp index 1be5f283..cfaec899 100644 --- a/include/session/config.hpp +++ b/include/session/config.hpp @@ -178,6 +178,8 @@ class ConfigMessage { verify_callable verifier = nullptr, sign_callable signer = nullptr, int lag = DEFAULT_DIFF_LAGS, + std::function + custom_conflict_resolver = nullptr, std::function error_handler = nullptr); /// Returns a read-only reference to the contained data. (To get a mutable config object use @@ -301,6 +303,8 @@ class MutableConfigMessage : public ConfigMessage { verify_callable verifier = nullptr, sign_callable signer = nullptr, int lag = DEFAULT_DIFF_LAGS, + std::function + custom_conflict_resolver = nullptr, std::function error_handler = nullptr); /// Wrapper around the above that takes a single string view to load a single message, doesn't diff --git a/include/session/config/base.hpp b/include/session/config/base.hpp index a6a53e4e..3959b5c6 100644 --- a/include/session/config/base.hpp +++ b/include/session/config/base.hpp @@ -869,6 +869,23 @@ class ConfigBase : public ConfigSig { std::unordered_set _merge( std::span>> configs); + /// API: base/ConfigBase::resolve_conflicts + /// + /// Subclasses can override this to add custom logic in order to resolve conflicts when merging + /// multiple configs that have the same seq_no. If this function returns false then the original + /// conflict resolution logic will be used. + /// + /// Inputs: + /// - `data` -- The config data to be updated to the resolved state. + /// - `diff` -- The diffs from the conflicting config update. + /// - `source` -- The config data that the diffs conflicted with. + /// + /// Outputs: + /// - `Bool` -- Returns true if the conflicts were resolved + virtual bool resolve_conflicts(dict& data, const oxenc::bt_dict& diff, const dict& source) { + return false; + }; + /// API: base/ConfigBase::extra_data /// /// Called when dumping to obtain any extra data that a subclass needs to store to reconstitute diff --git a/include/session/config/user_profile.hpp b/include/session/config/user_profile.hpp index 6b9dfa8e..bbb07620 100644 --- a/include/session/config/user_profile.hpp +++ b/include/session/config/user_profile.hpp @@ -24,6 +24,8 @@ using namespace std::literals; /// M - set to 1 if blinded message request retrieval is enabled, 0 if retrieval is *disabled*, and /// omitted if the setting has not been explicitly set (or has been explicitly cleared for some /// reason). +/// t - The unix timestamp (seconds) that the user last explicitly updated their public profile +/// information. class UserProfile : public ConfigBase { @@ -71,6 +73,23 @@ class UserProfile : public ConfigBase { /// - `const char*` - Will return "UserProfile" const char* encryption_domain() const override { return "UserProfile"; } + /// API: user_profile/UserProfile::resolve_conflicts + /// + /// The UserProfile config stores a timestamp indicating when the user explicitly updated their + /// profile information but there are other situations where the profile information can be + /// "automatically" updated by the clients (eg. re-uploading a display picture). If these + /// actions happen on two devices at the same time it can result in a conflict, in which case we + /// want the "explicit" update to to win the conflict resolution. + /// + /// Inputs: + /// - `data` -- The config data to be updated to the resolved state. + /// - `diff` -- The diffs from the conflicting config update. + /// - `source` -- The config data that the diffs conflicted with. + /// + /// Outputs: + /// - `Bool` -- Returns true if the conflicts were resolved + bool resolve_conflicts(dict& data, const oxenc::bt_dict& diff, const dict& source) override; + /// API: user_profile/UserProfile::get_name /// /// Returns the user profile name, or std::nullopt if there is no profile name set. @@ -199,6 +218,29 @@ class UserProfile : public ConfigBase { /// default). void set_blinded_msgreqs(std::optional enabled); + /// API: user_profile/UserProfile::get_public_profile_updated + /// + /// returns the timestamp that the user last updated their public profile information; or `0` if + /// it's never been updated. + /// + /// Inputs: None + /// + /// Outputs: + /// - `std::chrono::sys_seconds` - timestamp that the user last updated their public profile + /// information. Will be `0` if it's never been updated. + std::chrono::sys_seconds get_public_profile_updated() const; + + /// API: user_profile/UserProfile::set_public_profile_updated + /// + /// Sets the timestamp that the user last updated their public profile information (should be + /// updated by the clients when modifying public profile information via a user action, eg: + /// `name`, `profile_pic`, `set_blinded_msgreqs`) but not when an "automated" change occurs (eg. + /// re-uploading the display picture due to expiration). + /// + /// Inputs: + /// - `updated` -- timestamp that the user last updated their public profile information. + void set_public_profile_updated(std::chrono::sys_seconds updated); + bool accepts_protobuf() const override { return true; } }; diff --git a/src/config.cpp b/src/config.cpp index ee232910..035417f7 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -565,6 +565,7 @@ ConfigMessage::ConfigMessage( verify_callable verifier_, sign_callable signer_, int lag, + std::function custom_conflict_resolver, std::function error_handler) : verifier{std::move(verifier_)}, signer{std::move(signer_)}, lag{lag} { @@ -678,7 +679,10 @@ ConfigMessage::ConfigMessage( // ones for (const auto& [seqno_hash, ptrs] : replay) { const auto& [data, diff] = ptrs; - apply_diff(data_, *diff, *data); + + if (!custom_conflict_resolver(data_, *diff, *data)) + apply_diff(data_, *diff, *data); + lagged_diffs_.emplace_hint(lagged_diffs_.end(), seqno_hash, *diff); } @@ -694,12 +698,14 @@ MutableConfigMessage::MutableConfigMessage( verify_callable verifier, sign_callable signer, int lag, + std::function custom_conflict_resolver, std::function error_handler) : ConfigMessage{ serialized_confs, std::move(verifier), std::move(signer), lag, + std::move(custom_conflict_resolver), std::move(error_handler)} { if (!merged()) increment_impl(); @@ -715,6 +721,7 @@ MutableConfigMessage::MutableConfigMessage( std::move(verifier), std::move(signer), lag, + [](dict&, const oxenc::bt_dict&, const dict&) { return false; }, [](size_t, const config_error& e) { throw e; }} {} const oxenc::bt_dict& ConfigMessage::diff() { diff --git a/src/config/base.cpp b/src/config/base.cpp index a8851934..6e439351 100644 --- a/src/config/base.cpp +++ b/src/config/base.cpp @@ -531,6 +531,9 @@ std::unordered_set ConfigBase::_merge( _config->verifier, _config->signer, config_lags(), + [&](dict& data, const oxenc::bt_dict& diff, const dict& source) { + return resolve_conflicts(data, diff, source); + }, [&](size_t i, const config_error& e) { log::warning(cat, "{}", e.what()); assert(i > 0); // i == 0 means we can't deserialize our own serialization diff --git a/src/config/user_profile.cpp b/src/config/user_profile.cpp index f14b9255..f391e96f 100644 --- a/src/config/user_profile.cpp +++ b/src/config/user_profile.cpp @@ -83,6 +83,32 @@ std::optional UserProfile::get_blinded_msgreqs() const { return std::nullopt; } +std::chrono::sys_seconds UserProfile::get_public_profile_updated() const { + if (auto* t = data["t"].integer(); t) + return std::chrono::sys_seconds{std::chrono::seconds{*t}}; + return std::chrono::sys_seconds{}; +} + +void UserProfile::set_public_profile_updated(std::chrono::sys_seconds updated) { + if (updated.time_since_epoch().count() == 0) + data["t"].erase(); + else + data["t"] = static_cast(updated.time_since_epoch().count()); +} + +bool UserProfile::resolve_conflicts(dict& data, const oxenc::bt_dict& diff, const dict& source) { + auto diff_it = diff.find("t"); + auto source_it = source.find("t"); + + if (diff_it == diff.end() || source_it == source.end()) + return false; + + // TODO: Check which has the higher timestamp value and use those settings + // What if non-public values have also changed in the diff? + + return false; +}; + extern "C" { using namespace session; From 1f8cb1a3f4a53214d03e90aa39fcbf066cbf58ec Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 13 Aug 2025 14:11:36 +1000 Subject: [PATCH 2/4] Setup conflict resolution, added another version tracking value --- include/session/config.hpp | 8 +-- include/session/config/base.hpp | 11 ++-- include/session/config/user_profile.h | 50 +++++++++++++++ include/session/config/user_profile.hpp | 39 +++++++++--- src/config.cpp | 13 ++-- src/config/base.cpp | 4 +- src/config/user_profile.cpp | 81 +++++++++++++++++++++---- tests/test_configdata.cpp | 1 + 8 files changed, 168 insertions(+), 39 deletions(-) diff --git a/include/session/config.hpp b/include/session/config.hpp index cfaec899..d3a942f3 100644 --- a/include/session/config.hpp +++ b/include/session/config.hpp @@ -178,8 +178,8 @@ class ConfigMessage { verify_callable verifier = nullptr, sign_callable signer = nullptr, int lag = DEFAULT_DIFF_LAGS, - std::function - custom_conflict_resolver = nullptr, + std::function custom_conflict_resolver = + nullptr, std::function error_handler = nullptr); /// Returns a read-only reference to the contained data. (To get a mutable config object use @@ -303,8 +303,8 @@ class MutableConfigMessage : public ConfigMessage { verify_callable verifier = nullptr, sign_callable signer = nullptr, int lag = DEFAULT_DIFF_LAGS, - std::function - custom_conflict_resolver = nullptr, + std::function custom_conflict_resolver = + nullptr, std::function error_handler = nullptr); /// Wrapper around the above that takes a single string view to load a single message, doesn't diff --git a/include/session/config/base.hpp b/include/session/config/base.hpp index 3959b5c6..b280c12d 100644 --- a/include/session/config/base.hpp +++ b/include/session/config/base.hpp @@ -872,19 +872,16 @@ class ConfigBase : public ConfigSig { /// API: base/ConfigBase::resolve_conflicts /// /// Subclasses can override this to add custom logic in order to resolve conflicts when merging - /// multiple configs that have the same seq_no. If this function returns false then the original - /// conflict resolution logic will be used. + /// multiple configs that have the same seq_no. This function will remove any entries from + /// `diff` which have been handled by the custom resolution logic. /// /// Inputs: /// - `data` -- The config data to be updated to the resolved state. /// - `diff` -- The diffs from the conflicting config update. /// - `source` -- The config data that the diffs conflicted with. /// - /// Outputs: - /// - `Bool` -- Returns true if the conflicts were resolved - virtual bool resolve_conflicts(dict& data, const oxenc::bt_dict& diff, const dict& source) { - return false; - }; + /// Outputs: None + virtual void resolve_conflicts(dict& data, oxenc::bt_dict& diff, const dict& source) {}; /// API: base/ConfigBase::extra_data /// diff --git a/include/session/config/user_profile.h b/include/session/config/user_profile.h index 87d2c0ce..f4d09259 100644 --- a/include/session/config/user_profile.h +++ b/include/session/config/user_profile.h @@ -128,6 +128,31 @@ LIBSESSION_EXPORT user_profile_pic user_profile_get_pic(const config_object* con /// - `int` -- Returns 0 on success, non-zero on error LIBSESSION_EXPORT int user_profile_set_pic(config_object* conf, user_profile_pic pic); +/// API: user_profile/user_profile_profile_pic_content_version +/// +/// Returns the version of the profile picture content; or `0` if it's never been set. +/// +/// Inputs: None +/// +/// Outputs: +/// - `uint32_t` - version of the profile picture content. Will be `0` if it's never been set. +LIBSESSION_EXPORT uint32_t user_profile_get_profile_pic_content_version(const config_object* conf); + +/// API: user_profile/user_profile_profile_pic_content_version +/// +/// Sets the version for the profile picture content. This should be updated when a user sets a +/// new profile picture (or removes the current one), but now when re-uploading the current +/// profile picture. +/// +/// Inputs: +/// - `conf` -- [in] Pointer to the config object +/// - `version` -- [in] version for the profile picture content. +/// +/// Outputs: +/// - `int` -- Returns 0 on success, non-zero on error +LIBSESSION_EXPORT int user_profile_set_profile_pic_content_version( + config_object* conf, uint32_t version); + /// API: user_profile/user_profile_get_nts_priority /// /// Gets the current note-to-self priority level. Will be negative for hidden, 0 for unpinned, and > @@ -245,6 +270,31 @@ LIBSESSION_EXPORT int user_profile_get_blinded_msgreqs(const config_object* conf /// - `void` -- Returns Nothing LIBSESSION_EXPORT void user_profile_set_blinded_msgreqs(config_object* conf, int enabled); +/// API: user_profile/user_profile_get_profile_updated +/// +/// returns the timestamp that the user last updated their public profile information; or `0` if +/// it's never been updated. +/// +/// Inputs: None +/// +/// Outputs: +/// - `int64_t` - timestamp (unix seconds) that the user last updated their public profile +/// information. Will be `0` if it's never been updated. +LIBSESSION_EXPORT int64_t user_profile_get_profile_updated(config_object* conf); + +/// API: user_profile/user_profile_set_profile_updated +/// +/// Sets the timestamp that the user last updated their public profile information (should be +/// updated by the clients when modifying public profile information via a user action, eg: +/// `name`, `profile_pic`, `set_blinded_msgreqs`) but not when an "automated" change occurs (eg. +/// re-uploading the display picture due to expiration). +/// +/// Inputs: +/// - `conf` -- [in] Pointer to the config object +/// - `updated` -- [in] timestamp (unix seconds) that the user last updated their public profile +/// information. +LIBSESSION_EXPORT void user_profile_set_profile_updated(config_object* conf, int64_t updated); + #ifdef __cplusplus } // extern "C" #endif diff --git a/include/session/config/user_profile.hpp b/include/session/config/user_profile.hpp index bbb07620..0c2f77ad 100644 --- a/include/session/config/user_profile.hpp +++ b/include/session/config/user_profile.hpp @@ -18,14 +18,16 @@ using namespace std::literals; /// n - user profile name /// p - user profile url /// q - user profile decryption key (binary) +/// V - The version of the content of the profile picture, should be updated when the user uploads a +/// new profile picture (but not when re-uploading the current one). /// + - the priority value for the "Note to Self" pseudo-conversation (higher = higher in the /// conversation list). Omitted when 0. -1 means hidden. /// e - the expiry timer (in seconds) for the "Note to Self" pseudo-conversation. Omitted when 0. /// M - set to 1 if blinded message request retrieval is enabled, 0 if retrieval is *disabled*, and /// omitted if the setting has not been explicitly set (or has been explicitly cleared for some /// reason). -/// t - The unix timestamp (seconds) that the user last explicitly updated their public profile -/// information. +/// t - The unix timestamp (seconds) that the user last explicitly updated their profile information +/// (should be updated when changing `name`, `profile_pic` or `set_blinded_msgreqs`). class UserProfile : public ConfigBase { @@ -86,9 +88,8 @@ class UserProfile : public ConfigBase { /// - `diff` -- The diffs from the conflicting config update. /// - `source` -- The config data that the diffs conflicted with. /// - /// Outputs: - /// - `Bool` -- Returns true if the conflicts were resolved - bool resolve_conflicts(dict& data, const oxenc::bt_dict& diff, const dict& source) override; + /// Outputs: None + void resolve_conflicts(dict& data, oxenc::bt_dict& diff, const dict& source) override; /// API: user_profile/UserProfile::get_name /// @@ -148,6 +149,26 @@ class UserProfile : public ConfigBase { void set_profile_pic(std::string_view url, std::span key); void set_profile_pic(profile_pic pic); + /// API: user_profile/UserProfile::profile_pic_content_version + /// + /// Returns the version of the profile picture content; or `0` if it's never been set. + /// + /// Inputs: None + /// + /// Outputs: + /// - `uint32_t` - version of the profile picture content. Will be `0` if it's never been set. + uint32_t get_profile_pic_content_version() const; + + /// API: user_profile/UserProfile::profile_pic_content_version + /// + /// Sets the version for the profile picture content. This should be updated when a user sets a + /// new profile picture (or removes the current one), but now when re-uploading the current + /// profile picture. + /// + /// Inputs: + /// - `version` -- version for the profile picture content. + void set_profile_pic_content_version(uint32_t version); + /// API: user_profile/UserProfile::get_nts_priority /// /// Gets the Note-to-self conversation priority. Negative means hidden; 0 means unpinned; @@ -218,7 +239,7 @@ class UserProfile : public ConfigBase { /// default). void set_blinded_msgreqs(std::optional enabled); - /// API: user_profile/UserProfile::get_public_profile_updated + /// API: user_profile/UserProfile::get_profile_updated /// /// returns the timestamp that the user last updated their public profile information; or `0` if /// it's never been updated. @@ -228,9 +249,9 @@ class UserProfile : public ConfigBase { /// Outputs: /// - `std::chrono::sys_seconds` - timestamp that the user last updated their public profile /// information. Will be `0` if it's never been updated. - std::chrono::sys_seconds get_public_profile_updated() const; + std::chrono::sys_seconds get_profile_updated() const; - /// API: user_profile/UserProfile::set_public_profile_updated + /// API: user_profile/UserProfile::set_profile_updated /// /// Sets the timestamp that the user last updated their public profile information (should be /// updated by the clients when modifying public profile information via a user action, eg: @@ -239,7 +260,7 @@ class UserProfile : public ConfigBase { /// /// Inputs: /// - `updated` -- timestamp that the user last updated their public profile information. - void set_public_profile_updated(std::chrono::sys_seconds updated); + void set_profile_updated(std::chrono::sys_seconds updated); bool accepts_protobuf() const override { return true; } }; diff --git a/src/config.cpp b/src/config.cpp index 035417f7..5182520e 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -565,7 +565,7 @@ ConfigMessage::ConfigMessage( verify_callable verifier_, sign_callable signer_, int lag, - std::function custom_conflict_resolver, + std::function custom_conflict_resolver, std::function error_handler) : verifier{std::move(verifier_)}, signer{std::move(signer_)}, lag{lag} { @@ -679,10 +679,13 @@ ConfigMessage::ConfigMessage( // ones for (const auto& [seqno_hash, ptrs] : replay) { const auto& [data, diff] = ptrs; + auto mutable_diff = *diff; + + if (custom_conflict_resolver) + custom_conflict_resolver(data_, mutable_diff, *data); + + apply_diff(data_, mutable_diff, *data); - if (!custom_conflict_resolver(data_, *diff, *data)) - apply_diff(data_, *diff, *data); - lagged_diffs_.emplace_hint(lagged_diffs_.end(), seqno_hash, *diff); } @@ -698,7 +701,7 @@ MutableConfigMessage::MutableConfigMessage( verify_callable verifier, sign_callable signer, int lag, - std::function custom_conflict_resolver, + std::function custom_conflict_resolver, std::function error_handler) : ConfigMessage{ serialized_confs, diff --git a/src/config/base.cpp b/src/config/base.cpp index 6e439351..a774e585 100644 --- a/src/config/base.cpp +++ b/src/config/base.cpp @@ -531,8 +531,8 @@ std::unordered_set ConfigBase::_merge( _config->verifier, _config->signer, config_lags(), - [&](dict& data, const oxenc::bt_dict& diff, const dict& source) { - return resolve_conflicts(data, diff, source); + [&](dict& data, oxenc::bt_dict& diff, const dict& source) { + resolve_conflicts(data, diff, source); }, [&](size_t i, const config_error& e) { log::warning(cat, "{}", e.what()); diff --git a/src/config/user_profile.cpp b/src/config/user_profile.cpp index f391e96f..8ac714ea 100644 --- a/src/config/user_profile.cpp +++ b/src/config/user_profile.cpp @@ -52,6 +52,14 @@ void UserProfile::set_profile_pic(profile_pic pic) { set_profile_pic(pic.url, pic.key); } +uint32_t UserProfile::get_profile_pic_content_version() const { + return data["V"].integer_or(0); +} + +void UserProfile::set_profile_pic_content_version(uint32_t version) { + set_nonzero_int(data["V"], version); +} + void UserProfile::set_nts_priority(int priority) { set_nonzero_int(data["+"], priority); } @@ -83,30 +91,56 @@ std::optional UserProfile::get_blinded_msgreqs() const { return std::nullopt; } -std::chrono::sys_seconds UserProfile::get_public_profile_updated() const { +std::chrono::sys_seconds UserProfile::get_profile_updated() const { if (auto* t = data["t"].integer(); t) return std::chrono::sys_seconds{std::chrono::seconds{*t}}; return std::chrono::sys_seconds{}; } -void UserProfile::set_public_profile_updated(std::chrono::sys_seconds updated) { +void UserProfile::set_profile_updated(std::chrono::sys_seconds updated) { if (updated.time_since_epoch().count() == 0) data["t"].erase(); else data["t"] = static_cast(updated.time_since_epoch().count()); } -bool UserProfile::resolve_conflicts(dict& data, const oxenc::bt_dict& diff, const dict& source) { - auto diff_it = diff.find("t"); - auto source_it = source.find("t"); - - if (diff_it == diff.end() || source_it == source.end()) - return false; - - // TODO: Check which has the higher timestamp value and use those settings - // What if non-public values have also changed in the diff? +void UserProfile::resolve_conflicts(dict& data, oxenc::bt_dict& diff, const dict& source) { + // The UserProfile config stores a timestamp indicating when the user explicitly updated their + // profile information but there are other situations where the profile information can be + // "automatically" updated by the clients (eg. re-uploading a display picture). This hook + // pre-processes a conflict between these public profile values and removes any keys from the + // diff that should be ignored. + static const std::set relevant_keys = {"n", "p", "q", "t", "V"}; + + // No need to do anything if none of the relevant keys were modified + bool has_public_keys = false; + for (const auto& [key, _] : diff) { + if (relevant_keys.count(key)) { + has_public_keys = true; + break; + } + } - return false; + if (!has_public_keys) + return; + + // A higher content version should win, in the case that they both match then we should only + // keep the local state if it has a higher timestamp + const auto local_content_version = int_or_0(data, "V"); + const auto local_timestamp = ts_or_epoch(data, "t"); + const auto incoming_full_content_version = int_or_0(source, "V"); + const auto incoming_full_timestamp = ts_or_epoch(source, "t"); + auto local_state_wins = + ((local_content_version > incoming_full_content_version) || + (local_content_version == incoming_full_content_version && + local_timestamp > incoming_full_timestamp)); + + // If the local state wins then we should remove the `relevant_keys` from the diff (ie. keep the + // local state), otherwise the standard `apply_diff` logic should result in the incoming values + // overriding the local ones + if (local_state_wins) + for (const auto& key : relevant_keys) + diff.erase(key); }; extern "C" { @@ -167,6 +201,21 @@ LIBSESSION_C_API int user_profile_set_pic(config_object* conf, user_profile_pic static_cast(SESSION_ERR_BAD_VALUE)); } +LIBSESSION_C_API uint32_t user_profile_get_profile_pic_content_version(const config_object* conf) { + return unbox(conf)->get_profile_pic_content_version(); +} + +LIBSESSION_C_API int user_profile_set_profile_pic_content_version( + config_object* conf, uint32_t version) { + return wrap_exceptions( + conf, + [&] { + unbox(conf)->set_profile_pic_content_version(version); + return 0; + }, + static_cast(SESSION_ERR_BAD_VALUE)); +} + LIBSESSION_C_API int user_profile_get_nts_priority(const config_object* conf) { return unbox(conf)->get_nts_priority(); } @@ -196,4 +245,12 @@ LIBSESSION_C_API void user_profile_set_blinded_msgreqs(config_object* conf, int unbox(conf)->set_blinded_msgreqs(std::move(val)); } +LIBSESSION_C_API int64_t user_profile_get_profile_updated(config_object* conf) { + return unbox(conf)->get_profile_updated().time_since_epoch().count(); +} + +LIBSESSION_C_API void user_profile_set_profile_updated(config_object* conf, int64_t updated) { + unbox(conf)->set_profile_updated(to_sys_seconds(updated)); +} + } // extern "C" \ No newline at end of file diff --git a/tests/test_configdata.cpp b/tests/test_configdata.cpp index 24948943..376a8b0d 100644 --- a/tests/test_configdata.cpp +++ b/tests/test_configdata.cpp @@ -422,6 +422,7 @@ TEST_CASE("config message signature", "[config][signing]") { verifier, nullptr, ConfigMessage::DEFAULT_DIFF_LAGS, + [](config::dict&, oxenc::bt_dict&, const config::dict&) {}, [](size_t, const auto& exc) { throw exc; }), config::config_error, Message("Config signature failed verification")); From 2cefac922f5c05890cab1df9c88310d4f26d6ec4 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 13 Aug 2025 14:18:29 +1000 Subject: [PATCH 3/4] Also handle changes to the `blinded_msgreqs` setting --- src/config/user_profile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/user_profile.cpp b/src/config/user_profile.cpp index 8ac714ea..9d61ea2a 100644 --- a/src/config/user_profile.cpp +++ b/src/config/user_profile.cpp @@ -110,7 +110,7 @@ void UserProfile::resolve_conflicts(dict& data, oxenc::bt_dict& diff, const dict // "automatically" updated by the clients (eg. re-uploading a display picture). This hook // pre-processes a conflict between these public profile values and removes any keys from the // diff that should be ignored. - static const std::set relevant_keys = {"n", "p", "q", "t", "V"}; + static const std::set relevant_keys = {"n", "p", "q", "M", "t", "V"}; // No need to do anything if none of the relevant keys were modified bool has_public_keys = false; From 4a3952409a23b198cad2d0197699d01b7e01821c Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 21 Aug 2025 13:36:19 +1000 Subject: [PATCH 4/4] Reverted previous changes, added new re-upload fields to UserProfile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Reverted the previous changes • Added "t" to indicate the timestamp the profile pic was last updated via a user-driven change • Added new "P", "Q" and "T" keys to UserProfile to represent re-uploaded profile pic data • Added logic to auto-update the "t" and "T" timestamps when content is changed --- include/session/config.hpp | 4 - include/session/config/base.hpp | 14 ---- include/session/config/user_profile.h | 50 +++++-------- include/session/config/user_profile.hpp | 79 +++++++------------- src/config.cpp | 11 +-- src/config/base.cpp | 3 - src/config/user_profile.cpp | 98 +++++++++---------------- tests/test_configdata.cpp | 1 - 8 files changed, 81 insertions(+), 179 deletions(-) diff --git a/include/session/config.hpp b/include/session/config.hpp index d3a942f3..1be5f283 100644 --- a/include/session/config.hpp +++ b/include/session/config.hpp @@ -178,8 +178,6 @@ class ConfigMessage { verify_callable verifier = nullptr, sign_callable signer = nullptr, int lag = DEFAULT_DIFF_LAGS, - std::function custom_conflict_resolver = - nullptr, std::function error_handler = nullptr); /// Returns a read-only reference to the contained data. (To get a mutable config object use @@ -303,8 +301,6 @@ class MutableConfigMessage : public ConfigMessage { verify_callable verifier = nullptr, sign_callable signer = nullptr, int lag = DEFAULT_DIFF_LAGS, - std::function custom_conflict_resolver = - nullptr, std::function error_handler = nullptr); /// Wrapper around the above that takes a single string view to load a single message, doesn't diff --git a/include/session/config/base.hpp b/include/session/config/base.hpp index b280c12d..a6a53e4e 100644 --- a/include/session/config/base.hpp +++ b/include/session/config/base.hpp @@ -869,20 +869,6 @@ class ConfigBase : public ConfigSig { std::unordered_set _merge( std::span>> configs); - /// API: base/ConfigBase::resolve_conflicts - /// - /// Subclasses can override this to add custom logic in order to resolve conflicts when merging - /// multiple configs that have the same seq_no. This function will remove any entries from - /// `diff` which have been handled by the custom resolution logic. - /// - /// Inputs: - /// - `data` -- The config data to be updated to the resolved state. - /// - `diff` -- The diffs from the conflicting config update. - /// - `source` -- The config data that the diffs conflicted with. - /// - /// Outputs: None - virtual void resolve_conflicts(dict& data, oxenc::bt_dict& diff, const dict& source) {}; - /// API: base/ConfigBase::extra_data /// /// Called when dumping to obtain any extra data that a subclass needs to store to reconstitute diff --git a/include/session/config/user_profile.h b/include/session/config/user_profile.h index f4d09259..d3c2cece 100644 --- a/include/session/config/user_profile.h +++ b/include/session/config/user_profile.h @@ -92,7 +92,8 @@ LIBSESSION_EXPORT int user_profile_set_name(config_object* conf, const char* nam /// /// Obtains the current profile pic. The pointers in the returned struct will be NULL if a profile /// pic is not currently set, and otherwise should be copied right away (they will not be valid -/// beyond other API calls on this config object). +/// beyond other API calls on this config object). The returned value will be the latest profile pic +/// between when the user last set their profile and when it was last re-uploaded. /// /// Declaration: /// ```cpp @@ -110,7 +111,7 @@ LIBSESSION_EXPORT user_profile_pic user_profile_get_pic(const config_object* con /// API: user_profile/user_profile_set_pic /// -/// Sets a user profile +/// Sets a user profile pic /// /// Declaration: /// ```cpp @@ -128,30 +129,25 @@ LIBSESSION_EXPORT user_profile_pic user_profile_get_pic(const config_object* con /// - `int` -- Returns 0 on success, non-zero on error LIBSESSION_EXPORT int user_profile_set_pic(config_object* conf, user_profile_pic pic); -/// API: user_profile/user_profile_profile_pic_content_version +/// API: user_profile/user_profile_set_reupload_pic /// -/// Returns the version of the profile picture content; or `0` if it's never been set. +/// Sets a user profile pic when reuploading /// -/// Inputs: None -/// -/// Outputs: -/// - `uint32_t` - version of the profile picture content. Will be `0` if it's never been set. -LIBSESSION_EXPORT uint32_t user_profile_get_profile_pic_content_version(const config_object* conf); - -/// API: user_profile/user_profile_profile_pic_content_version -/// -/// Sets the version for the profile picture content. This should be updated when a user sets a -/// new profile picture (or removes the current one), but now when re-uploading the current -/// profile picture. +/// Declaration: +/// ```cpp +/// INT user_profile_set_reupload_pic( +/// [in] config_object* conf, +/// [in] user_profile_pic pic +/// ); +/// ``` /// /// Inputs: /// - `conf` -- [in] Pointer to the config object -/// - `version` -- [in] version for the profile picture content. +/// - `pic` -- [in] Pointer to the pic /// /// Outputs: /// - `int` -- Returns 0 on success, non-zero on error -LIBSESSION_EXPORT int user_profile_set_profile_pic_content_version( - config_object* conf, uint32_t version); +LIBSESSION_EXPORT int user_profile_set_reupload_pic(config_object* conf, user_profile_pic pic); /// API: user_profile/user_profile_get_nts_priority /// @@ -272,8 +268,9 @@ LIBSESSION_EXPORT void user_profile_set_blinded_msgreqs(config_object* conf, int /// API: user_profile/user_profile_get_profile_updated /// -/// returns the timestamp that the user last updated their public profile information; or `0` if -/// it's never been updated. +/// Returns the timestamp that the user last updated their profile information; or `0` if it's +/// never been updated. This value will return the latest timestamp between when the user last +/// set their profile and when it was last re-uploaded. /// /// Inputs: None /// @@ -282,19 +279,6 @@ LIBSESSION_EXPORT void user_profile_set_blinded_msgreqs(config_object* conf, int /// information. Will be `0` if it's never been updated. LIBSESSION_EXPORT int64_t user_profile_get_profile_updated(config_object* conf); -/// API: user_profile/user_profile_set_profile_updated -/// -/// Sets the timestamp that the user last updated their public profile information (should be -/// updated by the clients when modifying public profile information via a user action, eg: -/// `name`, `profile_pic`, `set_blinded_msgreqs`) but not when an "automated" change occurs (eg. -/// re-uploading the display picture due to expiration). -/// -/// Inputs: -/// - `conf` -- [in] Pointer to the config object -/// - `updated` -- [in] timestamp (unix seconds) that the user last updated their public profile -/// information. -LIBSESSION_EXPORT void user_profile_set_profile_updated(config_object* conf, int64_t updated); - #ifdef __cplusplus } // extern "C" #endif diff --git a/include/session/config/user_profile.hpp b/include/session/config/user_profile.hpp index 0c2f77ad..e2140fd7 100644 --- a/include/session/config/user_profile.hpp +++ b/include/session/config/user_profile.hpp @@ -18,8 +18,6 @@ using namespace std::literals; /// n - user profile name /// p - user profile url /// q - user profile decryption key (binary) -/// V - The version of the content of the profile picture, should be updated when the user uploads a -/// new profile picture (but not when re-uploading the current one). /// + - the priority value for the "Note to Self" pseudo-conversation (higher = higher in the /// conversation list). Omitted when 0. -1 means hidden. /// e - the expiry timer (in seconds) for the "Note to Self" pseudo-conversation. Omitted when 0. @@ -27,7 +25,12 @@ using namespace std::literals; /// omitted if the setting has not been explicitly set (or has been explicitly cleared for some /// reason). /// t - The unix timestamp (seconds) that the user last explicitly updated their profile information -/// (should be updated when changing `name`, `profile_pic` or `set_blinded_msgreqs`). +/// (automatically updates when changing `name`, `profile_pic` or `set_blinded_msgreqs`). +/// P - user profile url after re-uploading (should take precedence over `p` when `T > t`). +/// Q - user profile decryption key (binary) after re-uploading (should take precedence over `q` +/// when `T > t`). +/// T - The unix timestamp (seconds) that the user last re-uploaded their profile information +/// (automatically updates when calling `set_reupload_profile_pic`). class UserProfile : public ConfigBase { @@ -75,22 +78,6 @@ class UserProfile : public ConfigBase { /// - `const char*` - Will return "UserProfile" const char* encryption_domain() const override { return "UserProfile"; } - /// API: user_profile/UserProfile::resolve_conflicts - /// - /// The UserProfile config stores a timestamp indicating when the user explicitly updated their - /// profile information but there are other situations where the profile information can be - /// "automatically" updated by the clients (eg. re-uploading a display picture). If these - /// actions happen on two devices at the same time it can result in a conflict, in which case we - /// want the "explicit" update to to win the conflict resolution. - /// - /// Inputs: - /// - `data` -- The config data to be updated to the resolved state. - /// - `diff` -- The diffs from the conflicting config update. - /// - `source` -- The config data that the diffs conflicted with. - /// - /// Outputs: None - void resolve_conflicts(dict& data, oxenc::bt_dict& diff, const dict& source) override; - /// API: user_profile/UserProfile::get_name /// /// Returns the user profile name, or std::nullopt if there is no profile name set. @@ -121,7 +108,8 @@ class UserProfile : public ConfigBase { /// API: user_profile/UserProfile::get_profile_pic /// /// Gets the user's current profile pic URL and decryption key. The returned object will - /// evaluate as false if the URL and/or key are not set. + /// evaluate as false if the URL and/or key are not set. The returned value will be the latest + /// profile pic between when the user last set their profile and when it was last re-uploaded. /// /// Inputs: None /// @@ -131,8 +119,8 @@ class UserProfile : public ConfigBase { /// API: user_profile/UserProfile::set_profile_pic /// - /// Sets the user's current profile pic to a new URL and decryption key. Clears both if either - /// one is empty. + /// Sets the user's current profile pic to a new URL and decryption key. Clears both as well as + /// the reupload values if either one is empty. /// /// Declaration: /// ```cpp @@ -149,25 +137,24 @@ class UserProfile : public ConfigBase { void set_profile_pic(std::string_view url, std::span key); void set_profile_pic(profile_pic pic); - /// API: user_profile/UserProfile::profile_pic_content_version + /// API: user_profile/UserProfile::set_reupload_profile_pic /// - /// Returns the version of the profile picture content; or `0` if it's never been set. + /// Sets the user's profile pic to a new URL and decryption key after reuploading. /// - /// Inputs: None - /// - /// Outputs: - /// - `uint32_t` - version of the profile picture content. Will be `0` if it's never been set. - uint32_t get_profile_pic_content_version() const; - - /// API: user_profile/UserProfile::profile_pic_content_version - /// - /// Sets the version for the profile picture content. This should be updated when a user sets a - /// new profile picture (or removes the current one), but now when re-uploading the current - /// profile picture. + /// Declaration: + /// ```cpp + /// void set_reupload_profile_pic(std::string_view url, std::span key); + /// void set_reupload_profile_pic(profile_pic pic); + /// ``` /// /// Inputs: - /// - `version` -- version for the profile picture content. - void set_profile_pic_content_version(uint32_t version); + /// - First function: + /// - `url` -- URL pointing to the profile pic + /// - `key` -- Decryption key + /// - Second function: + /// - `pic` -- Profile pic object + void set_reupload_profile_pic(std::string_view url, std::span key); + void set_reupload_profile_pic(profile_pic pic); /// API: user_profile/UserProfile::get_nts_priority /// @@ -241,27 +228,17 @@ class UserProfile : public ConfigBase { /// API: user_profile/UserProfile::get_profile_updated /// - /// returns the timestamp that the user last updated their public profile information; or `0` if - /// it's never been updated. + /// Returns the timestamp that the user last updated their profile information; or `0` if it's + /// never been updated. This value will return the latest timestamp between when the user last + /// set their profile and when it was last re-uploaded. /// /// Inputs: None /// /// Outputs: - /// - `std::chrono::sys_seconds` - timestamp that the user last updated their public profile + /// - `std::chrono::sys_seconds` - timestamp that the user last updated their profile /// information. Will be `0` if it's never been updated. std::chrono::sys_seconds get_profile_updated() const; - /// API: user_profile/UserProfile::set_profile_updated - /// - /// Sets the timestamp that the user last updated their public profile information (should be - /// updated by the clients when modifying public profile information via a user action, eg: - /// `name`, `profile_pic`, `set_blinded_msgreqs`) but not when an "automated" change occurs (eg. - /// re-uploading the display picture due to expiration). - /// - /// Inputs: - /// - `updated` -- timestamp that the user last updated their public profile information. - void set_profile_updated(std::chrono::sys_seconds updated); - bool accepts_protobuf() const override { return true; } }; diff --git a/src/config.cpp b/src/config.cpp index 5182520e..a8e2cf1f 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -565,7 +565,6 @@ ConfigMessage::ConfigMessage( verify_callable verifier_, sign_callable signer_, int lag, - std::function custom_conflict_resolver, std::function error_handler) : verifier{std::move(verifier_)}, signer{std::move(signer_)}, lag{lag} { @@ -679,12 +678,7 @@ ConfigMessage::ConfigMessage( // ones for (const auto& [seqno_hash, ptrs] : replay) { const auto& [data, diff] = ptrs; - auto mutable_diff = *diff; - - if (custom_conflict_resolver) - custom_conflict_resolver(data_, mutable_diff, *data); - - apply_diff(data_, mutable_diff, *data); + apply_diff(data_, *diff, *data); lagged_diffs_.emplace_hint(lagged_diffs_.end(), seqno_hash, *diff); } @@ -701,14 +695,12 @@ MutableConfigMessage::MutableConfigMessage( verify_callable verifier, sign_callable signer, int lag, - std::function custom_conflict_resolver, std::function error_handler) : ConfigMessage{ serialized_confs, std::move(verifier), std::move(signer), lag, - std::move(custom_conflict_resolver), std::move(error_handler)} { if (!merged()) increment_impl(); @@ -724,7 +716,6 @@ MutableConfigMessage::MutableConfigMessage( std::move(verifier), std::move(signer), lag, - [](dict&, const oxenc::bt_dict&, const dict&) { return false; }, [](size_t, const config_error& e) { throw e; }} {} const oxenc::bt_dict& ConfigMessage::diff() { diff --git a/src/config/base.cpp b/src/config/base.cpp index a774e585..a8851934 100644 --- a/src/config/base.cpp +++ b/src/config/base.cpp @@ -531,9 +531,6 @@ std::unordered_set ConfigBase::_merge( _config->verifier, _config->signer, config_lags(), - [&](dict& data, oxenc::bt_dict& diff, const dict& source) { - resolve_conflicts(data, diff, source); - }, [&](size_t i, const config_error& e) { log::warning(cat, "{}", e.what()); assert(i > 0); // i == 0 means we can't deserialize our own serialization diff --git a/src/config/user_profile.cpp b/src/config/user_profile.cpp index 9d61ea2a..611156bc 100644 --- a/src/config/user_profile.cpp +++ b/src/config/user_profile.cpp @@ -28,6 +28,9 @@ void UserProfile::set_name(std::string_view new_name) { if (new_name.size() > contact_info::MAX_NAME_LENGTH) throw std::invalid_argument{"Invalid profile name: exceeds maximum length"}; set_nonempty_str(data["n"], new_name); + + const auto target_timestamp = (data["T"].integer_or(0) > data["t"].integer_or(0) ? "T" : "t"); + data[target_timestamp] = static_cast(std::chrono::system_clock::now().time_since_epoch().count()); } void UserProfile::set_name_truncated(std::string new_name) { set_name(utf8_truncate(std::move(new_name), contact_info::MAX_NAME_LENGTH)); @@ -35,9 +38,14 @@ void UserProfile::set_name_truncated(std::string new_name) { profile_pic UserProfile::get_profile_pic() const { profile_pic pic{}; - if (auto* url = data["p"].string(); url && !url->empty()) + + const bool use_primary_keys = (data["T"].integer_or(0) > data["t"].integer_or(0)); + const auto url_key = (use_primary_keys ? "p" : "P"); + const auto key_key = (use_primary_keys ? "q" : "Q"); + + if (auto* url = data[url_key].string(); url && !url->empty()) pic.url = *url; - if (auto* key = data["q"].string(); key && key->size() == 32) + if (auto* key = data[key_key].string(); key && key->size() == 32) pic.key.assign( reinterpret_cast(key->data()), reinterpret_cast(key->data()) + 32); @@ -46,18 +54,26 @@ profile_pic UserProfile::get_profile_pic() const { void UserProfile::set_profile_pic(std::string_view url, std::span key) { set_pair_if(!url.empty() && key.size() == 32, data["p"], url, data["q"], key); + + // If the profile was removed then we should remove the "reupload" version as well + if (url.empty() || key.size() != 32) { + set_reupload_profile_pic({}); + } + + data["t"] = static_cast(std::chrono::system_clock::now().time_since_epoch().count()); } void UserProfile::set_profile_pic(profile_pic pic) { set_profile_pic(pic.url, pic.key); } -uint32_t UserProfile::get_profile_pic_content_version() const { - return data["V"].integer_or(0); +void UserProfile::set_reupload_profile_pic(std::string_view url, std::span key) { + set_pair_if(!url.empty() && key.size() == 32, data["P"], url, data["Q"], key); + data["T"] = static_cast(std::chrono::system_clock::now().time_since_epoch().count()); } -void UserProfile::set_profile_pic_content_version(uint32_t version) { - set_nonzero_int(data["V"], version); +void UserProfile::set_reupload_profile_pic(profile_pic pic) { + set_reupload_profile_pic(pic.url, pic.key); } void UserProfile::set_nts_priority(int priority) { @@ -83,6 +99,9 @@ void UserProfile::set_blinded_msgreqs(std::optional value) { data["M"].erase(); else data["M"] = static_cast(*value); + + const auto target_timestamp = (data["T"].integer_or(0) > data["t"].integer_or(0) ? "T" : "t"); + data[target_timestamp] = static_cast(std::chrono::system_clock::now().time_since_epoch().count()); } std::optional UserProfile::get_blinded_msgreqs() const { @@ -92,57 +111,14 @@ std::optional UserProfile::get_blinded_msgreqs() const { } std::chrono::sys_seconds UserProfile::get_profile_updated() const { - if (auto* t = data["t"].integer(); t) + if (auto* t = data["t"].integer(); t) { + if (auto* T = data["T"].integer(); T && T > t) + return std::chrono::sys_seconds{std::chrono::seconds{*T}}; return std::chrono::sys_seconds{std::chrono::seconds{*t}}; + } return std::chrono::sys_seconds{}; } -void UserProfile::set_profile_updated(std::chrono::sys_seconds updated) { - if (updated.time_since_epoch().count() == 0) - data["t"].erase(); - else - data["t"] = static_cast(updated.time_since_epoch().count()); -} - -void UserProfile::resolve_conflicts(dict& data, oxenc::bt_dict& diff, const dict& source) { - // The UserProfile config stores a timestamp indicating when the user explicitly updated their - // profile information but there are other situations where the profile information can be - // "automatically" updated by the clients (eg. re-uploading a display picture). This hook - // pre-processes a conflict between these public profile values and removes any keys from the - // diff that should be ignored. - static const std::set relevant_keys = {"n", "p", "q", "M", "t", "V"}; - - // No need to do anything if none of the relevant keys were modified - bool has_public_keys = false; - for (const auto& [key, _] : diff) { - if (relevant_keys.count(key)) { - has_public_keys = true; - break; - } - } - - if (!has_public_keys) - return; - - // A higher content version should win, in the case that they both match then we should only - // keep the local state if it has a higher timestamp - const auto local_content_version = int_or_0(data, "V"); - const auto local_timestamp = ts_or_epoch(data, "t"); - const auto incoming_full_content_version = int_or_0(source, "V"); - const auto incoming_full_timestamp = ts_or_epoch(source, "t"); - auto local_state_wins = - ((local_content_version > incoming_full_content_version) || - (local_content_version == incoming_full_content_version && - local_timestamp > incoming_full_timestamp)); - - // If the local state wins then we should remove the `relevant_keys` from the diff (ie. keep the - // local state), otherwise the standard `apply_diff` logic should result in the incoming values - // overriding the local ones - if (local_state_wins) - for (const auto& key : relevant_keys) - diff.erase(key); -}; - extern "C" { using namespace session; @@ -201,16 +177,16 @@ LIBSESSION_C_API int user_profile_set_pic(config_object* conf, user_profile_pic static_cast(SESSION_ERR_BAD_VALUE)); } -LIBSESSION_C_API uint32_t user_profile_get_profile_pic_content_version(const config_object* conf) { - return unbox(conf)->get_profile_pic_content_version(); -} +LIBSESSION_C_API int user_profile_set_reupload_pic(config_object* conf, user_profile_pic pic) { + std::string_view url{pic.url}; + std::span key; + if (!url.empty()) + key = {pic.key, 32}; -LIBSESSION_C_API int user_profile_set_profile_pic_content_version( - config_object* conf, uint32_t version) { return wrap_exceptions( conf, [&] { - unbox(conf)->set_profile_pic_content_version(version); + unbox(conf)->set_reupload_profile_pic(url, key); return 0; }, static_cast(SESSION_ERR_BAD_VALUE)); @@ -249,8 +225,4 @@ LIBSESSION_C_API int64_t user_profile_get_profile_updated(config_object* conf) { return unbox(conf)->get_profile_updated().time_since_epoch().count(); } -LIBSESSION_C_API void user_profile_set_profile_updated(config_object* conf, int64_t updated) { - unbox(conf)->set_profile_updated(to_sys_seconds(updated)); -} - } // extern "C" \ No newline at end of file diff --git a/tests/test_configdata.cpp b/tests/test_configdata.cpp index 376a8b0d..24948943 100644 --- a/tests/test_configdata.cpp +++ b/tests/test_configdata.cpp @@ -422,7 +422,6 @@ TEST_CASE("config message signature", "[config][signing]") { verifier, nullptr, ConfigMessage::DEFAULT_DIFF_LAGS, - [](config::dict&, oxenc::bt_dict&, const config::dict&) {}, [](size_t, const auto& exc) { throw exc; }), config::config_error, Message("Config signature failed verification"));