Skip to content

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

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 9 commits into
base: main
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 src/Makefile.qt.include
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ QML_RES_FONTS = \

QML_RES_ICONS = \
qml/res/icons/add-wallet-dark.png \
qml/res/icons/alert-filled.png \
qml/res/icons/arrow-down.png \
qml/res/icons/arrow-up.png \
qml/res/icons/bitcoin-circle.png \
Expand Down
1 change: 1 addition & 0 deletions src/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
2 changes: 2 additions & 0 deletions src/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 src/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"));
}
} 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()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

could remove this case for now as it allows removing the circular dependency?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this might be a good idea and I can just do this validation when creating the transaction in the wallet model

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think im going to leave the circular dependency in for now.and take another look when doing validation with multiple recipients

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 src/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 src/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 @@ -137,7 +139,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 src/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 src/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 src/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 src/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