…m some …

a52f1d1 walletdb: Use SQLiteDatabase for mock wallet databases (Andrew Chow)
a78c229 tests: Place into mapWallet in coinselector_tests (Andrew Chow)

Pull request description:

  #23288 changed coinselector_tests to use `DescriptorScriptPubKeyMan`, but it also ended up significantly slowing down the test, from 4 seconds to over 1 minute. It appears that the source of this slow down is with `CWallet::AddToWallet`, and primarily due to writing data to the mock wallet database. Because the only thing that is actually needed is for the created transaction to be placed into `CWallet::mapWallet`, this PR removes the call to `AddToWallet` and just places the transaction into `mapWallet` directly. This reduces the test time to 5 seconds.

  To speed things up further, `CreateMockWalletDatabase` is changed to make a `SQLiteDatabase` instead of a `BerkeleyDatabase`. This is safe because there are no tests that require a specific mock database type.

ACKs for top commit:
  brunoerg:
    tACK a52f1d1
  lsilva01:
    tACK a52f1d1. Performed 74.36% better on Ubuntu 20.04 (VM, 12 MB, 8vCPU).
  glozow:
    utACK a52f1d1

Tree-SHA512: da77936bfd2e816d2e71703567b9389d0ee79f3a4a690802ffe3469df5bed371b296cb822b897f625654dab9436d91fd6bc02364a518a47d746e487d70a72595

a52f1d1 walletdb: Use SQLiteDatabase for mock wallet databases (Andrew Chow)
a78c229 tests: Place into mapWallet in coinselector_tests (Andrew Chow)

Pull request description:

  bitcoin#23288 changed coinselector_tests to use `DescriptorScriptPubKeyMan`, but it also ended up significantly slowing down the test, from 4 seconds to over 1 minute. It appears that the source of this slow down is with `CWallet::AddToWallet`, and primarily due to writing data to the mock wallet database. Because the only thing that is actually needed is for the created transaction to be placed into `CWallet::mapWallet`, this PR removes the call to `AddToWallet` and just places the transaction into `mapWallet` directly. This reduces the test time to 5 seconds.

  To speed things up further, `CreateMockWalletDatabase` is changed to make a `SQLiteDatabase` instead of a `BerkeleyDatabase`. This is safe because there are no tests that require a specific mock database type.

ACKs for top commit:
  brunoerg:
    tACK a52f1d1
  lsilva01:
    tACK a52f1d1. Performed 74.36% better on Ubuntu 20.04 (VM, 12 MB, 8vCPU).
  glozow:
    utACK a52f1d1

Tree-SHA512: da77936bfd2e816d2e71703567b9389d0ee79f3a4a690802ffe3469df5bed371b296cb822b897f625654dab9436d91fd6bc02364a518a47d746e487d70a72595

…sts/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

…_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#23288) introduced an intermittent failure in the `psbt_wallet_tests/psbt_updater_test` unit test. See 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#23368 (comment)):
  > Yes, as I said before this looks like a race where the wallet is deleted before stopping the scheduler: [bitcoin#23368 (comment)](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#23368.

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

Tree-SHA512: d9103f6252aab807453628159dec243bc543a2595eecaa04ec761dca3c2370085592c55d6f50967d69a4ac6e8b5827eec30dd9b025132c99b0bb9aa5911ad915

…scheduler

Summary:
> The order should be:
>  - stop scheduler
>  - delete wallet
>  - delete scheduler

This is a partial backport of [[bitcoin/bitcoin#23403 | core#23403]]
bitcoin/bitcoin@68018e4

The other commit in that pull request is not applicable to Bitcoin ABC, for now. It has a dependency on [[bitcoin/bitcoin#23288 | core#23288]], which has itself a lot of dependencies on recent descriptor wallets developments.

Test Plan:
With TSAN:
`ninja && ninja check`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10883

9a5b4d7 doc: Delete old line of code that was commented out (Michael Folkson)

Pull request description:

  In #23288 MarcoFalke [highlighted](https://github.com/bitcoin/bitcoin/pull/23288/files#r739817055) an old BOOST_CHECK that was commented out and replaced by a different BOOST_CHECK. I think this can be deleted and wasn't deliberately left in by achow101.

ACKs for top commit:
  achow101:
    ACK 9a5b4d7
  jonatack:
    ACK 9a5b4d7

Tree-SHA512: 69aa71d362bb190c75d617766f6af945a43211ddb315ccfef5ebab2beefe020032d99f0d0d4b312aa349e605ba712a8bc91d7f0a22427681e08c95f8a6ea7e64

9a5b4d7 doc: Delete old line of code that was commented out (Michael Folkson)

Pull request description:

  In bitcoin#23288 MarcoFalke [highlighted](https://github.com/bitcoin/bitcoin/pull/23288/files#r739817055) an old BOOST_CHECK that was commented out and replaced by a different BOOST_CHECK. I think this can be deleted and wasn't deliberately left in by achow101.

ACKs for top commit:
  achow101:
    ACK 9a5b4d7
  jonatack:
    ACK 9a5b4d7

Tree-SHA512: 69aa71d362bb190c75d617766f6af945a43211ddb315ccfef5ebab2beefe020032d99f0d0d4b312aa349e605ba712a8bc91d7f0a22427681e08c95f8a6ea7e64