💬 jonatack commented on pull request "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet":
(https://github.com/bitcoin/bitcoin/pull/27279#discussion_r1161770541)
Thanks for looking! I'm not sure whether compilers optimize for it but happy to drop that unrelated change, will do.

💬 ryanofsky commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1161771202)
> [8cd7730](https://github.com/bitcoin/bitcoin/commit/8cd77302305a2387368aabe8744b1859caf9956a): paging @ryanofsky: do you think we should move all this complexity here to the interface?

Right, probably it would be better to move the new code from the `wallet:WalletImpl::getAvailableBalance()` method in `src/wallet/interfaces.cpp` into the `wallet::GetAvailableBalance()` function in `src/wallet/spend.cpp` or into another function there like `wallet::GetSelectedCoinsBalance`. Reasoning is

-
...

💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1161777870)
Changed here and elsewhere

💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1161777986)
Done as suggested

💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1161778269)
Done as suggested

💬 achow101 commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1161778333)
Done as suggested

💬 jonatack commented on pull request "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet":
(https://github.com/bitcoin/bitcoin/pull/27279#discussion_r1161787781)
What do you think about: `Multiple messages will each be delimited by a newline (LF) character.`

(`\n` is the escape sequence (for use in a string literal) and `0A` is the OS-specific hex encoded representation)

- https://en.cppreference.com/w/cpp/language/escape
- https://en.wikipedia.org/wiki/Newline
- https://www.ibm.com/docs/en/zos/2.4.0?topic=output-escape-sequences

💬 jonatack commented on pull request "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet":
(https://github.com/bitcoin/bitcoin/pull/27279#discussion_r1161800526)
`WALLET_FLAG_AVOID_REUSE` is defined in `walletutil.h`, not in `wallet.h`, and per `git grep "#include <wallet/wallet.h>" | wc -l` this commit would avoid needlessly adding `WALLET_FLAG_CAVEATS` to nearly fifty compilation units.

I'll add `#include <wallet/walletutil.h>` to `wallet/rpc/wallet.cpp` in this change.

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1161800943)
If they're no longer active, we don't know internalness.

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1161805097)
So they aren't repeatedly written when the same key shows up in multiple descriptors.

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1161806010)
The encrypted secrets are all in the same format for both legacy wallet keys, descriptor keys, and now hd ckeys. I suppose this could be documented somewhere, although it's not immediately obvious where.

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1161810575)
Done

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1161810707)
Changed

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1161810822)
Done

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1161810914)
Done

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1161811025)
Done

💬 jonatack commented on pull request "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet":
(https://github.com/bitcoin/bitcoin/pull/27279#discussion_r1161811758)
Either way someone will ask why it isn't done the other way, i.e. https://github.com/bitcoin/bitcoin/pull/17578#pullrequestreview-338232718, so I went with the smallest diff.

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

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

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