Skip to content

Add validation to the Send form (address and amount) #490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: qt6
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions qml/bitcoin_qml.qrc
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
</qresource>
<qresource prefix="/icons">
<file alias="add-wallet-dark">res/icons/add-wallet-dark.png</file>
<file alias="alert-filled">res/icons/alert-filled.png</file>
<file alias="arrow-down">res/icons/arrow-down.png</file>
<file alias="arrow-up">res/icons/arrow-up.png</file>
<file alias="bitcoin-circle">res/icons/bitcoin-circle.png</file>
Expand Down
4 changes: 2 additions & 2 deletions qml/bitcoinamount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ qint64 BitcoinAmount::satoshi() const

void BitcoinAmount::setSatoshi(qint64 new_amount)
{
m_isSet = true;
if (m_satoshi != new_amount) {
if (m_satoshi != new_amount || m_isSet != true) {
m_isSet = true;
m_satoshi = new_amount;
Q_EMIT amountChanged();
}
Expand Down
2 changes: 2 additions & 0 deletions qml/controls/LabeledTextInput.qml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ Item {
property alias iconSource: icon.source
property alias customIcon: iconContainer.data
property alias enabled: input.enabled
property alias validator: input.validator
property alias maximumLength: input.maximumLength

signal iconClicked
signal textEdited
Expand Down
74 changes: 72 additions & 2 deletions qml/models/sendrecipient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@
#include <qml/models/sendrecipient.h>

#include <qml/bitcoinamount.h>
#include <qml/models/walletqmlmodel.h>

SendRecipient::SendRecipient(QObject* parent)
: QObject(parent), m_amount(new BitcoinAmount(this))
#include <key_io.h>

SendRecipient::SendRecipient(WalletQmlModel* wallet, QObject* parent)
: QObject(parent), m_wallet(wallet), m_amount(new BitcoinAmount(this))
{
connect(m_amount, &BitcoinAmount::amountChanged, this, &SendRecipient::validateAmount);
}

QString SendRecipient::address() const
Expand All @@ -21,6 +25,20 @@ void SendRecipient::setAddress(const QString& address)
if (m_address != address) {
m_address = address;
Q_EMIT addressChanged();
validateAddress();
}
}

QString SendRecipient::addressError() const
{
return m_addressError;
}

void SendRecipient::setAddressError(const QString& error)
{
if (m_addressError != error) {
m_addressError = error;
Q_EMIT addressErrorChanged();
}
}

Expand All @@ -42,6 +60,19 @@ BitcoinAmount* SendRecipient::amount() const
return m_amount;
}

QString SendRecipient::amountError() const
{
return m_amountError;
}

void SendRecipient::setAmountError(const QString& error)
{
if (m_amountError != error) {
m_amountError = error;
Q_EMIT amountErrorChanged();
}
}

QString SendRecipient::message() const
{
return m_message;
Expand Down Expand Up @@ -77,3 +108,42 @@ void SendRecipient::clear()
Q_EMIT messageChanged();
Q_EMIT amount()->amountChanged();
}

void SendRecipient::validateAddress()
{
if (!m_address.isEmpty() && !IsValidDestinationString(m_address.toStdString())) {
if (IsValidDestinationString(m_address.toStdString(), *CChainParams::Main())) {
setAddressError(tr("Address is valid for mainnet, not the current network"));
} else if (IsValidDestinationString(m_address.toStdString(), *CChainParams::TestNet())) {
setAddressError(tr("Address is valid for testnet, not the current network"));
} else {
setAddressError(tr("Invalid address format"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
setAddressError(tr("Invalid address format"));
setAddressError(tr("Invalid address"));

maybe better?

}
} else {
setAddressError("");
}

Q_EMIT isValidChanged();
}

void SendRecipient::validateAmount()
{
if (m_amount->isSet()) {
if (m_amount->satoshi() <= 0) {
setAmountError(tr("Amount must be greater than zero"));
} else if (m_amount->satoshi() > MAX_MONEY) {
setAmountError(tr("Amount exceeds maximum limit of 21,000,000 BTC"));
} else if (m_amount->satoshi() > m_wallet->balanceSatoshi()) {
setAmountError(tr("Amount exceeds available balance"));
} else {
setAmountError("");
}
}

Q_EMIT isValidChanged();
}

bool SendRecipient::isValid() const
{
return m_addressError.isEmpty() && m_amountError.isEmpty() && m_amount->satoshi() > 0 && !m_address.isEmpty();
}
23 changes: 22 additions & 1 deletion qml/models/sendrecipient.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include <QObject>
#include <QString>

class WalletQmlModel;

class SendRecipient : public QObject
{
Q_OBJECT
Expand All @@ -18,17 +20,25 @@ class SendRecipient : public QObject
Q_PROPERTY(QString message READ message WRITE setMessage NOTIFY messageChanged)
Q_PROPERTY(BitcoinAmount* amount READ amount CONSTANT)

Q_PROPERTY(QString addressError READ addressError NOTIFY addressErrorChanged)
Q_PROPERTY(QString amountError READ amountError NOTIFY amountErrorChanged)
Q_PROPERTY(bool isValid READ isValid NOTIFY isValidChanged)

public:
explicit SendRecipient(QObject* parent = nullptr);
explicit SendRecipient(WalletQmlModel* wallet, QObject* parent = nullptr);

QString address() const;
void setAddress(const QString& address);
QString addressError() const;
void setAddressError(const QString& error);

QString label() const;
void setLabel(const QString& label);

BitcoinAmount* amount() const;
void setAmount(const QString& amount);
QString amountError() const;
void setAmountError(const QString& error);

QString message() const;
void setMessage(const QString& message);
Expand All @@ -37,18 +47,29 @@ class SendRecipient : public QObject

bool subtractFeeFromAmount() const;

bool isValid() const;

Q_INVOKABLE void clear();

Q_SIGNALS:
void addressChanged();
void addressErrorChanged();
void amountErrorChanged();
void labelChanged();
void messageChanged();
void isValidChanged();

private:
void validateAddress();
void validateAmount();

const WalletQmlModel* m_wallet;
QString m_address{""};
QString m_addressError{""};
QString m_label{""};
QString m_message{""};
BitcoinAmount* m_amount;
QString m_amountError{""};
bool m_subtractFeeFromAmount{false};
};

Expand Down
8 changes: 5 additions & 3 deletions qml/models/sendrecipientslistmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <qml/models/sendrecipientslistmodel.h>
#include <qml/models/walletqmlmodel.h>

#include <qml/models/sendrecipient.h>

SendRecipientsListModel::SendRecipientsListModel(QObject* parent)
: QAbstractListModel(parent)
{
auto* recipient = new SendRecipient(this);
m_wallet = qobject_cast<WalletQmlModel*>(parent);
auto* recipient = new SendRecipient(m_wallet, this);
connect(recipient->amount(), &BitcoinAmount::amountChanged,
this, &SendRecipientsListModel::updateTotalAmount);
m_recipients.append(recipient);
Expand Down Expand Up @@ -50,7 +52,7 @@ void SendRecipientsListModel::add()
{
const int row = m_recipients.size();
beginInsertRows(QModelIndex(), row, row);
auto* recipient = new SendRecipient(this);
auto* recipient = new SendRecipient(m_wallet, this);
connect(recipient->amount(), &BitcoinAmount::amountChanged,
this, &SendRecipientsListModel::updateTotalAmount);
if (m_recipients.size() > 0) {
Expand Down Expand Up @@ -139,7 +141,7 @@ void SendRecipientsListModel::clear()
m_current = 0;
m_totalAmount = 0;

auto* recipient = new SendRecipient(this);
auto* recipient = new SendRecipient(m_wallet, this);
connect(recipient->amount(), &BitcoinAmount::amountChanged,
this, &SendRecipientsListModel::updateTotalAmount);
m_recipients.append(recipient);
Expand Down
8 changes: 4 additions & 4 deletions qml/models/sendrecipientslistmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
#ifndef BITCOIN_QML_MODELS_SENDRECIPIENTSLISTMODEL_H
#define BITCOIN_QML_MODELS_SENDRECIPIENTSLISTMODEL_H

#include <qml/models/sendrecipient.h>

#include <QAbstractListModel>
#include <QList>
#include <qobjectdefs.h>

class SendRecipient;
class WalletQmlModel;

class SendRecipientsListModel : public QAbstractListModel
{
Expand Down Expand Up @@ -58,6 +57,7 @@ class SendRecipientsListModel : public QAbstractListModel
private:
void updateTotalAmount();

WalletQmlModel* m_wallet;
QList<SendRecipient*> m_recipients;
int m_current{0};
qint64 m_totalAmount{0};
Expand Down
8 changes: 8 additions & 0 deletions qml/models/walletqmlmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ QString WalletQmlModel::balance() const
return BitcoinUnits::format(BitcoinUnits::Unit::BTC, m_wallet->getBalance());
}

CAmount WalletQmlModel::balanceSatoshi() const
{
if (!m_wallet) {
return 0;
}
return m_wallet->getBalance();
}

QString WalletQmlModel::name() const
{
if (!m_wallet) {
Expand Down
11 changes: 7 additions & 4 deletions qml/models/walletqmlmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,21 @@
#ifndef BITCOIN_QML_MODELS_WALLETQMLMODEL_H
#define BITCOIN_QML_MODELS_WALLETQMLMODEL_H

#include <interfaces/handler.h>
#include <interfaces/wallet.h>
#include <qml/models/activitylistmodel.h>
#include <qml/models/coinslistmodel.h>
#include <qml/models/sendrecipient.h>
#include <qml/models/sendrecipientslistmodel.h>
#include <qml/models/walletqmlmodeltransaction.h>

#include <consensus/amount.h>
#include <interfaces/handler.h>
#include <interfaces/wallet.h>
#include <wallet/coincontrol.h>

#include <QObject>
#include <memory>
#include <vector>

class ActivityListModel;
#include <QObject>

class WalletQmlModel : public QObject
{
Expand All @@ -39,6 +40,8 @@ class WalletQmlModel : public QObject

QString name() const;
QString balance() const;
CAmount balanceSatoshi() const;

ActivityListModel* activityListModel() const { return m_activity_list_model; }
CoinsListModel* coinsListModel() const { return m_coins_list_model; }
SendRecipientsListModel* sendRecipientList() const { return m_send_recipients; }
Expand Down
3 changes: 3 additions & 0 deletions qml/models/walletqmlmodeltransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

#include <qml/models/walletqmlmodeltransaction.h>

#include <qml/models/sendrecipient.h>
#include <qml/models/sendrecipientslistmodel.h>

#include <policy/policy.h>

WalletQmlModelTransaction::WalletQmlModelTransaction(const SendRecipientsListModel* recipient, QObject* parent)
Expand Down
Loading
Loading