wallet: bugfix: ensure atomicity in settings updates

- Settings updates were not thread-safe, as they were executed in
  three separate steps:

  1) Obtain settings value while acquiring the settings lock.
  2) Modify settings value.
  3) Overwrite settings value while acquiring the settings lock.

  This approach allowed concurrent threads to modify the same base value
  simultaneously, leading to data loss. When this occurred, the final
  settings state would only reflect the changes from the last thread
  that completed the operation, overwriting updates from other threads.

  Fix this by making the settings update operation atomic.

- Add test coverage for this behavior.

Co-authored-by: furszy <matiasfurszyfer@protonmail.com>

1 files changed, 21 insertions, 3 deletions

diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index af45f81f95..be596b1765 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -96,6 +96,17 @@ struct BlockInfo {
     BlockInfo(const uint256& hash LIFETIMEBOUND) : hash(hash) {}
 };
 
+//! The action to be taken after updating a settings value.
+//! WRITE indicates that the updated value must be written to disk,
+//! while SKIP_WRITE indicates that the change will be kept in memory-only
+//! without persisting it.
+enum class SettingsAction {
+    WRITE,
+    SKIP_WRITE
+};
+
+using SettingsUpdate = std::function<std::optional<interfaces::SettingsAction>(common::SettingsValue&)>;
+
 //! Interface giving clients (wallet processes, maybe other analysis tools in
 //! the future) ability to access to the chain state, receive notifications,
 //! estimate fees, and submit transactions.
@@ -344,9 +355,16 @@ public:
     //! Return <datadir>/settings.json setting value.
     virtual common::SettingsValue getRwSetting(const std::string& name) = 0;
 
-    //! Write a setting to <datadir>/settings.json. Optionally just update the
-    //! setting in memory and do not write the file.
-    virtual bool updateRwSetting(const std::string& name, const common::SettingsValue& value, bool write=true) = 0;
+    //! Updates a setting in <datadir>/settings.json.
+    //! Depending on the action returned by the update function, this will either
+    //! update the setting in memory or write the updated settings to disk.
+    virtual bool updateRwSetting(const std::string& name, const SettingsUpdate& update_function) = 0;
+
+    //! Replace a setting in <datadir>/settings.json with a new value.
+    virtual bool overwriteRwSetting(const std::string& name, common::SettingsValue& value, bool write = true) = 0;
+
+    //! Delete a given setting in <datadir>/settings.json.
+    virtual bool deleteRwSettings(const std::string& name, bool write = true) = 0;
 
     //! Synchronously send transactionAddedToMempool notifications about all
     //! current mempool transactions to the specified handler and return after