Skip to content

feat: implement exponential retention for wallet backups #7005

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

Open
PastaPastaPasta wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

feat: implement exponential retention for wallet backups #7005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c6f23e2
feat: implement exponential retention for wallet backups
PastaPastaPasta Nov 23, 2025
8e06dde
fix: missing file
PastaPastaPasta Nov 25, 2025
dc0a7a3
fix: apply changes as suggested by coderabbit
PastaPastaPasta Nov 25, 2025
0efabe0
fix: coderabbit suggestions in test
PastaPastaPasta Nov 25, 2025
a646f91
fix: whitespace
PastaPastaPasta Nov 25, 2025
8df14ca
fix: use ToString and fix trailing whitespace
PastaPastaPasta Nov 28, 2025
ce185aa
fix: redundant declaration
PastaPastaPasta Nov 28, 2025
93768ed
fix: use rolling exponential ranges for backup retention
PastaPastaPasta Nov 28, 2025
e2e0006
fix: use time-based exponential retention for wallet backups
UdjinM6 Nov 29, 2025
ccec0c0
test: update backup tests for time-based exponential retention
UdjinM6 Nov 29, 2025
a628851
refactor: deduplicate backup initialization logic
UdjinM6 Nov 29, 2025
30a0fb2
fix: remove duplicate global nWalletBackups variable
UdjinM6 Nov 29, 2025
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
1 change: 1 addition & 0 deletions src/Makefile.test.include
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ BITCOIN_TESTS =\

if ENABLE_WALLET
BITCOIN_TESTS += \
wallet/test/backup_tests.cpp \
wallet/test/bip39_tests.cpp \
wallet/test/coinjoin_tests.cpp \
wallet/test/psbt_wallet_tests.cpp \
Expand Down
2 changes: 1 addition & 1 deletion src/coinjoin/client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ bool CCoinJoinClientManager::CheckAutomaticBackup()
// We don't need auto-backups for descriptor wallets
if (!m_wallet->IsLegacy()) return true;

switch (nWalletBackups) {
switch (CWallet::nWalletBackups) {
case 0:
strAutoDenomResult = _("Automatic backups disabled") + Untranslated(", ") + _("no mixing available.");
WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- %s\n", strAutoDenomResult.original);
Expand Down
4 changes: 4 additions & 0 deletions src/interfaces/wallet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ class Wallet
//! Get the number of keys since the last auto backup
virtual int64_t getKeysLeftSinceAutoBackup() = 0;

//! Get wallet backup status
//! Returns: 1..20 = number of backups to keep, 0 = disabled, -1 = error, -2 = locked
virtual int getWalletBackupStatus() = 0;

//! Get wallet name.
virtual std::string getWalletName() = 0;

Expand Down
14 changes: 8 additions & 6 deletions src/qt/overviewpage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,9 +520,10 @@ void OverviewPage::coinJoinStatus(bool fForce)
if (!fForce && (clientModel->node().shutdownRequested() || !clientModel->masternodeSync().isBlockchainSynced())) return;

// Disable any PS UI for masternode or when autobackup is disabled or failed for whatever reason
if (clientModel->node().isMasternode() || nWalletBackups <= 0) {
int backupStatus = walletModel->wallet().getWalletBackupStatus();
if (clientModel->node().isMasternode() || backupStatus <= 0) {
DisableCoinJoinCompletely();
if (nWalletBackups <= 0) {
if (backupStatus <= 0) {
ui->labelCoinJoinEnabled->setToolTip(tr("Automatic backups are disabled, no mixing available!"));
}
return;
Comment on lines 522 to 529
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Dec 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Differentiate “disabled” vs “error/locked” backup states in CoinJoin UI

Right now coinJoinStatus() treats every non‑positive backupStatus the same in the early guard:

  • backupStatus <= 0 disables CoinJoin completely and sets a generic tooltip “Automatic backups are disabled, no mixing available!”.
  • Later, you have more specific handling for backupStatus == -1 (fatal backup error) and backupStatus == -2 (wallet locked, keypool not replenished), but that code is only effective within the same invocation where the failure occurs. On subsequent timer ticks, the early backupStatus <= 0 guard runs first and overwrites those more informative messages with the generic one.

This makes it hard for users to distinguish “I turned backups off” from “backups are failing” or “unlock your wallet to replenish the keypool”, and for the -2 case you end up disabling the CoinJoin UI entirely on the next tick even though the later block was only warning.

Consider restricting the early guard to the truly “disabled” state and letting the negative sentinel states be handled exclusively by the later block:

-    int backupStatus = walletModel->wallet().getWalletBackupStatus();
-    if (clientModel->node().isMasternode() || backupStatus <= 0) {
-        DisableCoinJoinCompletely();
-        if (backupStatus <= 0) {
-            ui->labelCoinJoinEnabled->setToolTip(tr("Automatic backups are disabled, no mixing available!"));
-        }
-        return;
-    }
+    const int backupStatus = walletModel->wallet().getWalletBackupStatus();
+    if (clientModel->node().isMasternode()) {
+        DisableCoinJoinCompletely();
+        return;
+    }
+    if (backupStatus == 0) {
+        // Backups explicitly disabled via settings/CLI.
+        DisableCoinJoinCompletely();
+        ui->labelCoinJoinEnabled->setToolTip(tr("Automatic backups are disabled, no mixing available!"));
+        return;
+    }

With this change:

  • 0 (disabled) still fully disables CoinJoin with the generic tooltip.
  • -1 and -2 are handled only in the legacy‑wallet block later in the function, so the more detailed error/warning text is preserved and not overwritten on the next tick.
  • DisableCoinJoinCompletely() continues to use getWalletBackupStatus() <= 0 to decide when to show the red “(Disabled)” label, which remains appropriate for both “disabled” and error states.

Also applies to: 597-599, 642-655, 751-761

🧰 Tools
🪛 Cppcheck (2.18.0)

[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.

(toomanyconfigs)

Expand Down Expand Up @@ -617,7 +618,7 @@ void OverviewPage::coinJoinStatus(bool fForce)

// Warn user that wallet is running out of keys
// NOTE: we do NOT warn user and do NOT create autobackups if mixing is not running
if (walletModel->wallet().isLegacy() && nWalletBackups > 0 && walletModel->getKeysLeftSinceAutoBackup() < COINJOIN_KEYS_THRESHOLD_WARNING) {
if (walletModel->wallet().isLegacy() && walletModel->wallet().getWalletBackupStatus() > 0 && walletModel->getKeysLeftSinceAutoBackup() < COINJOIN_KEYS_THRESHOLD_WARNING) {
QSettings settings;
if(settings.value("fLowKeysWarning").toBool()) {
QString strWarn = tr("Very low number of keys left since last automatic backup!") + "<br><br>" +
Expand Down Expand Up @@ -661,7 +662,8 @@ void OverviewPage::coinJoinStatus(bool fForce)
ui->labelCoinJoinEnabled->setText(strEnabled);

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

Expand All @@ -671,7 +673,7 @@ void OverviewPage::coinJoinStatus(bool fForce)
ui->labelCoinJoinEnabled->setToolTip(strError);

return;
} else if(nWalletBackups == -2) {
} else if (backupStatus == -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);
Expand Down Expand Up @@ -776,7 +778,7 @@ void OverviewPage::DisableCoinJoinCompletely()

ui->toggleCoinJoin->setText("(" + tr("Disabled") + ")");
ui->frameCoinJoin->setEnabled(false);
if (nWalletBackups <= 0) {
if (walletModel && walletModel->wallet().getWalletBackupStatus() <= 0) {
ui->labelCoinJoinEnabled->setText("<span style='" + GUIUtil::getThemedStyleQString(GUIUtil::ThemedStyle::TS_ERROR) + "'>(" + tr("Disabled") + ")</span>");
}
walletModel->coinJoin()->stopMixing();
Expand Down
9 changes: 0 additions & 9 deletions src/util/system.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,6 @@ const int64_t nStartupTime = GetTime();
//Dash only features
const std::string gCoinJoinName = "CoinJoin";

/**
nWalletBackups:
1..10 - number of automatic backups to keep
0 - disabled by command-line
-1 - disabled because of some error during run-time
-2 - disabled because wallet was locked and we were not able to replenish keypool
*/
int nWalletBackups = 10;

const char * const BITCOIN_CONF_FILENAME = "dash.conf";
const char * const BITCOIN_SETTINGS_FILENAME = "settings.json";

Expand Down
1 change: 0 additions & 1 deletion src/util/system.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@

//Dash only features

extern int nWalletBackups;
extern const std::string gCoinJoinName;

class UniValue;
Expand Down
3 changes: 2 additions & 1 deletion src/wallet/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
{
argsman.AddArg("-avoidpartialspends", strprintf("Group outputs by address, selecting many (possibly all) or none, instead of selecting on a per-output basis. Privacy is improved as addresses are mostly swept with fewer transactions and outputs are aggregated in clean change addresses. It may result in higher fees due to less optimal coin selection caused by this added limitation and possibly a larger-than-necessary number of inputs being used. Always enabled for wallets with \"avoid_reuse\" enabled, otherwise default: %u.", DEFAULT_AVOIDPARTIALSPENDS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
argsman.AddArg("-consolidatefeerate=<amt>", strprintf("The maximum feerate (in %s/kvB) at which transaction building may use more inputs than strictly necessary so that the wallet's UTXO pool can be reduced (default: %s).", CURRENCY_UNIT, FormatMoney(DEFAULT_CONSOLIDATE_FEERATE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
argsman.AddArg("-createwalletbackups=<n>", strprintf("Number of automatic wallet backups (default: %u)", nWalletBackups), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
argsman.AddArg("-createwalletbackups=<n>", strprintf("Number of automatic wallet backups to keep most recent (default: %u, max: %u). Older backups are kept at exponentially spaced intervals.", DEFAULT_N_WALLET_BACKUPS, MAX_N_WALLET_BACKUPS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
Copy link
Copy Markdown
Collaborator

@knst knst Jan 8, 2026

Choose a reason for hiding this comment

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

I think doc/managing-wallets.md should include detailed explanation how exactly exponential intervals works; this short explanation is not enough without looking to code, but init.cpp is not a place for detailed help

Copy link
Copy Markdown

@thepastaclaw thepastaclaw Apr 5, 2026

Choose a reason for hiding this comment

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

💬 Nitpick: Awkward grammar in -createwalletbackups help text

The help text reads "Number of automatic wallet backups to keep most recent" which is grammatically unclear — "keep most recent" lacks an article and reads ambiguously.

💡 Suggested change
Suggested change
argsman.AddArg("-createwalletbackups=<n>", strprintf("Number of automatic wallet backups to keep most recent (default: %u, max: %u). Older backups are kept at exponentially spaced intervals.", DEFAULT_N_WALLET_BACKUPS, MAX_N_WALLET_BACKUPS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
argsman.AddArg("-createwalletbackups=<n>", strprintf("Number of most recent automatic wallet backups to keep (default: %u, max: %u). Older backups are kept at exponentially spaced intervals.", DEFAULT_N_WALLET_BACKUPS, MAX_N_WALLET_BACKUPS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);

source: ['claude']

argsman.AddArg("-maxwalletbackups=<n>", strprintf("Maximum total number of automatic wallet backups to keep (default: %u)", DEFAULT_MAX_BACKUPS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
argsman.AddArg("-disablewallet", "Do not load the wallet and disable wallet RPC calls", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
#if HAVE_SYSTEM
argsman.AddArg("-instantsendnotify=<cmd>", "Execute command when a wallet InstantSend transaction is successfully locked. %s in cmd is replaced by TxID and %w is replaced by wallet name. %w is not currently implemented on Windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
Expand Down
1 change: 1 addition & 0 deletions src/wallet/interfaces.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ class WalletImpl : public Wallet
return m_wallet->AutoBackupWallet(wallet_path, error_string, warnings);
}
int64_t getKeysLeftSinceAutoBackup() override { return m_wallet->nKeysLeftSinceAutoBackup; }
int getWalletBackupStatus() override { return CWallet::nWalletBackups; }
std::string getWalletName() override { return m_wallet->GetName(); }
util::Result<CTxDestination> getNewDestination(const std::string& label) override
{
Expand Down
Loading
Loading