Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 Sjors commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3547497692)
cc @ryanofsky, @plebhash
🤔 janb84 reviewed a pull request: "clang-format: Set InsertNewlineAtEOF: true"
(https://github.com/bitcoin/bitcoin/pull/33896#pullrequestreview-3477661462)
ACK fa1bf6818f0910f997e5235a197ff51f2e18780d

PR unifies the enforcement of the new line at end of file to also include the rule in the clang-format rules (is already in the linter and in the .editorconfig)

Tested and works as advertised + learned of the existence a new script in contrib :)
💬 m3dwards commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3547524681)
As I understand it, this will produce the cross-compile toolchain using GCC 13. Should we wait on this PR until we have GCC 14] (https://github.com/bitcoin/bitcoin/pull/33775) being as CI for UCRT (https://github.com/bitcoin/bitcoin/pull/33764) will be based on Trixie and GCC 14?
💬 stickies-v commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2538107778)
> Are you looking for a complete header sync implementation first?

No, that's not necessary. I'm just highlighting that in my view there's a gap between what we're expecting of API changes (having clarity on how the new APIs will be used to achieve goals) and then not document (in PR or commits) how we're expecting the API to be used. Just "validation" seems concise.

Anyway, it seems like I'm bringing stop energy and that was not the goal, so I'll take a break here and perhaps revisit late
...
💬 TheCharlatan commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2538120529)
> No, that's not necessary

I think that would be perfectly reasonable as a prerequisite for these kind of deep changes actually. The commit and PR description should be updated too to better explain what is going on with the validation changes here in my view.
💬 yuvicc commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2538138138)
Makes sense @TheCharlatan @stickies-v, I will update the PR description for better understanding of the changes here.
💬 hodlinator commented on pull request "doc: Improve CI docs on env and qemu-user-static":
(https://github.com/bitcoin/bitcoin/pull/33887#discussion_r2538144538)
Taken in latest push (wrapped at 80 chars).
💬 janb84 commented on pull request "ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG":
(https://github.com/bitcoin/bitcoin/pull/33888#issuecomment-3547578829)

> while moving from cirrus-ci to GHA.

Just to clarify, is this for the forked repos that run on GHA because the main repo still runs on Cirrus runners right ? see :
https://github.com/bitcoin/bitcoin/actions/runs/19437220743/job/55611173427?pr=33888#step:1:2
💬 maflcko commented on pull request "ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG":
(https://github.com/bitcoin/bitcoin/pull/33888#issuecomment-3547591517)
> > while moving from cirrus-ci to GHA.
>
> Just to clarify, is this for the forked repos that run on GHA because the main repo still runs on Cirrus runners right ?

No, it is about the platform switch from `cirrus-ci.com` to the GHA platform, which goes through `github.com`. The GHA runner type should be unrelated.
🤔 janb84 reviewed a pull request: "doc: Improve CI docs on env and qemu-user-static"
(https://github.com/bitcoin/bitcoin/pull/33887#pullrequestreview-3477801343)
ACK 552eb90071fd246ba40037f74329403b72453047

changes since last ACK:
- grammar changes.

Thanks for incorporating my NIT.
💬 yuvicc commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#issuecomment-3547611060)
Thanks for the review @stickies-v
- Address usage of `LogError` instead of `LogDebug` [comment](https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2534865290)
- Addressed some nits and removed docstrings.
💬 maflcko commented on pull request "doc: Improve CI docs on env and qemu-user-static":
(https://github.com/bitcoin/bitcoin/pull/33887#issuecomment-3547619341)
lgtm ACK 552eb90071fd246ba40037f74329403b72453047
💬 laanwj commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3547626871)
> Linux 5.10.113-scw1 #1 SMP PREEMPT Mon Jul 15 10:10:04 UTC 2024

It looks like the kernel is fixed and provided externally. It doesn't actually use one from the distro.
💬 l0rinc commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3547653722)
Thanks for restarting the build @maflcko.

@achow101, I can reproduce the same locally, even on Clang 22 using libstdc++ 15.

<details>
<summary>Details</summary>

```
[183/612] Building CXX object src/CMakeFiles/bitcoin_node.dir/node/txorphanage.cpp.o 10:44:13 [1552/3036]
FAILED: src/CMakeFiles/bitcoin_node.di
...
💬 Sjors commented on issue "IPC via TCP Sockets":
(https://github.com/bitcoin/bitcoin/issues/32802#issuecomment-3547680115)
Ok, keep us posted on what you learn.
📝 waketraindev opened a pull request: "net: Decouple `CConnman::GetAddresses` from `CNode`"
(https://github.com/bitcoin/bitcoin/pull/33900)
Decouple `CConnman::GetAddresses` from `CNode` which was used only for getting network_key
💬 djkazic commented on pull request "guix: update per-host disk-space estimates in build gate":
(https://github.com/bitcoin/bitcoin/pull/33889#discussion_r2538317201)
@maflcko I proposed the changes to the build gates by just running a build and checking before/after snapshots of disk usage. Do we need more detailed info like tracking which files the extra disk space use comes from?
💬 djkazic commented on pull request "guix: update per-host disk-space estimates in build gate":
(https://github.com/bitcoin/bitcoin/pull/33889#discussion_r2538322036)
I've updated my PR description to be more in line with our discussions here.
👍 stickies-v approved a pull request: "contrib: turn off compression of macOS SDK to fix determinism (across distros)"
(https://github.com/bitcoin/bitcoin/pull/32009#pullrequestreview-3477969031)
tACK 5513cd0941a2300c0b78758d980ef5eee5079b4c

Determinism across Python versions (3.10-3.14) is now restored on my machine. New approach of just avoiding gzip entirely seems straightforward and preferable.
💬 stickies-v commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2538362360)
The CI script is still using the `.tar.gz` name: https://github.com/bitcoin/bitcoin/blob/2444488f6ad32dcbbed51a73cd4f59ff3a239e32/ci/test/01_base_install.sh#L93

Perhaps we can temporarily add both the `.tar.gz` and `.tar` files to the `SDK_URL` and update all of this in one go?