Bitcoin Core Github
43 subscribers
123K links
Download Telegram
fanquake closed a pull request: "Bitcoin v1"
(https://github.com/bitcoin/bitcoin/pull/33149)
⚠️ maflcko opened an issue: "intermittent TSAN failure in lockmanager_tests::blockmanager_readblock_hash_mismatch"
(https://github.com/bitcoin/bitcoin/issues/33150)
TSAN failure (https://github.com/bitcoin/bitcoin/actions/runs/16651908244/job/47126650397?pr=32989#step:8:6056):
```bash
2025-07-31T14:39:40.396402Z [test] [net.cpp:2419] [void CConnman::SetTryNewOutboundPeer(bool)] [net] setting try another outbound peer=false
2025-07-31T14:39:40.396438Z [test] [net.cpp:3192] [void==================
WARNING: ThreadSanitizer: data race (pid=11584)
Write of size 8 at 0x7234000016f0 by main thread:
#0 blockmanager_tests::blockmanager_readblock_hash_mi
...
💬 maflcko commented on issue "intermittent TSAN failure in lockmanager_tests::blockmanager_readblock_hash_mismatch":
(https://github.com/bitcoin/bitcoin/issues/33150#issuecomment-3163078227)
A bit odd that this only happens in the pull changing the CI system, but it seems to be frequent: https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3160730521

It would be good to look into this, so I created this issue.
👍 ismaelsadeeq approved a pull request: "refactor: CFeeRate encapsulates FeeFrac internally"
(https://github.com/bitcoin/bitcoin/pull/32750#pullrequestreview-3096018187)
re-ACK d3b8a54a81209420ef6447dd4581e1b6b8550647 📦
💬 maflcko commented on pull request "CI: silent merge check":
(https://github.com/bitcoin/bitcoin/pull/33145#issuecomment-3163086012)
Other stuff to check would be that this works at all and doesn't just re-run an ancient commit, like https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3133536724
👋 fanquake's pull request is ready for review: "ci: Use mlc `v1` and fix typos"
(https://github.com/bitcoin/bitcoin/pull/33125)
💬 ismaelsadeeq commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2259553163)
Yeah you are right 👍🏾
💬 fanquake commented on pull request "ci: Use mlc `v1` and fix typos":
(https://github.com/bitcoin/bitcoin/pull/33125#issuecomment-3163087672)
It can be.
📝 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