👍 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.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1658060325)
> With that, this PR could add only DIRTY coins and ignore the FRESH flag, which would be simpler.

I'm not sure how that would be simpler. The two flags are coupled and so adding one and not the other would be a more complex diff. Perhaps I don't understand what you are suggesting?

I could just replace the single FRESH-but-not-DIRTY check with an assertion and remove this comment about tracking FRESH coins?

💬 maflcko commented on pull request "doc: external-signer, example 'createwallet' RPC call requires eight argument":
(https://github.com/bitcoin/bitcoin/pull/30354#discussion_r1658229564)
Could use named arguments?

🤔 alfonsoromanz reviewed a pull request: "test: Added coverage to Block not found error using gettxoutsetinfo"
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2147191277)
Re ACK 82c454c65658439481b50fe71ed097ecb8d70737