Skip to content

Commit

Permalink
Merge bitcoin-core/gui#120: Fix multiwallet transaction notifications
Browse files Browse the repository at this point in the history
2414342 refactor: qt: Use vQueueNotifications.clear() (João Barbosa)
989e579 qt: Make transaction notification queue wallet specific (João Barbosa)
7b3b230 move-only: Define TransactionNotification before  TransactionTablePriv (João Barbosa)

Pull request description:

  Currently `vQueueNotifications` holds transactions of any wallet, but the queue is dispatched on a given wallet and it assumes notifications are of that wallet.

  This means that some transactions can be missed if multiple wallets are loaded.

  Fix this by having a queue for each wallet.

ACKs for top commit:
  jonasschnelli:
    utACK 2414342
  hebasto:
    ACK 2414342, I have reviewed the code and it looks OK, I agree it can be merged.
  ryanofsky:
    Code review ACK 2414342. Only change is dropping one commit

Tree-SHA512: 61beac5a16ed659e3a25ad145dbceafcef963aaf8f9838355298949ec2324e2bd760f59353cd251d30cf0334d8dc1642a1f3821d8a9eec092533b581f6ce86db
  • Loading branch information
jonasschnelli committed Nov 12, 2020
2 parents 8a48615 + 2414342 commit 9bd1316
Showing 1 changed file with 39 additions and 36 deletions.
75 changes: 39 additions & 36 deletions src/qt/transactiontablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,30 @@ struct TxLessThan
}
};

// queue notifications to show a non freezing progress dialog e.g. for rescan
struct TransactionNotification
{
public:
TransactionNotification() {}
TransactionNotification(uint256 _hash, ChangeType _status, bool _showTransaction):
hash(_hash), status(_status), showTransaction(_showTransaction) {}

void invoke(QObject *ttm)
{
QString strHash = QString::fromStdString(hash.GetHex());
qDebug() << "NotifyTransactionChanged: " + strHash + " status= " + QString::number(status);
bool invoked = QMetaObject::invokeMethod(ttm, "updateTransaction", Qt::QueuedConnection,
Q_ARG(QString, strHash),
Q_ARG(int, status),
Q_ARG(bool, showTransaction));
assert(invoked);
}
private:
uint256 hash;
ChangeType status;
bool showTransaction;
};

// Private implementation
class TransactionTablePriv
{
Expand All @@ -71,6 +95,12 @@ class TransactionTablePriv
*/
QList<TransactionRecord> cachedWallet;

bool fQueueNotifications = false;
std::vector< TransactionNotification > vQueueNotifications;

void NotifyTransactionChanged(const uint256 &hash, ChangeType status);
void ShowProgress(const std::string &title, int nProgress);

/* Query entire wallet anew from core.
*/
void refreshWallet(interfaces::Wallet& wallet)
Expand Down Expand Up @@ -674,34 +704,7 @@ void TransactionTableModel::updateDisplayUnit()
Q_EMIT dataChanged(index(0, Amount), index(priv->size()-1, Amount));
}

// queue notifications to show a non freezing progress dialog e.g. for rescan
struct TransactionNotification
{
public:
TransactionNotification() {}
TransactionNotification(uint256 _hash, ChangeType _status, bool _showTransaction):
hash(_hash), status(_status), showTransaction(_showTransaction) {}

void invoke(QObject *ttm)
{
QString strHash = QString::fromStdString(hash.GetHex());
qDebug() << "NotifyTransactionChanged: " + strHash + " status= " + QString::number(status);
bool invoked = QMetaObject::invokeMethod(ttm, "updateTransaction", Qt::QueuedConnection,
Q_ARG(QString, strHash),
Q_ARG(int, status),
Q_ARG(bool, showTransaction));
assert(invoked);
}
private:
uint256 hash;
ChangeType status;
bool showTransaction;
};

static bool fQueueNotifications = false;
static std::vector< TransactionNotification > vQueueNotifications;

static void NotifyTransactionChanged(TransactionTableModel *ttm, const uint256 &hash, ChangeType status)
void TransactionTablePriv::NotifyTransactionChanged(const uint256 &hash, ChangeType status)
{
// Find transaction in wallet
// Determine whether to show transaction or not (determine this here so that no relocking is needed in GUI thread)
Expand All @@ -714,10 +717,10 @@ static void NotifyTransactionChanged(TransactionTableModel *ttm, const uint256 &
vQueueNotifications.push_back(notification);
return;
}
notification.invoke(ttm);
notification.invoke(parent);
}

static void ShowProgress(TransactionTableModel *ttm, const std::string &title, int nProgress)
void TransactionTablePriv::ShowProgress(const std::string &title, int nProgress)
{
if (nProgress == 0)
fQueueNotifications = true;
Expand All @@ -726,27 +729,27 @@ static void ShowProgress(TransactionTableModel *ttm, const std::string &title, i
{
fQueueNotifications = false;
if (vQueueNotifications.size() > 10) { // prevent balloon spam, show maximum 10 balloons
bool invoked = QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true));
bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true));
assert(invoked);
}
for (unsigned int i = 0; i < vQueueNotifications.size(); ++i)
{
if (vQueueNotifications.size() - i <= 10) {
bool invoked = QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false));
bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false));
assert(invoked);
}

vQueueNotifications[i].invoke(ttm);
vQueueNotifications[i].invoke(parent);
}
std::vector<TransactionNotification >().swap(vQueueNotifications); // clear
vQueueNotifications.clear();
}
}

void TransactionTableModel::subscribeToCoreSignals()
{
// Connect signals to wallet
m_handler_transaction_changed = walletModel->wallet().handleTransactionChanged(std::bind(NotifyTransactionChanged, this, std::placeholders::_1, std::placeholders::_2));
m_handler_show_progress = walletModel->wallet().handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2));
m_handler_transaction_changed = walletModel->wallet().handleTransactionChanged(std::bind(&TransactionTablePriv::NotifyTransactionChanged, priv, std::placeholders::_1, std::placeholders::_2));
m_handler_show_progress = walletModel->wallet().handleShowProgress(std::bind(&TransactionTablePriv::ShowProgress, priv, std::placeholders::_1, std::placeholders::_2));
}

void TransactionTableModel::unsubscribeFromCoreSignals()
Expand Down

0 comments on commit 9bd1316

Please sign in to comment.