diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 61d50f0a0b08..38553aced04c 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -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 \ diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 6474cbd708bb..0192ce5b7759 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -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); diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 161b148ea81d..e159c3f2aec1 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 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; diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 7f11fde0f9a8..380bc89766ce 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -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; @@ -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!") + "

" + @@ -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(); @@ -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); @@ -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("(" + tr("Disabled") + ")"); } walletModel->coinJoin()->stopMixing(); diff --git a/src/util/system.cpp b/src/util/system.cpp index d769dbd055be..3ef5c4e925fe 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -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"; diff --git a/src/util/system.h b/src/util/system.h index 08722d5f57bc..be5cfd24f056 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 UniValue; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 7bb836c40083..9837179659ed 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 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("-maxwalletbackups=", 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=", "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); diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index f6dd6f7a0a8b..0bbecd53c624 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -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 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..8ea85d3fd6cc --- /dev/null +++ b/src/wallet/test/backup_tests.cpp @@ -0,0 +1,302 @@ +#include +#include + +#include + +#include +#include +#include +#include + +namespace wallet { + +BOOST_FIXTURE_TEST_SUITE(backup_tests, WalletTestingSetup) + +BOOST_AUTO_TEST_CASE(time_based_exponential_retention) +{ + // Capture "now" once so all timestamps are relative to the same point + auto now = std::chrono::file_clock::now(); + + // Helper to create a timestamp N days ago from the captured "now" + auto make_time = [&now](int64_t days_ago) { + auto days_duration = std::chrono::duration>(days_ago); + return fs::file_time_type{now - days_duration}; + }; + + // Helper to create a dummy path + auto make_path = [](int i) { + return fs::u8path("backup_" + ToString(i) + ".dat"); + }; + + std::multimap backups; + + // Case 1: Less than nWalletBackups (10) + for (int i = 0; i < 5; ++i) { + backups.insert({make_time(i), make_path(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({make_time(i), make_path(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({make_time(0), make_path(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({make_time(0), make_path(i)}); + } + backups.insert({make_time(1), make_path(10)}); // [1,2) days + backups.insert({make_time(2), make_path(11)}); // [2,4) days + backups.insert({make_time(3), make_path(12)}); // [2,4) days + backups.insert({make_time(5), make_path(13)}); // [4,8) days + backups.insert({make_time(7), make_path(14)}); // [4,8) days + backups.insert({make_time(10), make_path(15)}); // [8,16) days + backups.insert({make_time(15), make_path(16)}); // [8,16) days + backups.insert({make_time(20), make_path(17)}); // [16,32) days + backups.insert({make_time(25), make_path(18)}); // [16,32) days + backups.insert({make_time(30), make_path(19)}); // [16,32) days + + to_delete = GetBackupsToDelete(backups, 10, 50); + + // Should keep: + // - Latest 10 (by count, indices 0-9): backup_0 through backup_9 + // - Oldest in [1,2): backup_10 (1 day) + // - Oldest in [2,4): backup_12 (3 days) + // - Oldest in [4,8): backup_14 (7 days) + // - Oldest in [8,16): backup_16 (15 days) + // - Oldest in [16,32): backup_19 (30 days) + // Total: 15 kept, 5 deleted + BOOST_CHECK_EQUAL(to_delete.size(), 5); + + // Verify exactly which backups are deleted + std::set expected_deletions = { + "backup_11.dat", // 2 days, not oldest in [2,4) + "backup_13.dat", // 5 days, not oldest in [4,8) + "backup_15.dat", // 10 days, not oldest in [8,16) + "backup_17.dat", // 20 days, not oldest in [16,32) + "backup_18.dat" // 25 days, not oldest in [16,32) + }; + std::set actual_deletions; + for (const auto& path : to_delete) { + actual_deletions.insert(fs::PathToString(path.filename())); + } + 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({make_time(i), make_path(i)}); + } + + to_delete = GetBackupsToDelete(backups, 10, 50); + + // Should keep: + // - Latest 10 (by count): backup_0 through backup_9 + // - Oldest in [1,2): backup_1 is 1 day (already in latest 10) + // - Oldest in [2,4): backup_3 is 3 days (already in latest 10) + // - Oldest in [4,8): backup_7 is 7 days (already in latest 10) + // - Oldest in [8,16): backup_15 (15 days) + // - Oldest in [16,32): backup_31 (31 days) + // - Oldest in [32,64): backup_63 (63 days) + // - Oldest in [64,128): backup_99 (99 days) + // Total: 14 kept, 86 deleted + BOOST_CHECK_EQUAL(to_delete.size(), 86); + + // Verify specific kept backups in exponential ranges + std::set expected_kept = { + "backup_0.dat", "backup_1.dat", "backup_2.dat", "backup_3.dat", "backup_4.dat", + "backup_5.dat", "backup_6.dat", "backup_7.dat", "backup_8.dat", "backup_9.dat", + "backup_15.dat", "backup_31.dat", "backup_63.dat", "backup_99.dat" + }; + 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(fs::PathToString(path.filename())); + } + } + BOOST_CHECK(expected_kept == actual_kept); +} + +BOOST_AUTO_TEST_CASE(hard_max_limit) +{ + auto now = std::chrono::file_clock::now(); + auto make_time = [&now](int64_t days_ago) { + auto days_duration = std::chrono::duration>(days_ago); + return fs::file_time_type{now - days_duration}; + }; + auto make_path = [](int i) { + return fs::u8path("backup_" + ToString(i) + ".dat"); + }; + + std::multimap backups; + + // Create 100 daily backups and set maxBackups=15 + for (int i = 0; i < 100; ++i) { + backups.insert({make_time(i), make_path(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 = { + "backup_0.dat", "backup_1.dat", "backup_2.dat", "backup_3.dat", "backup_4.dat", + "backup_5.dat", "backup_6.dat", "backup_7.dat", "backup_8.dat", "backup_9.dat", + "backup_15.dat", "backup_31.dat", "backup_63.dat", "backup_99.dat" + }; + 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(fs::PathToString(path.filename())); + } + } + 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 = { + "backup_0.dat", "backup_1.dat", "backup_2.dat", "backup_3.dat", "backup_4.dat", + "backup_5.dat", "backup_6.dat", "backup_7.dat", "backup_8.dat", "backup_9.dat", + "backup_15.dat", "backup_31.dat" + }; + 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(fs::PathToString(path.filename())); + } + } + BOOST_CHECK(expected_kept_12 == actual_kept_12); +} + +BOOST_AUTO_TEST_CASE(irregular_backup_schedule) +{ + auto now = std::chrono::file_clock::now(); + auto make_time = [&now](int64_t days_ago) { + auto days_duration = std::chrono::duration>(days_ago); + return fs::file_time_type{now - days_duration}; + }; + auto make_path = [](int i) { + return fs::u8path("backup_" + ToString(i) + ".dat"); + }; + + 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({make_time(0), make_path(i)}); + } + // Day 1: 3 backups + for (int i = 5; i < 8; ++i) { + backups.insert({make_time(1), make_path(i)}); + } + // Day 2: 2 backups + for (int i = 8; i < 10; ++i) { + backups.insert({make_time(2), make_path(i)}); + } + // Day 10: 1 backup (gap) + backups.insert({make_time(10), make_path(10)}); + // Day 20: 1 backup (gap) + backups.insert({make_time(20), make_path(11)}); + + 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 = { + "backup_0.dat", "backup_1.dat", "backup_2.dat", "backup_3.dat", "backup_4.dat", + "backup_5.dat", "backup_6.dat", "backup_7.dat", "backup_8.dat", "backup_9.dat", + "backup_10.dat", "backup_11.dat" + }; + std::set actual_kept; + for (const auto& [time, path] : backups) { + actual_kept.insert(fs::PathToString(path.filename())); + } + BOOST_CHECK(expected_kept == actual_kept); +} + +BOOST_AUTO_TEST_CASE(long_inactivity_period) +{ + auto now = std::chrono::file_clock::now(); + auto make_time = [&now](int64_t days_ago) { + auto days_duration = std::chrono::duration>(days_ago); + return fs::file_time_type{now - days_duration}; + }; + auto make_path = [](int i) { + return fs::u8path("backup_" + ToString(i) + ".dat"); + }; + + std::multimap backups; + + // 15 backups created 60 days ago, then nothing until today + for (int i = 0; i < 15; ++i) { + backups.insert({make_time(60), make_path(i)}); + } + // New backup today + backups.insert({make_time(0), make_path(15)}); + + 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 + // Sorted order: backup_15 (0 days), then backup_14-0 in reverse insertion order (all 60 days) + // Latest 10 by count: backup_15, backup_14, backup_13, ..., backup_6 + // Oldest in [32,64): All backups 0-14 are 60 days old, backup_0 is oldest by insertion order + std::set expected_kept = { + "backup_0.dat", // oldest in [32,64) range + "backup_6.dat", "backup_7.dat", "backup_8.dat", "backup_9.dat", + "backup_10.dat", "backup_11.dat", "backup_12.dat", "backup_13.dat", "backup_14.dat", + "backup_15.dat" // newest + }; + 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(fs::PathToString(path.filename())); + } + } + BOOST_CHECK(expected_kept == actual_kept); +} + +BOOST_AUTO_TEST_SUITE_END() + +} // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 773d1dd6cc7d..481b19e3bba9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -56,6 +56,13 @@ using interfaces::FoundBlock; namespace wallet { +// Static wallet backup configuration +// Ensure compile-time invariant: nWalletBackups <= nMaxWalletBackups when nMaxWalletBackups > 0 +static_assert(DEFAULT_MAX_BACKUPS == 0 || DEFAULT_N_WALLET_BACKUPS <= DEFAULT_MAX_BACKUPS, + "DEFAULT_N_WALLET_BACKUPS must not exceed DEFAULT_MAX_BACKUPS when DEFAULT_MAX_BACKUPS > 0"); +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 " @@ -3489,13 +3496,104 @@ void CWallet::postInitProcess() chain().requestMempoolTransactions(*this); } -void CWallet::InitAutoBackup() +std::vector GetBackupsToDelete(const std::multimap& backups, int nWalletBackups, int maxBackups) +{ + std::vector paths_to_delete; + + // Early guards + if (maxBackups <= 0) { + return paths_to_delete; + } + + if (backups.size() <= (size_t)nWalletBackups) { + return paths_to_delete; + } + + // Sort by time descending (newest first) + std::vector> sorted_backups(backups.rbegin(), backups.rend()); + + std::set indices_to_keep; + + // Always keep the latest nWalletBackups (by count, not time) + for (size_t i = 0; i < std::min(sorted_backups.size(), (size_t)nWalletBackups); ++i) { + indices_to_keep.insert(i); + } + + // For older backups (beyond the latest nWalletBackups), apply time-based exponential retention + if (sorted_backups.size() > (size_t)nWalletBackups) { + auto now = sorted_backups[0].first; // newest backup time + + using days = std::chrono::duration>; + + // Define exponential day ranges: [1,2), [2,4), [4,8), [8,16), [16,32), [32,64), etc. + // We start from 1 day because the latest nWalletBackups are already kept + int range_start_days = 1; + int range_end_days = 2; + + // Process exponential time ranges until we hit maxBackups limit or run out of old backups + while (indices_to_keep.size() < (size_t)maxBackups) { + std::optional oldest_in_range; + + // Search through backups beyond the latest nWalletBackups + for (size_t i = nWalletBackups; i < sorted_backups.size(); ++i) { + auto age_duration = now - sorted_backups[i].first; + auto age_days = std::chrono::duration_cast(age_duration).count(); + + if (age_days >= range_start_days && age_days < range_end_days) { + // Keep searching - we want the oldest (highest index) in this range + oldest_in_range = i; + } + } + + if (oldest_in_range.has_value()) { + indices_to_keep.insert(*oldest_in_range); + } else { + // No backups in this range, check if there are any older backups left + bool has_older_backups = false; + for (size_t i = nWalletBackups; i < sorted_backups.size(); ++i) { + auto age_duration = now - sorted_backups[i].first; + auto age_days = std::chrono::duration_cast(age_duration).count(); + if (age_days >= range_end_days) { + has_older_backups = true; + break; + } + } + if (!has_older_backups) { + break; // No more old backups to consider + } + } + + // Move to next exponential range + range_start_days = range_end_days; + range_end_days *= 2; + } + } + + // Build deletion list + for (size_t i = 0; i < sorted_backups.size(); ++i) { + if (indices_to_keep.find(i) == indices_to_keep.end()) { + paths_to_delete.push_back(sorted_backups[i].second); + } + } + + return paths_to_delete; +} + +void CWallet::InitAutoBackup(const ArgsManager& args) { - if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) + 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 @@ -3627,20 +3725,15 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error } } - // 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; } } @@ -3848,7 +3941,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 0544840047ee..96e8742b60c9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -82,6 +82,21 @@ 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 = DEFAULT_MAX_BACKUPS / 3; +static constexpr int MAX_N_WALLET_BACKUPS = 20; + +/** + * Determine which backup files to delete based on retention policy. + * Keeps the latest nWalletBackups by count. + * For older backups, keeps the oldest backup in each exponential time range (in days): + * [1,2), [2,4), [4,8), [8,16), [16,32), [32,64), etc. + * This allows backups to accumulate naturally over time with exponential spacing. + * Enforces a hard limit of maxBackups. + */ +std::vector GetBackupsToDelete(const std::multimap& backups, int nWalletBackups, int maxBackups = DEFAULT_MAX_BACKUPS); + //! -paytxfee default constexpr CAmount DEFAULT_PAY_TX_FEE = 0; //! -fallbackfee default @@ -507,6 +522,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati std::set setLockedCoins GUARDED_BY(cs_wallet); int64_t nKeysLeftSinceAutoBackup; + static int nWalletBackups; + static int nMaxWalletBackups; /** Registered interfaces::Chain::Notifications handler. */ std::unique_ptr m_chain_notifications_handler; @@ -929,7 +946,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void postInitProcess(); /* AutoBackup functionality */ - static void InitAutoBackup(); + static void InitAutoBackup(const ArgsManager& args = gArgs); bool AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error_string, std::vector& warnings); bool BackupWallet(const std::string& strDest) const;