💬 ryanofsky commented on pull request "fuzz: Fix misplaced SeedRand::ZEROS":
(https://github.com/bitcoin/bitcoin/pull/31521#issuecomment-2554669015)
Thanks for the explanations. The reason I was confused earlier is that I mistakenly thought `CheckGlobalsImpl` would crash if a fuzz test ever used the global RNG without seeding it with zeroes first.

But (unfortunately?) the checker doesn't really check that. It only checks that if a fuzz test uses the RNG, then it must seed it with zeros at some point during the test. The checker doesn't check that seeding happens **before** using the RNG.

So this PR does fix some nondeterminism, but the
...

💬 ryanofsky commented on pull request "fuzz: Abort when using global PRNG without re-seed":
(https://github.com/bitcoin/bitcoin/pull/31486#discussion_r1892560661)
re: https://github.com/bitcoin/bitcoin/pull/31486#discussion_r1891383934

> So the boilerplate can't be removed

To clarify, in the context of #31521 specifically, I think the change I'm suggesting would have prevented the nondeterminism there, and would avoid the need for a `SeedRandomStateForTest(SeedRand::ZEROS);` call at the top of the `utxo_total_supply` test.

But the `utxo_total_supply` test is just an unusual test, because most fuzz tests use test setup objects that get shared acro
...

👍 theuni approved a pull request: "refactor: std::span compat fixes"
(https://github.com/bitcoin/bitcoin/pull/31540#pullrequestreview-2515272272)
utACK fa494a1d53f3f030fafe7b533d72b2200428a0fd.

I haven't verified that this assortment of changes is all that's needed for the conversion, but they all look good to me.

💬 theuni commented on pull request "refactor: std::span compat fixes":
(https://github.com/bitcoin/bitcoin/pull/31540#discussion_r1892519435)
😬

💬 theuni commented on pull request "refactor: std::span compat fixes":
(https://github.com/bitcoin/bitcoin/pull/31540#discussion_r1892536668)
Why drop the assert? Though I have no idea who/what tests with `-DDEBUG` anyway.

👍 adamandrews1 approved a pull request: "refactor: std::span compat fixes"
(https://github.com/bitcoin/bitcoin/pull/31540#pullrequestreview-2515386534)
utACK https://github.com/bitcoin/bitcoin/commit/fa494a1d53f3f030fafe7b533d72b2200428a0fd

📝 hebasto opened a pull request: "qa: Use `sys.executable` when invoking other Python scripts"
(https://github.com/bitcoin/bitcoin/pull/31541)
This PR fixes the `rpc_signer.py` and `wallet_signer.py` functional tests on systems where `python3` is not available in the `PATH`, causing the shebang `#!/usr/bin/env python3` to fail.

Here are logs on NetBSD 10.0:
- without this PR (only https://github.com/bitcoin/bitcoin/pull/31537 branch):
```
$ python3.12 ./build/test/functional/test_runner.py rpc_signer.py wallet_signer.py
Temporary test directory at /tmp/test_runner_₿_🏃_20241219_160538
Remaining jobs: [rpc_signer.py, wallet_sign
...

💬 maflcko commented on pull request "refactor: std::span compat fixes":
(https://github.com/bitcoin/bitcoin/pull/31540#discussion_r1892617595)
The assert is not dropped. In fact, there are now two asserts:

* https://github.com/bitcoin/bitcoin/blob/fa494a1d53f3f030fafe7b533d72b2200428a0fd/src/span.h#L184
* https://github.com/bitcoin/bitcoin/blob/fa494a1d53f3f030fafe7b533d72b2200428a0fd/src/span.h#L207

The first one is the one you left a comment on. And the second one is left for the compiler to drop, if it wants to.

💬 hebasto commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2554942049)
A CI log from https://github.com/hebasto/bitcoin-core-nightly has been added to the PR description.

💬 theuni commented on pull request "depends: Fix compiling `libevent` package on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31500#issuecomment-2555160770)
LGTM, but yeah, it'd be helpful to upstream this.

FWIW c23 adds `typeof()`, so technically guards for that could be added. Not worth the complexity though.

💬 maflcko commented on pull request "qa: Use `sys.executable` when invoking other Python scripts":
(https://github.com/bitcoin/bitcoin/pull/31541#issuecomment-2555161398)
lgtm. even without netbsd, this is required to pass down the python version that was selected to run the tests. This is also used in all other places in the tests, like https://github.com/bitcoin/bitcoin/blob/bb57017b2945d5e0bbd95c7f1a9369a8ab7c6fcd/test/functional/test_runner.py#L632

👋 hebasto's pull request is ready for review: "qa: Use `sys.executable` when invoking other Python scripts"
(https://github.com/bitcoin/bitcoin/pull/31541)

💬 hebasto commented on pull request "qa: Use `sys.executable` when invoking other Python scripts":
(https://github.com/bitcoin/bitcoin/pull/31541#issuecomment-2555184010)
> lgtm. even without netbsd

Thanks! Rebased on the master branch.

💬 maflcko commented on pull request "fuzz: Fix misplaced SeedRand::ZEROS":
(https://github.com/bitcoin/bitcoin/pull/31521#issuecomment-2555198249)
> But (unfortunately?) the checker doesn't really check that. It only checks that if a fuzz test uses the RNG, then it must seed it with zeros at some point during the test. The checker doesn't check that seeding happens **before** using the RNG.

Sure, makes sense to add this assert as well

👍 theuni approved a pull request: "guix: latest 2.31 glibc"
(https://github.com/bitcoin/bitcoin/pull/31529#pullrequestreview-2515661465)
utACK b8710201fbd01bf2cba3c3b1fd312316b1ae22af

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1892975685)
> wen `s/Span/std::span/` all over the place?

now: https://github.com/bitcoin/bitcoin/pull/31519

💬 vasild commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#issuecomment-2555525700)
Concept ACK

📝 maflcko opened a pull request: " test: Embed univalue json tests in binary "
(https://github.com/bitcoin/bitcoin/pull/31542)
All other benchmarks and tests have their data embedded, except for the univalue json tests.

This is not only confusing, but also problematic, when the test binary is moved to a different system for testing, because one has to put the test files in the source dir that was used at compile-time.

Fix all issues by embedding them. Also, re-enable a disabled test. Also, fix an issue in the GenerateHeaderFromJson.cmake.

Requested in https://github.com/bitcoin/bitcoin/pull/31434/files#r1876000
...

📝 hebasto opened a pull request: "cmake: Always provide `RPATH` on NetBSD"
(https://github.com/bitcoin/bitcoin/pull/31543)
Apparently, runtime paths cannot be skipped on NetBSD, even for system-wide packages.

On NetBSD 10.0:
- on the master branch @ bb57017b2945d5e0bbd95c7f1a9369a8ab7c6fcd:
```
$ cmake -B build -DCMAKE_C_COMPILER="/usr/pkg/gcc14/bin/gcc" -DCMAKE_CXX_COMPILER="/usr/pkg/gcc14/bin/g++"
$ cmake --build build
$ ./build/src/bitcoin-wallet -version
./build/src/bitcoin-wallet: Shared object "libsqlite3.so.0" not found
$ cmake --install build --prefix /home/hebasto/INSTALL
$ /home/hebasto/INSTALL/
...

💬 hebasto commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2555661793)
Tracked in https://github.com/hebasto/bitcoin-core-nightly/issues/6.