qt: Use fixed-point arithmetic in amount spinbox

Fixes various issues and cleans up code

- Fixes issue #4500: Amount widget +/- has floating point rounding artifacts
- Amount box can now be emptied again, without clearing to 0

Also aligns the amount to the right, as in other places.

src/Makefile.qt.include

@@ -145,6 +145,7 @@ BITCOIN_MM = \
 
 QT_MOC = \
   qt/bitcoin.moc \
+  qt/bitcoinamountfield.moc \
   qt/intro.moc \
   qt/overviewpage.moc \
   qt/rpcconsole.moc

src/qt/bitcoinamountfield.cpp

@@ -9,63 +9,185 @@
 #include "qvaluecombobox.h"
 
 #include <QApplication>
-#include <QDoubleSpinBox>
+#include <QAbstractSpinBox>
 #include <QHBoxLayout>
 #include <QKeyEvent>
-#include <qmath.h> // for qPow()
+#include <QLineEdit>
 
-// QDoubleSpinBox that shows SI-style thin space thousands separators
+/** QSpinBox that uses fixed-point numbers internally and uses our own
-class AmountSpinBox: public QDoubleSpinBox
+ * formatting/parsing functions.
+ */
+class AmountSpinBox: public QAbstractSpinBox
 {
+    Q_OBJECT
 public:
     explicit AmountSpinBox(QWidget *parent):
-        QDoubleSpinBox(parent)
+        QAbstractSpinBox(parent),
+        currentUnit(BitcoinUnits::BTC),
+        singleStep(100000) // satoshis
     {
+        setAlignment(Qt::AlignRight);
+
+        connect(lineEdit(), SIGNAL(textEdited(QString)), this, SIGNAL(valueChanged()));
+    }
+
+    QValidator::State validate(QString &text, int &pos) const
+    {
+        if(text.isEmpty())
+            return QValidator::Intermediate;
+        bool valid = false;
+        parse(text, &valid);
+        /* Make sure we return Intermediate so that fixup() is called on defocus */
+        return valid ? QValidator::Intermediate : QValidator::Invalid;
+    }
+
+    void fixup(QString &input) const
+    {
+        bool valid = false;
+        qint64 val = parse(input, &valid);
+        if(valid)
+        {
+            input = BitcoinUnits::format(currentUnit, val, false, BitcoinUnits::separatorAlways);
+            lineEdit()->setText(input);
+        }
     }
-    QString textFromValue(double value) const
+
+    qint64 value(bool *valid_out=0) const
     {
-        QStringList parts = QDoubleSpinBox::textFromValue(value).split(".");
+        return parse(text(), valid_out);
-        QString quotient_str = parts[0];
+    }
-        QString remainder_str;
+
-        if(parts.size() > 1)
+    void setValue(qint64 value)
-            remainder_str = parts[1];
+    {
-
+        lineEdit()->setText(BitcoinUnits::format(currentUnit, value, false, BitcoinUnits::separatorAlways));
-        // Code duplication between here and BitcoinUnits::format
+        emit valueChanged();
-        // TODO: Figure out how to share this code
+    }
-        QChar thin_sp(THIN_SP_CP);
+
-        int q_size = quotient_str.size();
+    void stepBy(int steps)
-        if (q_size > 4)
+    {
-            for (int i = 3; i < q_size; i += 3)
+        bool valid = false;
-                quotient_str.insert(q_size - i, thin_sp);
+        qint64 val = value(&valid);
-
+        val = val + steps * singleStep;
-        int r_size = remainder_str.size();
+        val = qMin(qMax(val, Q_INT64_C(0)), BitcoinUnits::maxMoney());
-        if (r_size > 4)
+        setValue(val);
-            for (int i = 3, adj = 0; i < r_size; i += 3, adj++)
+    }
-                remainder_str.insert(i + adj, thin_sp);
+
-
+    StepEnabled stepEnabled() const
-        if(remainder_str.isEmpty())
+    {
-            return quotient_str;
+        StepEnabled rv = 0;
+        if(text().isEmpty()) // Allow step-up with empty field
+            return StepUpEnabled;
+        bool valid = false;
+        qint64 val = value(&valid);
+        if(valid)
+        {
+            if(val > 0)
+                rv |= StepDownEnabled;
+            if(val < BitcoinUnits::maxMoney())
+                rv |= StepUpEnabled;
+        }
+        return rv;
+    }
+
+    void setDisplayUnit(int unit)
+    {
+        bool valid = false;
+        qint64 val = value(&valid);
+
+        currentUnit = unit;
+
+        if(valid)
+            setValue(val);
         else
-            return quotient_str + QString(".") + remainder_str;
+            clear();
     }
-    QValidator::State validate (QString &text, int &pos) const
+
+    void setSingleStep(qint64 step)
     {
-        QString s(BitcoinUnits::removeSpaces(text));
+        singleStep = step;
-        return QDoubleSpinBox::validate(s, pos);
+    }
+
+    QSize minimumSizeHint() const
+    {
+        if(cachedMinimumSizeHint.isEmpty())
+        {
+            ensurePolished();
+
+            const QFontMetrics fm(fontMetrics());
+            int h = lineEdit()->minimumSizeHint().height();
+            int w = fm.width(BitcoinUnits::format(BitcoinUnits::BTC, BitcoinUnits::maxMoney(), false, BitcoinUnits::separatorAlways));
+            w += 2; // cursor blinking space
+
+            QStyleOptionSpinBox opt;
+            initStyleOption(&opt);
+            QSize hint(w, h);
+            QSize extra(35, 6);
+            opt.rect.setSize(hint + extra);
+            extra += hint - style()->subControlRect(QStyle::CC_SpinBox, &opt,
+                                                    QStyle::SC_SpinBoxEditField, this).size();
+            // get closer to final result by repeating the calculation
+            opt.rect.setSize(hint + extra);
+            extra += hint - style()->subControlRect(QStyle::CC_SpinBox, &opt,
+                                                    QStyle::SC_SpinBoxEditField, this).size();
+            hint += extra;
+
+            opt.rect = rect();
+
+            cachedMinimumSizeHint = style()->sizeFromContents(QStyle::CT_SpinBox, &opt, hint, this)
+                                    .expandedTo(QApplication::globalStrut());
+        }
+        return cachedMinimumSizeHint;
+    }
+private:
+    int currentUnit;
+    qint64 singleStep;
+    mutable QSize cachedMinimumSizeHint;
+
+    /**
+     * Parse a string into a number of base monetary units and
+     * return validity.
+     * @note Must return 0 if !valid.
+     */
+    qint64 parse(const QString &text, bool *valid_out=0) const
+    {
+        qint64 val = 0;
+        bool valid = BitcoinUnits::parse(currentUnit, text, &val);
+        if(valid)
+        {
+            if(val < 0 || val > BitcoinUnits::maxMoney())
+                valid = false;
+        }
+        if(valid_out)
+            *valid_out = valid;
+        return valid ? val : 0;
     }
-    double valueFromText(const QString& text) const
+
+protected:
+    bool event(QEvent *event)
     {
-        return QDoubleSpinBox::valueFromText(BitcoinUnits::removeSpaces(text));
+        if (event->type() == QEvent::KeyPress || event->type() == QEvent::KeyRelease)
+        {
+            QKeyEvent *keyEvent = static_cast<QKeyEvent *>(event);
+            if (keyEvent->key() == Qt::Key_Comma)
+            {
+                // Translate a comma into a period
+                QKeyEvent periodKeyEvent(event->type(), Qt::Key_Period, keyEvent->modifiers(), ".", keyEvent->isAutoRepeat(), keyEvent->count());
+                return QAbstractSpinBox::event(&periodKeyEvent);
+            }
+        }
+        return QAbstractSpinBox::event(event);
     }
+
+signals:
+    void valueChanged();
 };
 
+#include "bitcoinamountfield.moc"
+
 BitcoinAmountField::BitcoinAmountField(QWidget *parent) :
     QWidget(parent),
-    amount(0),
+    amount(0)
-    currentUnit(-1)
 {
-    nSingleStep = 100000; // satoshis
-
     amount = new AmountSpinBox(this);
     amount->setLocale(QLocale::c());
     amount->installEventFilter(this);
@@ -85,21 +207,13 @@ BitcoinAmountField::BitcoinAmountField(QWidget *parent) :
     setFocusProxy(amount);
 
     // If one if the widgets changes, the combined content changes as well
-    connect(amount, SIGNAL(valueChanged(QString)), this, SIGNAL(textChanged()));
+    connect(amount, SIGNAL(valueChanged()), this, SIGNAL(valueChanged()));
     connect(unit, SIGNAL(currentIndexChanged(int)), this, SLOT(unitChanged(int)));
 
     // Set default based on configuration
     unitChanged(unit->currentIndex());
 }
 
-void BitcoinAmountField::setText(const QString &text)
-{
-    if (text.isEmpty())
-        amount->clear();
-    else
-        amount->setValue(BitcoinUnits::removeSpaces(text).toDouble());
-}
-
 void BitcoinAmountField::clear()
 {
     amount->clear();
@@ -108,16 +222,9 @@ void BitcoinAmountField::clear()
 
 bool BitcoinAmountField::validate()
 {
-    bool valid = true;
+    bool valid = false;
-    if (amount->value() == 0.0)
+    value(&valid);
-        valid = false;
-    else if (!BitcoinUnits::parse(currentUnit, text(), 0))
-        valid = false;
-    else if (amount->value() > BitcoinUnits::maxAmount(currentUnit))
-        valid = false;
-
     setValid(valid);
-
     return valid;
 }
 
@@ -129,14 +236,6 @@ void BitcoinAmountField::setValid(bool valid)
         amount->setStyleSheet(STYLE_INVALID);
 }
 
-QString BitcoinAmountField::text() const
-{
-    if (amount->text().isEmpty())
-        return QString();
-    else
-        return amount->text();
-}
-
 bool BitcoinAmountField::eventFilter(QObject *object, QEvent *event)
 {
     if (event->type() == QEvent::FocusIn)
@@ -144,17 +243,6 @@ bool BitcoinAmountField::eventFilter(QObject *object, QEvent *event)
         // Clear invalid flag on focus
         setValid(true);
     }
-    else if (event->type() == QEvent::KeyPress || event->type() == QEvent::KeyRelease)
-    {
-        QKeyEvent *keyEvent = static_cast<QKeyEvent *>(event);
-        if (keyEvent->key() == Qt::Key_Comma)
-        {
-            // Translate a comma into a period
-            QKeyEvent periodKeyEvent(event->type(), Qt::Key_Period, keyEvent->modifiers(), ".", keyEvent->isAutoRepeat(), keyEvent->count());
-            QApplication::sendEvent(object, &periodKeyEvent);
-            return true;
-        }
-    }
     return QWidget::eventFilter(object, event);
 }
 
@@ -167,18 +255,12 @@ QWidget *BitcoinAmountField::setupTabChain(QWidget *prev)
 
 qint64 BitcoinAmountField::value(bool *valid_out) const
 {
-    qint64 val_out = 0;
+    return amount->value(valid_out);
-    bool valid = BitcoinUnits::parse(currentUnit, text(), &val_out);
-    if (valid_out)
-    {
-        *valid_out = valid;
-    }
-    return val_out;
 }
 
 void BitcoinAmountField::setValue(qint64 value)
 {
-    setText(BitcoinUnits::format(currentUnit, value));
+    amount->setValue(value);
 }
 
 void BitcoinAmountField::setReadOnly(bool fReadOnly)
@@ -195,28 +277,7 @@ void BitcoinAmountField::unitChanged(int idx)
     // Determine new unit ID
     int newUnit = unit->itemData(idx, BitcoinUnits::UnitRole).toInt();
 
-    // Parse current value and convert to new unit
+    amount->setDisplayUnit(newUnit);
-    bool valid = false;
-    qint64 currentValue = value(&valid);
-
-    currentUnit = newUnit;
-
-    // Set max length after retrieving the value, to prevent truncation
-    amount->setDecimals(BitcoinUnits::decimals(currentUnit));
-    amount->setMaximum(qPow(10, BitcoinUnits::amountDigits(currentUnit)) - qPow(10, -amount->decimals()));
-    amount->setSingleStep((double)nSingleStep / (double)BitcoinUnits::factor(currentUnit));
-
-    if (valid)
-    {
-        // If value was valid, re-place it in the widget with the new unit
-        setValue(currentValue);
-    }
-    else
-    {
-        // If current value is invalid, just clear field
-        setText("");
-    }
-    setValid(true);
 }
 
 void BitcoinAmountField::setDisplayUnit(int newUnit)
@@ -226,6 +287,5 @@ void BitcoinAmountField::setDisplayUnit(int newUnit)
 
 void BitcoinAmountField::setSingleStep(qint64 step)
 {
-    nSingleStep = step;
+    amount->setSingleStep(step);
-    unitChanged(unit->currentIndex());
 }

src/qt/bitcoinamountfield.h

@@ -8,17 +8,18 @@
 #include <QWidget>
 
 QT_BEGIN_NAMESPACE
-class QDoubleSpinBox;
 class QValueComboBox;
 QT_END_NAMESPACE
 
+class AmountSpinBox;
+
 /** Widget for entering bitcoin amounts.
   */
 class BitcoinAmountField: public QWidget
 {
     Q_OBJECT
 
-    Q_PROPERTY(qint64 value READ value WRITE setValue NOTIFY textChanged USER true)
+    Q_PROPERTY(qint64 value READ value WRITE setValue NOTIFY valueChanged USER true)
 
 public:
     explicit BitcoinAmountField(QWidget *parent = 0);
@@ -49,20 +50,15 @@ public:
     QWidget *setupTabChain(QWidget *prev);
 
 signals:
-    void textChanged();
+    void valueChanged();
 
 protected:
     /** Intercept focus-in event and ',' key presses */
     bool eventFilter(QObject *object, QEvent *event);
 
 private:
-    QDoubleSpinBox *amount;
+    AmountSpinBox *amount;
     QValueComboBox *unit;
-    int currentUnit;
-    qint64 nSingleStep;
-
-    void setText(const QString &text);
-    QString text() const;
 
 private slots:
     void unitChanged(int idx);

src/qt/bitcoinunits.cpp

@@ -4,6 +4,8 @@
 
 #include "bitcoinunits.h"
 
+#include "core.h"
+
 #include <QStringList>
 
 BitcoinUnits::BitcoinUnits(QObject *parent):
@@ -250,3 +252,8 @@ QVariant BitcoinUnits::data(const QModelIndex &index, int role) const
     }
     return QVariant();
 }
+
+qint64 BitcoinUnits::maxMoney()
+{
+    return MAX_MONEY;
+}

src/qt/bitcoinunits.h

@@ -120,6 +120,9 @@ public:
         return text;
     }
 
+    //! Return maximum number of base units (Satoshis)
+    static qint64 maxMoney();
+
 private:
     QList<BitcoinUnits::Unit> unitlist;
 };

src/qt/sendcoinsentry.cpp

@@ -72,7 +72,7 @@ void SendCoinsEntry::setModel(WalletModel *model)
     if (model && model->getOptionsModel())
         connect(model->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit()));
 
-    connect(ui->payAmount, SIGNAL(textChanged()), this, SIGNAL(payAmountChanged()));
+    connect(ui->payAmount, SIGNAL(valueChanged()), this, SIGNAL(payAmountChanged()));
     connect(ui->deleteButton, SIGNAL(clicked()), this, SLOT(deleteClicked()));
     connect(ui->deleteButton_is, SIGNAL(clicked()), this, SLOT(deleteClicked()));
     connect(ui->deleteButton_s, SIGNAL(clicked()), this, SLOT(deleteClicked()));
@@ -130,6 +130,13 @@ bool SendCoinsEntry::validate()
         retval = false;
     }
 
+    // Sending a zero amount is invalid
+    if (ui->payAmount->value(0) <= 0)
+    {
+        ui->payAmount->setValid(false);
+        retval = false;
+    }
+
     // Reject dust outputs:
     if (retval && GUIUtil::isDust(ui->payTo->text(), ui->payAmount->value())) {
         ui->payAmount->setValid(false);