💬 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.
(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.
(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).
(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
(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.
(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.
(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.
(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
(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.
(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
...
(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.
(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
(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?
(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.
(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.
(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?
(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?
💬 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_r2538309245)
nit: could be cleaned up a bit more
<details>
<summary>git diff on 5513cd0941</summary>
```diff
diff --git a/contrib/macdeploy/gen-sdk b/contrib/macdeploy/gen-sdk
index 0cfd2b1379..426d82e46c 100755
--- a/contrib/macdeploy/gen-sdk
+++ b/contrib/macdeploy/gen-sdk
@@ -2,9 +2,7 @@
import argparse
import plistlib
import pathlib
-import sys
import tarfile
-import gzip
import os
import contextlib
@@ -23,7 +21,7 @@ def run():
description=__doc__, formatter_class=
...
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2538309245)
nit: could be cleaned up a bit more
<details>
<summary>git diff on 5513cd0941</summary>
```diff
diff --git a/contrib/macdeploy/gen-sdk b/contrib/macdeploy/gen-sdk
index 0cfd2b1379..426d82e46c 100755
--- a/contrib/macdeploy/gen-sdk
+++ b/contrib/macdeploy/gen-sdk
@@ -2,9 +2,7 @@
import argparse
import plistlib
import pathlib
-import sys
import tarfile
-import gzip
import os
import contextlib
@@ -23,7 +21,7 @@ def run():
description=__doc__, formatter_class=
...
💬 maflcko commented on pull request "net: Decouple `CConnman::GetAddresses` from `CNode`":
(https://github.com/bitcoin/bitcoin/pull/33900#issuecomment-3547819287)
it probably makes sense to check if those refactors are in line with the Net Split WG (ref https://achow101.com/ircmeetings/2025/bitcoin-core-dev.2025-11-13_16_00.log.html and the coredev meeting notes).
Otherwise, this may be touched again soon after.
(https://github.com/bitcoin/bitcoin/pull/33900#issuecomment-3547819287)
it probably makes sense to check if those refactors are in line with the Net Split WG (ref https://achow101.com/ircmeetings/2025/bitcoin-core-dev.2025-11-13_16_00.log.html and the coredev meeting notes).
Otherwise, this may be touched again soon after.
💬 maflcko commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2538390511)
Not sure it this is possible, but if the file was renamed, the existing linters will pick up and lint this file:
```
git mv ./contrib/macdeploy/gen-sdk ./contrib/macdeploy/gen-sdk.py
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2538390511)
Not sure it this is possible, but if the file was renamed, the existing linters will pick up and lint this file:
```
git mv ./contrib/macdeploy/gen-sdk ./contrib/macdeploy/gen-sdk.py
💬 fanquake commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2538402920)
> Perhaps we can temporarily add both the
We will upload the new SDK before this PR is merged (leaving the existing one in place), I don't think we need interim code changes here.
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2538402920)
> Perhaps we can temporarily add both the
We will upload the new SDK before this PR is merged (leaving the existing one in place), I don't think we need interim code changes here.