Skip to content

fix: surface specific autobackup-failure tooltips in CoinJoin status #7515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 2 commits into from
Aug 3, 2026
Merged

fix: surface specific autobackup-failure tooltips in CoinJoin status #7515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f16aea4
fix: surface specific autobackup-failure tooltips in CoinJoin status …
PastaPastaPasta Aug 2, 2026
0d7a9d6
test: add regression coverage for autobackup-failure CoinJoin tooltips
PastaPastaPasta Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 12 additions & 17 deletions src/qt/overviewpage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,14 @@ void OverviewPage::coinJoinStatus(bool fForce)
// Disable any PS UI for masternode or when autobackup is disabled or failed for whatever reason
if (clientModel->node().isMasternode() || nWalletBackups <= 0) {
DisableCoinJoinCompletely();
if (nWalletBackups <= 0) {
if (nWalletBackups == 0) {
ui->labelCoinJoinEnabled->setToolTip(tr("Automatic backups are disabled, no mixing available!"));
} else if (nWalletBackups == -1) {
ui->labelCoinJoinEnabled->setToolTip(tr("ERROR! Failed to create automatic backup") + ", " +
tr("see debug.log for details.") + "<br><br>" +
tr("Mixing is disabled, please close your wallet and fix the issue!"));
Comment on lines +531 to +534

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop CoinJoin in the backup-failure invocation

When AutoBackupWallet() sets nWalletBackups to -1 while mixing is active, this relocated guard has already been passed. Removing the later -1 block therefore lets the current invocation continue, display the status as enabled, and leave the client mixing until a subsequent GUI timer tick reaches this guard; CheckAutomaticBackup() merely returns false for -1 and does not call stopMixing(). Preserve the immediate DisableCoinJoinCompletely() path so the modal's claim that mixing is disabled is true as soon as the backup fails.

Useful? React with 👍 / 👎.

} else if (nWalletBackups == -2) {
ui->labelCoinJoinEnabled->setToolTip(tr("WARNING! Failed to replenish keypool, please unlock your wallet to do so."));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +535 to +536

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid directing users to unlock the wrong wallet

In a multiwallet GUI, this branch also runs on OverviewPages belonging to wallets that did not encounter the failure: nWalletBackups is process-global, while each WalletView owns a separate OverviewPage. If locked legacy wallet A sets the global to -2, switching to an already-unlocked or descriptor wallet B now shows “please unlock your wallet”; acting on B cannot replenish A's keypool, so the warning gives no usable recovery path. Track the failure per wallet or make the tooltip identify that another loaded wallet may need unlocking.

Useful? React with 👍 / 👎.

Comment on lines 526 to +536

@thepastaclaw thepastaclaw Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Add regression coverage for backup-status tooltip selection

This three-way branch (nWalletBackups == 0 / -1 / -2) is the core of the bug fix, but nothing verifies it stays correct going forward — a future refactor could silently reintroduce the original bug where the generic tooltip overwrites the specific ones. src/qt/test/wallettests.cpp already builds an OverviewPage wired to ClientModel and WalletModel in TestGUI, so this is directly testable: set OverviewPage's nWalletBackups to each of 0, -1, -2, call coinJoinStatus(true), and assert ui->labelCoinJoinEnabled->toolTip() matches the expected string for each. Restore the prior nWalletBackups value afterward so it doesn't leak into other assertions in the same test.

source: ['codex']

@PastaPastaPasta PastaPastaPasta Aug 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added in 0d7a9d6

}
return;
}
Expand Down Expand Up @@ -666,22 +672,11 @@ void OverviewPage::coinJoinStatus(bool fForce)
if(fShowAdvancedCJUI && !strKeysLeftText.isEmpty()) strEnabled += ", " + strKeysLeftText;
ui->labelCoinJoinEnabled->setText(strEnabled);

if (walletModel->wallet().isLegacy()) {
if(nWalletBackups == -1) {
// Automatic backup failed, nothing else we can do until user fixes the issue manually
DisableCoinJoinCompletely();

QString strError = tr("ERROR! Failed to create automatic backup") + ", " +
tr("see debug.log for details.") + "<br><br>" +
tr("Mixing is disabled, please close your wallet and fix the issue!");
ui->labelCoinJoinEnabled->setToolTip(strError);

return;
} else if(nWalletBackups == -2) {
// We were able to create automatic backup but keypool was not replenished because wallet is locked.
QString strWarning = tr("WARNING! Failed to replenish keypool, please unlock your wallet to do so.");
ui->labelCoinJoinEnabled->setToolTip(strWarning);
}
if (walletModel->wallet().isLegacy() && nWalletBackups == -1) {
// Automatic backup failed, nothing else we can do until user fixes the issue manually.
// Stop mixing right away; the guard above sets the matching tooltip on the next timer tick.
DisableCoinJoinCompletely();
return;
}

// check coinjoin status and unlock if needed
Expand Down
25 changes: 25 additions & 0 deletions src/qt/test/wallettests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <qt/walletmodel.h>
#include <key_io.h>
#include <test/util/setup_common.h>
#include <util/system.h>
#include <validation.h>
#include <wallet/wallet.h>
#include <qt/overviewpage.h>
Expand Down Expand Up @@ -193,6 +194,30 @@ void TestGUI(interfaces::Node& node)
QString balanceComparison = BitcoinUnits::floorHtmlWithPrivacy(unit, balance, BitcoinUnits::SeparatorStyle::ALWAYS, false);
QCOMPARE(balanceText, balanceComparison);

// Check that each autobackup failure state selects its specific tooltip on the CoinJoin status label
{
QLabel* coinJoinLabel = overviewPage.findChild<QLabel*>("labelCoinJoinEnabled");
QVERIFY(coinJoinLabel != nullptr);
const int nWalletBackupsOld = nWalletBackups;

nWalletBackups = 0;
overviewPage.coinJoinStatus(/*fForce=*/true);
QCOMPARE(coinJoinLabel->toolTip(), QString("Automatic backups are disabled, no mixing available!"));

nWalletBackups = -1;
overviewPage.coinJoinStatus(/*fForce=*/true);
QCOMPARE(coinJoinLabel->toolTip(),
QString("ERROR! Failed to create automatic backup, see debug.log for details.<br><br>Mixing is "
"disabled, please close your wallet and fix the issue!"));

nWalletBackups = -2;
overviewPage.coinJoinStatus(/*fForce=*/true);
QCOMPARE(coinJoinLabel->toolTip(),
QString("WARNING! Failed to replenish keypool, please unlock your wallet to do so."));

nWalletBackups = nWalletBackupsOld;
}

// Check Request Payment button
ReceiveCoinsDialog receiveCoinsDialog;
receiveCoinsDialog.setModel(&walletModel);
Expand Down
Loading