💬 mzumsande commented on pull request "validation: Improve error handling when VerifyDB fails due to insufficient dbcache":
(https://github.com/bitcoin/bitcoin/pull/25574#issuecomment-1428684154)
[f5724d1 ](https://github.com/bitcoin/bitcoin/commit/f5724d18878c292e4cdc766ac93d28d4252dd157)to [080f83e](https://github.com/bitcoin/bitcoin/commit/080f83e43ddc1fd2ddc57f6590b9166a373d38c2):

I added a commit to change the make the RPC return `false` in the case of unavailable data and improved the error messages. I left `init` behavior unchanged in this case (no aborts).

💬 ryanofsky commented on pull request "refactor, kernel: Remove gArgs accesses from dbwrapper and txdb":
(https://github.com/bitcoin/bitcoin/pull/25862#discussion_r1105021213)
re: https://github.com/bitcoin/bitcoin/pull/25862#discussion_r1087877059

In commit "refactor, validation: Add ChainstateManagerOpts db options" (cde3882cd4d4d1ee5682cab47d05e786cc798eb8)

> Not going against this change, but this line removal doesn't look to be part of a "non-behavior change" commit.

Good catch. It looks like this line got dropped due to a bad rebase. It was not supposed to be deleted.

💬 achow101 commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#issuecomment-1428731690)
ACK 0bb90cabc8e2c6bbbc3c52ec1170e725435b625d

💬 sipa commented on pull request "Modernize rpcauth.py":
(https://github.com/bitcoin/bitcoin/pull/27081#discussion_r1105069472)
Thanks; I've made some similar changes there.

💬 sipa commented on pull request "Modernize rpcauth.py":
(https://github.com/bitcoin/bitcoin/pull/27081#discussion_r1105070609)
Done.

💬 jamesob commented on pull request "Modernize rpcauth.py":
(https://github.com/bitcoin/bitcoin/pull/27081#issuecomment-1428763175)
Github ACK https://github.com/bitcoin/bitcoin/pull/27081/commits/e4e17907b686c25dda91e569645a8f501ca48f90

🤔 TheCharlatan requested changes to a pull request: "refactor: wallet, remove global 'ArgsManager' dependency"
(https://github.com/bitcoin/bitcoin/pull/26889)

💬 TheCharlatan commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1105059343)
You can also get rid of the `system.h` include in this file.

💬 TheCharlatan commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1105070733)
I think you can remove the `ArgsManager` from the arguments here.

👍 Lokoluis approved a pull request: "test: Fix intermittent sync issue in wallet_pruning"
(https://github.com/bitcoin/bitcoin/pull/27093)

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1105084574)
done

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1105084632)
done

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#issuecomment-1428788415)
Updated per feedback. Thanks. Tiny [diff](https://github.com/bitcoin/bitcoin/compare/ebb14064a0589bc56b71e267204696accc577036..fafaa7a7e15a49da0214ce9546a42117c63e611f)

💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1428803693)

> Noticed that we are calling cli-only commands with the "-" prefix. Would be good to follow the same convention here too. The renaming will let us detect them in a simpler manner by checking the first character without requiring to split the string etc.

This seems like something "nice to have" but there are other issues apart from what Pablo mentioned that makes me think that we shouldn´t do that:

- There are other console-only commands that already existed, like `help-console`
- Thi
...

💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1428811330)
> I agree with furszy's idea of introducing the functions `IsCliOnlyCommand ` and `ExecuteCliCommand `, to implement the `generate` cli-only command, for a cleaner implementation and with the advantage of leaving the structure ready for potential expansion of additional cli-only commands.

The provided method `executeConsoleOnlyCommand` is almost identical to the suggested `ExecuteCliCommand` and is also ready for potential future new console only commands, the only difference is that it detec
...

💬 sipa commented on pull request "net: avoid overriding non-virtual ToString() in CService and use better naming":
(https://github.com/bitcoin/bitcoin/pull/25619#issuecomment-1428811771)
utACK c9d548c91fb12fba516dee896f1f97692cfa2104

💬 rserranon commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1428851086)
testing ACK

👍 ponury1990 approved a pull request: "wallet: ensure the wallet is unlocked when needed for rescanning"
(https://github.com/bitcoin/bitcoin/pull/26347)

💬 w0xlt commented on pull request "[Draft / POC] Silent Payments":
(https://github.com/bitcoin/bitcoin/pull/24897#issuecomment-1429142222)
Update: The silentpayment index has been removed from the codebase.

`scantxoutset` has been changed to use `txindex` and `CBlockUndo`. So the scan works without silentpayment index.

`getsilentpaymentblockdata` also works without any indexes, using only `CBlockUndo`. The code is relatively simple. Therefore, even serving light clients, there is no need for indexing.

Seems to work fine. Not sure about the performance.

A new PR, with the index, will be added soon.

💬 darosior commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1105320169)
Will do if i need to retouch.

💬 martinus commented on pull request "Add simulation-based `CCoinsViewCache` fuzzer":
(https://github.com/bitcoin/bitcoin/pull/27011#issuecomment-1429201435)
@sipa While rebasing #25325 I saw that the fuzzer in `coins_view.cpp` still uses `CCoinsMap coins_map;` and doesn't use the deterministic seeds, shouldn't this fuzzer do that as well?

I can add it while rebasing, I'm touching that line anyways. Unless you want to do this separately