👍 tdb3 approved a pull request: "test: deduplicates p2p_tx_download constants"
(https://github.com/bitcoin/bitcoin/pull/31758#pullrequestreview-2585444075)
ACK 2b94e1abbfdef058546221d63f53b0b58eea22ea

Nice cleanup

💬 tdb3 commented on pull request "test: deduplicates p2p_tx_download constants":
(https://github.com/bitcoin/bitcoin/pull/31758#discussion_r1936523040)
nit: Not a blocker, but it seems like it would make sense for all of these types of constants to live in `p2p.py`.

💬 tdb3 commented on pull request "test: Added coverage to the waitfornewblock rpc":
(https://github.com/bitcoin/bitcoin/pull/31746#issuecomment-2626098245)
> You could change it to:
>
> ```
> if (timeout < 0) {
> throw JSONRPCError(RPC_MISC_ERROR, "Negative timeout");
> }
> ```
>
> In which case coverage would go up, but that seems unnecessary.

Maybe my preference for multi line if-statements is biasing me, but in general this seems like it could be an argument for preferring multi line if-statements over single line ones.

The current [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-note
...

👍 tdb3 approved a pull request: "test: Added coverage to the waitfornewblock rpc"
(https://github.com/bitcoin/bitcoin/pull/31746#pullrequestreview-2585483278)
ACK 93747d934b8a5a1732732a958c0d7e2f5dd0b8c2

Thanks for adding coverage. Induced test failure by commenting out https://github.com/bitcoin/bitcoin/blob/8fa10edcd1706a1f0dc9d8c3adbc8efa3c7755bf/src/rpc/blockchain.cpp#L284

👋 pablomartin4btc's pull request is ready for review: "Add new "address type" column to the "receiving tab" address book page"
(https://github.com/bitcoin-core/gui/pull/753)

💬 pablomartin4btc commented on pull request "Add new "address type" column to the "receiving tab" address book page":
(https://github.com/bitcoin-core/gui/pull/753#issuecomment-2626204992)
The [issue](https://github.com/bitcoin-core/gui/pull/753#issuecomment-2598800192) has been fixed (+ manually tested - transparent/ no impact on the use case).

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1936795545)
(and these two defaults are very unlikely to change).

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1936806097)
Good point. `BlockAssembler::CreateNewBlock()` uses `nFees` internally. It's updated in `AddToBlock`. I could expose it via `CBlockTemplate`.

However this code will hopefully go away soon(tm) with Cluster Mempool, and I suspect it's extremely fast. So it's perhaps not worth refactoring. In any case it could be done in a followup without changing the interface.

💬 Sjors commented on pull request "test: Added coverage to the waitfornewblock rpc":
(https://github.com/bitcoin/bitcoin/pull/31746#issuecomment-2626512279)
> it could be an argument for preferring multi line if-statements over single line ones

Maybe, but it wouldn't catch `?` branches either. You could propose a linter change that insists on multi line if-statements. Though I expect ... strong opinions :-)

👋 hebasto's pull request is ready for review: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176)

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2626571182)
Rebased on top of the merged https://github.com/bitcoin/bitcoin/pull/31428.

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2626845471)
re https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2408594619

> Taking a step back, I wonder what was the last real issue found by Wine, which wasn't found by the native Windows task? If there are none, or not too many, I think this could also be made a nightly CI task, only run on master after a merge (without caching), or in a completely separate repo. The same is done for other stuff, like the valgrind tasks, or the s390x tasks.

Done in https://github.com/hebasto/bitcoin
...

💬 ryanofsky commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2627030718)
At this point (edc1fd44fa0d78168847b199d44ae107e33861e4) it seems like there are 2 failures left. One is the 403 download failure [[1]](https://cirrus-ci.com/task/4569652367458304), [[2]](https://cirrus-ci.com/task/4851127344168960), [[3]](https://cirrus-ci.com/task/4710389855813632)

The other is the [clang-tidy failure](https://cirrus-ci.com/task/6715899064877056) which I'm in the middle of fixing (slowly, since there a lot of warnings so I wanted to figure out how to get clang-tidy and iwyu
...

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2627079274)
Updated 01a43b24436e0aed7b8f79d3857630a4bf6a0545 -> 10e71b4c47e1b199622280d100155ed5d6ef6d66 ([kernelApi_18](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_18) -> [kernelApi_19](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_19), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_18..kernelApi_19))

* Hooked up kernel tests to cmake so the CI can run them through `ctest`
* Added `test_kernel`, `libbitcoinkernel.so`, and `kernel/bitcoin-chainstate` to some more
...

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1937145120)
I added it to the cross compiled windows job now, but it is going to take extra work (#31158) to add it to the native job too.

💬 hebasto commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2627116832)
The "clang-tidy" CI job shouldn't process sources in the `src/ipc/libmultiprocess/example/` [directory](https://github.com/hebasto/bitcoin/actions/runs/13072023868/job/36476232260):
```
[07:11:14.250] [ 13/719][0.1s] clang-tidy-19 -p=/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/ipc/libmultiprocess/example/printer.capnp.proxy-client.c++
[07:11:14.250] error: no input files [clang-
...

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2627119867)
Rebased 10e71b4c47e1b199622280d100155ed5d6ef6d66 -> f4cdffba0d8c60ff6e6eb9732bd1b1f02fcada56 ([kernelApi_19](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_19) -> [kernelApi_20](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_20), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_19..kernelApi_20))

💬 ryanofsky commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2627152623)
> The "clang-tidy" CI job shouldn't process sources in the `src/ipc/libmultiprocess/example/` [directory](https://github.com/hebasto/bitcoin/actions/runs/13072023868/job/36476232260)

Maybe not, but I didn't see an easy way to exclude them, so I thought I would just fix the clang-tidy errors. I fixed the missing inputs problem by just adding necessary targets back to all:

```diff
--- a/src/ipc/CMakeLists.txt
+++ b/src/ipc/CMakeLists.txt
@@ -9,6 +9,10 @@ if (NOT WITH_LIBMULTIPROCESS)

...

⚠️ GregTonoski opened an issue: "incrementalrelayfee configuration option should be present in bitcoin.conf"
(https://github.com/bitcoin/bitcoin/issues/31769)
### Please describe the feature you'd like to see added.

... just like minrelaytxfee and others. Ideally, bitcoin.conf should encompass all configuration options.

### Is your feature related to a problem, if so please describe it.

As a user, I would like to specify value of the incrementalrelayfee configuration option in bitcoin.conf file so that I don't have to type it in a command line each time I start bitcoind.

In Bitcoin 28.1, the option is hidden, i.e. there isn't any description of th
...

💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2627276197)
ARM, TSAN and no wallet passed this time. I didn't wait for MSAN and VS 2022 before pushing your next fix...