Merge bitcoin/bitcoin#23403: test: Fix segfault in the psbt_wallet_tests/psbt_updater_test

68018e4 test: Avoid excessive locking of `cs_wallet` (Hennadii Stepanov)
7986faf test: Fix segfault in the psbt_wallet_tests/psbt_updater_test (Hennadii Stepanov)

Pull request description:

  The dcd6eeb commit (bitcoin/bitcoin#23288) introduced an intermittent failure in the `psbt_wallet_tests/psbt_updater_test` unit test. See bitcoin/bitcoin#23368.

  The test failure can be easily made reproducible with the following patch:
  ```diff
  --- a/src/scheduler.cpp
  +++ b/src/scheduler.cpp
  @@ -57,6 +57,8 @@ void CScheduler::serviceQueue()
               Function f = taskQueue.begin()->second;
               taskQueue.erase(taskQueue.begin());

  +            UninterruptibleSleep(100ms);
  +
               {
                   // Unlock before calling f, so it can reschedule itself or another task
                   // without deadlocking:
  ```

  This PR implements an idea which was mentioned in the [comment](bitcoin/bitcoin#23368 (comment)):
  > Yes, as I said before this looks like a race where the wallet is deleted before stopping the scheduler: [#23368 (comment)](bitcoin/bitcoin#23368 (comment))
  >
  > IIRC, the order should be:
  >
  >    * stop scheduler
  >
  >    * delete wallet
  >
  >    * delete scheduler

  The second commit introduces a refactoring with no behavior change.

  Fixes bitcoin/bitcoin#23368.

ACKs for top commit:
  mjdietzx:
    Code review ACK 68018e4

Tree-SHA512: d9103f6252aab807453628159dec243bc543a2595eecaa04ec761dca3c2370085592c55d6f50967d69a4ac6e8b5827eec30dd9b025132c99b0bb9aa5911ad915

Author: MarcoFalke

src/wallet/test/psbt_wallet_tests.cpp

@@ -14,8 +14,9 @@
 BOOST_FIXTURE_TEST_SUITE(psbt_wallet_tests, WalletTestingSetup)
 
 static void import_descriptor(CWallet& wallet, const std::string& descriptor)
+    EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
 {
-    LOCK(wallet.cs_wallet);
+    AssertLockHeld(wallet.cs_wallet);
     FlatSigningProvider provider;
     std::string error;
     std::unique_ptr<Descriptor> desc = Parse(descriptor, provider, error, /* require_checksum=*/ false);

src/wallet/test/wallet_test_fixture.cpp

@@ -4,6 +4,8 @@
 
 #include <wallet/test/wallet_test_fixture.h>
 
+#include <scheduler.h>
+
 WalletTestingSetup::WalletTestingSetup(const std::string& chainName)
     : TestingSetup(chainName),
       m_wallet(m_node.chain.get(), "", CreateMockWalletDatabase())
@@ -12,3 +14,8 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName)
     m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} });
     m_wallet_client->registerRpcs();
 }
+
+WalletTestingSetup::~WalletTestingSetup()
+{
+    if (m_node.scheduler) m_node.scheduler->stop();
+}

src/wallet/test/wallet_test_fixture.h

@@ -19,6 +19,7 @@
  */
 struct WalletTestingSetup : public TestingSetup {
     explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
+    ~WalletTestingSetup();
 
     std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_node.chain, *Assert(m_node.args));
     CWallet m_wallet;