🤔 stickies-v reviewed a pull request: "kernel: Allow null arguments for serialized data"
(https://github.com/bitcoin/bitcoin/pull/33853#pullrequestreview-3454620265)
Post-merge ACK a3ac59a4316305fb38a5338b48940682889d0dc2

It seems like the same reasoning should apply to `btck_chainstate_manager_options_create`'s `data_directory` and `blocks_directory`? Perhaps useful for a follow-up, with a brief docstring on how to use and interpret `BITCOINKERNEL_ARG_NONNULL` so we're clear on its usage? E.g.:

```
/**
* BITCOINKERNEL_ARG_NONNULL is a compiler attribute used to indicate that
* certain pointer arguments to a function are not expected to be null.

...

💬 stickies-v commented on pull request "kernel: Allow null arguments for serialized data":
(https://github.com/bitcoin/bitcoin/pull/33853#discussion_r2519176246)
Probably useful to `LogError` here, and in the other 2 places?

✅ maflcko closed an issue: "Enable C++26 P3471 "Standard Library Hardening" ahead of time?"
(https://github.com/bitcoin/bitcoin/issues/32265)

💬 maflcko commented on issue "Enable C++26 P3471 "Standard Library Hardening" ahead of time?":
(https://github.com/bitcoin/bitcoin/issues/32265#issuecomment-3523076321)
This seems nice, but probably has limited benefit in practise, given the tradeoffs. Closing for now.

👍 l0rinc approved a pull request: "txgraph: drop move assignment operator"
(https://github.com/bitcoin/bitcoin/pull/33862#pullrequestreview-3454502686)
code review ACK aef40b93cf057d2a27d61881b0858d491206bcd3

Simplifies the public interface of `TxGraph` by deleting the move assignment operator only used in tests.
The usage seems correct as it is currently, but left a question about the move-constructor which the fuzzers seem to be confused about.

💬 l0rinc commented on pull request "txgraph: drop move assignment operator":
(https://github.com/bitcoin/bitcoin/pull/33862#discussion_r2519205585)
This seems used to me, but https://storage.googleapis.com/oss-fuzz-coverage/bitcoin-core/reports/20251112/linux/src/bitcoin-core/src/txgraph.cpp.html#L3469 seems confused by it.

💬 instagibbs commented on pull request "txgraph: drop move assignment operator":
(https://github.com/bitcoin/bitcoin/pull/33862#issuecomment-3523123058)
link to discussion, I think https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2518940184

📝 ismaelsadeeq opened a pull request: "scripted-diff: fix leftover references to `policy/fees.h`"
(https://github.com/bitcoin/bitcoin/pull/33864)
Fixes #33863

ryanofsky wrote
> I still see some references to the src/policy/fees.h file removed by this PR:

```
$ git grep -n policy/fees.h
src/wallet/rpc/spend.cpp:206: * @param[in] conf_target UniValue integer; confirmation target in blocks, values between 1 and 1008 are valid per policy/fees.h;
test/functional/rpc_estimatefee.py:39: # max value of 1008 per src/policy/fees.h
test/functional/rpc_psbt.py:604: assert_raises_rpc_error(-8, "Invalid conf_
...

👋 ismaelsadeeq's pull request is ready for review: "scripted-diff: fix leftover references to `policy/fees.h`"
(https://github.com/bitcoin/bitcoin/pull/33864)

💬 ismaelsadeeq commented on issue "fees: leftover references to `policy/fees.cpp`":
(https://github.com/bitcoin/bitcoin/issues/33863#issuecomment-3523137257)
Thanks @ryanofsky @fanquake I opened #33864 with a fix

💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3523154890)
I believe at this point I've responded to all the review feedback either in this PR or for non-blocking suggestions, in the followups PR (#33591) -- please let me know if any reviewers think I've missed anything that should be addressed here.

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