💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2650680523)
That would be `DynSock::Pipe::GetBytes()` and `DynSock::Pipe::PushBytes()`. `Sock::Send()` and `Sock::Recv()` also use raw pointer + length, but they were not introduced in this PR and, more importantly, are supposed to mimic 1:1 the `send(2)` and `recv(2)` syscalls, in return value, arguments and semantic.

For `GetBytes()` and `PushBytes()` the change would be:

```diff
diff --git i/src/test/util/net.h w/src/test/util/net.h
index 3e717341d8..a28c442035 100644
--- i/src/test/util/net.h

...

🚀 ryanofsky merged a pull request: "build: disable bitcoin-node if daemon is not built"
(https://github.com/bitcoin/bitcoin/pull/31834)

💬 hodlinator commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2650784726)
> > Got a big diff, indicating a lot of test non-determinism:
>
> I haven't looked in detail, but I presume much is due to randomness. On one hand, one could go with #31486 for the unit tests as well. On the other hand, some unit tests are fuzz tests (drawing inputs from a random seed, which is recorded), so they'd be a bit more limited if the randomness was made deterministic. But I think either way is fine.

This gives me an 8.4K line diff instead of 39k, so there is some hope:
```
RAND
...

💬 ryanofsky commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2650797960)
re: What do you think? PR? Leave it alone?

Whichever you prefer, I'd happily review it. I wanted to make suggestion more as an idea to keep in mind when updating this code or writing new code, because of benefits described in https://github.com/bitcoin/bitcoin/issues/31272

Also ideally the change would go further IMO: replace `std::vector<uint8_t>` with `std::vector<std::byte>`, replace `memcpy` with `std::vector::assign`, replace Recv/Send arguments with `span` instead of `void*`. I don't
...

👍 hodlinator approved a pull request: "logging: Ensure -debug=0/none behaves consistently with -nodebug"
(https://github.com/bitcoin/bitcoin/pull/31767#pullrequestreview-2608732876)
re-ACK 7afeaa24693039730c9ae00c325c2b9d0e2deda1

Changes since [previous ACK](https://github.com/bitcoin/bitcoin/pull/31767#pullrequestreview-2599809742):
- Broke out initial commit which tests prior behavior (with minor improvements).
- Terser test for last occurrence of negated arg suggested by self.

💬 0xB10C commented on pull request "ci: Skip read-write of default env vars":
(https://github.com/bitcoin/bitcoin/pull/31678#issuecomment-2650860541)
> > I want to check if this helps [#30852 (comment)](https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2558198430) when used with #31545 by not having to keep the base images around anymore.
>
> @0xB10C Are you waiting for me to rebase this once more? I've held back, because there is already one review, but I am happy to rebase.

Thanks for the reminder. I played around with this a bit and it does indeed help:

When using https://github.com/bitcoin/bitcoin/pull/31545, the base
...

📝 maflcko opened a pull request: "test: Clear scheduler after init to increase fuzz stability"
(https://github.com/bitcoin/bitcoin/pull/31841)
Leaking a scheduler with a non-empty queue from the fuzz initialization phase into the fuzz target execution phase is problematic, because it messes with coverage data. This in turn is problematic, because it leads to:

* Decrease in fuzz target execution stability (non-determinism when running the fuzz target).
* Decrease in fuzz input merge stability (non-determinism when selecting a minimum set of fuzz input to reach maximum coverage), which leads to qa-assets bloat.

Fix one such issue.
...

💬 0xB10C commented on pull request "ci: Skip read-write of default env vars":
(https://github.com/bitcoin/bitcoin/pull/31678#issuecomment-2650867558)
My understanding is that https://github.com/bitcoin/bitcoin/pull/28138 was only needed for `DANGER_RUN_CI_ON_HOST`. That's why removing it here isn't a regression to pre 28138, correct?

💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2650868943)
I've created https://github.com/bitcoin/bitcoin/pull/31841 to make it easier to test this on a real patch.

💬 maflcko commented on pull request "ci: Skip read-write of default env vars":
(https://github.com/bitcoin/bitcoin/pull/31678#issuecomment-2650891965)
> My understanding is that #28138 was only needed for `DANGER_RUN_CI_ON_HOST`. That's why removing it here isn't a regression to pre 28138, correct?

I do not know if they were ever needed to be set inside the CI pod, given that they fall back to the default anyway (see pull description). This may have just been added out of caution, as on the outside the pod, they are required sometimes to be set, see the docs:

https://github.com/bitcoin/bitcoin/blame/86528937e5c4da2e12c46085fc41e87ed75925
...

💬 eval-exec commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1950895954)
Thank you, updated.

💬 0xB10C commented on pull request "ci: Skip read-write of default env vars":
(https://github.com/bitcoin/bitcoin/pull/31678#issuecomment-2650904601)
Makes sense, thanks.

ACK fa952acdb6ef1b07b7927202d1373fa7479fd5e4

💬 davidgumberg commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2650954493)
> > Summarizing #31476, the CentOS CI task has been configuring a build with python `3.9` (the latest version in the Stream 9 core repo) which is below the minimum version of `3.10`
>
> I don't think this is entirely right. appstream ships with 3.11 and 3.12 on centos. (You can install them just like you can install g++-14 on centos 9).

By 'core repo' I meant what I guess CentOS calls the 'BaseOS' repository, fixed in the description.

> > Making this matter a bit worse, we now also [use
...

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1950980811)
> it's possible to start the node and quickly shut it down. If that happens then `m_tip_block` remains null.
> If you call `createNewBlock()` during that time I'm not sure what happens

The RPC is not yet serving requests when [`m_tip_block` is assigned a value](https://github.com/bitcoin/bitcoin/blob/86528937e5c4da2e12c46085fc41e87ed759258e/src/node/kernel_notifications.cpp#L53-L58), but IPC is started before that. I put a long sleep just before assigning to `m_tip_block` but I am not sure h
...

💬 sipa commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1951006504)
There is one difference, in that `SeedHardwareFast` only tries to get 64 bits of entropy, while `SeedHardwareSlow` tries to get a full (and fresh) 256 bits, if possible. But without the rndrrs instruction, the difference isn't all that significant (especially as `SeedSlow` also gets 256 bit of entropy from the OS.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1951009074)
> it's possible

In theory, I don't think this is easy to hit in practice. And #31785 should fix it entirely. You could try modifying `bitcoin-mine` in #30437.

⚠️ hebasto opened an issue: "Wallet symlinks are not rejected as expected"
(https://github.com/bitcoin/bitcoin/issues/31842)
Since https://github.com/bitcoin/bitcoin/pull/10885, `bitcoind` is expected to reject symlinks to wallets. This behaviour is also intended to be tested in the following test:https://github.com/bitcoin/bitcoin/blob/86528937e5c4da2e12c46085fc41e87ed759258e/test/functional/wallet_multiwallet.py#L168-L170

However, this is not the case:
```
$ ls -l ~/.bitcoin/regtest/wallets/
total 3
drwx------ 2 hebasto hebasto 3 Feb 11 14:49 w8
lrwxrwxrwx 1 hebasto hebasto 41 Feb 11 14:11 w8_symlink_abs -> /home/
...

⚠️ fanquake opened an issue: "build: `-static-pie` builds no-longer working with CMake"
(https://github.com/bitcoin/bitcoin/issues/31843)
Using 28.x, it's easy to produce a `-static-pie` binary doing:
```bash
# aarch64 alpine, 28.x (32efe850438ef22e2de39e562af557872a402c31)
make -C depends/ NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_USDT=1 NO_UPNP=1 NO_NATPMP=1 LDFLAGS="-static-pie"
./autogen.sh
CONFIG_SITE=/bitcoin/depends/aarch64-unknown-linux-musl/share/config.site ./configure
make src/bitcoind
# link line
libtool: link: /usr/bin/ccache g++ -std=c++20 <snip> -fPIE <snip> -pie -static-pie -o bitcoind bitcoind-bitcoind.o ....
# file src/bit
...

🤔 laanwj reviewed a pull request: "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop"
(https://github.com/bitcoin/bitcoin/pull/31826#pullrequestreview-2609096562)
Code review ACK a2ae62fb4db5e014c03a7ad41ba94ca49fd3806e
i do not have RNDR-supporting hardware to test this

💬 sipa commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1951059294)
Nit: the entire loop can be skipped here (replace `if (g_rndr_supported) {` with `if (g_rndrrs_support) {`).