From f07c8c57cb35160066d2d17d3568234108746b3d Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 21 Jan 2021 22:27:28 +0200 Subject: [PATCH 1/5] qt: Use QVariant instead of int for BitcoinUnit in QSettings This change improves type safety. --- src/qt/bitcoin.cpp | 2 ++ src/qt/bitcoinunits.cpp | 37 +++++++++++++++++++++++++++++++++++++ src/qt/bitcoinunits.h | 5 +++++ src/qt/optionsmodel.cpp | 24 +++++++++++++++--------- src/qt/optionsmodel.h | 7 ++++--- 5 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 69948402d08..fb5a95fe2d8 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -88,6 +88,8 @@ static void RegisterMetaTypes() qRegisterMetaType>("std::function"); qRegisterMetaType("QMessageBox::Icon"); qRegisterMetaType("interfaces::BlockAndHeaderTipInfo"); + + qRegisterMetaTypeStreamOperators("BitcoinUnit"); } static QString GetLangTerritory() diff --git a/src/qt/bitcoinunits.cpp b/src/qt/bitcoinunits.cpp index 9660ba99f78..0e95529967c 100644 --- a/src/qt/bitcoinunits.cpp +++ b/src/qt/bitcoinunits.cpp @@ -248,3 +248,40 @@ CAmount BitcoinUnits::maxMoney() { return MAX_MONEY; } + +namespace { +qint8 ToQint8(BitcoinUnit unit) +{ + switch (unit) { + case BitcoinUnits::BTC: return 0; + case BitcoinUnits::mBTC: return 1; + case BitcoinUnits::uBTC: return 2; + case BitcoinUnits::SAT: return 3; + } // no default case, so the compiler can warn about missing cases + assert(false); +} + +BitcoinUnit FromQint8(qint8 num) +{ + switch (num) { + case 0: return BitcoinUnits::BTC; + case 1: return BitcoinUnits::mBTC; + case 2: return BitcoinUnits::uBTC; + case 3: return BitcoinUnits::SAT; + } + assert(false); +} +} // namespace + +QDataStream& operator<<(QDataStream& out, const BitcoinUnit& unit) +{ + return out << ToQint8(unit); +} + +QDataStream& operator>>(QDataStream& in, BitcoinUnit& unit) +{ + qint8 input; + in >> input; + unit = FromQint8(input); + return in; +} diff --git a/src/qt/bitcoinunits.h b/src/qt/bitcoinunits.h index e22ba0a9382..a67d991cd13 100644 --- a/src/qt/bitcoinunits.h +++ b/src/qt/bitcoinunits.h @@ -8,6 +8,7 @@ #include #include +#include #include // U+2009 THIN SPACE = UTF-8 E2 80 89 @@ -45,6 +46,7 @@ class BitcoinUnits: public QAbstractListModel uBTC, SAT }; + Q_ENUM(Unit) enum class SeparatorStyle { @@ -111,4 +113,7 @@ class BitcoinUnits: public QAbstractListModel }; typedef BitcoinUnits::Unit BitcoinUnit; +QDataStream& operator<<(QDataStream& out, const BitcoinUnit& unit); +QDataStream& operator>>(QDataStream& in, BitcoinUnit& unit); + #endif // BITCOIN_QT_BITCOINUNITS_H diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index abdf9e9ae6f..39b82361c1d 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -24,6 +24,7 @@ #include #include #include +#include const char *DEFAULT_GUI_PROXY_HOST = "127.0.0.1"; @@ -71,9 +72,15 @@ void OptionsModel::Init(bool resetSettings) fMinimizeOnClose = settings.value("fMinimizeOnClose").toBool(); // Display - if (!settings.contains("nDisplayUnit")) - settings.setValue("nDisplayUnit", BitcoinUnits::BTC); - nDisplayUnit = settings.value("nDisplayUnit").toInt(); + if (!settings.contains("DisplayBitcoinUnit")) { + settings.setValue("DisplayBitcoinUnit", QVariant::fromValue(BitcoinUnit::BTC)); + } + QVariant unit = settings.value("DisplayBitcoinUnit"); + if (unit.isValid()) { + m_display_bitcoin_unit = unit.value(); + } else { + m_display_bitcoin_unit = BitcoinUnit::BTC; + } if (!settings.contains("strThirdPartyTxUrls")) settings.setValue("strThirdPartyTxUrls", ""); @@ -328,7 +335,7 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const return settings.value("bSpendZeroConfChange"); #endif case DisplayUnit: - return nDisplayUnit; + return QVariant::fromValue(m_display_bitcoin_unit); case ThirdPartyTxUrls: return strThirdPartyTxUrls; case Language: @@ -515,12 +522,11 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in /** Updates current unit in memory, settings and emits displayUnitChanged(newUnit) signal */ void OptionsModel::setDisplayUnit(const QVariant &value) { - if (!value.isNull()) - { + if (!value.isNull()) { QSettings settings; - nDisplayUnit = value.toInt(); - settings.setValue("nDisplayUnit", nDisplayUnit); - Q_EMIT displayUnitChanged(nDisplayUnit); + m_display_bitcoin_unit = value.value(); + settings.setValue("DisplayBitcoinUnit", QVariant::fromValue(m_display_bitcoin_unit)); + Q_EMIT displayUnitChanged(static_cast(m_display_bitcoin_unit)); } } diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index 4d012a9b8f4..242023b6a66 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -6,12 +6,13 @@ #define BITCOIN_QT_OPTIONSMODEL_H #include -#include +#include #include #include #include +#include namespace interfaces { class Node; @@ -83,7 +84,7 @@ class OptionsModel : public QAbstractListModel bool getShowTrayIcon() const { return m_show_tray_icon; } bool getMinimizeToTray() const { return fMinimizeToTray; } bool getMinimizeOnClose() const { return fMinimizeOnClose; } - int getDisplayUnit() const { return nDisplayUnit; } + int getDisplayUnit() const { return static_cast(m_display_bitcoin_unit); } QString getThirdPartyTxUrls() const { return strThirdPartyTxUrls; } bool getUseEmbeddedMonospacedFont() const { return m_use_embedded_monospaced_font; } bool getCoinControlFeatures() const { return fCoinControlFeatures; } @@ -107,7 +108,7 @@ class OptionsModel : public QAbstractListModel bool fMinimizeToTray; bool fMinimizeOnClose; QString language; - int nDisplayUnit; + BitcoinUnit m_display_bitcoin_unit; QString strThirdPartyTxUrls; bool m_use_embedded_monospaced_font; bool fCoinControlFeatures; From a11cfe9690e2534ee0b66dbbe4ae95b197eaa0d7 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 21 Jan 2021 22:28:16 +0200 Subject: [PATCH 2/5] qt, refactor: Make BitcoinUnits::Unit a scoped enum --- src/qt/bitcoinamountfield.cpp | 21 ++++--- src/qt/bitcoinamountfield.h | 3 +- src/qt/bitcoingui.cpp | 13 ++-- src/qt/bitcoingui.h | 5 +- src/qt/bitcoinunits.cpp | 100 +++++++++++++++---------------- src/qt/bitcoinunits.h | 29 +++++---- src/qt/coincontroldialog.cpp | 4 +- src/qt/guiutil.cpp | 5 +- src/qt/optionsmodel.cpp | 2 +- src/qt/optionsmodel.h | 6 +- src/qt/overviewpage.cpp | 10 ++-- src/qt/psbtoperationsdialog.cpp | 4 +- src/qt/sendcoinsdialog.cpp | 3 +- src/qt/test/wallettests.cpp | 15 ++--- src/qt/transactiondesc.cpp | 2 +- src/qt/transactiondesc.h | 4 +- src/qt/transactiontablemodel.cpp | 3 +- src/qt/walletview.h | 3 +- 18 files changed, 119 insertions(+), 113 deletions(-) diff --git a/src/qt/bitcoinamountfield.cpp b/src/qt/bitcoinamountfield.cpp index ba59bcc6609..a65acc4680a 100644 --- a/src/qt/bitcoinamountfield.cpp +++ b/src/qt/bitcoinamountfield.cpp @@ -9,11 +9,14 @@ #include #include -#include #include +#include #include #include #include +#include + +#include /** QSpinBox that uses fixed-point numbers internally and uses our own * formatting/parsing functions. @@ -96,7 +99,7 @@ class AmountSpinBox: public QAbstractSpinBox setValue(val); } - void setDisplayUnit(int unit) + void setDisplayUnit(BitcoinUnit unit) { bool valid = false; CAmount val = value(&valid); @@ -122,7 +125,7 @@ class AmountSpinBox: public QAbstractSpinBox const QFontMetrics fm(fontMetrics()); int h = lineEdit()->minimumSizeHint().height(); - int w = GUIUtil::TextWidth(fm, BitcoinUnits::format(BitcoinUnits::BTC, BitcoinUnits::maxMoney(), false, BitcoinUnits::SeparatorStyle::ALWAYS)); + int w = GUIUtil::TextWidth(fm, BitcoinUnits::format(BitcoinUnit::BTC, BitcoinUnits::maxMoney(), false, BitcoinUnits::SeparatorStyle::ALWAYS)); w += 2; // cursor blinking space QStyleOptionSpinBox opt; @@ -148,7 +151,7 @@ class AmountSpinBox: public QAbstractSpinBox } private: - int currentUnit{BitcoinUnits::BTC}; + BitcoinUnit currentUnit{BitcoinUnit::BTC}; CAmount singleStep{CAmount(100000)}; // satoshis mutable QSize cachedMinimumSizeHint; bool m_allow_empty{true}; @@ -326,14 +329,14 @@ void BitcoinAmountField::unitChanged(int idx) unit->setToolTip(unit->itemData(idx, Qt::ToolTipRole).toString()); // Determine new unit ID - int newUnit = unit->itemData(idx, BitcoinUnits::UnitRole).toInt(); - - amount->setDisplayUnit(newUnit); + QVariant new_unit = unit->currentData(BitcoinUnits::UnitRole); + assert(new_unit.isValid()); + amount->setDisplayUnit(new_unit.value()); } -void BitcoinAmountField::setDisplayUnit(int newUnit) +void BitcoinAmountField::setDisplayUnit(BitcoinUnit new_unit) { - unit->setValue(newUnit); + unit->setValue(QVariant::fromValue(new_unit)); } void BitcoinAmountField::setSingleStep(const CAmount& step) diff --git a/src/qt/bitcoinamountfield.h b/src/qt/bitcoinamountfield.h index c60d9a2c90e..cdbbd2b08be 100644 --- a/src/qt/bitcoinamountfield.h +++ b/src/qt/bitcoinamountfield.h @@ -6,6 +6,7 @@ #define BITCOIN_QT_BITCOINAMOUNTFIELD_H #include +#include #include @@ -52,7 +53,7 @@ class BitcoinAmountField: public QWidget bool validate(); /** Change unit used to display amount. */ - void setDisplayUnit(int unit); + void setDisplayUnit(BitcoinUnit new_unit); /** Make field empty and ready for new input. */ void clear(); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 858010a38cb..bde4426b883 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -1202,7 +1202,7 @@ void BitcoinGUI::showEvent(QShowEvent *event) } #ifdef ENABLE_WALLET -void BitcoinGUI::incomingTransaction(const QString& date, int unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName) +void BitcoinGUI::incomingTransaction(const QString& date, BitcoinUnit unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName) { // On new transaction, make an info balloon QString msg = tr("Date: %1\n").arg(date) + @@ -1457,11 +1457,10 @@ UnitDisplayStatusBarControl::UnitDisplayStatusBarControl(const PlatformStyle *pl { createContextMenu(); setToolTip(tr("Unit to show amounts in. Click to select another unit.")); - QList units = BitcoinUnits::availableUnits(); + QList units = BitcoinUnits::availableUnits(); int max_width = 0; const QFontMetrics fm(font()); - for (const BitcoinUnits::Unit unit : units) - { + for (const BitcoinUnit unit : units) { max_width = qMax(max_width, GUIUtil::TextWidth(fm, BitcoinUnits::longName(unit))); } setMinimumSize(max_width, 0); @@ -1491,8 +1490,8 @@ void UnitDisplayStatusBarControl::changeEvent(QEvent* e) void UnitDisplayStatusBarControl::createContextMenu() { menu = new QMenu(this); - for (const BitcoinUnits::Unit u : BitcoinUnits::availableUnits()) { - menu->addAction(BitcoinUnits::longName(u))->setData(QVariant(u)); + for (const BitcoinUnit u : BitcoinUnits::availableUnits()) { + menu->addAction(BitcoinUnits::longName(u))->setData(QVariant::fromValue(u)); } connect(menu, &QMenu::triggered, this, &UnitDisplayStatusBarControl::onMenuSelection); } @@ -1513,7 +1512,7 @@ void UnitDisplayStatusBarControl::setOptionsModel(OptionsModel *_optionsModel) } /** When Display Units are changed on OptionsModel it will refresh the display text of the control on the status bar */ -void UnitDisplayStatusBarControl::updateDisplayUnit(int newUnits) +void UnitDisplayStatusBarControl::updateDisplayUnit(BitcoinUnit newUnits) { setText(BitcoinUnits::longName(newUnits)); } diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 65c5331d32f..d57cb8dc478 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -9,6 +9,7 @@ #include #endif +#include #include #include @@ -260,7 +261,7 @@ public Q_SLOTS: bool handlePaymentRequest(const SendCoinsRecipient& recipient); /** Show incoming transaction notification for new transactions. */ - void incomingTransaction(const QString& date, int unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName); + void incomingTransaction(const QString& date, BitcoinUnit unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName); #endif // ENABLE_WALLET private: @@ -348,7 +349,7 @@ class UnitDisplayStatusBarControl : public QLabel private Q_SLOTS: /** When Display Units are changed on OptionsModel it will refresh the display text of the control on the status bar */ - void updateDisplayUnit(int newUnits); + void updateDisplayUnit(BitcoinUnit newUnits); /** Tells underlying optionsModel to update its current display unit. */ void onMenuSelection(QAction* action); }; diff --git a/src/qt/bitcoinunits.cpp b/src/qt/bitcoinunits.cpp index 0e95529967c..12ecce5abfe 100644 --- a/src/qt/bitcoinunits.cpp +++ b/src/qt/bitcoinunits.cpp @@ -16,89 +16,89 @@ BitcoinUnits::BitcoinUnits(QObject *parent): { } -QList BitcoinUnits::availableUnits() +QList BitcoinUnits::availableUnits() { - QList unitlist; - unitlist.append(BTC); - unitlist.append(mBTC); - unitlist.append(uBTC); - unitlist.append(SAT); + QList unitlist; + unitlist.append(Unit::BTC); + unitlist.append(Unit::mBTC); + unitlist.append(Unit::uBTC); + unitlist.append(Unit::SAT); return unitlist; } -bool BitcoinUnits::valid(int unit) +bool BitcoinUnits::valid(Unit unit) { switch(unit) { - case BTC: - case mBTC: - case uBTC: - case SAT: + case Unit::BTC: + case Unit::mBTC: + case Unit::uBTC: + case Unit::SAT: return true; default: return false; } } -QString BitcoinUnits::longName(int unit) +QString BitcoinUnits::longName(Unit unit) { switch(unit) { - case BTC: return QString("BTC"); - case mBTC: return QString("mBTC"); - case uBTC: return QString::fromUtf8("µBTC (bits)"); - case SAT: return QString("Satoshi (sat)"); + case Unit::BTC: return QString("BTC"); + case Unit::mBTC: return QString("mBTC"); + case Unit::uBTC: return QString::fromUtf8("µBTC (bits)"); + case Unit::SAT: return QString("Satoshi (sat)"); default: return QString("???"); } } -QString BitcoinUnits::shortName(int unit) +QString BitcoinUnits::shortName(Unit unit) { switch(unit) { - case uBTC: return QString::fromUtf8("bits"); - case SAT: return QString("sat"); + case Unit::uBTC: return QString::fromUtf8("bits"); + case Unit::SAT: return QString("sat"); default: return longName(unit); } } -QString BitcoinUnits::description(int unit) +QString BitcoinUnits::description(Unit unit) { switch(unit) { - case BTC: return QString("Bitcoins"); - case mBTC: return QString("Milli-Bitcoins (1 / 1" THIN_SP_UTF8 "000)"); - case uBTC: return QString("Micro-Bitcoins (bits) (1 / 1" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)"); - case SAT: return QString("Satoshi (sat) (1 / 100" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)"); + case Unit::BTC: return QString("Bitcoins"); + case Unit::mBTC: return QString("Milli-Bitcoins (1 / 1" THIN_SP_UTF8 "000)"); + case Unit::uBTC: return QString("Micro-Bitcoins (bits) (1 / 1" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)"); + case Unit::SAT: return QString("Satoshi (sat) (1 / 100" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)"); default: return QString("???"); } } -qint64 BitcoinUnits::factor(int unit) +qint64 BitcoinUnits::factor(Unit unit) { switch(unit) { - case BTC: return 100000000; - case mBTC: return 100000; - case uBTC: return 100; - case SAT: return 1; - default: return 100000000; + case Unit::BTC: return 100'000'000; + case Unit::mBTC: return 100'000; + case Unit::uBTC: return 100; + case Unit::SAT: return 1; + default: return 100'000'000; } } -int BitcoinUnits::decimals(int unit) +int BitcoinUnits::decimals(Unit unit) { switch(unit) { - case BTC: return 8; - case mBTC: return 5; - case uBTC: return 2; - case SAT: return 0; + case Unit::BTC: return 8; + case Unit::mBTC: return 5; + case Unit::uBTC: return 2; + case Unit::SAT: return 0; default: return 0; } } -QString BitcoinUnits::format(int unit, const CAmount& nIn, bool fPlus, SeparatorStyle separators, bool justify) +QString BitcoinUnits::format(Unit unit, const CAmount& nIn, bool fPlus, SeparatorStyle separators, bool justify) { // Note: not using straight sprintf here because we do NOT want // localized number formatting. @@ -145,19 +145,19 @@ QString BitcoinUnits::format(int unit, const CAmount& nIn, bool fPlus, Separator // Please take care to use formatHtmlWithUnit instead, when // appropriate. -QString BitcoinUnits::formatWithUnit(int unit, const CAmount& amount, bool plussign, SeparatorStyle separators) +QString BitcoinUnits::formatWithUnit(Unit unit, const CAmount& amount, bool plussign, SeparatorStyle separators) { return format(unit, amount, plussign, separators) + QString(" ") + shortName(unit); } -QString BitcoinUnits::formatHtmlWithUnit(int unit, const CAmount& amount, bool plussign, SeparatorStyle separators) +QString BitcoinUnits::formatHtmlWithUnit(Unit unit, const CAmount& amount, bool plussign, SeparatorStyle separators) { QString str(formatWithUnit(unit, amount, plussign, separators)); str.replace(QChar(THIN_SP_CP), QString(THIN_SP_HTML)); return QString("%1").arg(str); } -QString BitcoinUnits::formatWithPrivacy(int unit, const CAmount& amount, SeparatorStyle separators, bool privacy) +QString BitcoinUnits::formatWithPrivacy(Unit unit, const CAmount& amount, SeparatorStyle separators, bool privacy) { assert(amount >= 0); QString value; @@ -169,7 +169,7 @@ QString BitcoinUnits::formatWithPrivacy(int unit, const CAmount& amount, Separat return value + QString(" ") + shortName(unit); } -bool BitcoinUnits::parse(int unit, const QString &value, CAmount *val_out) +bool BitcoinUnits::parse(Unit unit, const QString& value, CAmount* val_out) { if(!valid(unit) || value.isEmpty()) return false; // Refuse to parse invalid unit or empty string @@ -208,7 +208,7 @@ bool BitcoinUnits::parse(int unit, const QString &value, CAmount *val_out) return ok; } -QString BitcoinUnits::getAmountColumnTitle(int unit) +QString BitcoinUnits::getAmountColumnTitle(Unit unit) { QString amountTitle = QObject::tr("Amount"); if (BitcoinUnits::valid(unit)) @@ -238,7 +238,7 @@ QVariant BitcoinUnits::data(const QModelIndex &index, int role) const case Qt::ToolTipRole: return QVariant(description(unit)); case UnitRole: - return QVariant(static_cast(unit)); + return QVariant::fromValue(unit); } } return QVariant(); @@ -253,10 +253,10 @@ namespace { qint8 ToQint8(BitcoinUnit unit) { switch (unit) { - case BitcoinUnits::BTC: return 0; - case BitcoinUnits::mBTC: return 1; - case BitcoinUnits::uBTC: return 2; - case BitcoinUnits::SAT: return 3; + case BitcoinUnit::BTC: return 0; + case BitcoinUnit::mBTC: return 1; + case BitcoinUnit::uBTC: return 2; + case BitcoinUnit::SAT: return 3; } // no default case, so the compiler can warn about missing cases assert(false); } @@ -264,10 +264,10 @@ qint8 ToQint8(BitcoinUnit unit) BitcoinUnit FromQint8(qint8 num) { switch (num) { - case 0: return BitcoinUnits::BTC; - case 1: return BitcoinUnits::mBTC; - case 2: return BitcoinUnits::uBTC; - case 3: return BitcoinUnits::SAT; + case 0: return BitcoinUnit::BTC; + case 1: return BitcoinUnit::mBTC; + case 2: return BitcoinUnit::uBTC; + case 3: return BitcoinUnit::SAT; } assert(false); } diff --git a/src/qt/bitcoinunits.h b/src/qt/bitcoinunits.h index a67d991cd13..0785b74879c 100644 --- a/src/qt/bitcoinunits.h +++ b/src/qt/bitcoinunits.h @@ -39,8 +39,7 @@ class BitcoinUnits: public QAbstractListModel /** Bitcoin units. @note Source: https://en.bitcoin.it/wiki/Units . Please add only sensible ones */ - enum Unit - { + enum class Unit { BTC, mBTC, uBTC, @@ -62,29 +61,29 @@ class BitcoinUnits: public QAbstractListModel //! Get list of units, for drop-down box static QList availableUnits(); //! Is unit ID valid? - static bool valid(int unit); + static bool valid(Unit unit); //! Long name - static QString longName(int unit); + static QString longName(Unit unit); //! Short name - static QString shortName(int unit); + static QString shortName(Unit unit); //! Longer description - static QString description(int unit); + static QString description(Unit unit); //! Number of Satoshis (1e-8) per unit - static qint64 factor(int unit); + static qint64 factor(Unit unit); //! Number of decimals left - static int decimals(int unit); + static int decimals(Unit unit); //! Format as string - static QString format(int unit, const CAmount& amount, bool plussign = false, SeparatorStyle separators = SeparatorStyle::STANDARD, bool justify = false); + static QString format(Unit unit, const CAmount& amount, bool plussign = false, SeparatorStyle separators = SeparatorStyle::STANDARD, bool justify = false); //! Format as string (with unit) - static QString formatWithUnit(int unit, const CAmount& amount, bool plussign=false, SeparatorStyle separators=SeparatorStyle::STANDARD); + static QString formatWithUnit(Unit unit, const CAmount& amount, bool plussign = false, SeparatorStyle separators = SeparatorStyle::STANDARD); //! Format as HTML string (with unit) - static QString formatHtmlWithUnit(int unit, const CAmount& amount, bool plussign=false, SeparatorStyle separators=SeparatorStyle::STANDARD); + static QString formatHtmlWithUnit(Unit unit, const CAmount& amount, bool plussign = false, SeparatorStyle separators = SeparatorStyle::STANDARD); //! Format as string (with unit) of fixed length to preserve privacy, if it is set. - static QString formatWithPrivacy(int unit, const CAmount& amount, SeparatorStyle separators, bool privacy); + static QString formatWithPrivacy(Unit unit, const CAmount& amount, SeparatorStyle separators, bool privacy); //! Parse string to coin amount - static bool parse(int unit, const QString &value, CAmount *val_out); + static bool parse(Unit unit, const QString& value, CAmount* val_out); //! Gets title for amount column including current display unit if optionsModel reference available */ - static QString getAmountColumnTitle(int unit); + static QString getAmountColumnTitle(Unit unit); ///@} //! @name AbstractListModel implementation @@ -109,7 +108,7 @@ class BitcoinUnits: public QAbstractListModel static CAmount maxMoney(); private: - QList unitlist; + QList unitlist; }; typedef BitcoinUnits::Unit BitcoinUnit; diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 8ae06481410..2fbe8d2d98a 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -501,7 +501,7 @@ void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel * } // actually update labels - int nDisplayUnit = BitcoinUnits::BTC; + BitcoinUnit nDisplayUnit = BitcoinUnit::BTC; if (model && model->getOptionsModel()) nDisplayUnit = model->getOptionsModel()->getDisplayUnit(); @@ -584,7 +584,7 @@ void CoinControlDialog::updateView() QFlags flgCheckbox = Qt::ItemIsSelectable | Qt::ItemIsEnabled | Qt::ItemIsUserCheckable; QFlags flgTristate = Qt::ItemIsSelectable | Qt::ItemIsEnabled | Qt::ItemIsUserCheckable | Qt::ItemIsTristate; - int nDisplayUnit = model->getOptionsModel()->getDisplayUnit(); + BitcoinUnit nDisplayUnit = model->getOptionsModel()->getDisplayUnit(); for (const auto& coins : model->wallet().listCoins()) { CCoinControlWidgetItem* itemWalletAddress{nullptr}; diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 5ff40ae924e..da91a4cdb67 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -167,8 +167,7 @@ bool parseBitcoinURI(const QUrl &uri, SendCoinsRecipient *out) { if(!i->second.isEmpty()) { - if(!BitcoinUnits::parse(BitcoinUnits::BTC, i->second, &rv.amount)) - { + if (!BitcoinUnits::parse(BitcoinUnit::BTC, i->second, &rv.amount)) { return false; } } @@ -200,7 +199,7 @@ QString formatBitcoinURI(const SendCoinsRecipient &info) if (info.amount) { - ret += QString("?amount=%1").arg(BitcoinUnits::format(BitcoinUnits::BTC, info.amount, false, BitcoinUnits::SeparatorStyle::NEVER)); + ret += QString("?amount=%1").arg(BitcoinUnits::format(BitcoinUnit::BTC, info.amount, false, BitcoinUnits::SeparatorStyle::NEVER)); paramCount++; } diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 39b82361c1d..7c1e1f41d43 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -526,7 +526,7 @@ void OptionsModel::setDisplayUnit(const QVariant &value) QSettings settings; m_display_bitcoin_unit = value.value(); settings.setValue("DisplayBitcoinUnit", QVariant::fromValue(m_display_bitcoin_unit)); - Q_EMIT displayUnitChanged(static_cast(m_display_bitcoin_unit)); + Q_EMIT displayUnitChanged(m_display_bitcoin_unit); } } diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index 242023b6a66..3be0d6d614a 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -57,7 +57,7 @@ class OptionsModel : public QAbstractListModel ProxyUseTor, // bool ProxyIPTor, // QString ProxyPortTor, // int - DisplayUnit, // BitcoinUnits::Unit + DisplayUnit, // BitcoinUnit ThirdPartyTxUrls, // QString Language, // QString UseEmbeddedMonospacedFont, // bool @@ -84,7 +84,7 @@ class OptionsModel : public QAbstractListModel bool getShowTrayIcon() const { return m_show_tray_icon; } bool getMinimizeToTray() const { return fMinimizeToTray; } bool getMinimizeOnClose() const { return fMinimizeOnClose; } - int getDisplayUnit() const { return static_cast(m_display_bitcoin_unit); } + BitcoinUnit getDisplayUnit() const { return m_display_bitcoin_unit; } QString getThirdPartyTxUrls() const { return strThirdPartyTxUrls; } bool getUseEmbeddedMonospacedFont() const { return m_use_embedded_monospaced_font; } bool getCoinControlFeatures() const { return fCoinControlFeatures; } @@ -121,7 +121,7 @@ class OptionsModel : public QAbstractListModel // Check settings version and upgrade default values if required void checkAndMigrate(); Q_SIGNALS: - void displayUnitChanged(int unit); + void displayUnitChanged(BitcoinUnit unit); void coinControlFeaturesChanged(bool); void showTrayIconChanged(bool); void useEmbeddedMonospacedFontChanged(bool); diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 27783bdf87b..7511c3929a7 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -34,9 +34,9 @@ class TxViewDelegate : public QAbstractItemDelegate { Q_OBJECT public: - explicit TxViewDelegate(const PlatformStyle *_platformStyle, QObject *parent=nullptr): - QAbstractItemDelegate(parent), unit(BitcoinUnits::BTC), - platformStyle(_platformStyle) + explicit TxViewDelegate(const PlatformStyle* _platformStyle, QObject* parent = nullptr) + : QAbstractItemDelegate(parent), unit(BitcoinUnit::BTC), + platformStyle(_platformStyle) { connect(this, &TxViewDelegate::width_changed, this, &TxViewDelegate::sizeHintChanged); } @@ -126,7 +126,7 @@ class TxViewDelegate : public QAbstractItemDelegate return {DECORATION_SIZE + 8 + minimum_text_width, DECORATION_SIZE}; } - int unit; + BitcoinUnit unit; Q_SIGNALS: //! An intermediate signal for emitting from the `paint() const` member function. @@ -203,7 +203,7 @@ OverviewPage::~OverviewPage() void OverviewPage::setBalance(const interfaces::WalletBalances& balances) { - int unit = walletModel->getOptionsModel()->getDisplayUnit(); + BitcoinUnit unit = walletModel->getOptionsModel()->getDisplayUnit(); m_balances = balances; if (walletModel->wallet().isLegacy()) { if (walletModel->wallet().privateKeysDisabled()) { diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp index 99318c3bc0a..29b002862c9 100644 --- a/src/qt/psbtoperationsdialog.cpp +++ b/src/qt/psbtoperationsdialog.cpp @@ -166,7 +166,7 @@ std::string PSBTOperationsDialog::renderTransaction(const PartiallySignedTransac ExtractDestination(out.scriptPubKey, address); totalAmount += out.nValue; tx_description.append(tr(" * Sends %1 to %2") - .arg(BitcoinUnits::formatWithUnit(BitcoinUnits::BTC, out.nValue)) + .arg(BitcoinUnits::formatWithUnit(BitcoinUnit::BTC, out.nValue)) .arg(QString::fromStdString(EncodeDestination(address)))); tx_description.append("
"); } @@ -178,7 +178,7 @@ std::string PSBTOperationsDialog::renderTransaction(const PartiallySignedTransac tx_description.append(tr("Unable to calculate transaction fee or total transaction amount.")); } else { tx_description.append(tr("Pays transaction fee: ")); - tx_description.append(BitcoinUnits::formatWithUnit(BitcoinUnits::BTC, *analysis.fee)); + tx_description.append(BitcoinUnits::formatWithUnit(BitcoinUnit::BTC, *analysis.fee)); // add total amount in all subdivision units tx_description.append("
"); diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 160b43324f9..5aae0a49416 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -356,8 +356,7 @@ bool SendCoinsDialog::PrepareSendText(QString& question_string, QString& informa question_string.append("
"); CAmount totalAmount = m_current_transaction->getTotalTransactionAmount() + txFee; QStringList alternativeUnits; - for (const BitcoinUnits::Unit u : BitcoinUnits::availableUnits()) - { + for (const BitcoinUnit u : BitcoinUnits::availableUnits()) { if(u != model->getOptionsModel()->getDisplayUnit()) alternativeUnits.append(BitcoinUnits::formatHtmlWithUnit(u, totalAmount)); } diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 26f568b16a1..eec7330b9b3 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -7,24 +7,25 @@ #include #include +#include #include +#include #include #include +#include #include #include +#include +#include +#include #include #include #include #include #include -#include #include #include #include -#include -#include -#include -#include #include @@ -174,7 +175,7 @@ void TestGUI(interfaces::Node& node) // Check balance in send dialog QLabel* balanceLabel = sendCoinsDialog.findChild("labelBalance"); QString balanceText = balanceLabel->text(); - int unit = walletModel.getOptionsModel()->getDisplayUnit(); + BitcoinUnit unit = walletModel.getOptionsModel()->getDisplayUnit(); CAmount balance = walletModel.wallet().getBalance(); QString balanceComparison = BitcoinUnits::formatWithUnit(unit, balance, false, BitcoinUnits::SeparatorStyle::ALWAYS); QCOMPARE(balanceText, balanceComparison); @@ -200,7 +201,7 @@ void TestGUI(interfaces::Node& node) overviewPage.setWalletModel(&walletModel); QLabel* balanceLabel = overviewPage.findChild("labelBalance"); QString balanceText = balanceLabel->text().trimmed(); - int unit = walletModel.getOptionsModel()->getDisplayUnit(); + BitcoinUnit unit = walletModel.getOptionsModel()->getDisplayUnit(); CAmount balance = walletModel.wallet().getBalance(); QString balanceComparison = BitcoinUnits::formatWithUnit(unit, balance, false, BitcoinUnits::SeparatorStyle::ALWAYS); QCOMPARE(balanceText, balanceComparison); diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 02d220db20f..7f891916916 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -81,7 +81,7 @@ bool GetPaymentRequestMerchant(const std::string& pr, QString& merchant) return false; } -QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wallet, TransactionRecord *rec, int unit) +QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wallet, TransactionRecord* rec, BitcoinUnit unit) { int numBlocks; interfaces::WalletTxStatus status; diff --git a/src/qt/transactiondesc.h b/src/qt/transactiondesc.h index cf955a433c8..ecd5b95c5e9 100644 --- a/src/qt/transactiondesc.h +++ b/src/qt/transactiondesc.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_QT_TRANSACTIONDESC_H #define BITCOIN_QT_TRANSACTIONDESC_H +#include + #include #include @@ -24,7 +26,7 @@ class TransactionDesc: public QObject Q_OBJECT public: - static QString toHTML(interfaces::Node& node, interfaces::Wallet& wallet, TransactionRecord *rec, int unit); + static QString toHTML(interfaces::Node& node, interfaces::Wallet& wallet, TransactionRecord* rec, BitcoinUnit unit); private: TransactionDesc() {} diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index b68ceaedbba..f93ea04a017 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -232,7 +233,7 @@ class TransactionTablePriv return nullptr; } - QString describe(interfaces::Node& node, interfaces::Wallet& wallet, TransactionRecord *rec, int unit) + QString describe(interfaces::Node& node, interfaces::Wallet& wallet, TransactionRecord* rec, BitcoinUnit unit) { return TransactionDesc::toHTML(node, wallet, rec, unit); } diff --git a/src/qt/walletview.h b/src/qt/walletview.h index 68f8a5e95b5..6fa747ddf53 100644 --- a/src/qt/walletview.h +++ b/src/qt/walletview.h @@ -6,6 +6,7 @@ #define BITCOIN_QT_WALLETVIEW_H #include +#include #include @@ -125,7 +126,7 @@ public Q_SLOTS: /** HD-Enabled status of wallet changed (only possible during startup) */ void hdEnabledStatusChanged(); /** Notify that a new transaction appeared */ - void incomingTransaction(const QString& date, int unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName); + void incomingTransaction(const QString& date, BitcoinUnit unit, const CAmount& amount, const QString& type, const QString& address, const QString& label, const QString& walletName); /** Notify that the out of sync warning icon has been pressed */ void outOfSyncWarningClicked(); }; From 7a82cdc8c9104aeb6bde3cd5f789a94fce28b49a Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 21 Jan 2021 22:30:13 +0200 Subject: [PATCH 3/5] qt, refactor: Remove BitcoinUnits::valid function Since BitcoinUnits::Unit became a scoped enum, BitcoinUnits::valid function is no longer needed. --- src/qt/bitcoinunits.cpp | 26 +++----------------------- src/qt/bitcoinunits.h | 2 -- 2 files changed, 3 insertions(+), 25 deletions(-) diff --git a/src/qt/bitcoinunits.cpp b/src/qt/bitcoinunits.cpp index 12ecce5abfe..3e89fb2e16d 100644 --- a/src/qt/bitcoinunits.cpp +++ b/src/qt/bitcoinunits.cpp @@ -26,20 +26,6 @@ QList BitcoinUnits::availableUnits() return unitlist; } -bool BitcoinUnits::valid(Unit unit) -{ - switch(unit) - { - case Unit::BTC: - case Unit::mBTC: - case Unit::uBTC: - case Unit::SAT: - return true; - default: - return false; - } -} - QString BitcoinUnits::longName(Unit unit) { switch(unit) @@ -102,8 +88,6 @@ QString BitcoinUnits::format(Unit unit, const CAmount& nIn, bool fPlus, Separato { // Note: not using straight sprintf here because we do NOT want // localized number formatting. - if(!valid(unit)) - return QString(); // Refuse to format invalid unit qint64 n = (qint64)nIn; qint64 coin = factor(unit); int num_decimals = decimals(unit); @@ -171,8 +155,9 @@ QString BitcoinUnits::formatWithPrivacy(Unit unit, const CAmount& amount, Separa bool BitcoinUnits::parse(Unit unit, const QString& value, CAmount* val_out) { - if(!valid(unit) || value.isEmpty()) + if (value.isEmpty()) { return false; // Refuse to parse invalid unit or empty string + } int num_decimals = decimals(unit); // Ignore spaces and thin spaces when parsing @@ -210,12 +195,7 @@ bool BitcoinUnits::parse(Unit unit, const QString& value, CAmount* val_out) QString BitcoinUnits::getAmountColumnTitle(Unit unit) { - QString amountTitle = QObject::tr("Amount"); - if (BitcoinUnits::valid(unit)) - { - amountTitle += " ("+BitcoinUnits::shortName(unit) + ")"; - } - return amountTitle; + return QObject::tr("Amount") + " (" + shortName(unit) + ")"; } int BitcoinUnits::rowCount(const QModelIndex &parent) const diff --git a/src/qt/bitcoinunits.h b/src/qt/bitcoinunits.h index 0785b74879c..1d307c5733d 100644 --- a/src/qt/bitcoinunits.h +++ b/src/qt/bitcoinunits.h @@ -60,8 +60,6 @@ class BitcoinUnits: public QAbstractListModel //! Get list of units, for drop-down box static QList availableUnits(); - //! Is unit ID valid? - static bool valid(Unit unit); //! Long name static QString longName(Unit unit); //! Short name From 16e7d3f11847d00e902a90c117bb0fd85619cacb Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 21 Jan 2021 22:30:58 +0200 Subject: [PATCH 4/5] qt, refactor: Remove default cases for scoped enum --- src/qt/bitcoinunits.cpp | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/src/qt/bitcoinunits.cpp b/src/qt/bitcoinunits.cpp index 3e89fb2e16d..9accc39d6d8 100644 --- a/src/qt/bitcoinunits.cpp +++ b/src/qt/bitcoinunits.cpp @@ -28,60 +28,57 @@ QList BitcoinUnits::availableUnits() QString BitcoinUnits::longName(Unit unit) { - switch(unit) - { + switch (unit) { case Unit::BTC: return QString("BTC"); case Unit::mBTC: return QString("mBTC"); case Unit::uBTC: return QString::fromUtf8("µBTC (bits)"); case Unit::SAT: return QString("Satoshi (sat)"); - default: return QString("???"); - } + } // no default case, so the compiler can warn about missing cases + assert(false); } QString BitcoinUnits::shortName(Unit unit) { - switch(unit) - { - case Unit::uBTC: return QString::fromUtf8("bits"); + switch (unit) { + case Unit::BTC: return longName(unit); + case Unit::mBTC: return longName(unit); + case Unit::uBTC: return QString("bits"); case Unit::SAT: return QString("sat"); - default: return longName(unit); - } + } // no default case, so the compiler can warn about missing cases + assert(false); } QString BitcoinUnits::description(Unit unit) { - switch(unit) - { + switch (unit) { case Unit::BTC: return QString("Bitcoins"); case Unit::mBTC: return QString("Milli-Bitcoins (1 / 1" THIN_SP_UTF8 "000)"); case Unit::uBTC: return QString("Micro-Bitcoins (bits) (1 / 1" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)"); case Unit::SAT: return QString("Satoshi (sat) (1 / 100" THIN_SP_UTF8 "000" THIN_SP_UTF8 "000)"); - default: return QString("???"); - } + } // no default case, so the compiler can warn about missing cases + assert(false); } qint64 BitcoinUnits::factor(Unit unit) { - switch(unit) - { + switch (unit) { case Unit::BTC: return 100'000'000; case Unit::mBTC: return 100'000; case Unit::uBTC: return 100; case Unit::SAT: return 1; - default: return 100'000'000; - } + } // no default case, so the compiler can warn about missing cases + assert(false); } int BitcoinUnits::decimals(Unit unit) { - switch(unit) - { + switch (unit) { case Unit::BTC: return 8; case Unit::mBTC: return 5; case Unit::uBTC: return 2; case Unit::SAT: return 0; - default: return 0; - } + } // no default case, so the compiler can warn about missing cases + assert(false); } QString BitcoinUnits::format(Unit unit, const CAmount& nIn, bool fPlus, SeparatorStyle separators, bool justify) From 8eb1a34937990012d661d262a029e33c8cb67776 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 20 Apr 2021 22:15:39 +0300 Subject: [PATCH 5/5] qt: Skip displayUnitChanged signal if unit is not actually changed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Barbosa --- src/qt/optionsmodel.cpp | 14 ++++++-------- src/qt/optionsmodel.h | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 7c1e1f41d43..28090a2e7e5 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -519,15 +519,13 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in return successful; } -/** Updates current unit in memory, settings and emits displayUnitChanged(newUnit) signal */ -void OptionsModel::setDisplayUnit(const QVariant &value) +void OptionsModel::setDisplayUnit(const QVariant& new_unit) { - if (!value.isNull()) { - QSettings settings; - m_display_bitcoin_unit = value.value(); - settings.setValue("DisplayBitcoinUnit", QVariant::fromValue(m_display_bitcoin_unit)); - Q_EMIT displayUnitChanged(m_display_bitcoin_unit); - } + if (new_unit.isNull() || new_unit.value() == m_display_bitcoin_unit) return; + m_display_bitcoin_unit = new_unit.value(); + QSettings settings; + settings.setValue("DisplayBitcoinUnit", QVariant::fromValue(m_display_bitcoin_unit)); + Q_EMIT displayUnitChanged(m_display_bitcoin_unit); } void OptionsModel::setRestartRequired(bool fRequired) diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index 3be0d6d614a..bfef7e0f589 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -77,8 +77,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; - /** Updates current unit in memory, settings and emits displayUnitChanged(newUnit) signal */ - void setDisplayUnit(const QVariant &value); + /** Updates current unit in memory, settings and emits displayUnitChanged(new_unit) signal */ + void setDisplayUnit(const QVariant& new_unit); /* Explicit getters */ bool getShowTrayIcon() const { return m_show_tray_icon; }