Page MenuHomePhabricator

[gui] stop migrating QSettings from BitcoinABC 0.14.6 or Bitcoin Core
ClosedPublic

Authored by PiRK on Mar 12 2024, 08:53.

Details

Summary

This is a partial revert of D513. It is highly unlikely in 2024 that anyone will directly upgrade from v0.14.6 to 0.28.12 without any intermediate version. And I don't see a good reason to use Core's settings for a user who first installs Bitcoin ABC.

The affected settings are mostly unimportant GUI state (window geometry, fonts...), and a few parameters that we probably don't want to copy anyway (data dir location...).

("DisplayBitcoinUnit", "MainWindowGeometry", "PeersTabBanlistHeaderState", "PeersTabPeerHeaderState", "RPCConsoleWindowGeometry", "RPCConsoleWindowPeersTabSplitterSizes", "RecentRequestsViewHeaderState", "SubFeeFromAmount", "TransactionViewHeaderState", "UseEmbeddedMonospacedFont", "enable_psbt_controls", "fCoinControlFeatures", "fFeeSectionMinimized", "fHideTrayIcon", "fMinimizeOnClose", "fMinimizeToTray", "fReset", "fRestartRequired", "mask_values", "nConfTarget", "nFeeRadio", "nSettingsVersion", "nSmartFeeSliderPosition", "nTransactionFee", "strDataDir", "strThirdPartyTxUrls")

See https://github.com/Bitcoin-ABC/bitcoin-abc/issues/547 for more context and additional issues solved by this diff.

Test Plan

build and test bitcoin-qt locally:

cmake .. -GNinja
ninja
src/qt/bitcoin-qt

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Mar 12 2024, 08:53
src/qt/bitcoin.cpp
591–596 ↗(On Diff #46115)

I can't really figure out from the doc if this is a persistent change saved to the settings on disk, or if it only affects this particular QSettings instance in the local scope (in that case it only served a purpose during the key migration and it would be pointless to keep it)

src/qt/bitcoin.cpp
591–596 ↗(On Diff #46115)

It doesn't hurt in any case, but it might be a no-op here. I'm trying to find answers on Qt forums.

PiRK planned changes to this revision.Mar 12 2024, 10:10
PiRK added inline comments.
src/qt/bitcoin.cpp
591–596 ↗(On Diff #46115)

It is not persistent, so let's remove it.

$ python
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from PyQt5.QtCore import QSettings
>>> from PyQt5.QtWidgets import QApplication
>>> QApplication.setOrganizationName("Lemonade Inc.")
>>> QApplication.setOrganizationDomain("freshlemonade.com")
>>> QApplication.setApplicationName("glass")
>>>
>>> s1 = QSettings()
>>> s1.applicationName()
'glass'
>>> s1.fallbacksEnabled()
True
>>> s1.setFallbacksEnabled(False)
>>> s1.fallbacksEnabled()
False
>>>
>>> s2 = QSettings()
>>> s2.applicationName()
'glass'
>>> s2.fallbacksEnabled()
True
>>>
PiRK edited the summary of this revision. (Show Details)
PiRK edited the test plan for this revision. (Show Details)

remove no-op settings.setFallbacksEnabled(false);
This only affects the settings var in the local scope, which is unused.

This revision is now accepted and ready to land.Mar 12 2024, 10:28