Add settings.json prune-prev, proxy-prev, onion-prev settings

This provides a way for the GUI settings dialog box to retain previous pruning
and proxy settings when they are disabled, as requested by vasild:

bitcoin/bitcoin#15936 (review)
bitcoin/bitcoin#15936 (comment)
bitcoin/bitcoin#15936 (comment)

Importantly, while this PR changes the settings.json format, it changes it in a
fully backwards compatible way, so previous versious of bitcoind and bitcoin-qt
will correctly interpret prune, proxy, and onion settins written by new
versions of bitcoin-qt.

Author: ryanofsky

src/qt/optionsmodel.cpp

@@ -59,7 +59,7 @@ static const char* SettingName(OptionsModel::OptionID option)
 }
 
 /** Call node.updateRwSetting() with Bitcoin 22.x workaround. */
-static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID option, const util::SettingsValue& value)
+static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID option, const std::string& suffix, const util::SettingsValue& value)
 {
     if (value.isNum() &&
         (option == OptionsModel::DatabaseCache ||
@@ -73,9 +73,9 @@ static void UpdateRwSetting(interfaces::Node& node, OptionsModel::OptionID optio
         // in later releases by https://github.com/bitcoin/bitcoin/pull/24498.
         // If new numeric settings are added, they can be written as numbers
         // instead of strings, because bitcoin 22.x will not try to read these.
-        node.updateRwSetting(SettingName(option), value.getValStr());
+        node.updateRwSetting(SettingName(option) + suffix, value.getValStr());
     } else {
-        node.updateRwSetting(SettingName(option), value);
+        node.updateRwSetting(SettingName(option) + suffix, value);
     }
 }
 
@@ -131,13 +131,6 @@ void OptionsModel::addOverriddenOption(const std::string &option)
 bool OptionsModel::Init(bilingual_str& error)
 {
     // Initialize display settings from stored settings.
-    m_prune_size_gb = PruneSizeGB(node().getPersistentSetting("prune"));
-    ProxySetting proxy = ParseProxyString(SettingToString(node().getPersistentSetting("proxy"), GetDefaultProxyAddress().toStdString()));
-    m_proxy_ip = proxy.ip;
-    m_proxy_port = proxy.port;
-    ProxySetting onion = ParseProxyString(SettingToString(node().getPersistentSetting("onion"), GetDefaultProxyAddress().toStdString()));
-    m_onion_ip = onion.ip;
-    m_onion_port = onion.port;
     language = QString::fromStdString(SettingToString(node().getPersistentSetting("lang"), ""));
 
     checkAndMigrate();
@@ -318,8 +311,6 @@ void OptionsModel::SetPruneTargetGB(int prune_target_gb)
     const util::SettingsValue cur_value = node().getPersistentSetting("prune");
     const util::SettingsValue new_value = PruneSetting(prune_target_gb > 0, prune_target_gb);
 
-    m_prune_size_gb = prune_target_gb;
-
     // Force setting to take effect. It is still safe to change the value at
     // this point because this function is only called after the intro screen is
     // shown, before the node starts.
@@ -332,7 +323,12 @@ void OptionsModel::SetPruneTargetGB(int prune_target_gb)
         PruneSizeGB(cur_value) != PruneSizeGB(new_value)) {
         // Call UpdateRwSetting() instead of setOption() to avoid setting
         // RestartRequired flag
-        UpdateRwSetting(node(), Prune, new_value);
+        UpdateRwSetting(node(), Prune, "", new_value);
+    }
+
+    // Keep previous pruning size, if pruning was disabled.
+    if (PruneEnabled(cur_value)) {
+        UpdateRwSetting(node(), Prune, "-prev", PruneEnabled(new_value) ? util::SettingsValue{} : cur_value);
     }
 }
 
@@ -360,9 +356,9 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in
     return successful;
 }
 
-QVariant OptionsModel::getOption(OptionID option) const
+QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) const
 {
-    auto setting = [&]{ return node().getPersistentSetting(SettingName(option)); };
+    auto setting = [&]{ return node().getPersistentSetting(SettingName(option) + suffix); };
 
     QSettings settings;
     switch (option) {
@@ -389,19 +385,30 @@ QVariant OptionsModel::getOption(OptionID option) const
 
     // default proxy
     case ProxyUse:
+    case ProxyUseTor:
         return ParseProxyString(SettingToString(setting(), "")).is_set;
     case ProxyIP:
-        return m_proxy_ip;
+    case ProxyIPTor: {
+        ProxySetting proxy = ParseProxyString(SettingToString(setting(), ""));
+        if (proxy.is_set) {
+            return proxy.ip;
+        } else if (suffix.empty()) {
+            return getOption(option, "-prev");
+        } else {
+            return ParseProxyString(GetDefaultProxyAddress().toStdString()).ip;
+        }
+    }
     case ProxyPort:
-        return m_proxy_port;
-
-    // separate Tor proxy
-    case ProxyUseTor:
-        return ParseProxyString(SettingToString(setting(), "")).is_set;
-    case ProxyIPTor:
-        return m_onion_ip;
-    case ProxyPortTor:
-        return m_onion_port;
+    case ProxyPortTor: {
+        ProxySetting proxy = ParseProxyString(SettingToString(setting(), ""));
+        if (proxy.is_set) {
+            return proxy.port;
+        } else if (suffix.empty()) {
+            return getOption(option, "-prev");
+        } else {
+            return ParseProxyString(GetDefaultProxyAddress().toStdString()).port;
+        }
+    }
 
 #ifdef ENABLE_WALLET
     case SpendZeroConfChange:
@@ -426,7 +433,9 @@ QVariant OptionsModel::getOption(OptionID option) const
     case Prune:
         return PruneEnabled(setting());
     case PruneSize:
-        return m_prune_size_gb;
+        return PruneEnabled(setting()) ? PruneSizeGB(setting()) :
+               suffix.empty()          ? getOption(option, "-prev") :
+                                         DEFAULT_PRUNE_TARGET_GB;
     case DatabaseCache:
         return qlonglong(SettingToInt(setting(), nDefaultDbCache));
     case ThreadsScriptVerif:
@@ -440,10 +449,10 @@ QVariant OptionsModel::getOption(OptionID option) const
     }
 }
 
-bool OptionsModel::setOption(OptionID option, const QVariant& value)
+bool OptionsModel::setOption(OptionID option, const QVariant& value, const std::string& suffix)
 {
-    auto changed = [&] { return value.isValid() && value != getOption(option); };
-    auto update = [&](const util::SettingsValue& value) { return UpdateRwSetting(node(), option, value); };
+    auto changed = [&] { return value.isValid() && value != getOption(option, suffix); };
+    auto update = [&](const util::SettingsValue& value) { return UpdateRwSetting(node(), option, suffix, value); };
 
     bool successful = true; /* set to false on parse error */
     QSettings settings;
@@ -481,52 +490,60 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value)
     // default proxy
     case ProxyUse:
         if (changed()) {
-            update(ProxyString(value.toBool(), m_proxy_ip, m_proxy_port));
-            setRestartRequired(true);
+            if (suffix.empty() && !value.toBool()) setOption(option, true, "-prev");
+            update(ProxyString(value.toBool(), getOption(ProxyIP).toString(), getOption(ProxyPort).toString()));
+            if (suffix.empty() && value.toBool()) UpdateRwSetting(node(), option, "-prev", {});
+            if (suffix.empty()) setRestartRequired(true);
         }
         break;
     case ProxyIP:
         if (changed()) {
-            m_proxy_ip = value.toString();
-            if (getOption(ProxyUse).toBool()) {
-                update(ProxyString(true, m_proxy_ip, m_proxy_port));
-                setRestartRequired(true);
+            if (suffix.empty() && !getOption(ProxyUse).toBool()) {
+                setOption(option, value, "-prev");
+            } else {
+                update(ProxyString(true, value.toString(), getOption(ProxyPort).toString()));
             }
+            if (suffix.empty() && getOption(ProxyUse).toBool()) setRestartRequired(true);
         }
         break;
     case ProxyPort:
         if (changed()) {
-            m_proxy_port = value.toString();
-            if (getOption(ProxyUse).toBool()) {
-                update(ProxyString(true, m_proxy_ip, m_proxy_port));
-                setRestartRequired(true);
+            if (suffix.empty() && !getOption(ProxyUse).toBool()) {
+                setOption(option, value, "-prev");
+            } else {
+                update(ProxyString(true, getOption(ProxyIP).toString(), value.toString()));
             }
+            if (suffix.empty() && getOption(ProxyUse).toBool()) setRestartRequired(true);
         }
         break;
 
     // separate Tor proxy
     case ProxyUseTor:
         if (changed()) {
-            update(ProxyString(value.toBool(), m_onion_ip, m_onion_port));
-            setRestartRequired(true);
+            if (suffix.empty() && !value.toBool()) setOption(option, true, "-prev");
+            update(ProxyString(value.toBool(), getOption(ProxyIPTor).toString(), getOption(ProxyPortTor).toString()));
+            if (suffix.empty() && value.toBool()) UpdateRwSetting(node(), option, "-prev", {});
+            if (suffix.empty()) setRestartRequired(true);
         }
         break;
     case ProxyIPTor:
         if (changed()) {
-            m_onion_ip = value.toString();
-            if (getOption(ProxyUseTor).toBool()) {
-                update(ProxyString(true, m_onion_ip, m_onion_port));
-                setRestartRequired(true);
+            if (suffix.empty() && !getOption(ProxyUseTor).toBool()) {
+                setOption(option, value, "-prev");
+            } else {
+                update(ProxyString(true, value.toString(), getOption(ProxyPortTor).toString()));
             }
+            if (suffix.empty() && getOption(ProxyUseTor).toBool()) setRestartRequired(true);
         }
         break;
     case ProxyPortTor:
         if (changed()) {
-            m_onion_port = value.toString();
-            if (getOption(ProxyUseTor).toBool()) {
-                update(ProxyString(true, m_onion_ip, m_onion_port));
-                setRestartRequired(true);
+            if (suffix.empty() && !getOption(ProxyUseTor).toBool()) {
+                setOption(option, value, "-prev");
+            } else {
+                update(ProxyString(true, getOption(ProxyIPTor).toString(), value.toString()));
             }
+            if (suffix.empty() && getOption(ProxyUseTor).toBool()) setRestartRequired(true);
         }
         break;
 
@@ -580,17 +597,20 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value)
         break;
     case Prune:
         if (changed()) {
-            update(PruneSetting(value.toBool(), m_prune_size_gb));
-            setRestartRequired(true);
+            if (suffix.empty() && !value.toBool()) setOption(option, true, "-prev");
+            update(PruneSetting(value.toBool(), getOption(PruneSize).toInt()));
+            if (suffix.empty() && value.toBool()) UpdateRwSetting(node(), option, "-prev", {});
+            if (suffix.empty()) setRestartRequired(true);
         }
         break;
     case PruneSize:
         if (changed()) {
-            m_prune_size_gb = ParsePruneSizeGB(value);
-            if (getOption(Prune).toBool()) {
-                update(PruneSetting(true, m_prune_size_gb));
-                setRestartRequired(true);
+            if (suffix.empty() && !getOption(Prune).toBool()) {
+                setOption(option, value, "-prev");
+            } else {
+                update(PruneSetting(true, ParsePruneSizeGB(value)));
             }
+            if (suffix.empty() && getOption(Prune).toBool()) setRestartRequired(true);
         }
         break;
     case DatabaseCache:

src/qt/optionsmodel.h

@@ -81,8 +81,8 @@ class OptionsModel : public QAbstractListModel
     int rowCount(const QModelIndex & parent = QModelIndex()) const override;
     QVariant data(const QModelIndex & index, int role = Qt::DisplayRole) const override;
     bool setData(const QModelIndex & index, const QVariant & value, int role = Qt::EditRole) override;
-    QVariant getOption(OptionID option) const;
-    bool setOption(OptionID option, const QVariant& value);
+    QVariant getOption(OptionID option, const std::string& suffix="") const;
+    bool setOption(OptionID option, const QVariant& value, const std::string& suffix="");
     /** Updates current unit in memory, settings and emits displayUnitChanged(new_unit) signal */
     void setDisplayUnit(const QVariant& new_unit);
 
@@ -121,15 +121,6 @@ class OptionsModel : public QAbstractListModel
     bool m_sub_fee_from_amount;
     bool m_enable_psbt_controls;
 
-    //! In-memory settings for display. These are stored persistently by the
-    //! bitcoin node but it's also nice to store them in memory to prevent them
-    //! getting cleared when enable/disable toggles are used in the GUI.
-    int m_prune_size_gb;
-    QString m_proxy_ip;
-    QString m_proxy_port;
-    QString m_onion_ip;
-    QString m_onion_port;
-
     /* settings that were overridden by command-line */
     QString strOverriddenByCommandLine;