From 217a413bf881e75543bdb5c38e797f71630d8d37 Mon Sep 17 00:00:00 2001 From: tobtoht Date: Thu, 2 Mar 2023 13:41:33 +0100 Subject: [PATCH] libwalletqt: cleanup tx commit --- src/MainWindow.cpp | 31 +++++------ src/MainWindow.h | 1 + src/libwalletqt/PendingTransaction.cpp | 3 +- src/libwalletqt/Wallet.cpp | 74 +++++++++++++++----------- src/libwalletqt/Wallet.h | 9 ++-- 5 files changed, 66 insertions(+), 52 deletions(-) diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index 457fb53..8ab3521 100644 --- a/src/MainWindow.cpp +++ b/src/MainWindow.cpp @@ -448,19 +448,7 @@ void MainWindow::initWalletContext() { config()->set(Config::donateBeg, -1); }); - connect(m_wallet, &Wallet::multiBroadcast, this, [this](PendingTransaction *tx){ - quint64 count = tx->txCount(); - for (quint64 i = 0; i < count; i++) { - QString txData = tx->signedTxToHex(i); - - for (const auto& node: m_nodes->nodes()) { - QString address = node.toURL(); - qDebug() << QString("Relaying %1 to: %2").arg(tx->txid()[i], address); - m_rpc->setDaemonAddress(address); - m_rpc->sendRawTransaction(txData); - } - } - }); + connect(m_wallet, &Wallet::multiBroadcast, this, &MainWindow::onMultiBroadcast); } void MainWindow::menuToggleTabVisible(const QString &key){ @@ -661,6 +649,19 @@ void MainWindow::onOfflineMode(bool offline) { this->onConnectionStatusChanged(Wallet::ConnectionStatus_Disconnected); } +void MainWindow::onMultiBroadcast(const QMap &txHexMap) { + QMapIterator i(txHexMap); + while (i.hasNext()) { + i.next(); + for (const auto& node: m_nodes->nodes()) { + QString address = node.toURL(); + qDebug() << QString("Relaying %1 to: %2").arg(i.key(), address); + m_rpc->setDaemonAddress(address); + m_rpc->sendRawTransaction(i.value()); + } + } +} + void MainWindow::onSynchronized() { this->updateNetStats(); this->setStatusText("Synchronized"); @@ -808,8 +809,8 @@ void MainWindow::onCreateTransactionSuccess(PendingTransaction *tx, const QVecto } } -void MainWindow::onTransactionCommitted(bool status, PendingTransaction *tx, const QStringList& txid) { - if (status) { // success +void MainWindow::onTransactionCommitted(bool success, PendingTransaction *tx, const QStringList& txid) { + if (success) { QMessageBox msgBox{this}; QPushButton *showDetailsButton = msgBox.addButton("Show details", QMessageBox::ActionRole); msgBox.addButton(QMessageBox::Ok); diff --git a/src/MainWindow.h b/src/MainWindow.h index be84f59..99a261c 100644 --- a/src/MainWindow.h +++ b/src/MainWindow.h @@ -193,6 +193,7 @@ private slots: void showUpdateNotification(); void onProxySettingsChanged(); void onOfflineMode(bool offline); + void onMultiBroadcast(const QMap &txHexMap); private: friend WindowManager; diff --git a/src/libwalletqt/PendingTransaction.cpp b/src/libwalletqt/PendingTransaction.cpp index 0e07144..c4d4cd2 100644 --- a/src/libwalletqt/PendingTransaction.cpp +++ b/src/libwalletqt/PendingTransaction.cpp @@ -95,7 +95,8 @@ void PendingTransaction::refresh() } PendingTransaction::PendingTransaction(Monero::PendingTransaction *pt, QObject *parent) - : QObject(parent), m_pimpl(pt) + : QObject(parent) + , m_pimpl(pt) { } diff --git a/src/libwalletqt/Wallet.cpp b/src/libwalletqt/Wallet.cpp index 751d14e..d539404 100644 --- a/src/libwalletqt/Wallet.cpp +++ b/src/libwalletqt/Wallet.cpp @@ -80,6 +80,7 @@ Wallet::Wallet(Monero::Wallet *wallet, QObject *parent) connect(this, &Wallet::updated, this, &Wallet::onUpdated); connect(this, &Wallet::heightsRefreshed, this, &Wallet::onHeightsRefreshed); connect(this, &Wallet::transactionCreated, this, &Wallet::onTransactionCreated); + connect(this, &Wallet::transactionCommitted, this, &Wallet::onTransactionCommitted); } // #################### Status #################### @@ -693,17 +694,15 @@ void Wallet::createTransaction(const QString &address, quint64 amount, const QSt } qInfo() << "Creating transaction"; - m_scheduler.run([this, all, address, amount] { std::set subaddr_indices; - Monero::PendingTransaction * ptImpl = m_walletImpl->createTransaction(address.toStdString(), "", all ? Monero::optional() : Monero::optional(amount), constants::mixin, - static_cast(this->tx_priority), - currentSubaddressAccount(), subaddr_indices, m_selectedInputs); - PendingTransaction *tx = new PendingTransaction(ptImpl, this); + Monero::PendingTransaction *ptImpl = m_walletImpl->createTransaction(address.toStdString(), "", all ? Monero::optional() : Monero::optional(amount), constants::mixin, + static_cast(this->tx_priority), + currentSubaddressAccount(), subaddr_indices, m_selectedInputs); QVector addresses{address}; - emit transactionCreated(tx, addresses); + emit transactionCreated(ptImpl, addresses); }); emit initiateTransaction(); @@ -738,8 +737,8 @@ void Wallet::createTransactionMultiDest(const QVector &addresses, const Monero::PendingTransaction *ptImpl = m_walletImpl->createTransactionMultDest(dests, "", amount, constants::mixin, static_cast(this->tx_priority), currentSubaddressAccount(), subaddr_indices, m_selectedInputs); - PendingTransaction *tx = new PendingTransaction(ptImpl); - emit transactionCreated(tx, addresses); + + emit transactionCreated(ptImpl, addresses); }); emit initiateTransaction(); @@ -757,10 +756,9 @@ void Wallet::sweepOutputs(const QVector &keyImages, QString address, bo kis.push_back(key_image.toStdString()); } Monero::PendingTransaction *ptImpl = m_walletImpl->createTransactionSelected(kis, address.toStdString(), outputs, static_cast(this->tx_priority)); - PendingTransaction *tx = new PendingTransaction(ptImpl, this); QVector addresses {address}; - emit transactionCreated(tx, addresses); + emit transactionCreated(ptImpl, addresses); }); emit initiateTransaction(); @@ -773,9 +771,11 @@ void Wallet::onCreateTransactionError(const QString &msg) { emit endTransaction(); } -void Wallet::onTransactionCreated(PendingTransaction *tx, const QVector &address) { +void Wallet::onTransactionCreated(Monero::PendingTransaction *mtx, const QVector &address) { qDebug() << Q_FUNC_INFO; + PendingTransaction *tx = new PendingTransaction(mtx, this); + for (auto &addr : address) { if (addr == constants::donationAddress) { this->donationSending = true; @@ -795,14 +795,12 @@ void Wallet::commitTransaction(PendingTransaction *tx, const QString &descriptio // Clear list of selected transfers this->setSelectedInputs({}); - // Nodes - even well-connected, properly configured ones - consistently fail to relay transactions - // To mitigate transactions failing we just send the transaction to every node we know about over Tor - if (config()->get(Config::multiBroadcast).toBool()) { - // Let MainWindow handle this - emit multiBroadcast(tx); + QMap txHexMap; + for (int i = 0; i < tx->txCount(); i++) { + txHexMap[tx->txid()[i]] = tx->signedTxToHex(i); } - m_scheduler.run([this, tx, description] { + m_scheduler.run([this, tx, description, txHexMap] { auto txIdList = tx->txid(); // retrieve before commit bool success = tx->commit(); @@ -812,24 +810,36 @@ void Wallet::commitTransaction(PendingTransaction *tx, const QString &descriptio } } - // Store wallet immediately, so we don't risk losing tx key if wallet crashes - this->storeSafer(); - - this->history()->refresh(this->currentSubaddressAccount()); - this->coins()->refresh(this->currentSubaddressAccount()); - - this->updateBalance(); - - // this tx was a donation to Feather, stop our nagging - if (this->donationSending) { - this->donationSending = false; - emit donationSent(); - } - - emit transactionCommitted(success, tx, txIdList); + emit transactionCommitted(success, tx, txIdList, txHexMap); }); } +void Wallet::onTransactionCommitted(bool success, PendingTransaction *tx, const QStringList &txid, const QMap &txHexMap) { + // Store wallet immediately, so we don't risk losing tx key if wallet crashes + this->storeSafer(); + + this->history()->refresh(this->currentSubaddressAccount()); + this->coins()->refresh(this->currentSubaddressAccount()); + this->updateBalance(); + + if (!success) { + return; + } + + // Nodes - even well-connected, properly configured ones - consistently fail to relay transactions + // To mitigate transactions failing we just send the transaction to every node we know about over Tor + if (config()->get(Config::multiBroadcast).toBool()) { + // Let MainWindow handle this + emit multiBroadcast(txHexMap); + } + + // this tx was a donation to Feather, stop our nagging + if (this->donationSending) { + this->donationSending = false; + emit donationSent(); + } +} + void Wallet::disposeTransaction(PendingTransaction *t) { m_walletImpl->disposeTransaction(t->m_pimpl); delete t; diff --git a/src/libwalletqt/Wallet.h b/src/libwalletqt/Wallet.h index b165ab8..2a5fb36 100644 --- a/src/libwalletqt/Wallet.h +++ b/src/libwalletqt/Wallet.h @@ -302,6 +302,7 @@ public: void onCreateTransactionError(const QString &msg); void commitTransaction(PendingTransaction *tx, const QString &description=""); + void onTransactionCommitted(bool success, PendingTransaction *tx, const QStringList& txid, const QMap &txHexMap); //! deletes transaction and frees memory void disposeTransaction(PendingTransaction * t); @@ -410,13 +411,13 @@ signals: void deviceButtonPressed(); void deviceError(const QString &message); void walletPassphraseNeeded(bool onDevice); - void transactionCommitted(bool status, PendingTransaction *t, const QStringList& txid); + void transactionCommitted(bool status, PendingTransaction *t, const QStringList& txid, const QMap &txHexMap); void deviceShowAddressShowed(); void transactionProofVerified(TxProofResult result); void spendProofVerified(QPair result); // emitted when transaction is created async - void transactionCreated(PendingTransaction * transaction, QVector address); + void transactionCreated(Monero::PendingTransaction *ptImpl, QVector address); void connectionStatusChanged(int status) const; void currentSubaddressAccountChanged() const; @@ -437,7 +438,7 @@ signals: void selectedInputsChanged(const QStringList &selectedInputs); - void multiBroadcast(PendingTransaction *tx); + void multiBroadcast(const QMap &txHexMap); void heightsRefreshed(bool success, quint64 daemonHeight, quint64 targetHeight); private: @@ -451,7 +452,7 @@ private: void onRefreshed(bool success, const QString &message); // ##### Transactions ##### - void onTransactionCreated(PendingTransaction *tx, const QVector &address); + void onTransactionCreated(Monero::PendingTransaction *mtx, const QVector &address); private: friend class WalletManager;