diff --git a/doc/managing-wallets.md b/doc/managing-wallets.md index ca89186edd2f..57992cbc7093 100644 --- a/doc/managing-wallets.md +++ b/doc/managing-wallets.md @@ -106,7 +106,15 @@ This means that a single backup is enough to recover the coins at any time. It i Non-HD wallets must be backed up every 1000 keys used since the previous backup, or even more often to maintain the metadata. -### 1.6 Restoring the Wallet From a Backup +### 1.6 Automatic Backups + +For legacy wallets, Dash Core automatically creates a backup in the `backups` directory inside the data directory on every startup and whenever the keypool is replenished. Each file is named after the wallet with the backup time appended, e.g. `wallet.dat.2026-08-02-14-30`. + +Older backups are pruned as new ones are made. The most recent `-createwalletbackups` backups (default: 10, max: 20) are always kept. Beyond those, one backup is kept from each widening age range — 1-2 days old, 2-4 days, 4-8 days, and so on — up to `-maxwalletbackups` files in total (default: 30). This preserves restore points going back months while keeping the size of the `backups` directory predictable. Because the ranges are measured in days, several backups made within the same day do not each get their own range: only the most recent `-createwalletbackups` of them are kept. + +Setting either option to `0` disables automatic backups, in which case existing backups are left untouched. Renaming a backup also excludes it from pruning, which is a simple way to keep one indefinitely. + +### 1.7 Restoring the Wallet From a Backup To restore a wallet, the `restorewallet` RPC or the `Restore Wallet` GUI menu item (`File` -> `Restore Wallet…`) must be used. diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 5e9d47f8c4c7..e986f212bd0d 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -217,6 +217,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 \ diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 1a16f48acadd..5eb9085e6d30 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -843,7 +843,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: WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::CheckAutomaticBackup -- Automatic backups disabled, no mixing available.\n"); stopMixing(); diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 7300f4286e99..d973fa8d377a 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -115,6 +115,10 @@ class Wallet //! Get the number of keys since the last auto backup virtual int64_t getKeysLeftSinceAutoBackup() = 0; + //! Get automatic backup status: >0 = enabled, 0 = disabled, -1 = failed, + //! -2 = wallet locked. Process-wide, not per-wallet. + virtual int getWalletBackupStatus() = 0; + //! Get wallet name. virtual std::string getWalletName() = 0; diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 9ce945530a5b..c2cb3ae8ef0e 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -524,15 +524,16 @@ void OverviewPage::coinJoinStatus(bool fForce) if (fForce) nCachedNumBlocks = std::numeric_limits::max(); // Disable any PS UI for masternode or when autobackup is disabled or failed for whatever reason - if (clientModel->node().isMasternode() || nWalletBackups <= 0) { + const int backup_status{walletModel->wallet().getWalletBackupStatus()}; + if (clientModel->node().isMasternode() || backup_status <= 0) { DisableCoinJoinCompletely(); - if (nWalletBackups == 0) { + if (backup_status == 0) { ui->labelCoinJoinEnabled->setToolTip(tr("Automatic backups are disabled, no mixing available!")); - } else if (nWalletBackups == -1) { + } else if (backup_status == -1) { ui->labelCoinJoinEnabled->setToolTip(tr("ERROR! Failed to create automatic backup") + ", " + tr("see debug.log for details.") + "

" + tr("Mixing is disabled, please close your wallet and fix the issue!")); - } else if (nWalletBackups == -2) { + } else if (backup_status == -2) { ui->labelCoinJoinEnabled->setToolTip(tr("WARNING! Failed to replenish keypool, please unlock your wallet to do so.")); } return; @@ -628,7 +629,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!") + "

" + @@ -672,13 +673,6 @@ void OverviewPage::coinJoinStatus(bool fForce) if(fShowAdvancedCJUI && !strKeysLeftText.isEmpty()) strEnabled += ", " + strKeysLeftText; ui->labelCoinJoinEnabled->setText(strEnabled); - 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 if (nBestHeight != nCachedNumBlocks) { // Balance and number of transactions might have changed @@ -781,7 +775,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("(" + tr("Disabled") + ")"); } walletModel->withCoinJoin([](auto& client) { client.stopMixing(); }); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 1394ac1fdea1..295748127378 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -198,24 +198,24 @@ void TestGUI(interfaces::Node& node) { QLabel* coinJoinLabel = overviewPage.findChild("labelCoinJoinEnabled"); QVERIFY(coinJoinLabel != nullptr); - const int nWalletBackupsOld = nWalletBackups; + const int nWalletBackupsOld = CWallet::nWalletBackups; - nWalletBackups = 0; + CWallet::nWalletBackups = 0; overviewPage.coinJoinStatus(/*fForce=*/true); QCOMPARE(coinJoinLabel->toolTip(), QString("Automatic backups are disabled, no mixing available!")); - nWalletBackups = -1; + CWallet::nWalletBackups = -1; overviewPage.coinJoinStatus(/*fForce=*/true); QCOMPARE(coinJoinLabel->toolTip(), QString("ERROR! Failed to create automatic backup, see debug.log for details.

Mixing is " "disabled, please close your wallet and fix the issue!")); - nWalletBackups = -2; + CWallet::nWalletBackups = -2; overviewPage.coinJoinStatus(/*fForce=*/true); QCOMPARE(coinJoinLabel->toolTip(), QString("WARNING! Failed to replenish keypool, please unlock your wallet to do so.")); - nWalletBackups = nWalletBackupsOld; + CWallet::nWalletBackups = nWalletBackupsOld; } // Check Request Payment button diff --git a/src/util/system.cpp b/src/util/system.cpp index 347188e9f426..66c0b8e4be5f 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -67,15 +67,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"; diff --git a/src/util/system.h b/src/util/system.h index 4fff2c2f1e3c..ec5727aec0c1 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -35,7 +35,6 @@ //Dash only features -extern int nWalletBackups; extern const std::string gCoinJoinName; class ArgsManager; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index befccb33b3d1..a06a92e21ecb 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -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=", 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=", strprintf("Number of automatic wallet backups (default: %u)", nWalletBackups), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + argsman.AddArg("-createwalletbackups=", strprintf("Number of most recent automatic wallet backups to keep, 0 to disable (default: %u, max: %u). Older backups are additionally kept at exponentially spaced intervals, see doc/managing-wallets.md.", DEFAULT_N_WALLET_BACKUPS, MAX_N_WALLET_BACKUPS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + argsman.AddArg("-maxwalletbackups=", strprintf("Maximum total number of automatic wallet backups to keep, 0 to disable (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=", "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); @@ -214,7 +215,7 @@ void WalletInit::InitCoinJoinSettings(CCoinJoinClientManager& mgr) const void WalletInit::InitAutoBackup() const { - CWallet::InitAutoBackup(); + CWallet::InitAutoBackup(gArgs); } } // namespace wallet diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 099437d099a3..9fc99cc677e1 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -215,6 +215,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 getNewDestination(const std::string& label) override { diff --git a/src/wallet/test/backup_tests.cpp b/src/wallet/test/backup_tests.cpp new file mode 100644 index 000000000000..1fe1c83d7d76 --- /dev/null +++ b/src/wallet/test/backup_tests.cpp @@ -0,0 +1,374 @@ +#include +#include + +#include + +#include +#include +#include +#include + +namespace wallet { + +namespace { +//! Fixtures are anchored on a fixed date so that day-bucket boundaries are exact +//! rather than dependent on when the test happens to run. +constexpr std::chrono::sys_days BACKUP_ANCHOR{std::chrono::year{2026} / 8 / 4}; + +//! Timestamp of a backup taken `days_ago` days before the anchor. +std::chrono::system_clock::time_point MakeBackupTime(int days_ago) +{ + return BACKUP_ANCHOR - std::chrono::days{days_ago}; +} + +//! Backup filename AutoBackupWallet() writes for that timestamp. `sequence` distinguishes +//! several backups taken on the same day, as the minute field of the name. +fs::path MakeBackupPath(int days_ago, int sequence = 0) +{ + const std::chrono::year_month_day date{BACKUP_ANCHOR - std::chrono::days{days_ago}}; + return fs::u8path(strprintf("wallet.dat.%04i-%02u-%02u-00-%02i", int(date.year()), unsigned(date.month()), + unsigned(date.day()), sequence)); +} +} // namespace + +BOOST_FIXTURE_TEST_SUITE(backup_tests, WalletTestingSetup) + +BOOST_AUTO_TEST_CASE(time_based_exponential_retention) +{ + std::multimap backups; + + // Case 1: Less than nWalletBackups (10) + for (int i = 0; i < 5; ++i) { + backups.insert({MakeBackupTime(i), MakeBackupPath(i)}); + } + auto to_delete = GetBackupsToDelete(backups, 10, 50); + BOOST_CHECK(to_delete.empty()); + + // Case 2: Exactly nWalletBackups (10) + backups.clear(); + for (int i = 0; i < 10; ++i) { + backups.insert({MakeBackupTime(i), MakeBackupPath(i)}); + } + to_delete = GetBackupsToDelete(backups, 10, 50); + BOOST_CHECK(to_delete.empty()); + + // Case 3: 11 backups - all recent (< 1 day old) + // Since all are < 1 day old, no time-based retention applies + // Keep latest 10, but the 11th is also < 1 day so it doesn't get kept + backups.clear(); + for (int i = 0; i < 11; ++i) { + backups.insert({MakeBackupTime(0), MakeBackupPath(0, i)}); + } + to_delete = GetBackupsToDelete(backups, 10, 50); + // All backups are 0 days old, so none fall into [1,2) or later ranges + // Keep only latest 10, delete 1 + BOOST_CHECK_EQUAL(to_delete.size(), 1); + + // Case 4: 20 backups spanning multiple days + // Latest 10: 0 days old + // Older backups: 1, 2, 3, 5, 7, 10, 15, 20, 25, 30 days old + backups.clear(); + for (int i = 0; i < 10; ++i) { + backups.insert({MakeBackupTime(0), MakeBackupPath(0, i)}); + } + backups.insert({MakeBackupTime(1), MakeBackupPath(1)}); // [1,2) days + backups.insert({MakeBackupTime(2), MakeBackupPath(2)}); // [2,4) days + backups.insert({MakeBackupTime(3), MakeBackupPath(3)}); // [2,4) days + backups.insert({MakeBackupTime(5), MakeBackupPath(5)}); // [4,8) days + backups.insert({MakeBackupTime(7), MakeBackupPath(7)}); // [4,8) days + backups.insert({MakeBackupTime(10), MakeBackupPath(10)}); // [8,16) days + backups.insert({MakeBackupTime(15), MakeBackupPath(15)}); // [8,16) days + backups.insert({MakeBackupTime(20), MakeBackupPath(20)}); // [16,32) days + backups.insert({MakeBackupTime(25), MakeBackupPath(25)}); // [16,32) days + backups.insert({MakeBackupTime(30), MakeBackupPath(30)}); // [16,32) days + + to_delete = GetBackupsToDelete(backups, 10, 50); + + // Should keep: + // - Latest 10 (by count): all backups from today + // - Oldest in each populated exponential time range + // Total: 15 kept, 5 deleted + BOOST_CHECK_EQUAL(to_delete.size(), 5); + + // Verify exactly which backups are deleted + std::set expected_deletions = { + MakeBackupPath(2), // not oldest in [2,4) + MakeBackupPath(5), // not oldest in [4,8) + MakeBackupPath(10), // not oldest in [8,16) + MakeBackupPath(20), // not oldest in [16,32) + MakeBackupPath(25) // not oldest in [16,32) + }; + const std::set actual_deletions{to_delete.begin(), to_delete.end()}; + BOOST_CHECK(expected_deletions == actual_deletions); + + // Case 5: Test that we accumulate over time + // Simulate 100 days of daily backups + backups.clear(); + for (int i = 0; i < 100; ++i) { + backups.insert({MakeBackupTime(i), MakeBackupPath(i)}); + } + + to_delete = GetBackupsToDelete(backups, 10, 50); + + // Should keep: + // - Latest 10 by count and the oldest backup in each exponential time range + // Total: 14 kept, 86 deleted + BOOST_CHECK_EQUAL(to_delete.size(), 86); + + // Verify specific kept backups in exponential ranges + std::set expected_kept = {MakeBackupPath(0), MakeBackupPath(1), MakeBackupPath(2), MakeBackupPath(3), + MakeBackupPath(4), MakeBackupPath(5), MakeBackupPath(6), MakeBackupPath(7), + MakeBackupPath(8), MakeBackupPath(9), MakeBackupPath(15), MakeBackupPath(31), + MakeBackupPath(63), MakeBackupPath(99)}; + std::set actual_kept; + for (const auto& [time, path] : backups) { + if (std::find(to_delete.begin(), to_delete.end(), path) == to_delete.end()) { + actual_kept.insert(path); + } + } + BOOST_CHECK(expected_kept == actual_kept); +} + +BOOST_AUTO_TEST_CASE(hard_max_limit) +{ + std::multimap backups; + + // Create 100 daily backups and set maxBackups=15 + for (int i = 0; i < 100; ++i) { + backups.insert({MakeBackupTime(i), MakeBackupPath(i)}); + } + + auto to_delete = GetBackupsToDelete(backups, 10, 15); + + // Without maxBackups limit, we'd keep 14 backups (see Case 5 above) + // With maxBackups=15, we still keep 14 (under the limit) + BOOST_CHECK_EQUAL(to_delete.size(), 86); + + // Verify same backups kept as in Case 5 + std::set expected_kept_15 = {MakeBackupPath(0), MakeBackupPath(1), MakeBackupPath(2), + MakeBackupPath(3), MakeBackupPath(4), MakeBackupPath(5), + MakeBackupPath(6), MakeBackupPath(7), MakeBackupPath(8), + MakeBackupPath(9), MakeBackupPath(15), MakeBackupPath(31), + MakeBackupPath(63), MakeBackupPath(99)}; + std::set actual_kept_15; + for (const auto& [time, path] : backups) { + if (std::find(to_delete.begin(), to_delete.end(), path) == to_delete.end()) { + actual_kept_15.insert(path); + } + } + BOOST_CHECK(expected_kept_15 == actual_kept_15); + + // Now test with maxBackups=12 (less than natural retention) + to_delete = GetBackupsToDelete(backups, 10, 12); + + // Should cap at 12 backups: keep latest 10 + 2 oldest time ranges + // Total: 12 kept, 88 deleted + BOOST_CHECK_EQUAL(to_delete.size(), 88); + + // Verify exact backups kept when capped + std::set expected_kept_12 = {MakeBackupPath(0), MakeBackupPath(1), MakeBackupPath(2), + MakeBackupPath(3), MakeBackupPath(4), MakeBackupPath(5), + MakeBackupPath(6), MakeBackupPath(7), MakeBackupPath(8), + MakeBackupPath(9), MakeBackupPath(15), MakeBackupPath(31)}; + std::set actual_kept_12; + for (const auto& [time, path] : backups) { + if (std::find(to_delete.begin(), to_delete.end(), path) == to_delete.end()) { + actual_kept_12.insert(path); + } + } + BOOST_CHECK(expected_kept_12 == actual_kept_12); +} + +BOOST_AUTO_TEST_CASE(irregular_backup_schedule) +{ + std::multimap backups; + + // Test irregular schedule: multiple backups some days, gaps on others + // Day 0: 5 backups + for (int i = 0; i < 5; ++i) { + backups.insert({MakeBackupTime(0), MakeBackupPath(0, i)}); + } + // Day 1: 3 backups + for (int i = 5; i < 8; ++i) { + backups.insert({MakeBackupTime(1), MakeBackupPath(1, i - 5)}); + } + // Day 2: 2 backups + for (int i = 8; i < 10; ++i) { + backups.insert({MakeBackupTime(2), MakeBackupPath(2, i - 8)}); + } + // Day 10: 1 backup (gap) + backups.insert({MakeBackupTime(10), MakeBackupPath(10)}); + // Day 20: 1 backup (gap) + backups.insert({MakeBackupTime(20), MakeBackupPath(20)}); + + auto to_delete = GetBackupsToDelete(backups, 10, 50); + + // Should keep: + // - Latest 10 (5 from day 0, 3 from day 1, 2 from day 2) + // - Oldest in [8,16): day 10 + // - Oldest in [16,32): day 20 + // Total: 12 kept, 0 deleted + BOOST_CHECK_EQUAL(to_delete.size(), 0); + + // Verify all backups are kept + std::set expected_kept = {MakeBackupPath(0, 0), MakeBackupPath(0, 1), MakeBackupPath(0, 2), + MakeBackupPath(0, 3), MakeBackupPath(0, 4), MakeBackupPath(1, 0), + MakeBackupPath(1, 1), MakeBackupPath(1, 2), MakeBackupPath(2, 0), + MakeBackupPath(2, 1), MakeBackupPath(10), MakeBackupPath(20)}; + std::set actual_kept; + for (const auto& [time, path] : backups) { + actual_kept.insert(path); + } + BOOST_CHECK(expected_kept == actual_kept); +} + +BOOST_AUTO_TEST_CASE(long_inactivity_period) +{ + std::multimap backups; + + // 15 backups created 60 days ago, then nothing until today + for (int i = 0; i < 15; ++i) { + backups.insert({MakeBackupTime(60), MakeBackupPath(60, i)}); + } + // New backup today + backups.insert({MakeBackupTime(0), MakeBackupPath(0)}); + + auto to_delete = GetBackupsToDelete(backups, 10, 50); + + // Should keep: + // - Latest 10 (1 from today, 9 from 60 days ago) + // - Oldest in [32,64): 6 backups from 60 days ago qualify, keep the oldest + // Total: 11 kept, 5 deleted + BOOST_CHECK_EQUAL(to_delete.size(), 5); + + // Verify exact backups kept + // The current backup plus the nine latest at 60 days old are kept by count. + // The earliest 60-day backup is retained from the [32,64) range. + std::set expected_kept = {MakeBackupPath(0), // newest + MakeBackupPath(60, 0), // oldest in [32,64) range + MakeBackupPath(60, 6), MakeBackupPath(60, 7), MakeBackupPath(60, 8), + MakeBackupPath(60, 9), MakeBackupPath(60, 10), MakeBackupPath(60, 11), + MakeBackupPath(60, 12), MakeBackupPath(60, 13), MakeBackupPath(60, 14)}; + std::set actual_kept; + for (const auto& [time, path] : backups) { + if (std::find(to_delete.begin(), to_delete.end(), path) == to_delete.end()) { + actual_kept.insert(path); + } + } + BOOST_CHECK(expected_kept == actual_kept); +} + +BOOST_AUTO_TEST_CASE(non_positive_max_backups) +{ + std::multimap backups; + for (int i = 0; i < 20; ++i) { + backups.insert({MakeBackupTime(i), MakeBackupPath(i)}); + } + + // maxBackups <= 0 means "delete nothing", regardless of nWalletBackups + BOOST_CHECK(GetBackupsToDelete(backups, 10, 0).empty()); + BOOST_CHECK(GetBackupsToDelete(backups, 10, -1).empty()); + BOOST_CHECK(GetBackupsToDelete(backups, 0, 0).empty()); +} + +BOOST_AUTO_TEST_CASE(count_window_boundaries) +{ + std::multimap backups; + for (int i = 0; i < 20; ++i) { + backups.insert({MakeBackupTime(i), MakeBackupPath(i)}); + } + + // maxBackups == nWalletBackups leaves no room for time buckets, degrading to + // the pre-exponential "keep the N most recent" policy. + auto to_delete = GetBackupsToDelete(backups, 10, 10); + BOOST_CHECK_EQUAL(to_delete.size(), 10); + std::set expected_deleted; + for (int i = 10; i < 20; ++i) { + expected_deleted.insert(MakeBackupPath(i)); + } + BOOST_CHECK(expected_deleted == std::set(to_delete.begin(), to_delete.end())); + + // An empty count window leaves retention to the time buckets: the newest backup is + // kept as their anchor, then one per [1,2), [2,4), [4,8), [8,16), [16,32). + to_delete = GetBackupsToDelete(backups, 0, 30); + std::set expected_kept{MakeBackupPath(0), MakeBackupPath(1), MakeBackupPath(3), + MakeBackupPath(7), MakeBackupPath(15), MakeBackupPath(19)}; + std::set actual_kept; + for (const auto& [time, path] : backups) { + if (std::find(to_delete.begin(), to_delete.end(), path) == to_delete.end()) { + actual_kept.insert(path); + } + } + BOOST_CHECK(expected_kept == actual_kept); + + // Negative nWalletBackups is treated the same as an empty count window. + BOOST_CHECK(GetBackupsToDelete(backups, -1, 30) == to_delete); +} + +BOOST_AUTO_TEST_CASE(init_auto_backup_clamping) +{ + const int nWalletBackupsOrig = CWallet::nWalletBackups; + const int nMaxWalletBackupsOrig = CWallet::nMaxWalletBackups; + + auto init_with_args = [](const std::vector& argv) { + ArgsManager args; + args.AddArg("-createwalletbackups=", "", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + args.AddArg("-maxwalletbackups=", "", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + args.AddArg("-disablewallet", "", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + std::vector argv_full{"ignored"}; + argv_full.insert(argv_full.end(), argv.begin(), argv.end()); + std::string error; + BOOST_REQUIRE(args.ParseParameters(argv_full.size(), argv_full.data(), error)); + CWallet::InitAutoBackup(args); + }; + + // Defaults + init_with_args({}); + BOOST_CHECK_EQUAL(CWallet::nWalletBackups, DEFAULT_N_WALLET_BACKUPS); + BOOST_CHECK_EQUAL(CWallet::nMaxWalletBackups, DEFAULT_MAX_BACKUPS); + + // -createwalletbackups above MAX_N_WALLET_BACKUPS clamps to the max + init_with_args({"-createwalletbackups=100"}); + BOOST_CHECK_EQUAL(CWallet::nWalletBackups, MAX_N_WALLET_BACKUPS); + + // Negative -createwalletbackups clamps to 0 (disabled) + init_with_args({"-createwalletbackups=-5"}); + BOOST_CHECK_EQUAL(CWallet::nWalletBackups, 0); + + // -createwalletbackups is limited by -maxwalletbackups + init_with_args({"-createwalletbackups=15", "-maxwalletbackups=12"}); + BOOST_CHECK_EQUAL(CWallet::nWalletBackups, 12); + BOOST_CHECK_EQUAL(CWallet::nMaxWalletBackups, 12); + + // -maxwalletbackups=0 disables automatic backups entirely + init_with_args({"-maxwalletbackups=0"}); + BOOST_CHECK_EQUAL(CWallet::nWalletBackups, 0); + BOOST_CHECK_EQUAL(CWallet::nMaxWalletBackups, 0); + + CWallet::nWalletBackups = nWalletBackupsOrig; + CWallet::nMaxWalletBackups = nMaxWalletBackupsOrig; +} + +BOOST_AUTO_TEST_CASE(parse_backup_file_time) +{ + using namespace std::chrono; + + const auto parsed = ParseBackupFileTime(fs::u8path("wallet.dat.2026-08-02-14-30")); + BOOST_REQUIRE(parsed.has_value()); + const auto expected = sys_days{year{2026} / 8 / 2} + hours{14} + minutes{30}; + BOOST_CHECK(*parsed == expected); + + // Wrong shape or invalid fields + BOOST_CHECK(!ParseBackupFileTime(fs::u8path("wallet.dat")).has_value()); + BOOST_CHECK(!ParseBackupFileTime(fs::u8path("wallet.dat.bak")).has_value()); + BOOST_CHECK(!ParseBackupFileTime(fs::u8path("wallet.dat.2026-08-02")).has_value()); + BOOST_CHECK(!ParseBackupFileTime(fs::u8path("wallet.dat.2026-08-02-14-3x")).has_value()); + BOOST_CHECK(!ParseBackupFileTime(fs::u8path("wallet.dat.2026-13-02-14-30")).has_value()); + BOOST_CHECK(!ParseBackupFileTime(fs::u8path("wallet.dat.2026-02-30-14-30")).has_value()); + BOOST_CHECK(!ParseBackupFileTime(fs::u8path("wallet.dat.2026-08-02-24-30")).has_value()); + BOOST_CHECK(!ParseBackupFileTime(fs::u8path("wallet.dat.2026-08-02-14-60")).has_value()); +} + +BOOST_AUTO_TEST_SUITE_END() + +} // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e02442a653ec..e31298ea83a8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -52,6 +52,7 @@ #include #include +#include #include #include @@ -68,6 +69,9 @@ static isminetype InputIsMine(const CWallet& wallet, const CTxIn& txin) EXCLUSIV return ISMINE_NO; } +int CWallet::nWalletBackups = DEFAULT_N_WALLET_BACKUPS; +int CWallet::nMaxWalletBackups = DEFAULT_MAX_BACKUPS; + const std::map WALLET_FLAG_CAVEATS{ {WALLET_FLAG_AVOID_REUSE, "You need to rescan the blockchain in order to correctly mark used " @@ -3588,13 +3592,87 @@ void CWallet::postInitProcess() chain().requestMempoolTransactions(*this); } -void CWallet::InitAutoBackup() +std::vector GetBackupsToDelete(const std::multimap& backups, + int nWalletBackups, int maxBackups) +{ + if (maxBackups <= 0) return {}; + + // CWallet::nWalletBackups doubles as an error status and can be negative, so + // treat anything below zero as an empty count window rather than wrapping the cast. + const size_t keep_by_count{nWalletBackups > 0 ? static_cast(nWalletBackups) : 0}; + if (backups.size() <= keep_by_count) return {}; + + // Newest first, so age ascends with the index. + const std::vector> sorted_backups(backups.rbegin(), + backups.rend()); + const auto newest = sorted_backups[0].first; + + // Always keep the newest backup, which anchors the age ranges, plus the count window. + std::set indices_to_keep; + for (size_t i = 0; i < std::max(keep_by_count, 1); ++i) { + indices_to_keep.insert(i); + } + + // Of the rest, keep the oldest backup in each exponential age range in days: [1,2), + // [2,4), [4,8), ... std::bit_width() maps an age onto the 1-based index of its range, + // and since age ascends with the index, the last entry seen in a range is its oldest. + std::map oldest_per_range; + for (size_t i = keep_by_count; i < sorted_backups.size(); ++i) { + const auto age_days{std::chrono::duration_cast(newest - sorted_backups[i].first).count()}; + if (age_days < 1) continue; + oldest_per_range[std::bit_width(static_cast(age_days))] = i; + } + for (const auto& [range, index] : oldest_per_range) { + if (indices_to_keep.size() >= static_cast(maxBackups)) break; + indices_to_keep.insert(index); + } + + std::vector paths_to_delete; + for (size_t i = 0; i < sorted_backups.size(); ++i) { + if (!indices_to_keep.count(i)) paths_to_delete.push_back(sorted_backups[i].second); + } + return paths_to_delete; +} + +std::optional ParseBackupFileTime(const fs::path& backup_file) { - if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) + const std::string ext{fs::PathToString(backup_file.extension())}; + // Expect ".YYYY-MM-DD-HH-MM" as produced by AutoBackupWallet() + if (ext.size() != 17 || ext[0] != '.' || ext[5] != '-' || ext[8] != '-' || ext[11] != '-' || ext[14] != '-') { + return std::nullopt; + } + const auto to_int = [&ext](size_t pos, size_t len) -> std::optional { + int value{0}; + for (size_t i = pos; i < pos + len; ++i) { + if (!IsDigit(ext[i])) return std::nullopt; + value = value * 10 + (ext[i] - '0'); + } + return value; + }; + const auto year{to_int(1, 4)}, month{to_int(6, 2)}, day{to_int(9, 2)}, hour{to_int(12, 2)}, minute{to_int(15, 2)}; + if (!year || !month || !day || !hour || !minute) return std::nullopt; + if (*hour > 23 || *minute > 59) return std::nullopt; + const std::chrono::year_month_day ymd{std::chrono::year{*year}, std::chrono::month{static_cast(*month)}, + std::chrono::day{static_cast(*day)}}; + if (!ymd.ok()) return std::nullopt; + return std::chrono::sys_days{ymd} + std::chrono::hours{*hour} + std::chrono::minutes{*minute}; +} + +void CWallet::InitAutoBackup(const ArgsManager& args) +{ + if (args.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) return; - nWalletBackups = gArgs.GetIntArg("-createwalletbackups", 10); - nWalletBackups = std::max(0, std::min(10, nWalletBackups)); + nWalletBackups = args.GetIntArg("-createwalletbackups", DEFAULT_N_WALLET_BACKUPS); + nWalletBackups = std::max(0, std::min(MAX_N_WALLET_BACKUPS, nWalletBackups)); + + nMaxWalletBackups = args.GetIntArg("-maxwalletbackups", DEFAULT_MAX_BACKUPS); + nMaxWalletBackups = std::max(0, nMaxWalletBackups); + + // Enforce nWalletBackups <= nMaxWalletBackups + if (nWalletBackups > nMaxWalletBackups) { + nWalletBackups = nMaxWalletBackups; + } } bool CWallet::BackupWallet(const std::string& strDest) const @@ -3711,35 +3789,30 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error } } - // Keep only the last 10 backups, including the new one of course - std::multimap folder_set; - // Build map of backup files for current(!) wallet sorted by last write time - fs::path currentFile; + // Apply the retention policy to the backups of this wallet, keyed by the timestamp + // embedded in each filename rather than the filesystem mtime, which isn't preserved + // by all filesystems, cloud-sync tools or cross-device copies. Files that don't carry + // a timestamp we wrote ourselves are left alone entirely. + std::multimap folder_set; for (const auto& entry : fs::directory_iterator(backupsDir)) { // Only check regular files - if (entry.is_regular_file()) { - currentFile = entry.path().filename(); - // Only add the backups for the current wallet, e.g. wallet.dat.* - if (fs::PathToString(entry.path().stem()) == strWalletName) { - folder_set.insert(decltype(folder_set)::value_type(fs::last_write_time(entry.path()), entry)); - } + if (!entry.is_regular_file()) continue; + // Only add the backups for the current wallet, e.g. wallet.dat.* + if (fs::PathToString(entry.path().stem()) != strWalletName) continue; + if (const auto backup_time{ParseBackupFileTime(entry.path())}) { + folder_set.insert({*backup_time, entry.path()}); } } - // Loop backward through backup files and keep the N newest ones (1 <= N <= 10) - int counter{0}; - for (const auto& [entry_time, entry] : folder_set | std::views::reverse) { - counter++; - if (counter > nWalletBackups) { - // More than nWalletBackups backups: delete oldest one(s) - try { - fs::remove(entry); - WalletLogPrintf("Old backup deleted: %s\n", fs::PathToString(entry)); - } catch(fs::filesystem_error &error) { - warnings.push_back(strprintf(_("Failed to delete backup, error: %s"), fsbridge::get_filesystem_error_message(error))); - WalletLogPrintf("%s\n", Join(warnings, Untranslated("\n")).original); - return false; - } + std::vector backupsToDelete = GetBackupsToDelete(folder_set, nWalletBackups, nMaxWalletBackups); + for (const auto& path : backupsToDelete) { + try { + fs::remove(path); + WalletLogPrintf("Old backup deleted: %s\n", fs::PathToString(path)); + } catch(fs::filesystem_error &error) { + warnings.push_back(strprintf(_("Failed to delete backup, error: %s"), fsbridge::get_filesystem_error_message(error))); + WalletLogPrintf("%s\n", Join(warnings, Untranslated("\n")).original); + return false; } } @@ -3947,7 +4020,7 @@ bool CWallet::Unlock(const SecureString& strWalletPassphrase, bool fForMixingOnl if(nWalletBackups == -2) { TopUpKeyPool(); WalletLogPrintf("Keypool replenished, re-initializing automatic backups.\n"); - nWalletBackups = m_args.GetIntArg("-createwalletbackups", 10); + InitAutoBackup(m_args); } return true; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index fdc07a9cadfe..11732c005131 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -35,6 +35,7 @@ #include #include +#include #include #include #include @@ -82,6 +83,31 @@ void NotifyWalletLoading(WalletContext& context, const std::shared_ptr& void NotifyWalletLoaded(WalletContext& context, const std::shared_ptr& wallet); std::unique_ptr MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error); +//! Wallet backup configuration defaults +static constexpr int DEFAULT_MAX_BACKUPS = 30; +static constexpr int DEFAULT_N_WALLET_BACKUPS = 10; +static constexpr int MAX_N_WALLET_BACKUPS = 20; + +/** + * Pick which backups to delete: keeps the newest nWalletBackups by count, plus the oldest + * backup in each exponential age range in days relative to the newest backup ([1,2), + * [2,4), [4,8), ...), for at most maxBackups files in total. Beyond the count window + * retention goes purely by age, so same-day backups don't each get a range of their own. + * maxBackups <= 0 deletes nothing, and a non-positive nWalletBackups leaves retention + * entirely to the age ranges. Callers must keep nWalletBackups <= maxBackups for the + * total to hold; InitAutoBackup() enforces that. + */ +std::vector GetBackupsToDelete(const std::multimap& backups, + int nWalletBackups, int maxBackups = DEFAULT_MAX_BACKUPS); + +/** + * Read back the UTC timestamp AutoBackupWallet() embeds in backup filenames (e.g. + * "wallet.dat.2026-08-02-14-30"), or std::nullopt if the name isn't in that format — + * which is also how retention tells its own files apart from anything else a user may + * have put in the backups directory. + */ +std::optional ParseBackupFileTime(const fs::path& backup_file); + //! -paytxfee default constexpr CAmount DEFAULT_PAY_TX_FEE = 0; //! -fallbackfee default @@ -535,6 +561,10 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati std::set setLockedCoins GUARDED_BY(cs_wallet); int64_t nKeysLeftSinceAutoBackup; + //! Automatic backup settings, deliberately process-wide rather than per-wallet: + //! a backup failure on one wallet disables mixing for all of them. + static int nWalletBackups; + static int nMaxWalletBackups; /** Registered interfaces::Chain::Notifications handler. */ std::unique_ptr m_chain_notifications_handler; @@ -972,7 +1002,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void postInitProcess(); /* AutoBackup functionality */ - static void InitAutoBackup(); + static void InitAutoBackup(const ArgsManager& args); bool AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error_string, std::vector& warnings); bool BackupWallet(const std::string& strDest) const; diff --git a/test/util/data/non-backported.txt b/test/util/data/non-backported.txt index 7692da21aa71..2eb7e3686b20 100644 --- a/test/util/data/non-backported.txt +++ b/test/util/data/non-backported.txt @@ -79,4 +79,5 @@ src/util/wpipe.* src/wallet/bip39* src/wallet/coinjoin.* src/wallet/hdchain.* +src/wallet/test/backup_tests.cpp src/hash_x11.h