chain: ensure updateRwSetting doesn't update to a null settings

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

Author: ismaelsadeeq

src/interfaces/chain.h

@@ -356,6 +356,8 @@ class Chain
     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;

src/node/interfaces.cpp

@@ -819,10 +819,14 @@ class ChainImpl : public Chain
     {
         std::optional<interfaces::SettingsAction> action;
         args().LockSettings([&](common::Settings& settings) {
-            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 (auto* value = common::FindKey(settings.rw_settings, name)) {
+                action = update_settings_func(*value);
+                if (value->isNull()) settings.rw_settings.erase(name);
+            } else {
+                UniValue new_value;
+                action = update_settings_func(new_value);
+                if (!new_value.isNull()) settings.rw_settings[name] = std::move(new_value);
+            }
         });
         if (!action) return false;
         // Now dump value to disk if requested