← Watch feed

Moderate 58 Monero Committed Sep 23, 2026 · 07:32 UTC

Merge pull request #4690

XMR

Monero Project/Monero GUI

c72adf62 View source ↗

Public commit record

What the developer wrote

Authored by tobtoht

51/100 · Thin

Merge pull request #4690

3f2d979 Wallet: fix send confirmation stale transaction race (selsta)

ACKs: plowsof, jpk68

✓ Subject identifies a change✓ Provides an explanatory body✓ Links an issue, advisory, or supporting reference

The short version

What changed, and why it matters

This update fixes a timing bug in the Monero wallet's send screen. If a user quickly changed or cancelled a payment while a transaction was still being prepared in the background, the wallet could accidentally show or use the wrong transaction details. The fix adds request IDs so the app can tell old, stale transactions apart from the current one and safely discard them. It also cleans up transaction objects when the user rejects a payment, preventing possible crashes or memory waste.

Recommended action

Treat as a security-relevant bug fix and include in the next release. Users who create or cancel transactions rapidly in the GUI are the most likely to benefit. No immediate emergency response is indicated, but downstream packagers should apply the patch.

Security signals we found

Race condition between asynchronous transaction creation and UI state changes

Use-after-free / dangling pointer risk from stale PendingTransaction objects

Potential wrong-transaction confirmation or commit due to stale async result

Memory leak mitigation via explicit disposeTransaction on rejection and mismatch paths

UI state desynchronization in send confirmation flow

Technical analysis

Evidence from the diff

The patch addresses a stale-transaction race condition in the Monero GUI send flow. Previously, createTransactionAsync/createTransactionAllAsync emitted transactionCreated without correlating the async result to the originating request. If the user modified recipients, cancelled, or otherwise triggered a new transaction before the prior async job completed, the older result could overwrite or be committed as the current transaction. The fix introduces a monotonically increasing requestId passed through the async creation path and returned via transactionCreated. main.qml now tracks transactionRequestId and activeTransactionRequestId; onTransactionCreated discards results whose requestId does not match activeTransactionRequestId and disposes the stale PendingTransaction. Additional cleanup sets transaction to null and disposes it on rejection, password-dialog rejection, and view-only abort paths. The sweep-unmixable async path is also updated for consistency.

Changed components

main.qml send/confirmation flowsrc/libwalletqt/Wallet.cpp async transaction creation methodssrc/libwalletqt/Wallet.h async transaction creation signatures and transactionCreated signal

Inspect captured patch +57 / −24

### main.qml
@@ -72,6 +72,8 @@ ApplicationWindow {
     property var currentWallet;
     property bool disconnected: currentWallet ? currentWallet.disconnected : false
     property var transaction;
+    property double transactionRequestId: 0
+    property double activeTransactionRequestId: 0
     property var walletPassword
     property int restoreHeight:0
     property bool daemonSynced: false
@@ -895,8 +897,15 @@ ApplicationWindow {
         return false;
     }
 
-    function onTransactionCreated(pendingTransaction, addresses, paymentId, mixinCount) {
+    function onTransactionCreated(pendingTransaction, addresses, paymentId, mixinCount, requestId) {
         console.log("Transaction created");
+        if (requestId !== activeTransactionRequestId) {
+            console.log("Discarding stale transaction");
+            currentWallet.disposeTransaction(pendingTransaction);
+            return;
+        }
+
+        activeTransactionRequestId = 0;
         txConfirmationPopup.bottomText.text = "";
         transaction = pendingTransaction;
         // validate address;
@@ -909,12 +918,14 @@ ApplicationWindow {
             }
             // deleting transaction object, we don't want memleaks
             currentWallet.disposeTransaction(transaction);
+            transaction = null;
 
         } else if (transaction.txCount == 0) {
             console.error("Can't create transaction: ", transaction.errorString);
             txConfirmationPopup.errorText.text   = qsTr("No unmixable outputs to sweep") + translationManager.emptyString
             // deleting transaction object, we don't want memleaks
             currentWallet.disposeTransaction(transaction);
+            transaction = null;
         } else {
             console.log("Transaction created, amount: " + walletManager.displayAmount(transaction.amount)
                     + ", fee: " + walletManager.displayAmount(transaction.fee));
@@ -959,16 +970,27 @@ ApplicationWindow {
         txConfirmationPopup.transactionDescription = description;
         txConfirmationPopup.open();
 
+        const requestId = ++transactionRequestId;
+        activeTransactionRequestId = requestId;
+
         if (recipientAll) {
-            currentWallet.createTransactionAllAsync(recipientAll.address, paymentId, mixinCount, priority);
+            currentWallet.createTransactionAllAsync(recipientAll.address, paymentId, mixinCount, priority, requestId);
         } else {
             const addresses = recipients.map(function (recipient) {
                 return recipient.address;
             });
             const amountsxmr = recipients.map(function (recipient) {
                 return recipient.amount;
             });
-            currentWallet.createTransactionAsync(addresses, paymentId, amountsxmr, mixinCount, priority);
+            currentWallet.createTransactionAsync(addresses, paymentId, amountsxmr, mixinCount, priority, requestId);
+        }
+    }
+
+    function rejectPendingTransaction() {
+        activeTransactionRequestId = 0;
+        if (transaction) {
+            currentWallet.disposeTransaction(transaction);
+            transaction = null;
         }
     }
 
@@ -983,8 +1005,7 @@ ApplicationWindow {
             handleTransactionConfirmed()
         }
         onRejected: {
-            // do nothing
-
+            rejectPendingTransaction()
         }
 
     }
@@ -1000,12 +1021,14 @@ ApplicationWindow {
             txConfirmationPopup.errorText.text  = qsTr("Can't create transaction: ") + transaction.errorString + translationManager.emptyString
             // deleting transaction object, we don't want memleaks
             currentWallet.disposeTransaction(transaction);
+            transaction = null;
 
         } else if (transaction.txCount == 0) {
             console.error("No unmixable outputs to sweep");
             txConfirmationPopup.errorText.text  = qsTr("No unmixable outputs to sweep") + translationManager.emptyString
             // deleting transaction object, we don't want memleaks
             currentWallet.disposeTransaction(transaction);
+            transaction = null;
         } else {
             console.log("Transaction created, amount: " + walletManager.displayAmount(transaction.amount)
                     + ", fee: " + walletManager.displayAmount(transaction.fee));
@@ -1022,7 +1045,7 @@ ApplicationWindow {
         if(viewOnly){
             // No file specified - abort
             if(!saveTxDialog.fileUrl) {
-                currentWallet.disposeTransaction(transaction)
+                rejectPendingTransaction()
                 return;
             }
 
@@ -1032,7 +1055,9 @@ ApplicationWindow {
             transaction.setFilename(path);
         }
         appWindow.showProcessingSplash(qsTr("Sending transaction ..."));
-        currentWallet.commitTransactionAsync(transaction);
+        const pendingTransaction = transaction;
+        transaction = null;
+        currentWallet.commitTransactionAsync(pendingTransaction);
     }
 
     function onTransactionCommitted(success, transaction, txid) {
@@ -1677,7 +1702,7 @@ ApplicationWindow {
                     passwordDialog.showError(qsTr("Wrong password") + translationManager.emptyString);
                 }
             }
-            passwordDialog.onRejectedCallback = null;
+            passwordDialog.onRejectedCallback = rejectPendingTransaction;
             if(!persistentSettings.askPasswordBeforeSending) {
                 handleAccepted()
             } else {
@@ -1688,6 +1713,7 @@ ApplicationWindow {
                     appWindow.viewOnly ? "" : FontAwesome.arrowCircleRight);
             }
         }
+        onRejected: rejectPendingTransaction()
     }
 
     // Transaction successfully sent popup
@@ -2346,9 +2372,11 @@ ApplicationWindow {
         if (inputDialogVisible) inputDialog.close()
         remoteNodeDialog.close();
         informationPopup.close()
-        txConfirmationPopup.close()
-        txConfirmationPopup.clearFields()
-        txConfirmationPopup.rejected()
+        if (txConfirmationPopup.visible) {
+            txConfirmationPopup.close()
+            txConfirmationPopup.clearFields()
+            txConfirmationPopup.rejected()
+        }
         successfulTxPopup.close();
 
         if (currentWallet && currentWallet.getBackgroundSyncType() != Wallet.BackgroundSync_Off) {

### src/libwalletqt/Wallet.cpp
@@ -674,11 +674,12 @@ void Wallet::createTransactionAsync(
     const QString &payment_id,
     const QVector<QString> &destinationAmounts,
     quint32 mixin_count,
-    PendingTransaction::Priority priority)
+    PendingTransaction::Priority priority,
+    quint64 requestId)
 {
-    m_scheduler.run([this, destinationAddresses, payment_id, destinationAmounts, mixin_count, priority] {
+    m_scheduler.run([this, destinationAddresses, payment_id, destinationAmounts, mixin_count, priority, requestId] {
         PendingTransaction *tx = createTransaction(destinationAddresses, payment_id, destinationAmounts, mixin_count, priority);
-        emit transactionCreated(tx, destinationAddresses, payment_id, mixin_count);
+        emit transactionCreated(tx, destinationAddresses, payment_id, mixin_count, requestId);
     });
 }
 
@@ -695,11 +696,12 @@ PendingTransaction *Wallet::createTransactionAll(const QString &dst_addr, const
 
 void Wallet::createTransactionAllAsync(const QString &dst_addr, const QString &payment_id,
                                quint32 mixin_count,
-                               PendingTransaction::Priority priority)
+                               PendingTransaction::Priority priority,
+                               quint64 requestId)
 {
-    m_scheduler.run([this, dst_addr, payment_id, mixin_count, priority] {
+    m_scheduler.run([this, dst_addr, payment_id, mixin_count, priority, requestId] {
         PendingTransaction *tx = createTransactionAll(dst_addr, payment_id, mixin_count, priority);
-        emit transactionCreated(tx, {dst_addr}, payment_id, mixin_count);
+        emit transactionCreated(tx, {dst_addr}, payment_id, mixin_count, requestId);
     });
 }
 
@@ -710,11 +712,11 @@ PendingTransaction *Wallet::createSweepUnmixableTransaction()
     return result;
 }
 
-void Wallet::createSweepUnmixableTransactionAsync()
+void Wallet::createSweepUnmixableTransactionAsync(quint64 requestId)
 {
-    m_scheduler.run([this] {
+    m_scheduler.run([this, requestId] {
         PendingTransaction *tx = createSweepUnmixableTransaction();
-        emit transactionCreated(tx, {""}, "", 0);
+        emit transactionCreated(tx, {""}, "", 0, requestId);
     });
 }
 

### src/libwalletqt/Wallet.h
@@ -247,21 +247,23 @@ class Wallet : public QObject, public PassprasePrompter
         const QString &payment_id,
         const QVector<QString> &destinationAmounts,
         quint32 mixin_count,
-        PendingTransaction::Priority priority);
+        PendingTransaction::Priority priority,
+        quint64 requestId);
 
     //! creates transaction with all outputs
     Q_INVOKABLE PendingTransaction * createTransactionAll(const QString &dst_addr, const QString &payment_id,
                                                        quint32 mixin_count, PendingTransaction::Priority priority);
 
     //! creates async transaction with all outputs
     Q_INVOKABLE void createTransactionAllAsync(const QString &dst_addr, const QString &payment_id,
-                                               quint32 mixin_count, PendingTransaction::Priority priority);
+                                               quint32 mixin_count, PendingTransaction::Priority priority,
+                                               quint64 requestId);
 
     //! creates sweep unmixable transaction
     Q_INVOKABLE PendingTransaction * createSweepUnmixableTransaction();
 
     //! creates async sweep unmixable transaction
-    Q_INVOKABLE void createSweepUnmixableTransactionAsync();
+    Q_INVOKABLE void createSweepUnmixableTransactionAsync(quint64 requestId);
 
     //! Sign a transfer from file
     Q_INVOKABLE UnsignedTransaction * loadTxFile(const QString &fileName);
@@ -406,7 +408,8 @@ class Wallet : public QObject, public PassprasePrompter
         PendingTransaction *transaction,
         const QVector<QString> &addresses,
         const QString &paymentId,
-        quint32 mixinCount);
+        quint32 mixinCount,
+        quint64 requestId);
 
     void connectionStatusChanged(int status) const;
     void currentSubaddressAccountChanged() const;

Risk score

Why this scored 58/100

Our methodology →

Potential impact 18/30

Exploitability 12/25

Stealth signal 8/15

Affected reach 10/15

Confidence 7/10

Evidence quality 3/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 58/100

Community notes

Related watches

Merge pull request #4645

Merge pull request #4687

Merge pull request #4709