Bitcoin-ABC / bitcoin-abc

Bitcoin ABC develops node software and infrastructure for the eCash project. This a mirror of the official Bitcoin-ABC repository. Please see README.md
https://reviews.bitcoinabc.org
MIT License
1.24k stars 779 forks source link

QVariant::load: unknown user type with name BitcoinUnits::Unit. #547

Closed younicoin closed 8 months ago

younicoin commented 8 months ago

Hello, I built Bitcoin-ABC on Devuan Linux, g++ version 12.2, qt version 5.15.8. I see this message in terminal on every start:

./src/qt/bitcoin-qt
  QVariant::load: unknown user type with name BitcoinUnits::Unit.

Actually, wallet works, but I think it would be great to make it completely fine. And I started to dig. I found the reason here https://doc.qt.io/qt-5/qvariant.html

Warning: To make this function work with a custom type registered with qRegisterMetaType(), its comparison operator must be registered using QMetaType::registerComparators().

and here https://doc.qt.io/qt-6/qvariant.html

Note: If the stream contains types that aren't the built-in ones (see QMetaType::Type), those types must be registered using qRegisterMetaType() or QMetaType::registerType() before the variant can be properly loaded. If an unregistered type is found, QVariant will set the corrupt flag in the stream, stop processing and print a warning. For example, for QList it would print the following:

QVariant::load: unknown user type with name QList

I tried to solve this editing src/qt/bitcoingui.cpp, src/qt/bitcoinunits.cpp, but my debug messages are shown after that error, so, the error occurs before includes of src/qt/bitcoingui.cpp, src/qt/bitcoinunits.cpp, and I think that error message appears in early start of thread here src/qt/bitcoin.cpp. I saw the comment here src/qt/bitcoin.cpp :

// Note on how Qt works: it tries to directly invoke methods if the signal
    // is emitted on the same thread that the target object 'lives' on.
    // But if the target object 'lives' on another thread (executor here does)
    // the SLOT will be invoked asynchronously at a later time in the thread
    // of the target object.  So.. we pass a pointer around.  If you pass
    // a reference around (even if it's non-const) you'll get Qt generating
    // code to copy-construct the parameter in question (Q_DECLARE_METATYPE
    // and qRegisterMetaType generate this code).  For the Config class,
    // which is noncopyable, we can't do this.  So.. we have to pass
    // pointers to Config around.  Make sure Config &/Config * isn't a
    // temporary (eg it lives somewhere aside from the stack) or this will
    // crash because initialize() gets executed in another thread at some
    // unspecified time (after) requestedInitialize() is emitted!

I think, developers had some headache with QVariant, trying to register new type before it actually declared? Is that type BitcoinUnits::Unit should be registered before starting coreThread->start()? Finally, they could not hack it, and error message still appears. Could you note that issue again and continue to solve that?

younicoin commented 8 months ago

I noted that error occurs after invoking MigrateSettings() and exactly in line 553 of src/qt/bitcoin.cpp const QStringList legacyKeys(legacy.allKeys());

https://github.com/Bitcoin-ABC/bitcoin-abc/blob/7d99a4ee502feed3adc8c9ca84ac3a18c2822f0e/src/qt/bitcoin.cpp#L553

So, using QStringList containing elemenrs of class BitcoinUnits::Unit is wrong. This is wrong, because we run BitcoinThread in a strange way : with pointer to another Qt thread. This us why needs to check this process to be sure BitcoinUnits::Unit registered as MetaType, as QVariant requires.

Fabcien commented 8 months ago

Thanks for the report, we will look into this

younicoin commented 8 months ago

Adding one line to src/qt/bitcoin.cpp

   qRegisterMetaType<Config *>();
+   qRegisterMetaType<BitcoinUnits::Unit>("BitcoinUnits::Unit");

I compiled and got an error ./src/qt/bitcoin-qt QVariant::load: unable to load type 1038.

this error might happen here src/qt/bitcoin.cpp const QStringList legacyKeys(legacy.allKeys()); https://github.com/Bitcoin-ABC/bitcoin-abc/blob/0886c422af1e55749eb923f85e816629344487c7/src/qt/bitcoin.cpp#L644

Some people say in internet, that need to use QVariantList instead of QList, but making so I got error:

qt/bitcoinunits.h:52:12: error: ‘QVariantList’ {aka ‘QList’} is not a template 52 | static QVariantList availableUnits();

younicoin commented 8 months ago

replacing line https://github.com/Bitcoin-ABC/bitcoin-abc/blob/0886c422af1e55749eb923f85e816629344487c7/src/qt/bitcoin.cpp#L644 with this: const QStringList legacyKeys; so I just created empty variable. This solved issue. But I dont know what that variable use for and what actually it should contain. But the call off legacy.allKeys() works bad and need to dig there

younicoin commented 8 months ago

I added some debug lines and watched that variable legacyKeys:

vim src/qt/bitcoin.cpp 72:#include 575:const QStringList legacyKeys(legacy.allKeys()); //const QStringList legacyKeys; qDebug() << legacyKeys; ninja ./src/qt/bitcoin-qt QVariant::load: unknown user type with name BitcoinUnits::Unit. ("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")

As I can consume, this is used on ios and macos and may be some other operating systems for translation and may be some other functions, but the main is translation, and this is why we list all words for you can translate it if you want. This is useful, but gives error now. So, it is fine to stop call legacy.allKeys() that causes the error QVariant::load: unknown user type with name BitcoinUnits::Unit. or if i become Pro today i will somehow register class BitcoinUnits::Unit in bitcoin.cpp

younicoin commented 8 months ago

I just brainstormed this part and concreted this

  1. We run app with name Bitcoin-ABC
  2. We might used this app with other name "Bitcoin" (or "Bitcoin-Qt" if you are on macos or ios). And may be settings are still there
  3. We try to migrate settings from old app name "Bitcoin" to new app name "Bitcoin-ABC"
  4. But if you have no old app name "Bitcoin" registered in your system you got the error Solution: before copying settings need to check if app name "Bitcoin" actually exists. So, need to debug function MigrateSettings. https://github.com/Bitcoin-ABC/bitcoin-abc/blob/7d99a4ee502feed3adc8c9ca84ac3a18c2822f0e/src/qt/bitcoin.cpp#L538 This might work only on devices that had installed your old app name "Bitcoin" and now are switched to new app name "Bitcoin-ABC". But, on devices that had installed Bitcoin-ABC first time and have no "Bitcoin" this does not work. Check presence of "Bictoin" app first (or "Bitcoin-Qt").
PiRK commented 8 months ago

It is likely that the MigrateSettings function is no longer necessary. It was introduced for users upgrading from v0.14.6 to a newer version, and this is now very unlikely to happen as an upgrade has been mandatory every 6 months for more than 6 years. We are currently at 0.28.11 i'll have a look at this tomorrow, and remove that entire codepath if possible.

Thanks for the investigation.

PiRK commented 8 months ago

should be fixed by D15682

younicoin commented 8 months ago

hello. yes, built and run without warnings.

mkdir bitcoin-abc-2024-03-12
cd bitcoin-abc-2024-03-12/
git clone https://github.com/Bitcoin-ABC/bitcoin-abc.git
cd bitcoin-abc
mkdir build && cd build
cmake -GNinja ..
ninja
  [130/543] Generating bitcoin_tr.qm
  Removed plural forms as the target language has less forms.
  If this sounds wrong, possibly the target language is not set or recognized.
  [212/543] Building native src/secp256k1/gen_context
  [2/2] Linking C executable src/secp256k1/gen_context
  [543/543] Linking CXX executable src/qt/bitcoin-qt
./src/qt/bitcoin-qt
  ##no warnings