💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3252122010)
Also tested with `wrk` 4.1.0:
```
$ wrk --latency -t 1 -c 1 -d 10s http://localhost:8332/rest/txfromblock/$BLOCKHASH-5000.bin
Running 10s test @ http://localhost:8332/rest/txfromblock/000000000000000000017bfd05b5fa367a424c4a565a4baf7950d9e8605df8ec-5000.bin
1 threads and 1 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 137.29us 33.60us 2.78ms 93.01%
Req/Sec 7.20k 518.79 8.84k 73.27%
Latency Distribution
50% 136.00us
7
...
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3252122010)
Also tested with `wrk` 4.1.0:
```
$ wrk --latency -t 1 -c 1 -d 10s http://localhost:8332/rest/txfromblock/$BLOCKHASH-5000.bin
Running 10s test @ http://localhost:8332/rest/txfromblock/000000000000000000017bfd05b5fa367a424c4a565a4baf7950d9e8605df8ec-5000.bin
1 threads and 1 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 137.29us 33.60us 2.78ms 93.01%
Req/Sec 7.20k 518.79 8.84k 73.27%
Latency Distribution
50% 136.00us
7
...
💬 maflcko commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3252163887)
> There does not seem to be a way without signal handlers to delete the static, cached datadir when fuzzing is complete. Another issue is that multi-core fuzzing via AFL++ does not work because each worker will try to wipe the cached datadir (created via `-testdatadir` which wipes if the directory exists) which will cause other workers to crash if they are trying to copy it. I could not figure out a way for each worker to have their own cached datadir.
Is my understanding correct that AFL wil
...
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3252163887)
> There does not seem to be a way without signal handlers to delete the static, cached datadir when fuzzing is complete. Another issue is that multi-core fuzzing via AFL++ does not work because each worker will try to wipe the cached datadir (created via `-testdatadir` which wipes if the directory exists) which will cause other workers to crash if they are trying to copy it. I could not figure out a way for each worker to have their own cached datadir.
Is my understanding correct that AFL wil
...
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321022896)
> the info is not signaled to the caller of the function.
Yes, earlier versions of this PR did that, but I simplified it.
> I think this should at least be translatable for consistency with nearby init errors
Done in 1c40b32597712059d5d809925da0e9adccac0fb3
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321022896)
> the info is not signaled to the caller of the function.
Yes, earlier versions of this PR did that, but I simplified it.
> I think this should at least be translatable for consistency with nearby init errors
Done in 1c40b32597712059d5d809925da0e9adccac0fb3
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321023685)
Done in 1c40b32597712059d5d809925da0e9adccac0fb3. Thanks.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321023685)
Done in 1c40b32597712059d5d809925da0e9adccac0fb3. Thanks.
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321023871)
Done in 1c40b32597712059d5d809925da0e9adccac0fb3.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321023871)
Done in 1c40b32597712059d5d809925da0e9adccac0fb3.
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321024033)
Done in 1c40b32597712059d5d809925da0e9adccac0fb3.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321024033)
Done in 1c40b32597712059d5d809925da0e9adccac0fb3.
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321024201)
Done in 1c40b32597712059d5d809925da0e9adccac0fb3.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321024201)
Done in 1c40b32597712059d5d809925da0e9adccac0fb3.
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321032279)
I’ll leave the comment in for now.
I agree that clear, organized code is the best form of documentation, but I think helpful comments can still save valuable developer time.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321032279)
I’ll leave the comment in for now.
I agree that clear, organized code is the best form of documentation, but I think helpful comments can still save valuable developer time.
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321033785)
Done in https://github.com/bitcoin/bitcoin/commit/1c40b32597712059d5d809925da0e9adccac0fb3.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321033785)
Done in https://github.com/bitcoin/bitcoin/commit/1c40b32597712059d5d809925da0e9adccac0fb3.
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321034125)
Done in https://github.com/bitcoin/bitcoin/commit/1c40b32597712059d5d809925da0e9adccac0fb3.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321034125)
Done in https://github.com/bitcoin/bitcoin/commit/1c40b32597712059d5d809925da0e9adccac0fb3.
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321034340)
Done in https://github.com/bitcoin/bitcoin/commit/1c40b32597712059d5d809925da0e9adccac0fb3.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321034340)
Done in https://github.com/bitcoin/bitcoin/commit/1c40b32597712059d5d809925da0e9adccac0fb3.
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321034952)
Done in https://github.com/bitcoin/bitcoin/commit/1c40b32597712059d5d809925da0e9adccac0fb3. Thanks for the review.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2321034952)
Done in https://github.com/bitcoin/bitcoin/commit/1c40b32597712059d5d809925da0e9adccac0fb3. Thanks for the review.
💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3252244247)
re-ACK db52550507045402e89c6455bec680fcd61e26b6
I checked again that the test isn't skipped on the machines where we enable it. It's skipped in the [test each commit job](https://github.com/bitcoin/bitcoin/pull/33201#logs):
```
243/273 - interface_ipc.py skipped (capnp module not available.)
```
That's for the last commit it runs, 1b69f79d138eaf099b108aea6476a0c790d49de7 `ci: enable IPC tests in CI`.
I would be fine with leaving that for a followup.
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3252244247)
re-ACK db52550507045402e89c6455bec680fcd61e26b6
I checked again that the test isn't skipped on the machines where we enable it. It's skipped in the [test each commit job](https://github.com/bitcoin/bitcoin/pull/33201#logs):
```
243/273 - interface_ipc.py skipped (capnp module not available.)
```
That's for the last commit it runs, 1b69f79d138eaf099b108aea6476a0c790d49de7 `ci: enable IPC tests in CI`.
I would be fine with leaving that for a followup.
💬 Sjors commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-3252262528)
re-utACK 88db09bafe9ec363525e5e526c5f6cdd13691447
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-3252262528)
re-utACK 88db09bafe9ec363525e5e526c5f6cdd13691447
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3252313496)
Updated bce88ae28ab2cd12f32aead1fbf47153c50c3b05 -> d89d7bbe75d81971b6b314d99dd7b5d555da4dc2 ([kernelApi_60](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_60) -> [kernelApi_61](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_61), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_60..kernelApi_61))
* Changed ownership model from structs wrapping pointers and tracking their ownership to raw pointers and ownership being indicated by `const`ness of returned point
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3252313496)
Updated bce88ae28ab2cd12f32aead1fbf47153c50c3b05 -> d89d7bbe75d81971b6b314d99dd7b5d555da4dc2 ([kernelApi_60](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_60) -> [kernelApi_61](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_61), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_60..kernelApi_61))
* Changed ownership model from structs wrapping pointers and tracking their ownership to raw pointers and ownership being indicated by `const`ness of returned point
...
🤔 hodlinator reviewed a pull request: "headerssync: Correct unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3182000020)
Changes since last push:
* Rebased to resolve conflict with #33274.
* Moved checks for buffering-behavior during redownload from the first commit to after the refactoring commits.
* Since #33274 updated the `REDOWNLOAD_BUFFER_SIZE` constant to surpass `TARGET_BLOCKS` (as predicted), we now have to temporarily increase `TARGET_BLOCKS` for these new checks to pass.
* Switched from temporarily introducing `g_latest_result` and having commit at the end to remove it (53341ea10dc2f7df371b416060863
...
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3182000020)
Changes since last push:
* Rebased to resolve conflict with #33274.
* Moved checks for buffering-behavior during redownload from the first commit to after the refactoring commits.
* Since #33274 updated the `REDOWNLOAD_BUFFER_SIZE` constant to surpass `TARGET_BLOCKS` (as predicted), we now have to temporarily increase `TARGET_BLOCKS` for these new checks to pass.
* Switched from temporarily introducing `g_latest_result` and having commit at the end to remove it (53341ea10dc2f7df371b416060863
...
💬 hodlinator commented on pull request "headerssync: Correct unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2319904297)
Had my calculator set to hexadecimal when getting it to 38 commitments. :face_in_clouds:
The risk of spurious test failure is closer to $\frac{1}{2 ^ {25}}$ = one in 33,554,432. Added comment + `static_assert` in `sneaky_redownload` before the check that would fail.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2319904297)
Had my calculator set to hexadecimal when getting it to 38 commitments. :face_in_clouds:
The risk of spurious test failure is closer to $\frac{1}{2 ^ {25}}$ = one in 33,554,432. Added comment + `static_assert` in `sneaky_redownload` before the check that would fail.
💬 hodlinator commented on pull request "headerssync: Correct unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2321157714)
Done.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2321157714)
Done.
💬 hodlinator commented on pull request "headerssync: Correct unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2321157369)
Done.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2321157369)
Done.
💬 rkrux commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2321223622)
I see, I had missed the connection between the two commits.
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2321223622)
I see, I had missed the connection between the two commits.