From 27b432f524be4c32925aa86660d424629b2a0e20 Mon Sep 17 00:00:00 2001
From: Ava Chow <github@achow101.com>
Date: Tue, 20 Feb 2024 11:54:54 -0500
Subject: [PATCH] wallet, tests: Have CreateSyncedWallet use CWallet::Create

CWallet::Create will properly connect the wallet to the chain, so we
should be doing that rather than ad-hoc chain connection.
---
 src/wallet/test/spend_tests.cpp  | 11 +++++++++--
 src/wallet/test/util.cpp         | 25 +++++++++++++------------
 src/wallet/test/util.h           |  2 +-
 src/wallet/test/wallet_tests.cpp | 15 +++++++++------
 4 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp
index 963c0f838b1..fba65792a30 100644
--- a/src/wallet/test/spend_tests.cpp
+++ b/src/wallet/test/spend_tests.cpp
@@ -7,6 +7,7 @@
 #include <script/solver.h>
 #include <validation.h>
 #include <wallet/coincontrol.h>
+#include <wallet/context.h>
 #include <wallet/spend.h>
 #include <wallet/test/util.h>
 #include <wallet/test/wallet_test_fixture.h>
@@ -19,7 +20,10 @@ BOOST_FIXTURE_TEST_SUITE(spend_tests, WalletTestingSetup)
 BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup)
 {
     CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
-    auto wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), coinbaseKey);
+    WalletContext context;
+    context.chain = m_node.chain.get();
+    context.args = m_node.args;
+    auto wallet = CreateSyncedWallet(context, coinbaseKey);
 
     // Check that a subtract-from-recipient transaction slightly less than the
     // coinbase input amount does not create a change output (because it would
@@ -67,7 +71,10 @@ BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup)
 
     // Add 4 spendable UTXO, 50 BTC each, to the wallet (total balance 200 BTC)
     for (int i = 0; i < 4; i++) CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
-    auto wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), coinbaseKey);
+    WalletContext context;
+    context.chain = m_node.chain.get();
+    context.args = m_node.args;
+    auto wallet = CreateSyncedWallet(context, coinbaseKey);
 
     LOCK(wallet->cs_wallet);
     auto available_coins = AvailableCoins(*wallet);
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index 43fd91fe60d..2cd014ce1be 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -17,18 +17,17 @@
 #include <memory>
 
 namespace wallet {
-std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key)
+std::shared_ptr<CWallet> CreateSyncedWallet(WalletContext& context, const CKey& key)
 {
-    auto wallet = std::make_unique<CWallet>(&chain, "", CreateMockableWalletDatabase());
-    {
-        LOCK2(wallet->cs_wallet, ::cs_main);
-        wallet->SetLastBlockProcessed(cchain.Height(), cchain.Tip()->GetBlockHash());
-    }
+    bilingual_str error;
+    std::vector<bilingual_str> warnings;
+    auto wallet = CWallet::Create(context, "", CreateMockableWalletDatabase(), WALLET_FLAG_DESCRIPTORS, error, warnings);
+
+    // Allow the fallback fee with it's default
+    wallet->m_allow_fallback_fee = true;
+
     {
         LOCK(wallet->cs_wallet);
-        wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
-        wallet->SetupDescriptorScriptPubKeyMans();
-
         FlatSigningProvider provider;
         std::string error;
         auto descs = Parse("combo(" + EncodeSecret(key) + ")", provider, error, /* require_checksum=*/ false);
@@ -39,11 +38,13 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc
     }
     WalletRescanReserver reserver(*wallet);
     reserver.reserve();
-    CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false);
+    CWallet::ScanResult result = wallet->ScanForWalletTransactions(context.chain->getBlockHash(0), /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false);
     assert(result.status == CWallet::ScanResult::SUCCESS);
-    assert(result.last_scanned_block == cchain.Tip()->GetBlockHash());
-    assert(*result.last_scanned_height == cchain.Height());
+    int tip_height = context.chain->getHeight().value();
+    assert(*result.last_scanned_height == tip_height);
+    assert(result.last_scanned_block == context.chain->getBlockHash(tip_height));
     assert(result.last_failed_block.IsNull());
+
     return wallet;
 }
 
diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h
index c8a89c0e642..7d1407d7a05 100644
--- a/src/wallet/test/util.h
+++ b/src/wallet/test/util.h
@@ -36,7 +36,7 @@ static const DatabaseFormat DATABASE_FORMATS[] = {
 
 const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj";
 
-std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key);
+std::shared_ptr<CWallet> CreateSyncedWallet(WalletContext& chain, const CKey& key);
 
 std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context);
 std::shared_ptr<CWallet> TestLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, uint64_t create_flags);
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 19f2cd273c6..e9a18da016c 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -553,7 +553,10 @@ public:
     ListCoinsTestingSetup()
     {
         CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
-        wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), coinbaseKey);
+        WalletContext context;
+        context.chain = m_node.chain.get();
+        context.args = m_node.args;
+        wallet = CreateSyncedWallet(context, coinbaseKey);
     }
 
     ~ListCoinsTestingSetup()
@@ -577,17 +580,16 @@ public:
             blocktx = CMutableTransaction(*wallet->mapWallet.at(tx->GetHash()).tx);
         }
         CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
+        m_node.validation_signals->SyncWithValidationInterfaceQueue();
 
         LOCK(wallet->cs_wallet);
-        LOCK(Assert(m_node.chainman)->GetMutex());
-        wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, m_node.chainman->ActiveChain().Tip()->GetBlockHash());
         auto it = wallet->mapWallet.find(tx->GetHash());
         BOOST_CHECK(it != wallet->mapWallet.end());
-        it->second.SetState(TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/1});
+        BOOST_CHECK(it->second.state<TxStateConfirmed>());
         return it->second;
     }
 
-    std::unique_ptr<CWallet> wallet;
+    std::shared_ptr<CWallet> wallet;
 };
 
 BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
@@ -650,9 +652,10 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
 void TestCoinsResult(ListCoinsTest& context, OutputType out_type, CAmount amount,
                      std::map<OutputType, size_t>& expected_coins_sizes)
 {
-    LOCK(context.wallet->cs_wallet);
     util::Result<CTxDestination> dest = Assert(context.wallet->GetNewDestination(out_type, ""));
     CWalletTx& wtx = context.AddTx(CRecipient{*dest, amount, /*fSubtractFeeFromAmount=*/true});
+
+    LOCK(context.wallet->cs_wallet);
     CoinFilterParams filter;
     filter.skip_locked = false;
     CoinsResult available_coins = AvailableCoins(*context.wallet, nullptr, std::nullopt, filter);