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

💬 fjahr commented on pull request "test: Fix feature_pruning test after nTime typo fix":
(https://github.com/bitcoin/bitcoin/pull/32312#issuecomment-2879584074)
> I tested by using other blocks and not necessarily changing the node that is requesting blocks from node1.

> my personal preference would be to make node2 request block 773 (or some other valid block) from node1. but yes, this works :)

I don't think this is a good idea because it's not testing the same behavior that the test currently does. The test intends to check that the prune height doesn't change when the block data is available, but not the undo data of these blocks. To do this,
...

💬 maflcko commented on pull request "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`":
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-2879587037)
> So here the output types are changed from `std::string` to `std::string_view` so that the values are known at compile time or during the early stages of program startup, before main() execution.

Can you explain why this is needed? Seems a lot of shuffling and code changes for a trivial change that will later cast everything to `std::string` anyway?

👍 hebasto approved a pull request: "fuzz: Properly setup wallet in wallet_fees target"
(https://github.com/bitcoin/bitcoin/pull/32488#pullrequestreview-2839582440)
ACK fa427ffceeefd368a1ade273501ce4b01133ad4d, this change fixes the issue when building the "Debug" configuration with MSVC on Windows.

@maflcko

Thank you for picking this up!