Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 davidgumberg 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-3251165278)
Code Review re-ACK 88db09b

```console
$ git range-diff 4c53178...88db09b
```

https://github.com/bitcoin/bitcoin/commit/88db09bafe9ec363525e5e526c5f6cdd13691447 takes the [reviewer suggestion](https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2302222710) of moving the `rtmsg*` cast after checking if `hdr->nlmsg_type == NLMSG_DONE`which resolves https://github.com/bitcoin/bitcoin/issues/33245 and implements [a suggested refactor](https://github.com/bitcoin/bitcoin/pull/32159#discus
...
💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2320500599)
I think these have been squashed correctly to address these clumsy mistakes and that the final three commits in the pull request reflect this.
💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2320501230)
I believe this has already been addressed by squashing the relevant commits.
👍 ryanofsky approved a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3183296138)
Code review ACK db52550507045402e89c6455bec680fcd61e26b6, just making small suggested changes since last review: fixing up pycapnp install instructions, using --break-system-packages only where needed, making small test cleanups
💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2320865276)
It seems that OS block cache is quite effective (even with `/*cache_size=*/0`), so I am getting similar performance when using 3GiB DB cache compared to https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3250323045.

Tested with the patch below:
```
$ ab -k -c 1 -n 100000 http://localhost:8332/rest/txfromblock/$BLOCKHASH-5000.bin
...
Document Path: /rest/txfromblock/000000000000000000017bfd05b5fa367a424c4a565a4baf7950d9e8605df8ec-5000.bin
Document Length: 234 bytes
...
👋 romanz's pull request is ready for review: "index: store per-block transaction locations for efficient lookups"
(https://github.com/bitcoin/bitcoin/pull/32541)
💬 maflcko commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3252077408)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
💬 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.