💬 janb84 commented on pull request "guix: documented shasum gathering command":
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3345772428)
Addressed in :0a97da9

- added `uname -m`
- added script for tag-based checkouts.

💬 janb84 commented on pull request "guix: documented shasum gathering command":
(https://github.com/bitcoin/bitcoin/pull/33472#discussion_r2387191699)
Thanks Addressed in : 0a97da9

💬 TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3345899017)
Thanks for the rebase :)

I updated darosiors's [core_bdk_wallet](https://github.com/darosior/core_bdk_wallet?tab=readme-ov-file#generating-rust-ipc-interface-from-capnp-definition) to use this newest version and I ran into a few crashes again. This is the one I managed to get a reproducible backtrace for:

<details>
<summary>Crash backtrace + ipc debug logging</summary>

```
2025-09-29T09:13:03Z [ipc] {bitcoin-node-157078/b-capnp-loop-157084} IPC server recv request #1 Init.construct$P
...

💬 katesalazar commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3345907639)
ACK fab9ddaf554837624fa8f388e046a30d5bf7626f

💬 janb84 commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3346029371)
Can you explain the difference in ctest tests-cases? Is the script double discovering tests ?

Master : 150
This PR: 368

See also:
https://github.com/bitcoin/bitcoin/actions/runs/18044688187/job/51352444037?pr=33483#step:8:4795
https://github.com/bitcoin/bitcoin/actions/runs/17995985600/job/51235033214?pr=33443#step:8:4357

💬 maflcko commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3346048339)
> Can you explain the difference in ctest tests-cases? Is the script double discovering tests ?

Previously there was a single bench sanity check, now there is one bench sanity check for each bench target. (This is the goal of this pull request, but maybe it isn't obvious from the description)

💬 optout21 commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3346064980)
reACK d870caca33ac9f0fdd76969a7341535c5722417e
([prev](https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3171424512))

👍 willcl-ark approved a pull request: "ci: Turn CentOS config into Alpine musl config"
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3278870275)
ACK fa6b2e9efece2d728bdc257c36c95db03e1a7bc4

Alpine/musl feels much more useful to me than a frozen-in-time CentOS build.

Not tested locally as I'm a bit bandwidth-contrained at the moment, but the code changes and CI look good to me.

💬 willcl-ark commented on pull request "ci: Turn CentOS config into Alpine musl config":
(https://github.com/bitcoin/bitcoin/pull/33480#discussion_r2387353020)
What do you think about setting this to `alpine:latest` to automatically use new versions? Might give us the occasionaly false positive when the alpine version is auto-bumped, but feels useful to catch musl changes as early as possible?

Perhaps that's a jobs for a nightly though...

💬 willcl-ark commented on pull request "ci: Turn CentOS config into Alpine musl config":
(https://github.com/bitcoin/bitcoin/pull/33480#discussion_r2387381492)
I wonder if this needs to be `lg` any more? Could probably be `md` I think?

💬 maflcko commented on issue "How to backport libmultiprocess changes?":
(https://github.com/bitcoin/bitcoin/issues/33439#issuecomment-3346073946)
> (1) should be either _all_ of the changes or _none_ of them. At least for the next few weeks I don't think there will be any libmultiprocess changes that aren't either safe or desirable to backport, like bugfixes, documentation improvements, CI improvements, and error handling improvements. I think the only changes to avoid backporting might be PRs that add new features like [#32387](https://github.com/bitcoin/bitcoin/pull/32387) (adding windows support) or [bitcoin-core/libmultiprocess#209](h
...

💬 willcl-ark commented on issue "ci: GHA fallback centos task runs out of space":
(https://github.com/bitcoin/bitcoin/issues/33293#issuecomment-3346080573)
This will be "fixed" by #33480 (as a side-effect). If we decide not to merge that, I can open a PR with something like https://github.com/willcl-ark/bitcoin/commit/f5885d05160fe0ac167976f2b36b6d5bec373e08 in to fix the space issue on CentOS.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3346117320)
Updated the PR description. It referred to a older version of the PR.

💬 maflcko commented on pull request "ci: Turn CentOS config into Alpine musl config":
(https://github.com/bitcoin/bitcoin/pull/33480#discussion_r2387430697)
Sure, pushed a commit

💬 maflcko commented on pull request "ci: Turn CentOS config into Alpine musl config":
(https://github.com/bitcoin/bitcoin/pull/33480#discussion_r2387430936)
Yeah, it is already part of this nightly: https://github.com/maflcko/b-c-nightly/blob/9d6cf19851e57a9717da28c894b5dce6882017ca/.cirrus.yml#L147

I don't think it every found something.

Also, for consistency and reproducibility, I'd prefer to stick with the best-effort-pinned-versions, like in the other CI tasks.

💬 optout21 commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3346170120)
Concept ACK ec7c21534505bb727ad0344c7f6881836b4e9ec5

One doubt: adding a return type to `OpenNetworkConnection` is not described in the commit msg nor in the PR; and it seems to be needed only later by #29415 . I suggest leaving it out (as the return value is not used in this PR), or alternatively, just documenting it in the commit msg & PR description.

💬 maflcko commented on issue "ci: GHA fallback centos task runs out of space":
(https://github.com/bitcoin/bitcoin/issues/33293#issuecomment-3346177632)
> This will be "fixed" by [#33480](https://github.com/bitcoin/bitcoin/pull/33480) (as a side-effect).

Are you sure? The task config should be unchanged in this pull request. So if the pull works around it, it doesn't seem like something that can be relied upon.



> I can open a PR with something like [willcl-ark@f5885d0](https://github.com/willcl-ark/bitcoin/commit/f5885d05160fe0ac167976f2b36b6d5bec373e08) in to fix the space issue on CentOS.

That'd be fine. Maybe just apply it to all GHA run
...

💬 maflcko commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3346183668)
The CI seems to fail reliably?

💬 katesalazar commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3346192356)
I think it's best when as many CI scripts as possible can be run on _any_ host. But I can agree on not cahnging this if there is no clear underlining issue being fixed.

🤔 janb84 reviewed a pull request: "CMake: Add dynamic test discovery"
(https://github.com/bitcoin/bitcoin/pull/33483#pullrequestreview-3279048407)
Approach ACK 6318195b8ecea6d4ced57c8aac97e1ca713d9822

This PR changes how (en when) we do test discovery. From configure time -> Test time and by use of a CMake module.

NIT: Please extend the PR description of the intended changed behaviour of bench sanity check tests. From single bench sanity check to one bench sanity check for each bench target.