Skip to content

Commit

Permalink
Merge bitcoin-core/gui#603: Add settings.json prune-prev, proxy-prev,…
Browse files Browse the repository at this point in the history 
… onion-prev settings

9d3127b Add settings.json prune-prev, proxy-prev, onion-prev settings (Ryan Ofsky)

Pull request description:

  With dashpay#602, if proxy and pruning settings are disabled in the GUI and the GUI is restarted, proxy and prune values are not stored anywhere. So if these settings are enabled in the future, default values will be shown, not previous values.

  This PR stores previous values so they will preserved across restarts. I'm not sure I like this behavior because showing default values seems simpler and safer to me. Previous values may just have been set temporarily and may have never actually worked, and it adds some code complexity to store them.

  This PR is one way of resolving dashpay#596. Other solutions are possible and could be implemented as alternatives.

ACKs for top commit:
  hebasto:
    ACK 9d3127b, tested on Ubuntu 22.04.
  vasild:
    ACK 9d3127b
  jarolrod:
    tACK 9d3127b

Tree-SHA512: 1778d1819443490c880cfd5c1711d9c5ac75ea3ee8440e2f0ced81d293247163a78ae8aba6027215110aec6533bd7dc6472aeead6796bfbd51bf2354e28f24a9
  • Loading branch information
@hebasto
hebasto committed Feb 15, 2023
2 parents 68e484a + 9d3127b commit e43ff4e
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 66 deletions.
130 changes: 75 additions & 55 deletions src/qt/optionsmodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -320,8 +313,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.
Expand All @@ -334,7 +325,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);
}
}

Expand Down Expand Up @@ -362,9 +358,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) {
Expand All @@ -391,19 +387,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:
Expand All @@ -428,7 +435,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:
Expand All @@ -444,10 +453,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;
Expand Down Expand Up @@ -485,52 +494,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;

Expand Down Expand Up @@ -584,17 +601,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:
Expand Down
13 changes: 2 additions & 11 deletions src/qt/optionsmodel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,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);

Expand Down Expand Up @@ -123,15 +123,6 @@ class OptionsModel : public QAbstractListModel
bool m_enable_psbt_controls;
bool m_mask_values;

//! 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;

Expand Down

0 comments on commit e43ff4e

Please sign in to comment.