👍 theuni approved a pull request: "kernel: Remove key module from kernel library"
(https://github.com/bitcoin/bitcoin/pull/29252#pullrequestreview-2041686542)
Very nice change. ACK a885a166cec6d84d08600f12b25d912bd28af80e

💬 theuni commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1591560587)
This is really unfortunate, but I can't think of anything less hack-ish. And at least it's relegated to the test code.

🤔 furszy reviewed a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-2041703888)
Going slowly here, I want to go through the bdb code diffs too. But, so far, so good. Benchmark 8f2267ff24acdc shows no diff with master and also have created and migrated a v0.6.3 wallet without any problem.

💬 theuni commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1591572104)
Just noticed while reviewing #29252...

This was removed, but the `#include <util/signalinterrupt.h>` include wasn't. I can open a quick cleanup PR, or would you like to @ryanofsky ?

(Looks like `<memory>` can go too.)

📝 kevkevinpal opened a pull request: "test: added test coverage to loadtxoutset could not open file"
(https://github.com/bitcoin/bitcoin/pull/30053)
The functional test coverage did not cover the rpc error of "Couldn't open file..." for loadtxoutset and this test adds coverage for it

This adds coverage to this line
https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2777

💬 stickies-v commented on pull request "rpc: improve submitpackage documentation and other improvements":
(https://github.com/bitcoin/bitcoin/pull/29292#discussion_r1591649657)
You're right, force pushed to add single quotes, thanks.

💬 stickies-v commented on pull request "rpc: improve submitpackage documentation and other improvements":
(https://github.com/bitcoin/bitcoin/pull/29292#discussion_r1591650049)
I quite liked the symmetry/progression of the `* 0` notation but I don't really care either way and agree that `[]` is readable as well, so addressed in force push.

💬 stickies-v commented on pull request "rpc: improve submitpackage documentation and other improvements":
(https://github.com/bitcoin/bitcoin/pull/29292#issuecomment-2097090105)
Force pushed to address @instagibbs's review, only very minor changes.

🤔 ajtowns reviewed a pull request: "crypto: add `NUMS_H` const"
(https://github.com/bitcoin/bitcoin/pull/30048#pullrequestreview-2041832888)
Concept ACK

💬 ajtowns commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1591656490)
Add a comment indicating where this comes from and why it's believed no one knows the discrete log?

💬 luke-jr commented on pull request "refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition":
(https://github.com/bitcoin/bitcoin/pull/29086#issuecomment-2097281761)
Rebased

🚀 fanquake merged a pull request: "net: Replace ifname check with IFF_LOOPBACK in Discover"
(https://github.com/bitcoin/bitcoin/pull/29984)

💬 Saeed149 commented on pull request "[26.x] final changes for 26.1":
(https://github.com/bitcoin/bitcoin/pull/29719#issuecomment-2097333568)
Hi

💬 luke-jr commented on pull request "Remove redundant `-datacarrier` option":
(https://github.com/bitcoin/bitcoin/pull/29942#issuecomment-2097344916)
It seems there is a benefit to redundant options, in that we had to do the whole `prune-prev` thing to preserve the disabled size in the GUI (which is actually kindof weird, since disabling pruning is a pretty big change).

But the confusion isn't nothing either.

Inclined to leave it alone since it's been this way for years. = Weak Concept NACK.

💬 fjahr commented on pull request "rpc: improve submitpackage documentation and other improvements":
(https://github.com/bitcoin/bitcoin/pull/29292#issuecomment-2097394376)
re-ACK 78e52f663f3e3ac86260b913dad777fd7218f210

Trial diff to check: https://github.com/bitcoin/bitcoin/compare/311f52371f37123f1f6186ac818db0741f643aed..78e52f663f3e3ac86260b913dad777fd7218f210

💬 maflcko commented on pull request "test: added test coverage to loadtxoutset could not open file":
(https://github.com/bitcoin/bitcoin/pull/30053#issuecomment-2097465074)
ACK ee67bba76cca2355541f99bb731f58479981b29e

💬 maflcko commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1591851190)
> (Looks like `<memory>` can go too.)

The iwyu output from https://cirrus-ci.com/task/5795750523699200 / https://api.cirrus-ci.com/v1/task/5795750523699200/logs/ci.log :

```
kernel/context.h should add these lines:

kernel/context.h should remove these lines:
- #include <util/signalinterrupt.h> // lines 8-8
- #include <memory> // lines 10-10

The full include-list for kernel/context.h:
---

kernel/context.cpp should add these lines:

kernel/context.cpp should remove these lin
...

💬 maflcko commented on pull request "build, test, doc: Temporarily remove Android-related stuff":
(https://github.com/bitcoin/bitcoin/pull/30049#discussion_r1591863142)
if the images are re-added later, it should be done with a clean revert to avoid bloating the repo with identical-looking but binary-differing png blobs.

💬 maflcko commented on issue "Memory leak with `rest/block` REST endpoint and `getblock` RPC when verbosity >=2":
(https://github.com/bitcoin/bitcoin/issues/30052#issuecomment-2097527398)
Is this a duplicate of https://github.com/bitcoin/bitcoin/issues/25229#issuecomment-2059651519 ?

💬 maflcko commented on issue "Memory leak with `rest/block` REST endpoint and `getblock` RPC when verbosity >=2":
(https://github.com/bitcoin/bitcoin/issues/30052#issuecomment-2097529114)
Related https://github.com/bitcoin/bitcoin/issues/24542 ?