From 33c6245ac1ecdfe25b1ee4fd9e93c43393634ae3 Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Mon, 12 Dec 2022 14:54:01 -0500
Subject: [PATCH 1/8] Introduce MockableDatabase for wallet unit tests

MockableDatabase is a WalletDatabase that allows us to interact with the
records to change them independently from the wallet, as well as
changing the return values from within the tests. This will give us
greater flexibility in testing the wallet.
---
 .../libtest_util/libtest_util.vcxproj.in      |  1 +
 src/wallet/test/util.cpp                      | 90 +++++++++++++++++++
 src/wallet/test/util.h                        | 74 +++++++++++++++
 src/wallet/test/wallet_tests.cpp              | 53 +----------
 4 files changed, 167 insertions(+), 51 deletions(-)

diff --git a/build_msvc/libtest_util/libtest_util.vcxproj.in b/build_msvc/libtest_util/libtest_util.vcxproj.in
index b5e844010e9..64cfa82dccd 100644
--- a/build_msvc/libtest_util/libtest_util.vcxproj.in
+++ b/build_msvc/libtest_util/libtest_util.vcxproj.in
@@ -8,6 +8,7 @@
     <ConfigurationType>StaticLibrary</ConfigurationType>
   </PropertyGroup>
   <ItemGroup>
+    <ClCompile Include="..\..\src\wallet\test\util.cpp" />
 @SOURCE_FILES@
   </ItemGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index b7bf312edf1..7d0a814ed44 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -7,6 +7,7 @@
 #include <chain.h>
 #include <key.h>
 #include <key_io.h>
+#include <streams.h>
 #include <test/util/setup_common.h>
 #include <wallet/wallet.h>
 #include <wallet/walletdb.h>
@@ -79,4 +80,93 @@ CTxDestination getNewDestination(CWallet& w, OutputType output_type)
     return *Assert(w.GetNewDestination(output_type, ""));
 }
 
+DatabaseCursor::Status MockableCursor::Next(DataStream& key, DataStream& value)
+{
+    if (!m_pass) {
+        return Status::FAIL;
+    }
+    if (m_cursor == m_cursor_end) {
+        return Status::DONE;
+    }
+    const auto& [key_data, value_data] = *m_cursor;
+    key.write(key_data);
+    value.write(value_data);
+    m_cursor++;
+    return Status::MORE;
+}
+
+bool MockableBatch::ReadKey(DataStream&& key, DataStream& value)
+{
+    if (!m_pass) {
+        return false;
+    }
+    SerializeData key_data{key.begin(), key.end()};
+    const auto& it = m_records.find(key_data);
+    if (it == m_records.end()) {
+        return false;
+    }
+    value.write(it->second);
+    return true;
+}
+
+bool MockableBatch::WriteKey(DataStream&& key, DataStream&& value, bool overwrite)
+{
+    if (!m_pass) {
+        return false;
+    }
+    SerializeData key_data{key.begin(), key.end()};
+    SerializeData value_data{value.begin(), value.end()};
+    auto [it, inserted] = m_records.emplace(key_data, value_data);
+    if (!inserted && overwrite) { // Overwrite if requested
+        it->second = value_data;
+        inserted = true;
+    }
+    return inserted;
+}
+
+bool MockableBatch::EraseKey(DataStream&& key)
+{
+    if (!m_pass) {
+        return false;
+    }
+    SerializeData key_data{key.begin(), key.end()};
+    m_records.erase(key_data);
+    return true;
+}
+
+bool MockableBatch::HasKey(DataStream&& key)
+{
+    if (!m_pass) {
+        return false;
+    }
+    SerializeData key_data{key.begin(), key.end()};
+    return m_records.count(key_data) > 0;
+}
+
+bool MockableBatch::ErasePrefix(Span<const std::byte> prefix)
+{
+    if (!m_pass) {
+        return false;
+    }
+    auto it = m_records.begin();
+    while (it != m_records.end()) {
+        auto& key = it->first;
+        if (key.size() < prefix.size() || std::search(key.begin(), key.end(), prefix.begin(), prefix.end()) != key.begin()) {
+            it++;
+            continue;
+        }
+        it = m_records.erase(it);
+    }
+    return true;
+}
+
+std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(std::map<SerializeData, SerializeData> records)
+{
+    return std::make_unique<MockableDatabase>(records);
+}
+
+MockableDatabase& GetMockableDatabase(CWallet& wallet)
+{
+    return dynamic_cast<MockableDatabase&>(wallet.GetDatabase());
+}
 } // namespace wallet
diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h
index d726517e211..92405107cf2 100644
--- a/src/wallet/test/util.h
+++ b/src/wallet/test/util.h
@@ -6,6 +6,8 @@
 #define BITCOIN_WALLET_TEST_UTIL_H
 
 #include <script/standard.h>
+#include <wallet/db.h>
+
 #include <memory>
 
 class ArgsManager;
@@ -31,6 +33,78 @@ std::string getnewaddress(CWallet& w);
 /** Returns a new destination, of an specific type, from the wallet */
 CTxDestination getNewDestination(CWallet& w, OutputType output_type);
 
+class MockableCursor: public DatabaseCursor
+{
+public:
+    std::map<SerializeData, SerializeData>::const_iterator m_cursor;
+    std::map<SerializeData, SerializeData>::const_iterator m_cursor_end;
+    bool m_pass;
+
+    explicit MockableCursor(const std::map<SerializeData, SerializeData>& records, bool pass) : m_cursor(records.begin()), m_cursor_end(records.end()), m_pass(pass) {}
+    ~MockableCursor() {}
+
+    Status Next(DataStream& key, DataStream& value) override;
+};
+
+class MockableBatch : public DatabaseBatch
+{
+private:
+    std::map<SerializeData, SerializeData>& m_records;
+    bool m_pass;
+
+    bool ReadKey(DataStream&& key, DataStream& value) override;
+    bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite=true) override;
+    bool EraseKey(DataStream&& key) override;
+    bool HasKey(DataStream&& key) override;
+    bool ErasePrefix(Span<const std::byte> prefix) override;
+
+public:
+    explicit MockableBatch(std::map<SerializeData, SerializeData>& records, bool pass) : m_records(records), m_pass(pass) {}
+    ~MockableBatch() {}
+
+    void Flush() override {}
+    void Close() override {}
+
+    std::unique_ptr<DatabaseCursor> GetNewCursor() override
+    {
+        return std::make_unique<MockableCursor>(m_records, m_pass);
+    }
+    bool TxnBegin() override { return m_pass; }
+    bool TxnCommit() override { return m_pass; }
+    bool TxnAbort() override { return m_pass; }
+};
+
+/** A WalletDatabase whose contents and return values can be modified as needed for testing
+ **/
+class MockableDatabase : public WalletDatabase
+{
+public:
+    std::map<SerializeData, SerializeData> m_records;
+    bool m_pass{true};
+
+    MockableDatabase(std::map<SerializeData, SerializeData> records = {}) : WalletDatabase(), m_records(records) {}
+    ~MockableDatabase() {};
+
+    void Open() override {}
+    void AddRef() override {}
+    void RemoveRef() override {}
+
+    bool Rewrite(const char* pszSkip=nullptr) override { return m_pass; }
+    bool Backup(const std::string& strDest) const override { return m_pass; }
+    void Flush() override {}
+    void Close() override {}
+    bool PeriodicFlush() override { return m_pass; }
+    void IncrementUpdateCounter() override {}
+    void ReloadDbEnv() override {}
+
+    std::string Filename() override { return "mockable"; }
+    std::string Format() override { return "mock"; }
+    std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override { return std::make_unique<MockableBatch>(m_records, m_pass); }
+};
+
+std::unique_ptr<WalletDatabase> CreateMockableWalletDatabase(std::map<SerializeData, SerializeData> records = {});
+
+MockableDatabase& GetMockableDatabase(CWallet& wallet);
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_TEST_UTIL_H
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index da4b553394c..9cf045f0655 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -951,62 +951,13 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup)
     TestUnloadWallet(std::move(wallet));
 }
 
-class FailCursor : public DatabaseCursor
-{
-public:
-    Status Next(DataStream& key, DataStream& value) override { return Status::FAIL; }
-};
-
-/** RAII class that provides access to a FailDatabase. Which fails if needed. */
-class FailBatch : public DatabaseBatch
-{
-private:
-    bool m_pass{true};
-    bool ReadKey(DataStream&& key, DataStream& value) override { return m_pass; }
-    bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite = true) override { return m_pass; }
-    bool EraseKey(DataStream&& key) override { return m_pass; }
-    bool HasKey(DataStream&& key) override { return m_pass; }
-    bool ErasePrefix(Span<const std::byte> prefix) override { return m_pass; }
-
-public:
-    explicit FailBatch(bool pass) : m_pass(pass) {}
-    void Flush() override {}
-    void Close() override {}
-
-    std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<FailCursor>(); }
-    bool TxnBegin() override { return false; }
-    bool TxnCommit() override { return false; }
-    bool TxnAbort() override { return false; }
-};
-
-/** A dummy WalletDatabase that does nothing, only fails if needed.**/
-class FailDatabase : public WalletDatabase
-{
-public:
-    bool m_pass{true}; // false when this db should fail
-
-    void Open() override {};
-    void AddRef() override {}
-    void RemoveRef() override {}
-    bool Rewrite(const char* pszSkip=nullptr) override { return true; }
-    bool Backup(const std::string& strDest) const override { return true; }
-    void Close() override {}
-    void Flush() override {}
-    bool PeriodicFlush() override { return true; }
-    void IncrementUpdateCounter() override { ++nUpdateCounter; }
-    void ReloadDbEnv() override {}
-    std::string Filename() override { return "faildb"; }
-    std::string Format() override { return "faildb"; }
-    std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override { return std::make_unique<FailBatch>(m_pass); }
-};
-
 /**
  * Checks a wallet invalid state where the inputs (prev-txs) of a new arriving transaction are not marked dirty,
  * while the transaction that spends them exist inside the in-memory wallet tx map (not stored on db due a db write failure).
  */
 BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup)
 {
-    CWallet wallet(m_node.chain.get(), "", std::make_unique<FailDatabase>());
+    CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
     {
         LOCK(wallet.cs_wallet);
         wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
@@ -1053,7 +1004,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup)
     // 1) Make db always fail
     // 2) Try to add a transaction that spends the previously created transaction and
     //    verify that we are not moving forward if the wallet cannot store it
-    static_cast<FailDatabase&>(wallet.GetDatabase()).m_pass = false;
+    GetMockableDatabase(wallet).m_pass = false;
     mtx.vin.clear();
     mtx.vin.push_back(CTxIn(good_tx_id, 0));
     BOOST_CHECK_EXCEPTION(wallet.transactionAddedToMempool(MakeTransactionRef(mtx)),

From f67a385556c60b2e4788a378196a395fca0539f5 Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Mon, 12 Dec 2022 15:40:48 -0500
Subject: [PATCH 2/8] wallet, tests: Replace usage of dummy db with mockable db

---
 src/bench/coin_selection.cpp              |  5 +-
 src/wallet/test/ismine_tests.cpp          | 77 ++++++++++++-----------
 src/wallet/test/scriptpubkeyman_tests.cpp |  3 +-
 src/wallet/test/wallet_tests.cpp          | 18 +++---
 4 files changed, 53 insertions(+), 50 deletions(-)

diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp
index 11ce14728de..0e110a653af 100644
--- a/src/bench/coin_selection.cpp
+++ b/src/bench/coin_selection.cpp
@@ -9,6 +9,7 @@
 #include <wallet/coinselection.h>
 #include <wallet/spend.h>
 #include <wallet/wallet.h>
+#include <wallet/test/util.h>
 
 #include <set>
 
@@ -20,7 +21,7 @@ using wallet::CWallet;
 using wallet::CWalletTx;
 using wallet::CoinEligibilityFilter;
 using wallet::CoinSelectionParams;
-using wallet::CreateDummyWalletDatabase;
+using wallet::CreateMockableWalletDatabase;
 using wallet::OutputGroup;
 using wallet::SelectCoinsBnB;
 using wallet::TxStateInactive;
@@ -46,7 +47,7 @@ static void CoinSelection(benchmark::Bench& bench)
 {
     NodeContext node;
     auto chain = interfaces::MakeChain(node);
-    CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
+    CWallet wallet(chain.get(), "", CreateMockableWalletDatabase());
     std::vector<std::unique_ptr<CWalletTx>> wtxs;
     LOCK(wallet.cs_wallet);
 
diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp
index f6f2d423e4b..fd0718fbb9c 100644
--- a/src/wallet/test/ismine_tests.cpp
+++ b/src/wallet/test/ismine_tests.cpp
@@ -10,6 +10,7 @@
 #include <test/util/setup_common.h>
 #include <wallet/types.h>
 #include <wallet/wallet.h>
+#include <wallet/test/util.h>
 
 #include <boost/test/unit_test.hpp>
 
@@ -55,7 +56,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2PK compressed - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         scriptPubKey = GetScriptForRawPubKey(pubkeys[0]);
@@ -74,7 +75,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2PK compressed - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "pk(" + EncodeSecret(keys[0]) + ")";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, true);
@@ -86,7 +87,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2PK uncompressed - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey);
@@ -105,7 +106,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2PK uncompressed - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "pk(" + EncodeSecret(uncompressedKey) + ")";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, true);
@@ -117,7 +118,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2PKH compressed - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         scriptPubKey = GetScriptForDestination(PKHash(pubkeys[0]));
@@ -136,7 +137,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2PKH compressed - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "pkh(" + EncodeSecret(keys[0]) + ")";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, true);
@@ -148,7 +149,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2PKH uncompressed - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         scriptPubKey = GetScriptForDestination(PKHash(uncompressedPubkey));
@@ -167,7 +168,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2PKH uncompressed - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "pkh(" + EncodeSecret(uncompressedKey) + ")";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, true);
@@ -179,7 +180,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2SH - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
 
@@ -206,7 +207,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2SH - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "sh(pkh(" + EncodeSecret(keys[0]) + "))";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, true);
@@ -219,7 +220,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // (P2PKH inside) P2SH inside P2SH (invalid) - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
 
@@ -238,7 +239,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // (P2PKH inside) P2SH inside P2SH (invalid) - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "sh(sh(" + EncodeSecret(keys[0]) + "))";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, false);
@@ -247,7 +248,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // (P2PKH inside) P2SH inside P2WSH (invalid) - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
 
@@ -266,7 +267,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // (P2PKH inside) P2SH inside P2WSH (invalid) - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "wsh(sh(" + EncodeSecret(keys[0]) + "))";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, false);
@@ -275,7 +276,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WPKH inside P2WSH (invalid) - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
 
@@ -292,7 +293,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WPKH inside P2WSH (invalid) - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "wsh(wpkh(" + EncodeSecret(keys[0]) + "))";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, false);
@@ -301,7 +302,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // (P2PKH inside) P2WSH inside P2WSH (invalid) - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
 
@@ -320,7 +321,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // (P2PKH inside) P2WSH inside P2WSH (invalid) - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "wsh(wsh(" + EncodeSecret(keys[0]) + "))";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, false);
@@ -329,7 +330,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WPKH compressed - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -345,7 +346,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WPKH compressed - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "wpkh(" + EncodeSecret(keys[0]) + ")";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, true);
@@ -357,7 +358,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WPKH uncompressed - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
@@ -378,7 +379,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WPKH uncompressed (invalid) - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "wpkh(" + EncodeSecret(uncompressedKey) + ")";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, false);
@@ -387,7 +388,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // scriptPubKey multisig - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
 
@@ -422,7 +423,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // scriptPubKey multisig - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         std::string desc_str = "multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + ")";
 
         auto spk_manager = CreateDescriptor(keystore, desc_str, true);
@@ -434,7 +435,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2SH multisig - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
@@ -457,7 +458,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2SH multisig - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
 
         std::string desc_str = "sh(multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + "))";
 
@@ -471,7 +472,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WSH multisig with compressed keys - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -500,7 +501,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WSH multisig with compressed keys - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
 
         std::string desc_str = "wsh(multi(2, " + EncodeSecret(keys[0]) + ", " + EncodeSecret(keys[1]) + "))";
 
@@ -514,7 +515,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WSH multisig with uncompressed key - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
@@ -543,7 +544,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WSH multisig with uncompressed key (invalid) - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
 
         std::string desc_str = "wsh(multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + "))";
 
@@ -553,7 +554,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WSH multisig wrapped in P2SH - Legacy
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
 
@@ -583,7 +584,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // P2WSH multisig wrapped in P2SH - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
 
         std::string desc_str = "sh(wsh(multi(2, " + EncodeSecret(keys[0]) + ", " + EncodeSecret(keys[1]) + ")))";
 
@@ -598,7 +599,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // Combo - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
 
         std::string desc_str = "combo(" + EncodeSecret(keys[0]) + ")";
 
@@ -642,7 +643,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // Taproot - Descriptor
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
 
         std::string desc_str = "tr(" + EncodeSecret(keys[0]) + ")";
 
@@ -660,7 +661,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // OP_RETURN
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -675,7 +676,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // witness unspendable
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -690,7 +691,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // witness unknown
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -705,7 +706,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
 
     // Nonstandard
     {
-        CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
+        CWallet keystore(chain.get(), "", CreateMockableWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
         LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
diff --git a/src/wallet/test/scriptpubkeyman_tests.cpp b/src/wallet/test/scriptpubkeyman_tests.cpp
index 90042f52526..d4997c418ab 100644
--- a/src/wallet/test/scriptpubkeyman_tests.cpp
+++ b/src/wallet/test/scriptpubkeyman_tests.cpp
@@ -7,6 +7,7 @@
 #include <test/util/setup_common.h>
 #include <wallet/scriptpubkeyman.h>
 #include <wallet/wallet.h>
+#include <wallet/test/util.h>
 
 #include <boost/test/unit_test.hpp>
 
@@ -18,7 +19,7 @@ BOOST_FIXTURE_TEST_SUITE(scriptpubkeyman_tests, BasicTestingSetup)
 BOOST_AUTO_TEST_CASE(CanProvide)
 {
     // Set up wallet and keyman variables.
-    CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
+    CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
     LegacyScriptPubKeyMan& keyman = *wallet.GetOrCreateLegacyScriptPubKeyMan();
 
     // Make a 1 of 2 multisig script
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 9cf045f0655..0329a2487e3 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -98,7 +98,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
 
     // Verify ScanForWalletTransactions fails to read an unknown start block.
     {
-        CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
+        CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
         {
             LOCK(wallet.cs_wallet);
             LOCK(Assert(m_node.chainman)->GetMutex());
@@ -164,7 +164,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
     // Verify ScanForWalletTransactions only picks transactions in the new block
     // file.
     {
-        CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
+        CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
         {
             LOCK(wallet.cs_wallet);
             LOCK(Assert(m_node.chainman)->GetMutex());
@@ -192,7 +192,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
 
     // Verify ScanForWalletTransactions scans no blocks.
     {
-        CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
+        CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
         {
             LOCK(wallet.cs_wallet);
             LOCK(Assert(m_node.chainman)->GetMutex());
@@ -232,7 +232,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
     // before the missing block, and success for a key whose creation time is
     // after.
     {
-        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
+        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateMockableWalletDatabase());
         wallet->SetupLegacyScriptPubKeyMan();
         WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash()));
         WalletContext context;
@@ -298,7 +298,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
     {
         WalletContext context;
         context.args = &m_args;
-        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
+        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateMockableWalletDatabase());
         {
             auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan();
             LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore);
@@ -321,7 +321,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
     // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME
     // were scanned, and no prior blocks were scanned.
     {
-        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
+        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateMockableWalletDatabase());
         LOCK(wallet->cs_wallet);
         wallet->SetupLegacyScriptPubKeyMan();
 
@@ -355,7 +355,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
 // debit functions.
 BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
 {
-    CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase());
+    CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
 
     LOCK(wallet.cs_wallet);
     LOCK(Assert(m_node.chainman)->GetMutex());
@@ -709,7 +709,7 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, ListCoinsTest)
 BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
 {
     {
-        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
+        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateMockableWalletDatabase());
         wallet->SetupLegacyScriptPubKeyMan();
         wallet->SetMinVersion(FEATURE_LATEST);
         wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
@@ -717,7 +717,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
         BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, ""));
     }
     {
-        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase());
+        const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateMockableWalletDatabase());
         LOCK(wallet->cs_wallet);
         wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
         wallet->SetMinVersion(FEATURE_LATEST);

From 14aa4cb1e44f089a6022a2b14a98bca4a7dd9a01 Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Mon, 12 Dec 2022 15:43:00 -0500
Subject: [PATCH 3/8] wallet: Move DummyDatabase to salvage

It's only used by salvage, so make it local to that only.
---
 src/wallet/db.h         | 45 ---------------------------------------
 src/wallet/salvage.cpp  | 47 ++++++++++++++++++++++++++++++++++++++++-
 src/wallet/walletdb.cpp |  6 ------
 src/wallet/walletdb.h   |  3 ---
 4 files changed, 46 insertions(+), 55 deletions(-)

diff --git a/src/wallet/db.h b/src/wallet/db.h
index 6834ba69632..b4ccd13a9ac 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -174,51 +174,6 @@ public:
     virtual std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) = 0;
 };
 
-class DummyCursor : public DatabaseCursor
-{
-    Status Next(DataStream& key, DataStream& value) override { return Status::FAIL; }
-};
-
-/** RAII class that provides access to a DummyDatabase. Never fails. */
-class DummyBatch : public DatabaseBatch
-{
-private:
-    bool ReadKey(DataStream&& key, DataStream& value) override { return true; }
-    bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite = true) override { return true; }
-    bool EraseKey(DataStream&& key) override { return true; }
-    bool HasKey(DataStream&& key) override { return true; }
-    bool ErasePrefix(Span<const std::byte> prefix) override { return true; }
-
-public:
-    void Flush() override {}
-    void Close() override {}
-
-    std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<DummyCursor>(); }
-    bool TxnBegin() override { return true; }
-    bool TxnCommit() override { return true; }
-    bool TxnAbort() override { return true; }
-};
-
-/** A dummy WalletDatabase that does nothing and never fails. Only used by unit tests.
- **/
-class DummyDatabase : public WalletDatabase
-{
-public:
-    void Open() override {};
-    void AddRef() override {}
-    void RemoveRef() override {}
-    bool Rewrite(const char* pszSkip=nullptr) override { return true; }
-    bool Backup(const std::string& strDest) const override { return true; }
-    void Close() override {}
-    void Flush() override {}
-    bool PeriodicFlush() override { return true; }
-    void IncrementUpdateCounter() override { ++nUpdateCounter; }
-    void ReloadDbEnv() override {}
-    std::string Filename() override { return "dummy"; }
-    std::string Format() override { return "dummy"; }
-    std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override { return std::make_unique<DummyBatch>(); }
-};
-
 enum class DatabaseFormat {
     BERKELEY,
     SQLITE,
diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp
index 06c8c8bb377..ab73e67285c 100644
--- a/src/wallet/salvage.cpp
+++ b/src/wallet/salvage.cpp
@@ -23,6 +23,51 @@ static bool KeyFilter(const std::string& type)
     return WalletBatch::IsKeyType(type) || type == DBKeys::HDCHAIN;
 }
 
+class DummyCursor : public DatabaseCursor
+{
+    Status Next(DataStream& key, DataStream& value) override { return Status::FAIL; }
+};
+
+/** RAII class that provides access to a DummyDatabase. Never fails. */
+class DummyBatch : public DatabaseBatch
+{
+private:
+    bool ReadKey(DataStream&& key, DataStream& value) override { return true; }
+    bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite=true) override { return true; }
+    bool EraseKey(DataStream&& key) override { return true; }
+    bool HasKey(DataStream&& key) override { return true; }
+    bool ErasePrefix(Span<const std::byte> prefix) override { return true; }
+
+public:
+    void Flush() override {}
+    void Close() override {}
+
+    std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<DummyCursor>(); }
+    bool TxnBegin() override { return true; }
+    bool TxnCommit() override { return true; }
+    bool TxnAbort() override { return true; }
+};
+
+/** A dummy WalletDatabase that does nothing and never fails. Only used by salvage.
+ **/
+class DummyDatabase : public WalletDatabase
+{
+public:
+    void Open() override {};
+    void AddRef() override {}
+    void RemoveRef() override {}
+    bool Rewrite(const char* pszSkip=nullptr) override { return true; }
+    bool Backup(const std::string& strDest) const override { return true; }
+    void Close() override {}
+    void Flush() override {}
+    bool PeriodicFlush() override { return true; }
+    void IncrementUpdateCounter() override { ++nUpdateCounter; }
+    void ReloadDbEnv() override {}
+    std::string Filename() override { return "dummy"; }
+    std::string Format() override { return "dummy"; }
+    std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override { return std::make_unique<DummyBatch>(); }
+};
+
 bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings)
 {
     DatabaseOptions options;
@@ -135,7 +180,7 @@ bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bil
     }
 
     DbTxn* ptxn = env->TxnBegin();
-    CWallet dummyWallet(nullptr, "", CreateDummyWalletDatabase());
+    CWallet dummyWallet(nullptr, "", std::make_unique<DummyDatabase>());
     for (KeyValPair& row : salvagedData)
     {
         /* Filter for only private key type KV pairs to be added to the salvaged wallet */
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index 005592d7203..072357022e5 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -1263,12 +1263,6 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas
     return nullptr;
 }
 
-/** Return object for accessing dummy database with no read/write capabilities. */
-std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase()
-{
-    return std::make_unique<DummyDatabase>();
-}
-
 /** Return object for accessing temporary in-memory database. */
 std::unique_ptr<WalletDatabase> CreateMockWalletDatabase(DatabaseOptions& options)
 {
diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
index 72086e950a6..b2045403781 100644
--- a/src/wallet/walletdb.h
+++ b/src/wallet/walletdb.h
@@ -306,9 +306,6 @@ using KeyFilterFn = std::function<bool(const std::string&)>;
 //! Unserialize a given Key-Value pair and load it into the wallet
 bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr);
 
-/** Return object for accessing dummy database with no read/write capabilities. */
-std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase();
-
 /** Return object for accessing temporary in-memory database. */
 std::unique_ptr<WalletDatabase> CreateMockWalletDatabase(DatabaseOptions& options);
 std::unique_ptr<WalletDatabase> CreateMockWalletDatabase();

From 075962bc25a90661612fe4613cd50ea1cae21f52 Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Mon, 12 Dec 2022 16:43:49 -0500
Subject: [PATCH 4/8] wallet, tests: Include wallet/test/util.h

This will be needed for the following scripted-diff to work.
---
 build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj | 1 +
 src/bench/wallet_loading.cpp                       | 1 +
 src/qt/test/addressbooktests.cpp                   | 1 +
 src/qt/test/wallettests.cpp                        | 1 +
 src/wallet/test/coinselector_tests.cpp             | 1 +
 src/wallet/test/group_outputs_tests.cpp            | 1 +
 src/wallet/test/wallet_test_fixture.cpp            | 1 +
 7 files changed, 7 insertions(+)

diff --git a/build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj b/build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj
index 3a2540d549a..045386ea172 100644
--- a/build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj
+++ b/build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj
@@ -10,6 +10,7 @@
   <ItemGroup>
     <ClCompile Include="..\..\src\init\bitcoin-qt.cpp" />
     <ClCompile Include="..\..\src\test\util\setup_common.cpp" />
+    <ClCompile Include="..\..\src\wallet\test\util.cpp" />
     <ClCompile Include="..\..\src\qt\test\addressbooktests.cpp" />
     <ClCompile Include="..\..\src\qt\test\apptests.cpp" />
     <ClCompile Include="..\..\src\qt\test\optiontests.cpp" />
diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp
index 6b09adcc9df..b5c2fdaf06b 100644
--- a/src/bench/wallet_loading.cpp
+++ b/src/bench/wallet_loading.cpp
@@ -17,6 +17,7 @@
 #include <optional>
 
 using wallet::CWallet;
+using wallet::CreateMockableWalletDatabase;
 using wallet::DatabaseFormat;
 using wallet::DatabaseOptions;
 using wallet::TxStateInactive;
diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp
index 5706964cc9a..ed49e60dc08 100644
--- a/src/qt/test/addressbooktests.cpp
+++ b/src/qt/test/addressbooktests.cpp
@@ -19,6 +19,7 @@
 #include <key.h>
 #include <key_io.h>
 #include <wallet/wallet.h>
+#include <wallet/test/util.h>
 #include <walletinitinterface.h>
 
 #include <chrono>
diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp
index 072718fe151..c3775e1b8e5 100644
--- a/src/qt/test/wallettests.cpp
+++ b/src/qt/test/wallettests.cpp
@@ -26,6 +26,7 @@
 #include <qt/walletmodel.h>
 #include <test/util/setup_common.h>
 #include <validation.h>
+#include <wallet/test/util.h>
 #include <wallet/wallet.h>
 
 #include <chrono>
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 7f66179517a..d0620781caa 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -12,6 +12,7 @@
 #include <wallet/coincontrol.h>
 #include <wallet/coinselection.h>
 #include <wallet/spend.h>
+#include <wallet/test/util.h>
 #include <wallet/test/wallet_test_fixture.h>
 #include <wallet/wallet.h>
 
diff --git a/src/wallet/test/group_outputs_tests.cpp b/src/wallet/test/group_outputs_tests.cpp
index 283e87989c4..cc248eadd1f 100644
--- a/src/wallet/test/group_outputs_tests.cpp
+++ b/src/wallet/test/group_outputs_tests.cpp
@@ -6,6 +6,7 @@
 
 #include <wallet/coinselection.h>
 #include <wallet/spend.h>
+#include <wallet/test/util.h>
 #include <wallet/wallet.h>
 
 #include <boost/test/unit_test.hpp>
diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp
index 2dd8f9ad336..1071c4e6c4f 100644
--- a/src/wallet/test/wallet_test_fixture.cpp
+++ b/src/wallet/test/wallet_test_fixture.cpp
@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <wallet/test/util.h>
 #include <wallet/test/wallet_test_fixture.h>
 
 #include <scheduler.h>

From f0eecf5e408238c64b77b0a4974ba2b9edb17487 Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Mon, 12 Dec 2022 15:58:51 -0500
Subject: [PATCH 5/8] scripted-diff: Replace CreateMockWalletDB with
 CreateMockableWalletDB

Since we have a mockable wallet database, we don't really need to be
using BDB or SQLite's in-memory database capabilities. It doesn't really
help us to be using those as we aren't doing anything that requires one
type of db over the other, and will just prefer SQLite if it's
available.

MockableDatabase is suitable for these uses, so use
CreateMockableWalletDatabase to use that.

-BEGIN VERIFY SCRIPT-
sed -i "s/CreateMockWalletDatabase(options)/CreateMockableWalletDatabase()/" $(git grep -l "CreateMockWalletDatabase(options)" -- ":(exclude)src/wallet/walletdb.*")
sed -i "s/CreateMockWalletDatabase/CreateMockableWalletDatabase/" $(git grep -l "CreateMockWalletDatabase" -- ":(exclude)src/wallet/walletdb.*")
-END VERIFY SCRIPT-
---
 src/bench/wallet_balance.cpp            | 4 ++--
 src/bench/wallet_create_tx.cpp          | 6 +++---
 src/bench/wallet_loading.cpp            | 2 +-
 src/qt/test/addressbooktests.cpp        | 4 ++--
 src/qt/test/wallettests.cpp             | 6 +++---
 src/wallet/test/coinselector_tests.cpp  | 2 +-
 src/wallet/test/group_outputs_tests.cpp | 2 +-
 src/wallet/test/util.cpp                | 4 ++--
 src/wallet/test/wallet_test_fixture.cpp | 2 +-
 src/wallet/test/wallet_tests.cpp        | 2 +-
 src/wallet/test/walletload_tests.cpp    | 4 ++--
 11 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp
index d5d057e96dc..2c77b9b22b2 100644
--- a/src/bench/wallet_balance.cpp
+++ b/src/bench/wallet_balance.cpp
@@ -15,7 +15,7 @@
 #include <optional>
 
 using wallet::CWallet;
-using wallet::CreateMockWalletDatabase;
+using wallet::CreateMockableWalletDatabase;
 using wallet::DBErrors;
 using wallet::GetBalance;
 using wallet::WALLET_FLAG_DESCRIPTORS;
@@ -28,7 +28,7 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b
 
     const auto& ADDRESS_WATCHONLY = ADDRESS_BCRT1_UNSPENDABLE;
 
-    CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockWalletDatabase()};
+    CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase()};
     {
         LOCK(wallet.cs_wallet);
         wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp
index cb31421598c..13b0019fd2f 100644
--- a/src/bench/wallet_create_tx.cpp
+++ b/src/bench/wallet_create_tx.cpp
@@ -15,7 +15,7 @@
 #include <wallet/wallet.h>
 
 using wallet::CWallet;
-using wallet::CreateMockWalletDatabase;
+using wallet::CreateMockableWalletDatabase;
 using wallet::DBErrors;
 using wallet::WALLET_FLAG_DESCRIPTORS;
 
@@ -83,7 +83,7 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type
 {
     const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
 
-    CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockWalletDatabase()};
+    CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase()};
     {
         LOCK(wallet.cs_wallet);
         wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
@@ -136,7 +136,7 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type
 static void AvailableCoins(benchmark::Bench& bench, const std::vector<OutputType>& output_type)
 {
     const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
-    CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockWalletDatabase()};
+    CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase()};
     {
         LOCK(wallet.cs_wallet);
         wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp
index b5c2fdaf06b..a5222ad9b05 100644
--- a/src/bench/wallet_loading.cpp
+++ b/src/bench/wallet_loading.cpp
@@ -71,7 +71,7 @@ static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
         options.create_flags = WALLET_FLAG_DESCRIPTORS;
         options.require_format = DatabaseFormat::SQLITE;
     }
-    auto database = CreateMockWalletDatabase(options);
+    auto database = CreateMockableWalletDatabase();
     auto wallet = BenchLoadWallet(std::move(database), context, options);
 
     // Generate a bunch of transactions and addresses to put into the wallet
diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp
index ed49e60dc08..f17a6b7f74b 100644
--- a/src/qt/test/addressbooktests.cpp
+++ b/src/qt/test/addressbooktests.cpp
@@ -32,7 +32,7 @@
 
 using wallet::AddWallet;
 using wallet::CWallet;
-using wallet::CreateMockWalletDatabase;
+using wallet::CreateMockableWalletDatabase;
 using wallet::RemoveWallet;
 using wallet::WALLET_FLAG_DESCRIPTORS;
 using wallet::WalletContext;
@@ -76,7 +76,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
     auto wallet_loader = interfaces::MakeWalletLoader(*test.m_node.chain, *Assert(test.m_node.args));
     test.m_node.wallet_loader = wallet_loader.get();
     node.setContext(&test.m_node);
-    const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase());
+    const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockableWalletDatabase());
     wallet->LoadWallet();
     wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
     {
diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp
index c3775e1b8e5..5f789e400ef 100644
--- a/src/qt/test/wallettests.cpp
+++ b/src/qt/test/wallettests.cpp
@@ -47,7 +47,7 @@
 
 using wallet::AddWallet;
 using wallet::CWallet;
-using wallet::CreateMockWalletDatabase;
+using wallet::CreateMockableWalletDatabase;
 using wallet::RemoveWallet;
 using wallet::WALLET_FLAG_DESCRIPTORS;
 using wallet::WALLET_FLAG_DISABLE_PRIVATE_KEYS;
@@ -190,7 +190,7 @@ void SyncUpWallet(const std::shared_ptr<CWallet>& wallet, interfaces::Node& node
 
 std::shared_ptr<CWallet> SetupLegacyWatchOnlyWallet(interfaces::Node& node, TestChain100Setup& test)
 {
-    std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase());
+    std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockableWalletDatabase());
     wallet->LoadWallet();
     {
         LOCK(wallet->cs_wallet);
@@ -208,7 +208,7 @@ std::shared_ptr<CWallet> SetupLegacyWatchOnlyWallet(interfaces::Node& node, Test
 
 std::shared_ptr<CWallet> SetupDescriptorsWallet(interfaces::Node& node, TestChain100Setup& test)
 {
-    std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase());
+    std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockableWalletDatabase());
     wallet->LoadWallet();
     LOCK(wallet->cs_wallet);
     wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index d0620781caa..a5394d88164 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -177,7 +177,7 @@ inline std::vector<OutputGroup>& KnapsackGroupOutputs(const CoinsResult& availab
 
 static std::unique_ptr<CWallet> NewWallet(const node::NodeContext& m_node, const std::string& wallet_name = "")
 {
-    std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), wallet_name, CreateMockWalletDatabase());
+    std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), wallet_name, CreateMockableWalletDatabase());
     BOOST_CHECK(wallet->LoadWallet() == DBErrors::LOAD_OK);
     LOCK(wallet->cs_wallet);
     wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
diff --git a/src/wallet/test/group_outputs_tests.cpp b/src/wallet/test/group_outputs_tests.cpp
index cc248eadd1f..e6b25cc2161 100644
--- a/src/wallet/test/group_outputs_tests.cpp
+++ b/src/wallet/test/group_outputs_tests.cpp
@@ -18,7 +18,7 @@ static int nextLockTime = 0;
 
 static std::shared_ptr<CWallet> NewWallet(const node::NodeContext& m_node)
 {
-    std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase());
+    std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockableWalletDatabase());
     wallet->LoadWallet();
     LOCK(wallet->cs_wallet);
     wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index 7d0a814ed44..7b0b3fe033c 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -17,7 +17,7 @@
 namespace wallet {
 std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key)
 {
-    auto wallet = std::make_unique<CWallet>(&chain, "", CreateMockWalletDatabase());
+    auto wallet = std::make_unique<CWallet>(&chain, "", CreateMockableWalletDatabase());
     {
         LOCK2(wallet->cs_wallet, ::cs_main);
         wallet->SetLastBlockProcessed(cchain.Height(), cchain.Tip()->GetBlockHash());
@@ -47,7 +47,7 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc
 
 std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options)
 {
-    auto new_database = CreateMockWalletDatabase(options);
+    auto new_database = CreateMockableWalletDatabase();
 
     // Get a cursor to the original database
     auto batch = database.MakeBatch();
diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp
index 1071c4e6c4f..a72446d599c 100644
--- a/src/wallet/test/wallet_test_fixture.cpp
+++ b/src/wallet/test/wallet_test_fixture.cpp
@@ -11,7 +11,7 @@ namespace wallet {
 WalletTestingSetup::WalletTestingSetup(const std::string& chainName)
     : TestingSetup(chainName),
       m_wallet_loader{interfaces::MakeWalletLoader(*m_node.chain, *Assert(m_node.args))},
-      m_wallet(m_node.chain.get(), "", CreateMockWalletDatabase())
+      m_wallet(m_node.chain.get(), "", CreateMockableWalletDatabase())
 {
     m_wallet.LoadWallet();
     m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} });
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 0329a2487e3..241ac8f065c 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -119,7 +119,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
     // Verify ScanForWalletTransactions picks up transactions in both the old
     // and new block files.
     {
-        CWallet wallet(m_node.chain.get(), "", CreateMockWalletDatabase());
+        CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
         {
             LOCK(wallet.cs_wallet);
             LOCK(Assert(m_node.chainman)->GetMutex());
diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
index 9f5a4b14d3a..23934b11dd2 100644
--- a/src/wallet/test/walletload_tests.cpp
+++ b/src/wallet/test/walletload_tests.cpp
@@ -34,7 +34,7 @@ public:
 
 BOOST_FIXTURE_TEST_CASE(wallet_load_unknown_descriptor, TestingSetup)
 {
-    std::unique_ptr<WalletDatabase> database = CreateMockWalletDatabase();
+    std::unique_ptr<WalletDatabase> database = CreateMockableWalletDatabase();
     {
         // Write unknown active descriptor
         WalletBatch batch(*database, false);
@@ -84,7 +84,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup)
 
     {   // Context setup.
         // Create and encrypt legacy wallet
-        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", CreateMockWalletDatabase()));
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", CreateMockableWalletDatabase()));
         LOCK(wallet->cs_wallet);
         auto legacy_spkm = wallet->GetOrCreateLegacyScriptPubKeyMan();
         BOOST_CHECK(legacy_spkm->SetupGeneration(true));

From b3bb17d5d07f51ac2e501e4a7a3bbcd17144070f Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Mon, 12 Dec 2022 16:57:54 -0500
Subject: [PATCH 6/8] tests: Update DuplicateMockDatabase for MockableDatabase

---
 src/bench/wallet_loading.cpp         |  4 ++--
 src/wallet/test/util.cpp             | 23 ++---------------------
 src/wallet/test/util.h               |  3 +--
 src/wallet/test/walletload_tests.cpp |  2 +-
 4 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp
index a5222ad9b05..877af2c71de 100644
--- a/src/bench/wallet_loading.cpp
+++ b/src/bench/wallet_loading.cpp
@@ -79,7 +79,7 @@ static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
         AddTx(*wallet);
     }
 
-    database = DuplicateMockDatabase(wallet->GetDatabase(), options);
+    database = DuplicateMockDatabase(wallet->GetDatabase());
 
     // reload the wallet for the actual benchmark
     BenchUnloadWallet(std::move(wallet));
@@ -88,7 +88,7 @@ static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
         wallet = BenchLoadWallet(std::move(database), context, options);
 
         // Cleanup
-        database = DuplicateMockDatabase(wallet->GetDatabase(), options);
+        database = DuplicateMockDatabase(wallet->GetDatabase());
         BenchUnloadWallet(std::move(wallet));
     });
 }
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index 7b0b3fe033c..09e7979c02b 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -45,28 +45,9 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc
     return wallet;
 }
 
-std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options)
+std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database)
 {
-    auto new_database = CreateMockableWalletDatabase();
-
-    // Get a cursor to the original database
-    auto batch = database.MakeBatch();
-    std::unique_ptr<wallet::DatabaseCursor> cursor = batch->GetNewCursor();
-
-    // Get a batch for the new database
-    auto new_batch = new_database->MakeBatch();
-
-    // Read all records from the original database and write them to the new one
-    while (true) {
-        DataStream key{};
-        DataStream value{};
-        DatabaseCursor::Status status = cursor->Next(key, value);
-        assert(status != DatabaseCursor::Status::FAIL);
-        if (status == DatabaseCursor::Status::DONE) break;
-        new_batch->Write(key, value);
-    }
-
-    return new_database;
+    return std::make_unique<MockableDatabase>(dynamic_cast<MockableDatabase&>(database).m_records);
 }
 
 std::string getnewaddress(CWallet& w)
diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h
index 92405107cf2..01c2458a6c2 100644
--- a/src/wallet/test/util.h
+++ b/src/wallet/test/util.h
@@ -20,13 +20,12 @@ class Chain;
 
 namespace wallet {
 class CWallet;
-struct DatabaseOptions;
 class WalletDatabase;
 
 std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key);
 
 // Creates a copy of the provided database
-std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options);
+std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database);
 
 /** Returns a new encoded destination from the wallet (hardcoded to BECH32) */
 std::string getnewaddress(CWallet& w);
diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
index 23934b11dd2..082416d035b 100644
--- a/src/wallet/test/walletload_tests.cpp
+++ b/src/wallet/test/walletload_tests.cpp
@@ -100,7 +100,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup)
 
         DatabaseOptions options;
         for (int i=0; i < NUMBER_OF_TESTS; i++) {
-            dbs.emplace_back(DuplicateMockDatabase(wallet->GetDatabase(), options));
+            dbs.emplace_back(DuplicateMockDatabase(wallet->GetDatabase()));
         }
     }
 

From 80ace042d8fece9be50bfef1be64c6e5720e87e6 Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Mon, 12 Dec 2022 17:38:08 -0500
Subject: [PATCH 7/8] tests: Modify records directly in wallet ckey loading
 test

In the wallet ckey loading test, we modify various ckey records to test
corruption handling. As the database is now a mockable database, we can
modify the records that the database will be initialized with. This
avoids having to use the verbose database reading and writing functions.
---
 src/wallet/test/walletload_tests.cpp | 85 +++++++++++++---------------
 1 file changed, 38 insertions(+), 47 deletions(-)

diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
index 082416d035b..6823eafdfa7 100644
--- a/src/wallet/test/walletload_tests.cpp
+++ b/src/wallet/test/walletload_tests.cpp
@@ -70,38 +70,45 @@ bool HasAnyRecordOfType(WalletDatabase& db, const std::string& key)
     return false;
 }
 
-BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup)
+template<typename... Args>
+SerializeData MakeSerializeData(const Args&... args)
 {
-    // The test duplicates the db so each case has its own db instance.
-    int NUMBER_OF_TESTS = 4;
-    std::vector<std::unique_ptr<WalletDatabase>> dbs;
-    CKey first_key;
-    auto get_db = [](std::vector<std::unique_ptr<WalletDatabase>>& dbs) {
-        std::unique_ptr<WalletDatabase> db = std::move(dbs.back());
-        dbs.pop_back();
-        return db;
-    };
+    CDataStream s(0, 0);
+    SerializeMany(s, args...);
+    return {s.begin(), s.end()};
+}
 
-    {   // Context setup.
+
+BOOST_FIXTURE_TEST_CASE(wallet_load_ckey, TestingSetup)
+{
+    SerializeData ckey_record_key;
+    SerializeData ckey_record_value;
+    std::map<SerializeData, SerializeData> records;
+
+    {
+        // Context setup.
         // Create and encrypt legacy wallet
         std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", CreateMockableWalletDatabase()));
         LOCK(wallet->cs_wallet);
         auto legacy_spkm = wallet->GetOrCreateLegacyScriptPubKeyMan();
         BOOST_CHECK(legacy_spkm->SetupGeneration(true));
 
-        // Get the first key in the wallet
+        // Retrieve a key
         CTxDestination dest = *Assert(legacy_spkm->GetNewDestination(OutputType::LEGACY));
         CKeyID key_id = GetKeyForDestination(*legacy_spkm, dest);
+        CKey first_key;
         BOOST_CHECK(legacy_spkm->GetKey(key_id, first_key));
 
-        // Encrypt the wallet and duplicate database
+        // Encrypt the wallet
         BOOST_CHECK(wallet->EncryptWallet("encrypt"));
         wallet->Flush();
 
-        DatabaseOptions options;
-        for (int i=0; i < NUMBER_OF_TESTS; i++) {
-            dbs.emplace_back(DuplicateMockDatabase(wallet->GetDatabase()));
-        }
+        // Store a copy of all the records
+        records = GetMockableDatabase(*wallet).m_records;
+
+        // Get the record for the retrieved key
+        ckey_record_key = MakeSerializeData(DBKeys::CRYPTED_KEY, first_key.GetPubKey());
+        ckey_record_value = records.at(ckey_record_key);
     }
 
     {
@@ -112,7 +119,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup)
         // the records every time that 'CWallet::Unlock' gets called, which is not good.
 
         // Load the wallet and check that is encrypted
-        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", get_db(dbs)));
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", CreateMockableWalletDatabase(records)));
         BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK);
         BOOST_CHECK(wallet->IsCrypted());
         BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY));
@@ -127,18 +134,12 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup)
     {
         // Second test case:
         // Verify that loading up a 'ckey' with no checksum triggers a complete re-write of the crypted keys.
-        std::unique_ptr<WalletDatabase> db = get_db(dbs);
-        {
-            std::unique_ptr<DatabaseBatch> batch = db->MakeBatch(false);
-            std::pair<std::vector<unsigned char>, uint256> value;
-            BOOST_CHECK(batch->Read(std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey()), value));
 
-            const auto key = std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey());
-            BOOST_CHECK(batch->Write(key, value.first, /*fOverwrite=*/true));
-        }
+        // Cut off the 32 byte checksum from a ckey record
+        records[ckey_record_key].resize(ckey_record_value.size() - 32);
 
         // Load the wallet and check that is encrypted
-        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", std::move(db)));
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", CreateMockableWalletDatabase(records)));
         BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK);
         BOOST_CHECK(wallet->IsCrypted());
         BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY));
@@ -154,35 +155,25 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup)
     {
         // Third test case:
         // Verify that loading up a 'ckey' with an invalid checksum throws an error.
-        std::unique_ptr<WalletDatabase> db = get_db(dbs);
-        {
-            std::unique_ptr<DatabaseBatch> batch = db->MakeBatch(false);
-            std::vector<unsigned char> crypted_data;
-            BOOST_CHECK(batch->Read(std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey()), crypted_data));
 
-            // Write an invalid checksum
-            std::pair<std::vector<unsigned char>, uint256> value = std::make_pair(crypted_data, uint256::ONE);
-            const auto key = std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey());
-            BOOST_CHECK(batch->Write(key, value, /*fOverwrite=*/true));
-        }
+        // Cut off the 32 byte checksum from a ckey record
+        records[ckey_record_key].resize(ckey_record_value.size() - 32);
+        // Fill in the checksum space with 0s
+        records[ckey_record_key].resize(ckey_record_value.size());
 
-        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", std::move(db)));
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", CreateMockableWalletDatabase(records)));
         BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::CORRUPT);
     }
 
     {
         // Fourth test case:
         // Verify that loading up a 'ckey' with an invalid pubkey throws an error
-        std::unique_ptr<WalletDatabase> db = get_db(dbs);
-        {
-            CPubKey invalid_key;
-            BOOST_ASSERT(!invalid_key.IsValid());
-            const auto key = std::make_pair(DBKeys::CRYPTED_KEY, invalid_key);
-            std::pair<std::vector<unsigned char>, uint256> value;
-            BOOST_CHECK(db->MakeBatch(false)->Write(key, value, /*fOverwrite=*/true));
-        }
+        CPubKey invalid_key;
+        BOOST_ASSERT(!invalid_key.IsValid());
+        SerializeData key = MakeSerializeData(DBKeys::CRYPTED_KEY, invalid_key);
+        records[key] = ckey_record_value;
 
-        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", std::move(db)));
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", CreateMockableWalletDatabase(records)));
         BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::CORRUPT);
     }
 }

From 33e2b82a4fc990253ff77655f437c7aed336bc55 Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Wed, 3 May 2023 10:38:27 -0400
Subject: [PATCH 8/8] wallet, bench: Remove unused database options from
 WalletBenchLoading

---
 src/bench/wallet_loading.cpp | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp
index 877af2c71de..cf32fde51ea 100644
--- a/src/bench/wallet_loading.cpp
+++ b/src/bench/wallet_loading.cpp
@@ -18,18 +18,16 @@
 
 using wallet::CWallet;
 using wallet::CreateMockableWalletDatabase;
-using wallet::DatabaseFormat;
-using wallet::DatabaseOptions;
 using wallet::TxStateInactive;
 using wallet::WALLET_FLAG_DESCRIPTORS;
 using wallet::WalletContext;
 using wallet::WalletDatabase;
 
-static std::shared_ptr<CWallet> BenchLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, DatabaseOptions& options)
+static std::shared_ptr<CWallet> BenchLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, uint64_t create_flags)
 {
     bilingual_str error;
     std::vector<bilingual_str> warnings;
-    auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings);
+    auto wallet = CWallet::Create(context, "", std::move(database), create_flags, error, warnings);
     NotifyWalletLoaded(context, wallet);
     if (context.chain) {
         wallet->postInitProcess();
@@ -56,7 +54,6 @@ static void AddTx(CWallet& wallet)
 static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
 {
     const auto test_setup = MakeNoLogFileContext<TestingSetup>();
-    test_setup->m_args.ForceSetArg("-unsafesqlitesync", "1");
 
     WalletContext context;
     context.args = &test_setup->m_args;
@@ -64,15 +61,12 @@ static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
 
     // Setup the wallet
     // Loading the wallet will also create it
-    DatabaseOptions options;
-    if (legacy_wallet) {
-        options.require_format = DatabaseFormat::BERKELEY;
-    } else {
-        options.create_flags = WALLET_FLAG_DESCRIPTORS;
-        options.require_format = DatabaseFormat::SQLITE;
+    uint64_t create_flags = 0;
+    if (!legacy_wallet) {
+        create_flags = WALLET_FLAG_DESCRIPTORS;
     }
     auto database = CreateMockableWalletDatabase();
-    auto wallet = BenchLoadWallet(std::move(database), context, options);
+    auto wallet = BenchLoadWallet(std::move(database), context, create_flags);
 
     // Generate a bunch of transactions and addresses to put into the wallet
     for (int i = 0; i < 1000; ++i) {
@@ -85,7 +79,7 @@ static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
     BenchUnloadWallet(std::move(wallet));
 
     bench.epochs(5).run([&] {
-        wallet = BenchLoadWallet(std::move(database), context, options);
+        wallet = BenchLoadWallet(std::move(database), context, create_flags);
 
         // Cleanup
         database = DuplicateMockDatabase(wallet->GetDatabase());