libwalletqt: cleanup tx commit

src/MainWindow.cpp

@@ -448,19 +448,7 @@ void MainWindow::initWalletContext() {
         config()->set(Config::donateBeg, -1);
     });
     
-    connect(m_wallet, &Wallet::multiBroadcast,      this, [this](PendingTransaction *tx){
+    connect(m_wallet, &Wallet::multiBroadcast,      this, &MainWindow::onMultiBroadcast);
-        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);
-            }
-        }
-    });
 }
 
 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<QString, QString> &txHexMap) {
+    QMapIterator<QString, QString> 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) {
+void MainWindow::onTransactionCommitted(bool success, PendingTransaction *tx, const QStringList& txid) {
-    if (status) { // success
+    if (success) {
         QMessageBox msgBox{this};
         QPushButton *showDetailsButton = msgBox.addButton("Show details", QMessageBox::ActionRole);
         msgBox.addButton(QMessageBox::Ok);

src/MainWindow.h

@@ -193,6 +193,7 @@ private slots:
     void showUpdateNotification();
     void onProxySettingsChanged();
     void onOfflineMode(bool offline);
+    void onMultiBroadcast(const QMap<QString, QString> &txHexMap);
 
 private:
     friend WindowManager;

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)
 {
 
 }

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<uint32_t> subaddr_indices;
 
-        Monero::PendingTransaction * ptImpl = m_walletImpl->createTransaction(address.toStdString(), "", all ? Monero::optional<uint64_t>() : Monero::optional<uint64_t>(amount), constants::mixin,
+        Monero::PendingTransaction *ptImpl = m_walletImpl->createTransaction(address.toStdString(), "", all ? Monero::optional<uint64_t>() : Monero::optional<uint64_t>(amount), constants::mixin,
                                                                              static_cast<Monero::PendingTransaction::Priority>(this->tx_priority),
                                                                              currentSubaddressAccount(), subaddr_indices, m_selectedInputs);
-        PendingTransaction *tx = new PendingTransaction(ptImpl, this);
 
         QVector<QString> addresses{address};
-        emit transactionCreated(tx, addresses);
+        emit transactionCreated(ptImpl, addresses);
     });
 
     emit initiateTransaction();
@@ -738,8 +737,8 @@ void Wallet::createTransactionMultiDest(const QVector<QString> &addresses, const
         Monero::PendingTransaction *ptImpl = m_walletImpl->createTransactionMultDest(dests, "", amount, constants::mixin,
                                                                                      static_cast<Monero::PendingTransaction::Priority>(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<QString> &keyImages, QString address, bo
             kis.push_back(key_image.toStdString());
         }
         Monero::PendingTransaction *ptImpl = m_walletImpl->createTransactionSelected(kis, address.toStdString(), outputs, static_cast<Monero::PendingTransaction::Priority>(this->tx_priority));
-        PendingTransaction *tx = new PendingTransaction(ptImpl, this);
 
         QVector<QString> 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<QString> &address) {
+void Wallet::onTransactionCreated(Monero::PendingTransaction *mtx, const QVector<QString> &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
+    QMap<QString, QString> txHexMap;
-    // To mitigate transactions failing we just send the transaction to every node we know about over Tor
+    for (int i = 0; i < tx->txCount(); i++) {
-    if (config()->get(Config::multiBroadcast).toBool()) {
+        txHexMap[tx->txid()[i]] = tx->signedTxToHex(i);
-        // Let MainWindow handle this
-        emit multiBroadcast(tx);
     }
 
-    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,22 +810,34 @@ void Wallet::commitTransaction(PendingTransaction *tx, const QString &descriptio
             }
         }
 
+        emit transactionCommitted(success, tx, txIdList, txHexMap);
+    });
+}
+
+void Wallet::onTransactionCommitted(bool success, PendingTransaction *tx, const QStringList &txid, const QMap<QString, QString> &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();
     }
-
-        emit transactionCommitted(success, tx, txIdList);
-    });
 }
 
 void Wallet::disposeTransaction(PendingTransaction *t) {

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<QString, QString> &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<QString, QString> &txHexMap);
     void deviceShowAddressShowed();
     void transactionProofVerified(TxProofResult result);
     void spendProofVerified(QPair<bool, bool> result);
 
     // emitted when transaction is created async
-    void transactionCreated(PendingTransaction * transaction, QVector<QString> address);
+    void transactionCreated(Monero::PendingTransaction *ptImpl, QVector<QString> 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<QString, QString> &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<QString> &address);
+    void onTransactionCreated(Monero::PendingTransaction *mtx, const QVector<QString> &address);
 
 private:
     friend class WalletManager;