Bitcoin Core Github
43 subscribers
123K links
Download Telegram
📝 fanquake opened a pull request: "subtree: update crc32c subtree"
(https://github.com/bitcoin/bitcoin/pull/33151)
Sync the subtree with latest upstream. The changes here are a no-op, but pull them to fix the drive-by-typo-fixing: #33057.

Includes https://github.com/bitcoin-core/crc32c-subtree/pull/8.
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2259562535)
64d38b8e81d4fbbfac02f4e3e734d8ecd61dd1dc: i don't understand this. This reads as if the cache is only restored when not run on cirrus, which seems odd and unwanted?
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2259568457)
https://github.com/bitcoin/bitcoin/commit/64d38b8e81d4fbbfac02f4e3e734d8ecd61dd1dc: why is this needed? Shouldn't cirrus take care of this itself in its own action?
💬 fanquake commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3163218944)
`node_init_tests` will fail, if there is a `bitcoind` running. Not sure that's expected, or it's documented, that it will no-longer be possible to run the unit tests, if you're also running `bitcoind`:
```bash
../../../src/test/node_init_tests.cpp:14: Entering test suite "node_init_tests"
../../../src/test/node_init_tests.cpp:33: Entering test case "init_test"
2025-08-07T08:49:40.080515Z [unknown] [../../../../src/test/util/random.cpp:48] [void SeedRandomStateForTest(SeedRand)] Setting rando
...
🤔 maflcko reviewed a pull request: "ci: Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3096135811)
There are still some leftover files, which could be deleted: https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241782043

Also, I am not familiar with the github-cirrus-ccache-remote caching, so I can't review that one.

review ACK 36d3042512ed9e638b413866b5a36eec48895bcc 🐬

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqV
...
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2259625125)
nit in 993e59ca0c: Would be good to use naming consistent with all other CONTAINER_NAME?

For ci/test/00_setup_env_mac_native.sh:

export CONTAINER_NAME=ci_mac_native

For ci/test/00_setup_env_mac_native_fuzz.sh:

export CONTAINER_NAME=ci_mac_native_fuzz
💬 maflcko commented on pull request "subtree: update crc32c subtree":
(https://github.com/bitcoin/bitcoin/pull/33151#issuecomment-3163357166)
> The changes here are a no-op,

lgtm ACK 8ef8dd6871dd88ff8c5c6c2f354545ae73dea542

Indeed looks like no-op changes
💬 maflcko commented on pull request "ci: Use mlc `v1` and fix typos":
(https://github.com/bitcoin/bitcoin/pull/33125#issuecomment-3163414840)
lgtm ACK f28a94b40eea8669eafc1b4152823e1ab26fa618
🤔 janb84 reviewed a pull request: "subtree: update crc32c subtree"
(https://github.com/bitcoin/bitcoin/pull/33151#pullrequestreview-3096381930)
ACK 8ef8dd6871dd88ff8c5c6c2f354545ae73dea542

PR contains pull from upstream changes containing (mostly) spelling changes. The changes seem indeed no-op related.

- (simple) code-review
- build & ran tests.
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2259821971)
unused variable?
💬 Ataraxia009 commented on pull request "Removing Bitcoin core text where unnecessary":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3163451028)
> > Can we merge this, the builds seem fine
>
> No, it didn't. The build failed and you haven't tested this yourself at all?
>
> > However i cant run the drahbot build, if somebody could add the label for building, would be helpful
>
> A passing CI (or build) is not sufficient for a merge. Especially here, because a full guix build (with signing) isn't run by any CI. The burden to explain the changes (and why they are safe and correct, and how to test them) is on the pull request author
...
💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-3163458248)
Rebased 81387424d37650e186535b9868d71c5882bc7b20 -> 78ed83ba337c97798134f4bd0f9307facde3a59d ([kernelInternalLib_19](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_19) -> [kernelInternalLib_20](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_20), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_19..kernelInternalLib_20))

Fixed conflict with #33077
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3163466142)
Rebased 6a9fdf7ae58a85ccc08c5f6917f64f28f5a330ad -> ce8003578e725cf3c64a0f3e1447459e26955a3d ([kernelApi_49](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_49) -> [kernelApi_50](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_50), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_49..kernelApi_50))

* Fixed conflict with #33077

@stickies-v want to give the mono lib a try in your python bindings?
💬 TheCharlatan commented on pull request "Removing Bitcoin core text where unnecessary":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3163495792)
> Anyways, everything should be fine now, should attach a drahbot guix build request to be sure

Ideally you would do the full guix build yourself and post the build artifact hashes. Your dev environment should have no influence on that. Then reviewers can compare their results to yours.
💬 Ataraxia009 commented on pull request "Removing Bitcoin core text where unnecessary":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2259856491)
Changing it to `CFBundleName = \"BundleName\"` would be confusing or weird for people just running the raw deploy build, no?
💬 Ataraxia009 commented on pull request "Removing Bitcoin core text where unnecessary":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2259859427)
Decided @luke-jr 's solution is best here, so adopted the changes suggested here ^
💬 maflcko commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3163563858)
> are we just interested in the `notfound` branch and the rest is there mostly to avoid intermittent failure?

Yes.

I think mocktime could be used to remove the loop and just do a one-shot `notfound` message?
📝 hebasto opened a pull request: "Prepare "Open Transifex translations for v30.0" release step"
(https://github.com/bitcoin/bitcoin/pull/33152)
This PR follows our [Release Process](https://github.com/bitcoin/bitcoin/blob/53a996f122663e271efa52c45b173613b8ac635e/doc/release-process.md).

It is required to open Transifex translations for v30.0, as scheduled in https://github.com/bitcoin/bitcoin/issues/32275.

For reference, see the previous similar PR: https://github.com/bitcoin/bitcoin/pull/31809.

**Note for reviewers:**

To reproduce the diff in the last commit, run:
```
cmake --preset dev-mode
cmake --build build_dev_mode
...
🚀 fanquake merged a pull request: "ci: Use mlc `v1` and fix typos"
(https://github.com/bitcoin/bitcoin/pull/33125)
💬 hebasto commented on pull request "Release: Prepare "Open Transifex translations for v30.0" step":
(https://github.com/bitcoin/bitcoin/pull/33152#issuecomment-3163592440)
cc @stickies-v @pablomartin4btc @johnny9 @jarolrod as regular reviewers of similar previous PRs.