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, 24 insertions, 6 deletions

diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 9fe08eb3dd..54b986c926 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -814,14 +814,32 @@ public:
         });
         return result;
     }
-    bool updateRwSetting(const std::string& name, const common::SettingsValue& value, bool write) override
+    bool updateRwSetting(const std::string& name,
+                         const interfaces::SettingsUpdate& update_settings_func) override
     {
+        std::optional<interfaces::SettingsAction> action;
         args().LockSettings([&](common::Settings& settings) {
-            if (value.isNull()) {
-                settings.rw_settings.erase(name);
-            } else {
-                settings.rw_settings[name] = value;
-            }
+            auto* ptr_value = common::FindKey(settings.rw_settings, name);
+            // Create value if it doesn't exist
+            auto& value = ptr_value ? *ptr_value : settings.rw_settings[name];
+            action = update_settings_func(value);
+        });
+        if (!action) return false;
+        // Now dump value to disk if requested
+        return *action == interfaces::SettingsAction::SKIP_WRITE || args().WriteSettingsFile();
+    }
+    bool overwriteRwSetting(const std::string& name, common::SettingsValue& value, bool write) override
+    {
+        if (value.isNull()) return deleteRwSettings(name, write);
+        return updateRwSetting(name, [&](common::SettingsValue& settings) {
+            settings = std::move(value);
+            return write ? interfaces::SettingsAction::WRITE : interfaces::SettingsAction::SKIP_WRITE;
+        });
+    }
+    bool deleteRwSettings(const std::string& name, bool write) override
+    {
+        args().LockSettings([&](common::Settings& settings) {
+            settings.rw_settings.erase(name);
         });
         return !write || args().WriteSettingsFile();
     }