🚀 fanquake merged a pull request: "cmake: Remove unused `BUILD_TESTING` variable from "dev-mode" preset"
(https://github.com/bitcoin/bitcoin/pull/31544)
(https://github.com/bitcoin/bitcoin/pull/31544)
🚀 fanquake merged a pull request: "doc: Install `py3-zmq` port on OpenBSD for `interface_zmq.py`"
(https://github.com/bitcoin/bitcoin/pull/31535)
(https://github.com/bitcoin/bitcoin/pull/31535)
🚀 fanquake merged a pull request: "guix: latest 2.31 glibc"
(https://github.com/bitcoin/bitcoin/pull/31529)
(https://github.com/bitcoin/bitcoin/pull/31529)
👍 tdb3 approved a pull request: "rpc: increase the defaults for -rpcthreads and -rpcworkqueue"
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2517790394)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d
Saw performance improvement and no degradation.
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2517790394)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d
Saw performance improvement and no degradation.
📝 maflcko opened a pull request: "fuzz: Abort when global PRNG is used before SeedRand::ZEROS"
(https://github.com/bitcoin/bitcoin/pull/31548)
This adds one more check to abort when global PRNG is used before SeedRand::ZEROS in fuzz tests. This is achieved by carving out the two remaining uses. First, `g_rng_temp_path_init`, and second the random fallback for `RANDOM_CTX_SEED`, which isn't used in fuzz tests anyway.
Requested in https://github.com/bitcoin/bitcoin/pull/31521#issuecomment-2554669015
Can be tested by reverting fadd568931a2d21e0f80e1efaf2281f5164fa20e and observing an abort when running the `utxo_total_supply` fuzz t
...
(https://github.com/bitcoin/bitcoin/pull/31548)
This adds one more check to abort when global PRNG is used before SeedRand::ZEROS in fuzz tests. This is achieved by carving out the two remaining uses. First, `g_rng_temp_path_init`, and second the random fallback for `RANDOM_CTX_SEED`, which isn't used in fuzz tests anyway.
Requested in https://github.com/bitcoin/bitcoin/pull/31521#issuecomment-2554669015
Can be tested by reverting fadd568931a2d21e0f80e1efaf2281f5164fa20e and observing an abort when running the `utxo_total_supply` fuzz t
...
🤔 l0rinc reviewed a pull request: "test: fix TestShell initialization and reset()"
(https://github.com/bitcoin/bitcoin/pull/31415#pullrequestreview-2517777408)
I wanted to see what the π pull request contains, ended up reviewing it.
Could you please specify in the description of how the reviewers should test the code and why the extraction was necessary besides the absolute file computation?
(https://github.com/bitcoin/bitcoin/pull/31415#pullrequestreview-2517777408)
I wanted to see what the π pull request contains, ended up reviewing it.
Could you please specify in the description of how the reviewers should test the code and why the extraction was necessary besides the absolute file computation?
💬 l0rinc commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1894066689)
there are many other instances of `bitcoin/test` in the doc, could you please adjust the rest as well?
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1894066689)
there are many other instances of `bitcoin/test` in the doc, could you please adjust the rest as well?
💬 l0rinc commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1894075268)
Now that it's moved out of `TestShell` this (slightly awkward) comment should likely be updated.
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1894075268)
Now that it's moved out of `TestShell` this (slightly awkward) comment should likely be updated.
💬 l0rinc commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1894082388)
useless comment, the code already makes this obvious
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1894082388)
useless comment, the code already makes this obvious
💬 l0rinc commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1894076519)
Q: I'm not sure I understand the reason for extraction, can you please clarify that?
Nit: We're adding another global here which pollutes the namespace, can we rather inline it?
```suggestion
dummy_testshell_file = pathlib.Path(__file__).absolute().parent.parent / "testshell_dummy.py"
```
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1894076519)
Q: I'm not sure I understand the reason for extraction, can you please clarify that?
Nit: We're adding another global here which pollutes the namespace, can we rather inline it?
```suggestion
dummy_testshell_file = pathlib.Path(__file__).absolute().parent.parent / "testshell_dummy.py"
```
💬 hebasto commented on pull request "depends: Fix compiling `libevent` package on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31500#issuecomment-2557287932)
> Can you link to the upstream issue / PR.
> LGTM, but yeah, it'd be helpful to upstream this.
Sure thing!
Here is an upstream PR: https://github.com/libevent/libevent/pull/1768.
(https://github.com/bitcoin/bitcoin/pull/31500#issuecomment-2557287932)
> Can you link to the upstream issue / PR.
> LGTM, but yeah, it'd be helpful to upstream this.
Sure thing!
Here is an upstream PR: https://github.com/libevent/libevent/pull/1768.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2557298504)
`3522e897f2...964b7b0ad6`: pet clang-tidy
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2557298504)
`3522e897f2...964b7b0ad6`: pet clang-tidy
💬 hebasto commented on issue "qa: Intermittent `AssertionError: not(10.00000000 == 340)` in `wallet_assumeutxo.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/31546#issuecomment-2557299362)
> Is it reproducible?
It seems intermittent.
(https://github.com/bitcoin/bitcoin/issues/31546#issuecomment-2557299362)
> Is it reproducible?
It seems intermittent.
💬 vasild commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2557300801)
The closest thing to this is at https://github.com/bitcoin/bitcoin/pull/29415, waiting for testing and code review.
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2557300801)
The closest thing to this is at https://github.com/bitcoin/bitcoin/pull/29415, waiting for testing and code review.
💬 hebasto commented on pull request "doc: Install `net/py-pyzmq` port on FreeBSD for `interface_zmq.py`":
(https://github.com/bitcoin/bitcoin/pull/31526#issuecomment-2557307026)
Friendly ping @vasild :)
(https://github.com/bitcoin/bitcoin/pull/31526#issuecomment-2557307026)
Friendly ping @vasild :)
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2557349002)
I'm aware this is a big chunk of code, but perhaps this can aid review a bit.
The biggest commits is [txgraph: (feature) add initial version](https://github.com/bitcoin/bitcoin/pull/31363/commits/afc6a665770004411f1b57f4f43dda916dadd8f7). To get an idea of what is going on, I think it's useful to look (in order) at:
* The `src/txgraph.h` file added in that commit, as it defines the public interface.
* The `src/test/fuzz/txgraph.cpp` test added in the next commit ([txgraph: (tests) add simul
...
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2557349002)
I'm aware this is a big chunk of code, but perhaps this can aid review a bit.
The biggest commits is [txgraph: (feature) add initial version](https://github.com/bitcoin/bitcoin/pull/31363/commits/afc6a665770004411f1b57f4f43dda916dadd8f7). To get an idea of what is going on, I think it's useful to look (in order) at:
* The `src/txgraph.h` file added in that commit, as it defines the public interface.
* The `src/test/fuzz/txgraph.cpp` test added in the next commit ([txgraph: (tests) add simul
...
💬 hebasto commented on issue "depends: `capnp` package fails to build on NetBSD 10.0":
(https://github.com/bitcoin/bitcoin/issues/31499#issuecomment-2557362137)
> Shouldn't this be an upstream bug report, given that's where the issue is?
https://github.com/capnproto/capnproto/issues/2201.
(https://github.com/bitcoin/bitcoin/issues/31499#issuecomment-2557362137)
> Shouldn't this be an upstream bug report, given that's where the issue is?
https://github.com/capnproto/capnproto/issues/2201.
📝 marcofleon opened a pull request: "fuzz: Abort if system time is called without mock time being set"
(https://github.com/bitcoin/bitcoin/pull/31549)
This PR expands the `CheckGlobals` utility that was introduced in https://github.com/bitcoin/bitcoin/pull/31486 and should help with fuzz stability (https://github.com/bitcoin/bitcoin/issues/29018).
System time shouldn't be used when running a fuzz test, as it is likely to introduce instability (non-determinism). This PR identifies and fixes the targets that were calling system time without setting mock time at the start of an iteration.
Removing`SetMockTime()` from any one of these targe
...
(https://github.com/bitcoin/bitcoin/pull/31549)
This PR expands the `CheckGlobals` utility that was introduced in https://github.com/bitcoin/bitcoin/pull/31486 and should help with fuzz stability (https://github.com/bitcoin/bitcoin/issues/29018).
System time shouldn't be used when running a fuzz test, as it is likely to introduce instability (non-determinism). This PR identifies and fixes the targets that were calling system time without setting mock time at the start of an iteration.
Removing`SetMockTime()` from any one of these targe
...
💬 theuni commented on pull request "cmake: Always provide `RPATH` on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31543#issuecomment-2557510392)
So the `ld` finds the lib to link against, but the runtime loader then can't find it? That seems absurd.
Or is it because the toolchain you're using adds `/usr/pkg/lib/` to `ld`'s paths?
(https://github.com/bitcoin/bitcoin/pull/31543#issuecomment-2557510392)
So the `ld` finds the lib to link against, but the runtime loader then can't find it? That seems absurd.
Or is it because the toolchain you're using adds `/usr/pkg/lib/` to `ld`'s paths?
💬 hebasto commented on pull request "cmake: Always provide `RPATH` on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31543#issuecomment-2557513164)
> So the `ld` finds the lib to link against, but the runtime loader then can't find it? That seems absurd.
>
>
>
> Or is it because the toolchain you're using adds `/usr/pkg/lib/` to `ld`'s paths?
This is because CMake uses absolute paths to libraries during the linking stage.
(https://github.com/bitcoin/bitcoin/pull/31543#issuecomment-2557513164)
> So the `ld` finds the lib to link against, but the runtime loader then can't find it? That seems absurd.
>
>
>
> Or is it because the toolchain you're using adds `/usr/pkg/lib/` to `ld`'s paths?
This is because CMake uses absolute paths to libraries during the linking stage.