Merge bitcoin/bitcoin#30828: interfaces: #30697 follow ups

84663291275248fd52da644b0c2566bbf9cc780b chain: simplify `deleteRwSettings` code and improve it's doc (ismaelsadeeq)
f8d91f49c7091102138fcb123850399e8fadfbc7 chain: dont check for null settings value in `overwriteRwSetting` (ismaelsadeeq)
df601993f2d7454f081316d6a8ddefbcefa49b3d chain: ensure `updateRwSetting` doesn't update to a null settings (ismaelsadeeq)
c8e2eeeffb494d99d95c1c45efeda5a98dce94cd chain: uniformly use `SettingsAction` enum in settings methods (ismaelsadeeq)
1e9e735670f029c975d3b7486a54e5bb67135df7 chain: move new settings safely in `overwriteRwSetting` (ismaelsadeeq)
1c409004c80bc5f2314e20cc922076e22931cf73 test: remove wallet context from `write_wallet_settings_concurrently` (ismaelsadeeq)

Pull request description:

  This PR addresses the remaining review comments from #30697

  1. Disallowed overwriting settings values with a `null` value.
  2. Uniformly used the `SettingsAction` enum in all settings methods instead of a boolean parameter.
  3. Updated `overwriteRwSetting` to receive the `common::SettingsValue` parameter by value, enabling it to be moved safely.
  4. Removed wallet context from the `write_wallet_settings_concurrently` unit test, as it is not needed.

ACKs for top commit:
  achow101:
    ACK 84663291275248fd52da644b0c2566bbf9cc780b
  ryanofsky:
    Code review ACK 84663291275248fd52da644b0c2566bbf9cc780b. Looks good, thanks for taking suggestions and applying them to the right commits. Only changes since last review were documentation improvements and simplifying delete method.
  furszy:
    Code review ACK 84663291275248fd52da644b0c2566bbf9cc780b

Tree-SHA512: baf2f59ed5aac4a4bda0c84fb6554a466a40d1f7b52b61dc2ff293d83ae60e82b925b7003237b633fecb65eba3a4c108e69166046895d1295809fbe0de67b052

1 files changed, 9 insertions, 2 deletions

diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index be596b1765..7bc2d11b60 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -356,15 +356,22 @@ public:
     virtual common::SettingsValue getRwSetting(const std::string& name) = 0;
 
     //! Updates a setting in <datadir>/settings.json.
+    //! Null can be passed to erase the setting. There is intentionally no
+    //! support for writing null values to 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;
+    //! Null can be passed to erase the setting.
+    //! This method provides a simpler alternative to updateRwSetting when
+    //! atomically reading and updating the setting is not required.
+    virtual bool overwriteRwSetting(const std::string& name, common::SettingsValue value, SettingsAction action = SettingsAction::WRITE) = 0;
 
     //! Delete a given setting in <datadir>/settings.json.
-    virtual bool deleteRwSettings(const std::string& name, bool write = true) = 0;
+    //! This method provides a simpler alternative to overwriteRwSetting when
+    //! erasing a setting, for ease of use and readability.
+    virtual bool deleteRwSettings(const std::string& name, SettingsAction action = SettingsAction::WRITE) = 0;
 
     //! Synchronously send transactionAddedToMempool notifications about all
     //! current mempool transactions to the specified handler and return after