💬 hebasto commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#issuecomment-3523159466)
@ryanofsky

Would you like to take another look at this PR?

💬 kevkevinpal commented on pull request "scripted-diff: fix leftover references to `policy/fees.h`":
(https://github.com/bitcoin/bitcoin/pull/33864#issuecomment-3523327846)
ACK [b0a3887](https://github.com/bitcoin/bitcoin/pull/33864/commits/b0a38871546dfcdd3a578c1ae4c28a88b6ee32d5)

A doc change and scripted-diff are pretty straightforward

💬 cobratbq commented on pull request "doc: update interface, --stdin flag, IPC-command signtx (#31005)":
(https://github.com/bitcoin/bitcoin/pull/33765#discussion_r2519455789)
Oookaaaaaay .. that changes things considerably. That was not at all clear to me.

Any chance you can drop a link to the place in source-code (assuming it is somewhat concentrated), such that I can have a quick peek. I could search myself, but I suspect you are familiar with the code-base.

💬 willcl-ark commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#issuecomment-3523497358)
```
❯ uname -m; find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
x86_64
eacb251187a66b61d420c134b48b4081cac704cfaef3965bc10f61f2dedbe9b1 guix-build-79d5f120f4d1/output/aarch64-linux-gnu/SHA256SUMS.part
245e7592e13a90600f6ccff66442e98027d9f36b868942c88a117c05625e603e guix-build-79d5f120f4d1/output/aarch64-linux-gnu/bitcoin-79d5f120f4d1-aarch64-linux-gnu-debug.tar.gz
083a468e350320244bfb83b46f2015d28a31ab69845fd5f605e31742
...

🤔 w0xlt reviewed a pull request: "net_processing: reorder the code that handles the VERSION message"
(https://github.com/bitcoin/bitcoin/pull/33792#pullrequestreview-3455027702)
The reordering of `WTXIDRELAY` and `SENDADDRV2` looks safe to me and keeps them before the tx‑reconciliation announcement.

What’s the rationale for moving `m_addrman.Good(pfrom.addr)` earlier ?

📝 hebasto opened a pull request: "cmake: Specify Windows plugin path in `test_bitcoin-qt` property"
(https://github.com/bitcoin/bitcoin/pull/33865)
This PR simplifies testing on Windows by removing the need to set the `QT_PLUGIN_PATH` environment variable for different build configurations. For example, the paths might otherwise be:
- `C:/Users/hebasto/dev/bitcoin/build/vcpkg_installed/x64-windows/Qt6/plugins/` for "Release"
- `C:/Users/hebasto/dev/bitcoin/build/vcpkg_installed/x64-windows/debug/Qt6/plugins/` for "Debug"

🤔 w0xlt reviewed a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3455032313)
crACK https://github.com/bitcoin/bitcoin/pull/33565/commits/99898534477683ba60674473e421957487caab67

💬 janb84 commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#issuecomment-3523534979)
My Guix Build Output (matching)

**Host architecture:** `aarch64`
**Commit:** `79d5f120f4d1`

```shell
eacb251187a66b61d420c134b48b4081cac704cfaef3965bc10f61f2dedbe9b1 guix-build-79d5f120f4d1/output/aarch64-linux-gnu/SHA256SUMS.part
245e7592e13a90600f6ccff66442e98027d9f36b868942c88a117c05625e603e guix-build-79d5f120f4d1/output/aarch64-linux-gnu/bitcoin-79d5f120f4d1-aarch64-linux-gnu-debug.tar.gz
083a468e350320244bfb83b46f2015d28a31ab69845fd5f605e31742c9665e27 guix-build-79d5f120f4d
...

💬 maflcko commented on pull request "kernel: Allow null arguments for serialized data":
(https://github.com/bitcoin/bitcoin/pull/33853#discussion_r2519563595)
Ah sorry for missing this, but this doesn't work at all. A nullptr can never point to an object, so it is always invalid to create a non-zero span from it.

libc++ also complains about this.

You can reproduce via:

`rm -rf ./bld-cmake && cmake -B ./bld-cmake -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++;-stdlib=libc++;-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG' -DBUILD_GUI=OFF -DBUILD_FUZZ_BINARY=OFF -DBUILD_BENCH=OFF -DBUILD_KERNEL_LIB=ON -DENABLE_WALLET=OFF -DENABLE_IPC
...

🤔 janb84 reviewed a pull request: "scripted-diff: fix leftover references to `policy/fees.h`"
(https://github.com/bitcoin/bitcoin/pull/33864#pullrequestreview-3455154399)
ACK b0a38871546dfcdd3a578c1ae4c28a88b6ee32d5

Trivial cleanup PR.
This time I did some extra code searches to find any possible leftovers, could not find one.

💬 janb84 commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3523635392)
Since the "side-effect" of this pr will enable the kernel stuff is the PR title still fitting ?
Maybe something like: CI: base taks on `dev-mode`

💬 hebasto commented on pull request "cmake: Specify Windows plugin path in `test_bitcoin-qt` property":
(https://github.com/bitcoin/bitcoin/pull/33865#issuecomment-3523732124)
Friendly ping @purpleKarrot :)

💬 vicjuma commented on pull request "scripted-diff: fix leftover references to `policy/fees.h`":
(https://github.com/bitcoin/bitcoin/pull/33864#issuecomment-3523754781)
ACK the OP’s latest commit.

Confirmed that `git grep "policy/fees.h"` does not return any lines, meaning that the solution works.

> git grep -l "policy\/fees\.h" | xargs sed -i "s/policy\/fees.h/policy\/fees\/block_policy_estimator.h/g"

💬 TheCharlatan commented on pull request "kernel: Allow null arguments for serialized data":
(https://github.com/bitcoin/bitcoin/pull/33853#discussion_r2519703206)
How about keeping the checking, but testing it without going through the c++ wrapper?

💬 purpleKarrot commented on pull request "cmake: Specify Windows plugin path in `test_bitcoin-qt` property":
(https://github.com/bitcoin/bitcoin/pull/33865#discussion_r2519712187)
I think the `_$<CONFIG>` part is unnecessary, because the value of the `LOCATION` property is already config dependent.

`$<PATH:GET_*,...>` requires CMake version 3.24.

🤔 hebasto reviewed a pull request: "guix: build `bitcoin-qt` with static libxcb & utils"
(https://github.com/bitcoin/bitcoin/pull/33537#pullrequestreview-3455373185)
Tested `bitcoin-qt` from `bitcoin-79d5f120f4d1-x86_64-linux-gnu.tar.gz` on Fedora 42. It works without issues. `QXcbIntegrationPlugin` loaded correctly:
```
2025-11-12T20:21:43Z Bitcoin Core version v30.99.0-g79d5f120f4d10b681d3cebad1fcabff3c6f0c8fc (release build)
2025-11-12T20:21:43Z Qt 6.7.3 (static), plugin=xcb
2025-11-12T20:21:43Z Static plugins:
2025-11-12T20:21:43Z QMinimalIntegrationPlugin, version 395008
2025-11-12T20:21:43Z QXcbIntegrationPlugin, version 395008
2025-11-12T20:21:43Z S
...

💬 hebasto commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#discussion_r2519797731)
```suggestion
@@ -504,6 +504,7 @@ qt_config_compile_test(xcb_syslibs
```
to avoid
```
patching file qtbase/src/gui/configure.cmake
Hunk #1 succeeded at 504 (offset 5 lines).
```

💬 Crypt-iQ commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#discussion_r2519814656)
Is it always possible to split a 8 MWU template into two <= 4MB responses? Consider a (very) contrived example: a node for some reason has three non-standard jumbo taproot-spend txns in their mempool of sizes 2 MWU, 3 MWU, 3 MWU, and they go to generate a template for this. If the receiver doesn't have these txns, there's no way they can fit in the max message size. At least, this is what I gather is possible from reading BIP 342.

📝 TheCharlatan opened a pull request: "refactor: Let CCoinsViewCache::BatchWrite return void"
(https://github.com/bitcoin/bitcoin/pull/33866)
CCoinsViewCache::BatchWrite always returns true if called from a backed cache, so just return void instead. Also return void from ::Sync and ::Flush.

This allows for dropping a FatalError condition and simplifying some dead error handling code a bit.

Special case the coins view fuzz test since we now surface an exception instead of returning `false`, as previously implemented in `CCoinsView::BatchWrite`, when operating under a raw `CCoinsView`. This path should not be triggered in p
...

💬 plebhash commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3524052025)
> Ok, that's not really how you're supposed to use it :-)

hmm I guess this would indeed be a misuse of the interface if my end goal was to get Coinbase Tx data, and nothing more.

however, we **always need** to call `getBlock` specifically because it contains the `header` + `txdata` of the template... and as a byproduct, we happen to also get the Coinbase Tx data, which saves us one extra call to `getCoinbase`/`getCoinbaseTx`

at some point we'll start exploring the so called "Coinbase-only Sv2
...