🤔 janb84 reviewed a pull request: "kernel: Introduce C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3430058931)
Post merge ACK 6c7a34f3b0bd39ef7a1520aac56e12f78e5cc969

To test this PR I made a dotnet wrapper for libbitcoinkernel and that went pretty smooth, no major issues.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500462588)
On a system with 15 worker threads + main, it is likely that main will not be waiting much. The other 15 threads are busy setting newly fetch coins to ready so that the main thread can continuously read `true` for the ready flags.
On a system with only 3 worker threads + main, it is likely that the 3 worker threads will not be able to fetch and set ready coins fast enough where the main thread does not have to wait. For instance all 3 threads are fetching from disk, and the main thread reads th
...

💬 maflcko commented on pull request "test: skip interface_ipc if python version is freethreaded and PYTHON_GIL=0 is not set":
(https://github.com/bitcoin/bitcoin/pull/33795#discussion_r2500471963)
> > The correct fix would be to just require the GIL
>
> That's right, the text of the warning (from #33582) states "The global interpreter lock (GIL) has been enabled to load module 'capnp.lib.capnp'", so my understanding is when this test is run, the GIL is enabled and the test succeeds, except there is a spurious warning at the end which causes the test runner to fail.

Ok, this was not clear at all to me. It would be good to at least document, so that code-readers don't have to `git bla
...

💬 maflcko commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2500488831)
> There are 2 places in `validation.cpp` where `STR_INTERNAL_BUG` could be used too, but I suppose you didn't because it (afaik) requires us to abandon the compile time format string checking?

I think all format strings should be checked at compile time, even with `STR_INTERNAL_BUG`. I think `STR_INTERNAL_BUG` can't be used in the validation places, because `STR_INTERNAL_BUG` includes raw newlines and those are discouraged from logging. Though, it only happens on internal bugs anyway, so I ca
...

💬 ajtowns commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3499059367)
> Doe this mean that, due to `AreInputsStandard`, even more UTXOs were soft-confiscated when #12460 was added?

I think that's pretty likely, but haven't confirmed. Should mean that bare multisigs are only spendable if any data is pushed as a fake un/compressed pubkey.

👍 hodlinator approved a pull request: "util: Allow `Assert` (et al.) in contexts without __func__"
(https://github.com/bitcoin/bitcoin/pull/33785#pullrequestreview-3430025702)
re-ACK fad6efd3bef1d123f806d492f019e29530b03a5e

2 added commits since previous review (https://github.com/bitcoin/bitcoin/pull/33785#pullrequestreview-3427192513):
* Remove no longer needed disabling of linter according to CI. 👍 (Re-touching `NONFATAL_UNREACHABLE` but no big deal).
* De-duplicate code by using `STR_INTERNAL_BUG()`.

💬 hodlinator commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2500429489)
nit: This is a case of clang-format failing to understand what is happening inside a macro.

https://releases.llvm.org/17.0.1/tools/clang/docs/ClangFormatStyleOptions.html#cpp11bracedliststyle

https://github.com/bitcoin/bitcoin/blob/fada379589a17e86396aa7c2ce458ff2ff602b84/src/.clang-format#L87

If I in the same commit change this to...

```C++
throw NonFatalCheckError{"Unreachable code reached (non-fatal)", std::source_location::current()}
```

...and also add...
```C++
void Func_NONFATAL_
...

💬 flack commented on pull request "refactor: remove dead branches in `SingletonClusterImpl`":
(https://github.com/bitcoin/bitcoin/pull/33768#discussion_r2500676630)
this now always returns true. Is that needed for anything?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2500696484)
We discussed this out of band, here's the summary:
* the main thread is special because it has access to the dbcache, it can insert there without locking the cache.
* the threads each have their inputs now, each of which have a switch to signal when they've fetched the input and move on to the next
* the threads each compete for which input to fetch, marking them one-by-one as ready
* the main thread goes in order, spins until the next one is available, if it needs to wait, it does some fetc
...

💬 l0rinc commented on pull request "refactor: remove dead branches in `SingletonClusterImpl`":
(https://github.com/bitcoin/bitcoin/pull/33768#discussion_r2500745964)
Good question, I have already checked that, the sibling [GenericClusterImpl::Split](https://github.com/bitcoin/bitcoin/blob/master/src/txgraph.cpp#L1392) can return `false` which would [trigger deletion](https://github.com/bitcoin/bitcoin/blob/master/src/txgraph.cpp#L1773-L1777).

💬 sipa commented on pull request "refactor: remove dead branches in `SingletonClusterImpl`":
(https://github.com/bitcoin/bitcoin/pull/33768#discussion_r2500769009)
Yeah. It's indeed the case that within `SingletonClusterImpl` it'll always return `true`, but this isn't the case for all possible `Cluster` implementations.

⚠️ valentinewallace opened an issue: "REST equivalent for `gettxspendingprevout`"
(https://github.com/bitcoin/bitcoin/issues/33808)
This becomes more useful with #24539.

This would be useful for Lightning nodes so that they can use Bitcoin Core in situations where they currently use Electrum.

For example, an LSP has many mobile users who all connect to the LSP's Electrum instance to get information on whether their funding/commitment transactions have confirmed, etc.

Instead, they could be using the Core REST interface to retrieve this info, using `gettxspendingprevout` + @sstone’s PR.

A caveat is that resolving this
...

💬 valentinewallace commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-3499495731)
@sstone Question -- In LDK, when we confirm transactions, we require the transaction's index-in-block to be provided, for short channel ID construction. I'm curious how this API works for you without this information?

⚠️ TheBlueMatt opened an issue: "Cache headers in REST"
(https://github.com/bitcoin/bitcoin/issues/33809)
it would be nice to serve appropriate cache headers in the REST interface so that you can just slap nginx/cloudflare in front of it and have a nice caching proxy. Eg getXbyheight requests would get cached for 5 seconds and getYbyhash requests could get cached for a month/year.

💬 TheBlueMatt commented on issue "Cache headers in REST":
(https://github.com/bitcoin/bitcoin/issues/33809#issuecomment-3499514023)
@achow101 said she'd work on this

📝 hebasto opened a pull request: "ci: Add fast IWYU job"
(https://github.com/bitcoin/bitcoin/pull/33810)
This PR seeks to address feedback raised in the discussion of https://github.com/bitcoin/bitcoin/pull/33779, specifically from this [comment](https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3491515263):
> Maybe a better approach would be to run the enforced sections in a separate, faster job? Some of the linters are already a bit annoying to invoke locally, so I usually just run the lint job. Doing the same for the includes seems fine to me.

The new "iwyu" CI job runs very quickly
...

💬 hebasto commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3499831767)
> Maybe a better approach would be to run the enforced sections in a separate, faster job? Some of the linters are already a bit annoying to invoke locally, so I usually just run the lint job. Doing the same for the includes seems fine to me.

Done in https://github.com/bitcoin/bitcoin/pull/33810.

💬 marcofleon commented on issue "RFC: Add multiprocess fuzz target":
(https://github.com/bitcoin/bitcoin/issues/23015#issuecomment-3499863564)
I'd say fuzzamoto is well suited for this. I added a proof-of-concept scenario testing the mining interface, which just randomly calls the methods from the `Mining` and `BlockTemplate` interfaces for now. I'm sure we could come up with more sophisticated scenarios that involve the `Wallet` and `Chain` interfaces as well. The scenario includes a mining IPC client that connects to the `bitcoin-node` Unix socket. It's currently directly in the scenario file but could be extracted into a reusable mu
...

🤔 w0xlt reviewed a pull request: "ci: Add fast IWYU job"
(https://github.com/bitcoin/bitcoin/pull/33810#pullrequestreview-3431033031)
Concept ACK

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3499901560)
> it looks like in AcceptSingleTransaction() we acquire the lock and release it before the MemPoolAccept object is destroyed, causing a change to txgraph (when the changeset is destroyed) while the lock is not held. Will fix.

> Implemented a fix for the locking issue in https://github.com/bitcoin/bitcoin/commit/51d213d1449b61665628ce9b4e0209cff3c4f818.

IIUC we need to ensure we don’t release the mempool lock until after the changeset (i.e. SubPackageState) is destroyed.

The approach in
...