💬 hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1657807539)
Maybe go more general to increase MacOS/darwin compatibility:
```
...
+ "\nLoad wallet using absolute path (Unix):\n"
+ HelpExampleCli("loadwallet", "\"/path/to/specialWallet/\"")
+ HelpExampleRpc("loadwallet", "\"/path/to/specialWallet/\"")
+ "\nLoad wallet using absolute path (Windows):\n"
+ HelpExampleCli("loadwallet", "\"C:\\Users\\myusername\\specialWallet\\\"")
+ HelpExampleRpc("loadwallet", "\"C:\\Users\\myusername\\specialWallet\\\"")
...
```

👍 ryanofsky approved a pull request: "Mining interface followups, reduce cs_main locking, test rpc bug fix"
(https://github.com/bitcoin/bitcoin/pull/30335#pullrequestreview-2146507019)
Code review ACK a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed. Just new error string is added since last review, and a commit message was updated

💬 maflcko commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2195670730)
> Not a blocker at all, I just wonder if there is a verified recipe to get clang-16 for RISC-V platform?

Not sure what you mean, but this depends on your OS. You can install it normally. For example:

```
# apt install clang-16 && clang-16 --version
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
clang-16 is already the newest version (1:16.0.6-23ubuntu4).
0 upgraded, 0 newly installed, 0 to remove and 69 not upgraded.
Ubuntu clang ve
...

👍 tdb3 approved a pull request: "init: Add option for rpccookie permissions (replace 26088)"
(https://github.com/bitcoin/bitcoin/pull/28167#pullrequestreview-2146552397)
cr ACK 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe

🚀 ryanofsky merged a pull request: "init: Add option for rpccookie permissions (replace 26088)"
(https://github.com/bitcoin/bitcoin/pull/28167)

💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2195702113)
> Let me know if I'm missing something but at what point is the fuzzed wallet funded? I generated a coverage report and the test seems to blocked at SelectCoins in CreateTransactionInternal. It always results in an insufficient funds error, which means the fuzzer never gets past this line in CreateTransaction.

It is not being funded, I will add it. Thanks.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1657850742)
I don't think there is a need for that. If `parent == child` then this is a no-op (every transaction is already an ancestor and descendant of itself).

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1657857264)
Done.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1657857307)
Done.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1657857391)
Added a comment.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1657857455)
Done.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1657857579)
Done with a `static_assert`.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1657857631)
Done.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1657857760)
Done with a `static_assert`.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2195728404)
Addressed some of @instagibbs' comments, which involved moving some of the `src/test/fuzz/cluster_linearize.cpp` code to a common `src/test/util/cluster_linearize.h`, where it is also available to a newly-added (but pretty barebones) `src/test/cluster_linearize_tests.cpp`.

🚀 ryanofsky merged a pull request: "Mining interface followups, reduce cs_main locking, test rpc bug fix"
(https://github.com/bitcoin/bitcoin/pull/30335)

🤔 cbergqvist reviewed a pull request: "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB"
(https://github.com/bitcoin/bitcoin/pull/26596#pullrequestreview-2146541283)
`git range-diff master a8f8a2d 9700c217`

Agree with @furszy on reordering the commits to make it possible to retain the 3 asserts in `MigrateToDescriptor()`.

💬 cbergqvist commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1657873459)
This made me cringe when first looking at the code too, but I guess retaining the inconsistency makes it easier to search for `AddKeyPubKeyInner(const CKey& key, const CPubKey &pubkey)` and find all 4 places it exists. Changing it in this PR would arguably add noise (on the other hand - if not now, then when?).

💬 ajtowns commented on pull request "logging: Use LogFatal instead LogError for fatal errors":
(https://github.com/bitcoin/bitcoin/pull/30347#discussion_r1657886163)
The medium post describes five levels with the same names as ours currently: error, warn, info, debug, trace; the other six links all describe syslog's behaviour, which was designed for unix systems in the 1980s. Logging systems we actually use include python's (which has 5 default levels), leveldb (which does not have levels, so we treat it all as debug), and libevent (which has 4 levels). So, no, 6-8 levels is not standard, and for a custom subsystem we should be doing what's best for us, not
...

📝 cobratbq opened a pull request: "doc: external-signer, example 'createwallet' RPC call requires eight argument"
(https://github.com/bitcoin/bitcoin/pull/30354)
The eight argument 'true' is necessary to invoke the external signer upon creating the wallet. The parameter defaults to 'false' otherwise.