👍 maflcko approved a pull request: "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`"
(https://github.com/bitcoin/bitcoin/pull/32296#pullrequestreview-2839190115)
nice, lgtm

💬 maflcko commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088333853)
style nit: Could adjust the indent manually, while touching this, so that the `ser_writedata*` are aligned? Also, personally I prefer to use `uint8_t(a)` over the verbose static cast for integral types. See https://github.com/bitcoin/bitcoin/blob/f9d8910539a2f7c4542457e08cd2bdd2dcb9cd08/doc/developer-notes.md#L111

But just style nits, up to you.

💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2088367575)
Good point, otherwise it leaves `bitcoin.exe` behind because it's not inside the `daemon` subdirectory.

I'll add this to my Windows test:

```
diff --git a/share/setup.nsi.in b/share/setup.nsi.in
index 270468ad7b..2160cc05a1 100644
--- a/share/setup.nsi.in
+++ b/share/setup.nsi.in
@@ -130,6 +130,7 @@ done${UNSECTION_ID}:

# Uninstaller sections
Section /o -un.Main UNSEC0000
+ Delete /REBOOTOK $INSTDIR\@BITCOIN_WRAPPER_NAME@@EXEEXT@
Delete /REBOOTOK $INSTDIR\@BITCOIN_GUI_
...

💬 delta1 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2879319514)
@gmaxwell drahtbot has categorized your comment as nack instead of ack

👍 TheCharlatan approved a pull request: "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage"
(https://github.com/bitcoin/bitcoin/pull/32467#pullrequestreview-2839318576)
ACK da5d55227e6aead8a9fbb85e967801cfcd4283ce

💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2088424348)
Works like a charm.

I also briefly tested `bitcoin.exe gui`, `node` and `rpc`.

📝 fanquake opened a pull request: "build: document why we check for `std::system`"
(https://github.com/bitcoin/bitcoin/pull/32491)
It's probably debatable if we support targets like iOS, but for now, document why we are checking for this standard library feature.

Trying to use `std::system` for a `aarch64-darwin-ios` target results in:
```bash
test.cpp:7:10: error: 'system' is unavailable: not available on iOS
7 | std::system("some_command");
| ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/_stdlib.h:203:6: note: 'system' has been explicitly marked unavailable here
203 |
...

✅ fanquake closed an issue: "Mistake in `feature_pruning.py` functional test?"
(https://github.com/bitcoin/bitcoin/issues/32249)

💬 fanquake commented on issue "Mistake in `feature_pruning.py` functional test?":
(https://github.com/bitcoin/bitcoin/issues/32249#issuecomment-2879402851)
Closed via #32312.

💬 fanquake commented on pull request "test: Fix feature_pruning test after nTime typo fix":
(https://github.com/bitcoin/bitcoin/pull/32312#issuecomment-2879403090)
Backported to 29.x in #32292.

💬 fjahr commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#issuecomment-2879442591)
re-ACK da5d55227e6aead8a9fbb85e967801cfcd4283ce

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2088470479)
Yes, that is it. I don't think this will actually matter in the grand scheme. I think I'll drop it again.

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2088486315)
Pushed the compiler issue on a separate branch here: https://github.com/TheCharlatan/bitcoin/actions/runs/15016860100/job/42196724477#step:10:198 .

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2088492832)
> edit: I see this is already (partially) suggested and addressed in https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2078426205, I think adding the IsCoinRef concept could still be nice but less important than specifying the std::span

I did implement a concept initially, but did not find it really that helpful, since the two usable template variants are concretely defined anyway.

💬 polespinasa commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2088471060)
Nit: Will it be removed for sure? If there's no clear consensus on removal, maybe is worth deleting the last part of the sentence. I've seen a lot of controversy bc of that the last two days.

🤔 polespinasa reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2839410351)
code review ACK 35bcd8eed3

left one small comment to avoid further discussions...

💬 maflcko commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2879517285)
> drahtbot has categorized your comment as nack instead of ack

<details><summary>off-topic reply</summary>

When a comment contains both `ack` and `nack` (in uppercase), the bot just picks one. Obviously, it is off-topic to discuss here, but I think there is a misunderstanding what the goal of the summary comment by the bot is. The goal is *not* to have everyone "register" their "vote". It is simply a best-effort overview to have a clickable link to every reviewer's most recent review comme
...

💬 maflcko commented on pull request "doc: Add troubleshooting note about Guix on SELinux systems":
(https://github.com/bitcoin/bitcoin/pull/32442#discussion_r2088538753)
Are there steps to reproduce on a fresh install? I tried Ubuntu 24.04, but it seems to pass:

```
1 export DEBIAN_FRONTEND=noninteractive && apt update && apt install git vim htop guix bash curl make -y && groupadd --system guixbuild && for i in `seq -w 1 10`; do useradd -g guixbuild -G guixbuild -d /var/empty -s `which nologin` -c "Guix build user $i" --system guixbuilder$i; done
2 git config --global merge.conflictstyle zdiff3
3 guix-daemon --build-users-group=guixbuild
...

📝 fanquake opened a pull request: "test: add skip_if_running_under_valgrind()"
(https://github.com/bitcoin/bitcoin/pull/32492)
Enable it in the USDT tests. The context (from 0xB10C):

> every time the tracepoint is reached a SIGTRAP is fired.
> No matter the tracepoint contents, even with an empty one.
> Valgrind intercepts SIGTRAP and aborts.

See discussion in #32374.

💬 fanquake commented on issue "test: interface_usdt_net.py failure under --valgrind":
(https://github.com/bitcoin/bitcoin/issues/32374#issuecomment-2879582658)
Opened #32492 to add skipping (for now).