From 7e612ee3313457caec0c0f1d8c668ffd63bd01cb Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 6 May 2021 23:34:58 -0300
Subject: [PATCH] Walletdb: unify wallet backups cleanup.

---
 src/wallet/walletdb.cpp | 69 +++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 41 deletions(-)

diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index e85c0a51519032..5e6769ceff4feb 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -889,6 +889,27 @@ static folder_set_t buildBackupsMapSortedByLastWrite(const std::string& strWalle
     return folder_set;
 }
 
+static bool cleanWalletBackups(folder_set_t& folder_set, int nWalletBackups, std::string& strBackupWarning)
+{
+    // Loop backward through backup files and keep the N newest ones (1 <= N <= 10)
+    int counter = 0;
+    for (const std::pair<const std::time_t, fs::path>& file : reverse_iterate(folder_set)) {
+        counter++;
+        if (counter > nWalletBackups) {
+            // More than nWalletBackups backups: delete oldest one(s)
+            try {
+                fs::remove(file.second);
+                LogPrintf("Old backup deleted: %s\n", file.second);
+            } catch (fs::filesystem_error &error) {
+                strBackupWarning = strprintf(_("Failed to delete backup, error: %s"), error.what());
+                LogPrintf("%s\n", strBackupWarning);
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
 bool AutoBackupWallet(const std::string& strWalletFile, std::string& strBackupWarning, std::string& strBackupError)
 {
     strBackupWarning = strBackupError = "";
@@ -927,60 +948,26 @@ bool AutoBackupWallet(const std::string& strWalletFile, std::string& strBackupWa
 
     // Keep only the last 10 backups, including the new one of course
     folder_set_t folder_set = buildBackupsMapSortedByLastWrite(strWalletFile, backupsDir);
-
-    // Loop backward through backup files and keep the N newest ones (1 <= N <= 10)
-    int counter = 0;
-    for (const std::pair<const std::time_t, fs::path>& file : reverse_iterate(folder_set)) {
-        counter++;
-        if (counter > nWalletBackups) {
-            // More than nWalletBackups backups: delete oldest one(s)
-            try {
-                fs::remove(file.second);
-                LogPrintf("Old backup deleted: %s\n", file.second);
-            } catch (fs::filesystem_error &error) {
-                strBackupWarning = strprintf(_("Failed to delete backup, error: %s"), error.what());
-                LogPrintf("%s\n", strBackupWarning);
-                return false;
-            }
-        }
-    }
-    return true;
+    return cleanWalletBackups(folder_set, nWalletBackups, strBackupWarning);
 }
 
 void MultiBackup(const CWallet& wallet, fs::path pathCustom, fs::path pathWithFile, const fs::path& pathSrc)
 {
     int nThreshold = gArgs.GetArg("-custombackupthreshold", DEFAULT_CUSTOMBACKUPTHRESHOLD);
     if (nThreshold > 0) {
+        std::string strBackupWarning;
         pathCustom.make_preferred();
         folder_set_t folderSet = buildBackupsMapSortedByLastWrite(wallet.GetDBHandle().GetName(), pathCustom);
+        if (!cleanWalletBackups(folderSet, nThreshold, strBackupWarning)) {
+            NotifyBacked(wallet, false, strBackupWarning);
+        }
 
-        int counter = 0; //TODO: add seconds to avoid naming conflicts
-        for (auto entry : folderSet) {
-            counter++;
+        // TODO: add seconds to avoid naming conflicts
+        for (const auto& entry : folderSet) {
             if(entry.second == pathWithFile) {
                 pathWithFile += "(1)";
             }
         }
-
-        if (counter >= nThreshold) {
-            std::time_t oldestBackup = 0;
-            for(auto entry : folderSet) {
-                if(oldestBackup == 0 || entry.first < oldestBackup) {
-                    oldestBackup = entry.first;
-                }
-            }
-
-            try {
-                auto entry = folderSet.find(oldestBackup);
-                if (entry != folderSet.end()) {
-                    fs::remove(entry->second);
-                    LogPrintf("Old backup deleted: %s\n", (*entry).second);
-                }
-            } catch (const fs::filesystem_error& error) {
-                std::string strMessage = strprintf("Failed to delete backup %s\n", error.what());
-                NotifyBacked(wallet, false, strMessage);
-            }
-        }
     }
     AttemptBackupWallet(&wallet, pathSrc.string(), pathWithFile.string());
 }