💬 maflcko commented on pull request "gui: Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3252082868)
(ci failure can be ignored, see https://github.com/bitcoin/bitcoin/issues/33293)

💬 maflcko commented on issue "ci: GHA fallback centos task runs out of space":
(https://github.com/bitcoin/bitcoin/issues/33293#issuecomment-3252108363)
Yeah, I guess we could nuke the build dir object files, but it seems tedious and could make debugging later harder?

I'd say the underlying issue is with GitHub Actions. Conceptually they looked at the npm hell and implemented it for CI. The GHA runners are optimized so that you can test your "sort" module out of the box. The runners ship with npm, android, chromium, powershell, graalvm, dotnet, all popular docker images ... (at least the last time I looked).

So I think the correct fix is to re
...

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

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

💬 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

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

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

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

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

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

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

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

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

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

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

💬 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

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

🤔 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
...

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

💬 hodlinator commented on pull request "headerssync: Correct unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2321157714)
Done.