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

💬 janb84 commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#discussion_r2387475262)
This comment line, is this intended as a reminder to remove this module if the code is added upstream ?

if so, Nit: please make the intent more explicit. e.g

```suggestion
#TODO remove file if fixed upstream:
# https://gitlab.kitware.com/cmake/cmake/-/issues/26920
```

🤔 janb84 reviewed a pull request: "ci: Turn CentOS config into Alpine musl config"
(https://github.com/bitcoin/bitcoin/pull/33480#pullrequestreview-3279090957)
reACK 444409ff2b78d8f3e541bd6e883af8da7adfd264

changes since last ACK:
- changed container size to md. CI still works with this size. ✅

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

This took 29 minutes for an alpine depends, gui build, with no docker/ccache/depends caches (as no merges to master yet) which seems in line with other jobs.

📝 AmelkaDzikwk opened a pull request: "Create based point of view"
(https://github.com/bitcoin/bitcoin/pull/33490)
#2

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that
...

💬 AmelkaDzikwk commented on pull request "Create based point of view":
(https://github.com/bitcoin/bitcoin/pull/33490#issuecomment-3346308216)
not sure what does it mean?

💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3346353073)
> Thanks for the rebase :)
>
> I updated darosior'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:

Unfortunately, I don't think this is a meaningful backtrace, since it is just showing the trace of a thread at the time it receives a SIGPIPE signal (`Thread 3 "b-capnp-loop" receiv
...