💬 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.
💬 maflcko commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3252448008)
When testing the GHA caching, it seems down (at least yesterday and today):
https://github.com/maflcko/bitcoin-core-qa-assets/actions/runs/17436331249/job/49574416227#step:7:173
```
+ ./ci/test/02_run_container.sh
+ '[' -z '' ']'
+ MAYBE_CPUSET=
+ '[' '' ']'
+ echo 'Creating mirror.gcr.io/ubuntu:24.04 container to run in'
+ docker buildx build --file /home/runner/work/bitcoin-core-qa-assets/bitcoin-core-qa-assets/ci/test_imagefile --build-arg CI_IMAGE_NAME_TAG=mirror.gcr.io/ubuntu:24.04 --buil
...
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3252448008)
When testing the GHA caching, it seems down (at least yesterday and today):
https://github.com/maflcko/bitcoin-core-qa-assets/actions/runs/17436331249/job/49574416227#step:7:173
```
+ ./ci/test/02_run_container.sh
+ '[' -z '' ']'
+ MAYBE_CPUSET=
+ '[' '' ']'
+ echo 'Creating mirror.gcr.io/ubuntu:24.04 container to run in'
+ docker buildx build --file /home/runner/work/bitcoin-core-qa-assets/bitcoin-core-qa-assets/ci/test_imagefile --build-arg CI_IMAGE_NAME_TAG=mirror.gcr.io/ubuntu:24.04 --buil
...
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3252476641)
`4652f75bbf...f400e0bb82`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3252476641)
`4652f75bbf...f400e0bb82`: rebase due to conflicts
💬 maflcko commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3252544945)
Interestingly, the armhf task is using GHA runners even in this repo, and it fails with a different error message:
https://github.com/bitcoin/bitcoin/actions/runs/17447989750/job/49546894060?pr=33300#step:8:154:
```
+ echo 'Creating mirror.gcr.io/ubuntu:24.04 container to run in'
+ docker buildx build --file /home/runner/work/bitcoin/bitcoin/ci/test_imagefile --build-arg CI_IMAGE_NAME_TAG=mirror.gcr.io/ubuntu:24.04 --build-arg FILE_ENV=./ci/test/00_setup_env_arm.sh --build-arg BASE_ROOT_DIR=/h
...
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3252544945)
Interestingly, the armhf task is using GHA runners even in this repo, and it fails with a different error message:
https://github.com/bitcoin/bitcoin/actions/runs/17447989750/job/49546894060?pr=33300#step:8:154:
```
+ echo 'Creating mirror.gcr.io/ubuntu:24.04 container to run in'
+ docker buildx build --file /home/runner/work/bitcoin/bitcoin/ci/test_imagefile --build-arg CI_IMAGE_NAME_TAG=mirror.gcr.io/ubuntu:24.04 --build-arg FILE_ENV=./ci/test/00_setup_env_arm.sh --build-arg BASE_ROOT_DIR=/h
...
💬 maflcko commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3252572043)
For peak confusion, another task claims to have read the cache, even though I don't see any task that has created a cache, nor a cache in https://github.com/bitcoin/bitcoin/actions/caches.
https://github.com/bitcoin/bitcoin/actions/runs/17458152407/job/49576315120?pr=29641#step:7:153:
```
+ echo 'Creating mirror.gcr.io/ubuntu:24.04 container to run in'
+ docker buildx build --file /home/admin/actions-runner/_work/bitcoin/bitcoin/ci/test_imagefile --build-arg CI_IMAGE_NAME_TAG=mirror.gcr.io/ubu
...
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3252572043)
For peak confusion, another task claims to have read the cache, even though I don't see any task that has created a cache, nor a cache in https://github.com/bitcoin/bitcoin/actions/caches.
https://github.com/bitcoin/bitcoin/actions/runs/17458152407/job/49576315120?pr=29641#step:7:153:
```
+ echo 'Creating mirror.gcr.io/ubuntu:24.04 container to run in'
+ docker buildx build --file /home/admin/actions-runner/_work/bitcoin/bitcoin/ci/test_imagefile --build-arg CI_IMAGE_NAME_TAG=mirror.gcr.io/ubu
...
👋 fanquake's pull request is ready for review: "[29.x] *san CI backports"
(https://github.com/bitcoin/bitcoin/pull/33294)
(https://github.com/bitcoin/bitcoin/pull/33294)
💬 dergoegge commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3252595782)
> Is my understanding correct that AFL will run init (create the static dir), then fork into several fuzz processes, which run for N iterations and then shut down and delete the static dir? (On the next re-fork, it will fail to find the static dir)?
We removed the `__AFL_INIT` call, so it will actually fork prior to `init`, so I think all that's needed is to randomize the static dir name as you suggested as well.
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3252595782)
> Is my understanding correct that AFL will run init (create the static dir), then fork into several fuzz processes, which run for N iterations and then shut down and delete the static dir? (On the next re-fork, it will fail to find the static dir)?
We removed the `__AFL_INIT` call, so it will actually fork prior to `init`, so I think all that's needed is to randomize the static dir name as you suggested as well.
💬 dergoegge commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2321358588)
Wondering if we should always wipe the `partialBlock` (i.e. downgrade the request to a regular block request) when `FillBlock` fails. Then it would no longer be possible to process two `blocktxn` messages for the same block.
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2321358588)
Wondering if we should always wipe the `partialBlock` (i.e. downgrade the request to a regular block request) when `FillBlock` fails. Then it would no longer be possible to process two `blocktxn` messages for the same block.
🚀 fanquake merged a pull request: "build: set ENABLE_IPC to OFF when fuzzing"
(https://github.com/bitcoin/bitcoin/pull/33235)
(https://github.com/bitcoin/bitcoin/pull/33235)