Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ’¬ maflcko commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387159193)
No strong opinion, but:

* Such test coverage did not exist before either.
* I am not aware of this ever finding an issue. The version that matters most is the xcode toolchain version, as documented in the lines above.
πŸ’¬ maflcko commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387159665)
I think the meaningless bloat is just frustrating when glancing at and selecting CI results, because it makes it impossible to differentiate tasks on a glance:

<img width="622" height="316" alt="Screenshot 2025-09-29 at 10-34-08 Merge bitcoin_bitcoin#33484 doc rpc fix case typo in `finalizepsbt… Β· bitcoin_bitcoin@d8fe258" src="https://github.com/user-attachments/assets/c533c42c-5df8-403a-aaeb-cd8ebbe69d08" />


Also, it is unclear what value it adds. Has the macOS version or the macOS arch
...
πŸ’¬ 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?