💬 achow101 commented on pull request "tests: Run both descriptor and legacy tests within a single test invocation":
(https://github.com/bitcoin/bitcoin/pull/20892#discussion_r1183812824)
Sorry, I think I gave the wrong commit. It should be a936a3f194f855e269a4dc2c25e17dbc55eb7a18 "tests: Run both descriptor and legacy wallet modes in single invocation".

The tmpdir changes are needed for this commit because the entire test is restarted for the other wallet type, so it needs to be able to make a new datadir.

💬 achow101 commented on pull request "tests: Run both descriptor and legacy tests within a single test invocation":
(https://github.com/bitcoin/bitcoin/pull/20892#issuecomment-1533196488)
> Can you update the description detailing the approach you are now taking?

Done.

💬 furszy commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1183810238)
In 45e38158:

Could also remove the `DatabaseOptions` var by providing the `create_flags` uint64_t directly to `BenchLoadWallet`.

And remove the `using wallet::DatabaseFormat` at the top to make the CI happy.

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1183816539)
I copied this behavior from `ChainstateManagerOpts`. We expect the default constructed `Options` to contain defaults. If we don't have defaults, we assign on object construction. So, yes, in both instances a `const` qualifier (and maybe in others using the same pattern) should be added.

💬 achow101 commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1183817443)
Oops, fixed.

💬 achow101 commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1183821846)
Done

💬 achow101 commented on pull request "wallet: Refactor and document CoinControl":
(https://github.com/bitcoin/bitcoin/pull/26066#issuecomment-1533214936)
ACK daba95700b0b77a2e898299f218c47a69ed2c7d0

🚀 achow101 merged a pull request: "wallet: Refactor and document CoinControl"
(https://github.com/bitcoin/bitcoin/pull/26066)

🤔 theStack reviewed a pull request: "wallet: fix deadlock in bdb read write operation"
(https://github.com/bitcoin/bitcoin/pull/27556#pullrequestreview-1411142149)
Didn't look closer at the changes yet, but can confirm that running the newly introduced unit test via
```
$ ./src/test/test_bitcoin --log_level=all --run_test=walletdb_tests/walletdb_read_write_deadlock
```
succeeds on the PR branch head and hangs if the bugfix commit bda562ddba13ee8f94147734ab9901ab07357f2f is reverted.

👍 furszy approved a pull request: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1411147768)
ACK 00b6e0bb

💬 furszy commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1183848909)
Note:
If this is used in a read-write process, I'm quite sure that it will suffer from the same problematic tackled in #27556.

💬 achow101 commented on pull request "wallet: fix deadlock in bdb read write operation":
(https://github.com/bitcoin/bitcoin/pull/27556#issuecomment-1533259027)
ACK d3419f4108669d37a4ccecab77750e70c2f6237c

💬 achow101 commented on pull request "wallet: fix deadlock in bdb read write operation":
(https://github.com/bitcoin/bitcoin/pull/27556#issuecomment-1533261116)
Backport shouldn't be required as I don't think there's any user action that can trigger the deadlock.

💬 achow101 commented on pull request "wallet: fix deadlock in bdb read write operation":
(https://github.com/bitcoin/bitcoin/pull/27556#discussion_r1183870974)
We should just make the `batch` argument required to avoid this kind of issue in the future.

💬 furszy commented on pull request "wallet: fix deadlock in bdb read write operation":
(https://github.com/bitcoin/bitcoin/pull/27556#discussion_r1183886841)
sure

🤔 glozow reviewed a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1411212761)
Addressed comments, rebased, added a release note

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1183886938)
Done

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1183879477)
Brilliant, done

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1183879659)
Done

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1183878971)
Fixed

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1183878715)
Makes sense, taken