← Watch feed
Wallet: fix send confirmation stale transaction race
Public commit record
50/100 · Thin
What the developer wrote
Authored by selsta
Wallet: fix send confirmation stale transaction race
✓ Specific, descriptive subject✓ Names a concrete action or component! No meaningful explanatory body
What changed, and why it matters
This patch fixes a race condition in the Monero GUI wallet's send screen. If a user quickly changed send details or canceled a transaction while an earlier transaction was still being prepared in the background, the wallet could mix up the old and new transactions. The fix tags each send request with an ID and discards results from outdated requests, and it properly cleans up canceled transactions to avoid memory leaks or accidental use of the wrong transaction.
Recommended action
Treat as a bug-fix commit with possible security side effects. Reviewers should verify that all rejection/cancellation paths now call rejectPendingTransaction(), that no other callers of transactionCreated exist that ignore requestId, and that the requestId cannot overflow or be misused across wallet sessions. No immediate incident response is indicated unless a reproducible exploit path is demonstrated.
Security signals we found
01
Race condition between asynchronous transaction creation and UI state changes
02
Potential use of stale PendingTransaction object after cancellation/replacement
03
Memory leak via undisposed PendingTransaction objects on rejection paths
04
Possible wrong-transaction commit if stale callback updates UI or is committed
05
UI-level fix with request-id correlation and explicit cleanup
Evidence from the diff
The commit addresses a stale-transaction race in the asynchronous transaction creation flow. Previously, createTransactionAsync/createTransactionAllAsync emitted transactionCreated without correlating it to a specific user request. If the user triggered multiple sends, rejected a password dialog, or closed the confirmation popup before the async job finished, the callback could operate on a stale PendingTransaction, leading to use-after-free-like behavior, memory leaks, or committing the wrong transaction. The patch introduces a monotonically increasing transactionRequestId, passes it through the async C++ worker, and has QML ignore callbacks whose requestId does not match the active request. It also adds rejectPendingTransaction() to dispose of the pending transaction and clear the transaction property on rejection, password-dialog rejection, and popup close.
Changed components
main.qml send/confirmation flowsrc/libwalletqt/Wallet.cpp async transaction creation methodssrc/libwalletqt/Wallet.h signal signaturesInspect captured patch +57 / −24
diff --git a/main.qml b/main.qml
index 6a676d9..d9e542f 100644
--- a/main.qml
+++ b/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,8 +970,11 @@ 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;
@@ -968,7 +982,15 @@ ApplicationWindow {
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) {
diff --git a/src/libwalletqt/Wallet.cpp b/src/libwalletqt/Wallet.cpp
index 64f9687..dad9650 100644
--- a/src/libwalletqt/Wallet.cpp
+++ b/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);
});
}
diff --git a/src/libwalletqt/Wallet.h b/src/libwalletqt/Wallet.h
index 1cac75a..a2ce3f2 100644
--- a/src/libwalletqt/Wallet.h
+++ b/src/libwalletqt/Wallet.h
@@ -247,7 +247,8 @@ public:
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,
@@ -255,13 +256,14 @@ public:
//! 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 @@ signals:
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
Our methodology →Why this scored 42/100
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.