wallet/db: keep the BDB logs when the shutdown flush fails

CDBEnv::Flush(true) removed database/ (log_archive DB_ARCH_REMOVE, then
remove_all) once mapFileUseCount was empty. It did that even when the
checkpoint or lsn_reset before it had failed, or the environment had
panicked (BDB0060). In those states wallet.dat can be missing committed
changes that only the logs hold:
- a page write failed (EBADF/ESPIPE through an fd closed behind BDB's
  back, the wolfSSL CloseSocket bug), so the page stayed dirty in the
  mpool cache and the checkpoint never completed;
- pages written after the last lsn_reset carry real LSNs that point
  into those logs.
Deleting the logs turned a wallet that the next start's DB_RECOVER would
repair into one that silently lost those changes. A std::terminate kept
the logs; the clean shutdown that ThreadNotifyWallets now does on a
wallet DB error deleted them.

Remember a failed txn_checkpoint or lsn_reset (in Flush and in
CheckpointLSN, which the periodic flush and backupwallet use). Before
removing anything, also check the environment for a panic. get_flags
reports it, and itself fails with DB_RUNRECOVERY once the environment
has panicked. In any of those cases keep database/ and log why. If the
kept logs are unusable, the next start already moves database/ aside
and retries (CWallet::Verify).

WalletDbEnv tests:
- a clean shutdown still removes the logs;
- a panicked environment (DB_PANIC_ENVIRONMENT) keeps them;
- a failed lsn_reset keeps them.
With the old db.cpp only the first passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/Makefile.gtest.include

@@ -17,7 +17,8 @@ hush_gtest_SOURCES = \
 	gtest/test_stratum_jobid.cpp \
 	gtest/test_wallet_notedata.cpp \
 	gtest/test_net_closesocket.cpp \
-	gtest/test_fd_defense.cpp
+	gtest/test_fd_defense.cpp \
+	gtest/test_wallet_dbenv.cpp
 
 hush_gtest_CPPFLAGS = $(AM_CPPFLAGS) -DMULTICORE -fopenmp -DBINARY_OUTPUT -DCURVE_ALT_BN128 -DSTATIC $(BITCOIN_INCLUDES)
 hush_gtest_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)

src/gtest/test_wallet_dbenv.cpp

@@ -0,0 +1,58 @@
+// Copyright (c) 2016-2024 The Hush developers
+// Distributed under the GPLv3 software license, see the accompanying
+// file COPYING or https://www.gnu.org/licenses/gpl-3.0.en.html
+
+#include <gtest/gtest.h>
+
+#include <boost/filesystem.hpp>
+
+#include "wallet/db.h"
+
+namespace {
+
+// A fresh environment in a temp dir with one committed record in "wallet.dat", so database/ holds a log file.
+class WalletDbEnv : public ::testing::Test {
+protected:
+    boost::filesystem::path dir;
+    CDBEnv env;
+
+    void SetUp() override {
+        dir = boost::filesystem::temp_directory_path() / boost::filesystem::unique_path("dbenv-%%%%%%%%");
+        boost::filesystem::create_directories(dir);
+        ASSERT_TRUE(env.Open(dir));
+        Db db(env.dbenv, DB_CXX_NO_EXCEPTIONS);
+        ASSERT_EQ(0, db.open(NULL, "wallet.dat", "main", DB_BTREE, DB_CREATE | DB_AUTO_COMMIT, 0));
+        char k[] = "key", v[] = "value";
+        Dbt key(k, sizeof k), value(v, sizeof v);
+        ASSERT_EQ(0, db.put(NULL, &key, &value, 0));
+        ASSERT_EQ(0, db.close(0));
+        ASSERT_TRUE(boost::filesystem::exists(Log()));
+    }
+    void TearDown() override {
+        env.Close();
+        boost::system::error_code ec;
+        boost::filesystem::remove_all(dir, ec);
+    }
+    boost::filesystem::path Log() const { return dir / "database" / "log.0000000001"; }
+};
+
+} // namespace
+
+TEST_F(WalletDbEnv, CleanShutdownRemovesTheLogs) {
+    env.Flush(true);
+    EXPECT_FALSE(boost::filesystem::exists(dir / "database"));
+}
+
+// BDB0060 PANIC (a page write failed, e.g. through an fd closed behind BDB's back): the logs may hold committed
+// changes wallet.dat never got, and the next start's DB_RECOVER needs them.
+TEST_F(WalletDbEnv, PanickedEnvironmentKeepsTheLogs) {
+    ASSERT_EQ(0, env.dbenv->set_flags(DB_PANIC_ENVIRONMENT, 1));
+    env.Flush(true);
+    EXPECT_TRUE(boost::filesystem::exists(Log()));
+}
+
+TEST_F(WalletDbEnv, FailedCheckpointKeepsTheLogs) {
+    env.CheckpointLSN("no-such-file.dat");   // lsn_reset fails
+    env.Flush(true);
+    EXPECT_TRUE(boost::filesystem::exists(Log()));
+}

src/wallet/db.cpp

@@ -61,6 +61,7 @@ void CDBEnv::Reset()
     dbenv = new DbEnv(DB_CXX_NO_EXCEPTIONS);
     fDbEnvInit = false;
     fMockDb = false;
+    fKeepLogs = false;
 }
 
 CDBEnv::CDBEnv() : dbenv(NULL)
@@ -276,10 +277,12 @@ bool CDBEnv::Salvage(const std::string& strFile, bool fAggressive, std::vector<C
 
 void CDBEnv::CheckpointLSN(const std::string& strFile)
 {
-    dbenv->txn_checkpoint(0, 0, 0);
+    if (dbenv->txn_checkpoint(0, 0, 0) != 0)
+        fKeepLogs = true;
     if (fMockDb)
         return;
-    dbenv->lsn_reset(strFile.c_str(), 0);
+    if (dbenv->lsn_reset(strFile.c_str(), 0) != 0)
+        fKeepLogs = true;
 }
 
 
@@ -498,10 +501,16 @@ void CDBEnv::Flush(bool fShutdown)
                 // Move log data to the dat file
                 CloseDb(strFile);
                 LogPrint("db", "CDBEnv::Flush: %s checkpoint\n", strFile);
-                dbenv->txn_checkpoint(0, 0, 0);
+                int ret = dbenv->txn_checkpoint(0, 0, 0);
+                if (ret != 0) {
+                    LogPrintf("CDBEnv::Flush: %s checkpoint failed: %s\n", strFile, DbEnv::strerror(ret));
+                    fKeepLogs = true;
+                }
                 LogPrint("db", "CDBEnv::Flush: %s detach\n", strFile);
-                if (!fMockDb)
+                if (!fMockDb && (ret = dbenv->lsn_reset(strFile.c_str(), 0)) != 0) {
-                    dbenv->lsn_reset(strFile.c_str(), 0);
+                    LogPrintf("CDBEnv::Flush: %s lsn_reset failed: %s\n", strFile, DbEnv::strerror(ret));
+                    fKeepLogs = true;
+                }
                 LogPrint("db", "CDBEnv::Flush: %s closed\n", strFile);
                 mapFileUseCount.erase(mi++);
             } else
@@ -511,10 +520,24 @@ void CDBEnv::Flush(bool fShutdown)
         if (fShutdown) {
             char** listp;
             if (mapFileUseCount.empty()) {
-                dbenv->log_archive(&listp, DB_ARCH_REMOVE);
+                // After a failed flush, or once the environment has panicked (BDB0060, e.g. after a page
-                Close();
+                // write failed), wallet.dat can be missing committed changes that only the logs hold.
-                if (!fMockDb)
+                // Removing them would turn a wallet the next start recovers (DB_RECOVER) into one that
-                    boost::filesystem::remove_all(boost::filesystem::path(strPath) / "database");
+                // silently lost them. If the logs are unusable, the next start moves database/ aside.
+                // (get_flags itself fails with DB_RUNRECOVERY once the environment has panicked.)
+                u_int32_t envFlags = 0;
+                if (dbenv->get_flags(&envFlags) != 0 || (envFlags & DB_PANIC_ENVIRONMENT))
+                    fKeepLogs = true;
+                if (fKeepLogs && !fMockDb) {
+                    LogPrintf("CDBEnv::Flush: the wallet database was not flushed cleanly; keeping %s for recovery at the next start\n",
+                              (boost::filesystem::path(strPath) / "database").string());
+                    Close();
+                } else {
+                    dbenv->log_archive(&listp, DB_ARCH_REMOVE);
+                    Close();
+                    if (!fMockDb)
+                        boost::filesystem::remove_all(boost::filesystem::path(strPath) / "database");
+                }
             }
         }
     }

src/wallet/db.h

@@ -28,6 +28,7 @@
 #include "sync.h"
 #include "version.h"
 
+#include <atomic>
 #include <map>
 #include <string>
 #include <vector>
@@ -53,6 +54,9 @@ class CDBEnv
 private:
     bool fDbEnvInit;
     bool fMockDb;
+    // A checkpoint or lsn_reset failed: the logs in database/ may hold the only copy of committed
+    // wallet changes, so the shutdown flush must leave them for DB_RECOVER at the next start.
+    std::atomic<bool> fKeepLogs;
     // Don't change into boost::filesystem::path, as that can result in
     // shutdown problems/crashes caused by a static initialized internal pointer.
     std::string strPath;