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

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

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

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

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

Branch

migratesettings

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 27811
Build 55181: Build Diff	build-diff · lint-circular-dependencies · build-debug · build-without-wallet · build-clang · build-clang-tidy
Build 55180: arc lint + arc unit

Event Timeline

PiRK created this revision.Mar 12 2024, 08:53

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 12 2024, 08:53

PiRK requested review of this revision.Mar 12 2024, 08:53

PiRK edited the summary of this revision. (Show Details)Mar 12 2024, 08:55

PiRK added inline comments.Mar 12 2024, 09:01

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)

PiRK added inline comments.Mar 12 2024, 09:08

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.

Harbormaster completed remote builds in B27810: Diff 46115.Mar 12 2024, 09:17

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
>>>