Merge bitcoin/bitcoin#30697: Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface

1b41d45d462d856a9d0b44ae0039bbb2cd78407c wallet: bugfix: ensure atomicity in settings updates (ismaelsadeeq)

Pull request description:

  This PR fixes #30620.

  As outlined in the issue, creating two wallets with `load_on_startup=true` simultaneously results in only one wallet being added to the startup file.

  The current issue arises because the wallet settings update process involves:
  1. Obtaining the settings value while acquiring the settings lock.
  2. Modifying the settings value.
  3. Overwriting the settings value while acquiring the settings lock again.

  This sequence is not thread-safe. Different threads could modify the same base value simultaneously, overwriting data from other workers without realizing it.

  The PR attempts to  fix this by modifying the chain interface's `updateRwSetting` method to accept a function that will be called with the settings reference. This function will either update or delete the setting and return an enum indicating whether the settings need to be overwritten in this or not.

  Additionally, this PR introduces two new methods to the chain interface:
  - `overwriteRwSetting`: This method replaces the setting with a new value.
  Used in `VerifyWallets`
  - `deleteRwSettings`: This method completely erases a specified setting.
  This method is currently used only in `overwriteRwSetting`.

  These changes ensure that updates are race-free across all clients.

ACKs for top commit:
  achow101:
    ACK 1b41d45d462d856a9d0b44ae0039bbb2cd78407c
  furszy:
    self-code-ACK https://github.com/bitcoin/bitcoin/commit/1b41d45d462d856a9d0b44ae0039bbb2cd78407c

Tree-SHA512: 50cda612b782aeb5e03e2cf63cc44779a013de1c535b883b57af4de22f24b0de80b4edecbcda235413baec0a12bdf0e5750fb6731c9e67d32e742d8c63f08c13

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