✅ maflcko closed an issue: "Intermittent CI network issue downloading assets.json from GitHub"
(https://github.com/bitcoin/bitcoin/issues/33599)
(https://github.com/bitcoin/bitcoin/issues/33599)
💬 willcl-ark commented on issue "Intermittent CI network issue downloading assets.json from GitHub":
(https://github.com/bitcoin/bitcoin/issues/33599#issuecomment-3400340069)
I had a reply from Cirrus on this; apparently they have done some work over the weekend which should resolve this issue.
We should keep an eye on failures in case it happens again, but other than that hopefully things now fixed.
(https://github.com/bitcoin/bitcoin/issues/33599#issuecomment-3400340069)
I had a reply from Cirrus on this; apparently they have done some work over the weekend which should resolve this issue.
We should keep an eye on failures in case it happens again, but other than that hopefully things now fixed.
💬 ac12644 commented on pull request "contrib: Fix gen-manpages.py to check build options":
(https://github.com/bitcoin/bitcoin/pull/33617#issuecomment-3400388374)
> How is this different from #33085 ?
@maflcko made several important improvements:
Added USE_UPNP
Removed unused options - Addressed your feedback about ENABLE_BITCOIN_CHAINSTATE, ENABLE_FUZZ_BINARY, ENABLE_USDT_TRACEPOINTS
Fixed grammar - "Aborting generation of..." instead of "Aborting generating..."
(https://github.com/bitcoin/bitcoin/pull/33617#issuecomment-3400388374)
> How is this different from #33085 ?
@maflcko made several important improvements:
Added USE_UPNP
Removed unused options - Addressed your feedback about ENABLE_BITCOIN_CHAINSTATE, ENABLE_FUZZ_BINARY, ENABLE_USDT_TRACEPOINTS
Fixed grammar - "Aborting generation of..." instead of "Aborting generating..."
✅ ac12644 closed a pull request: "contrib: Fix gen-manpages.py to check build options"
(https://github.com/bitcoin/bitcoin/pull/33617)
(https://github.com/bitcoin/bitcoin/pull/33617)
💬 maflcko commented on pull request "contrib: Fix gen-manpages.py to check build options":
(https://github.com/bitcoin/bitcoin/pull/33617#issuecomment-3400420594)
It may be good to retain the original author in the commit. This can be achieved by cherry-picking or by adding the co-authored-by tag. Otherwise, it looks like you fully authored this yourself.
(https://github.com/bitcoin/bitcoin/pull/33617#issuecomment-3400420594)
It may be good to retain the original author in the commit. This can be achieved by cherry-picking or by adding the co-authored-by tag. Otherwise, it looks like you fully authored this yourself.
💬 maflcko commented on pull request "test: change log rate limit version gate from 299900 to 289900":
(https://github.com/bitcoin/bitcoin/pull/33612#discussion_r2428141151)
I don't understand this change. Neither 28.99, nor 29.0 had this change. So appending the arg will lead to a startup error or warning there?
```
$ bitcoin-core.daemon -regtest -nologratelimit
Error: Error parsing command line arguments: Invalid parameter -nologratelimit
$ bitcoin-core.daemon -version | grep version
Bitcoin Core daemon version v29.0.0
(https://github.com/bitcoin/bitcoin/pull/33612#discussion_r2428141151)
I don't understand this change. Neither 28.99, nor 29.0 had this change. So appending the arg will lead to a startup error or warning there?
```
$ bitcoin-core.daemon -regtest -nologratelimit
Error: Error parsing command line arguments: Invalid parameter -nologratelimit
$ bitcoin-core.daemon -version | grep version
Bitcoin Core daemon version v29.0.0
💬 vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2428267622)
I meant "schedule" in the more general English sense, however I can see that:
> "schedule" in this codebase almost always means putting a task on the `scheduler` ...
So, looking at "schedule (verb)" synonyms, what about:
```
ArrageTxForBroadcastToAll()
BookTxForBroadcastToAll()
LineUpTxForBroadcastToAll()
MarkTxForBroadcastToAll()
NoteTxForBroadcastToAll()
RegisterTxForBroadcastToAll()
SetTxForBroadcastToAll()
SetUpTxForBroadcastToAll()
```
?
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2428267622)
I meant "schedule" in the more general English sense, however I can see that:
> "schedule" in this codebase almost always means putting a task on the `scheduler` ...
So, looking at "schedule (verb)" synonyms, what about:
```
ArrageTxForBroadcastToAll()
BookTxForBroadcastToAll()
LineUpTxForBroadcastToAll()
MarkTxForBroadcastToAll()
NoteTxForBroadcastToAll()
RegisterTxForBroadcastToAll()
SetTxForBroadcastToAll()
SetUpTxForBroadcastToAll()
```
?
💬 maflcko commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3400587637)
Thanks! That works. Docker truly couldn't be more confusing. Neither `build`, nor `BUILDKIT=1` work and the docker build driver for buildx is called `default`, but apparently it is not used by default, so it doesn't work either. I went ahead and set `os.environ["BUILDX_BUILDER"] = "default"`, which should hopefully work with both docker and podman.
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3400587637)
Thanks! That works. Docker truly couldn't be more confusing. Neither `build`, nor `BUILDKIT=1` work and the docker build driver for buildx is called `default`, but apparently it is not used by default, so it doesn't work either. I went ahead and set `os.environ["BUILDX_BUILDER"] = "default"`, which should hopefully work with both docker and podman.
💬 maflcko commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3400597071)
> > I still think merging this PR is fine
>
> Yes, sounds good. I am not familiar with docker/GHA though, so I can't review it closely.
fwiw
lgtm ACK bc706955d740f8a59bec78e44d33e80d1cca373b
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3400597071)
> > I still think merging this PR is fine
>
> Yes, sounds good. I am not familiar with docker/GHA though, so I can't review it closely.
fwiw
lgtm ACK bc706955d740f8a59bec78e44d33e80d1cca373b
📝 maflcko opened a pull request: " ci: Build ci_native_base image layer "
(https://github.com/bitcoin/bitcoin/pull/33620)
Storage is cheap, so there was little value in extracting build layers in the CI images.
However, the GHA cache is limited to 10GB, so extracting a shared base layer could help to reduce the overall footprint. Possibly, it could even speed up image building, because installation is only done once.
(https://github.com/bitcoin/bitcoin/pull/33620)
Storage is cheap, so there was little value in extracting build layers in the CI images.
However, the GHA cache is limited to 10GB, so extracting a shared base layer could help to reduce the overall footprint. Possibly, it could even speed up image building, because installation is only done once.
📝 maflcko converted_to_draft a pull request: "ci: Build ci_native_base image layer"
(https://github.com/bitcoin/bitcoin/pull/33620)
Storage is cheap, so there was little value in extracting build layers in the CI images.
However, the GHA cache is limited to 10GB, so extracting a shared base layer could help to reduce the overall footprint. Possibly, it could even speed up image building, because installation is only done once.
(https://github.com/bitcoin/bitcoin/pull/33620)
Storage is cheap, so there was little value in extracting build layers in the CI images.
However, the GHA cache is limited to 10GB, so extracting a shared base layer could help to reduce the overall footprint. Possibly, it could even speed up image building, because installation is only done once.
💬 maflcko commented on pull request "ci: Build ci_native_base image layer":
(https://github.com/bitcoin/bitcoin/pull/33620#issuecomment-3400623153)
(currently a draft, because this is based on some other pulls)
(https://github.com/bitcoin/bitcoin/pull/33620#issuecomment-3400623153)
(currently a draft, because this is based on some other pulls)
💬 Sjors commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3400641744)
ACK 0626b90f507db68610a69feec86deb712dd095a1
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3400641744)
ACK 0626b90f507db68610a69feec86deb712dd095a1
💬 maflcko commented on pull request "ci: Build ci_native_base image layer":
(https://github.com/bitcoin/bitcoin/pull/33620#issuecomment-3400671936)
Also, it won't work anyway, due to `--cache-to`: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/18490105374/job/52681542962#step:6:149:
```
+ docker buildx ls --format '{{.DriverEndpoint}} {{.Name}}'
Using existing docker based buildx: default
Building ci_native_base image layer
+ docker buildx build --file=/home/runner/work/bitcoin-core-with-ci/bitcoin-core-with-ci/ci/test_imagefile_base --platform=linux --label=bitcoin-ci-test --tag=ci_native_base --cache-from type=gha,sco
...
(https://github.com/bitcoin/bitcoin/pull/33620#issuecomment-3400671936)
Also, it won't work anyway, due to `--cache-to`: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/18490105374/job/52681542962#step:6:149:
```
+ docker buildx ls --format '{{.DriverEndpoint}} {{.Name}}'
Using existing docker based buildx: default
Building ci_native_base image layer
+ docker buildx build --file=/home/runner/work/bitcoin-core-with-ci/bitcoin-core-with-ci/ci/test_imagefile_base --platform=linux --label=bitcoin-ci-test --tag=ci_native_base --cache-from type=gha,sco
...
💬 Raimo33 commented on pull request "p2p: Allow block downloads from peers without snapshot block after assumeutxo validation":
(https://github.com/bitcoin/bitcoin/pull/33604#issuecomment-3400714807)
Concept ACK
approach seems fine
(https://github.com/bitcoin/bitcoin/pull/33604#issuecomment-3400714807)
Concept ACK
approach seems fine
💬 Raimo33 commented on pull request "node: change a tx-relay on/off flag to enum":
(https://github.com/bitcoin/bitcoin/pull/33567#issuecomment-3400743635)
Concept NACK.
what would be a third option? it's either relay or not.
(https://github.com/bitcoin/bitcoin/pull/33567#issuecomment-3400743635)
Concept NACK.
what would be a third option? it's either relay or not.
👍 hodlinator approved a pull request: "ci: Add macOS cross task for arm64-apple-darwin"
(https://github.com/bitcoin/bitcoin/pull/33549#pullrequestreview-3334496537)
crACK fad5a7101cc3dccbb525cfe9afc105aace8da88e
(https://github.com/bitcoin/bitcoin/pull/33549#pullrequestreview-3334496537)
crACK fad5a7101cc3dccbb525cfe9afc105aace8da88e
💬 hodlinator commented on pull request "ci: Add macOS cross task for arm64-apple-darwin":
(https://github.com/bitcoin/bitcoin/pull/33549#discussion_r2428354982)
nits in fa8c750a0aff9c03270b71a91536639f3922eed8:
Can't see any reference in the configs/scripts for how this step is reused for Mac. Maybe there's some part of the Cirrus config I'm not finding, although I can't see it being run in the logs either (https://productionresultssa9.blob.core.windows.net/actions-results/b7d9a1c2-e433-4d61-91b6-663cde22c2f6/workflow-job-run-8d60d8fe-2ef0-566b-bad9-1d540924800c/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-10-14T09%3A00%3A24Z&sig=F60Lcc0LkB2xmwQ8QEI
...
(https://github.com/bitcoin/bitcoin/pull/33549#discussion_r2428354982)
nits in fa8c750a0aff9c03270b71a91536639f3922eed8:
Can't see any reference in the configs/scripts for how this step is reused for Mac. Maybe there's some part of the Cirrus config I'm not finding, although I can't see it being run in the logs either (https://productionresultssa9.blob.core.windows.net/actions-results/b7d9a1c2-e433-4d61-91b6-663cde22c2f6/workflow-job-run-8d60d8fe-2ef0-566b-bad9-1d540924800c/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-10-14T09%3A00%3A24Z&sig=F60Lcc0LkB2xmwQ8QEI
...
💬 hodlinator commented on pull request "ci: Add macOS cross task for arm64-apple-darwin":
(https://github.com/bitcoin/bitcoin/pull/33549#discussion_r2428360859)
reviewed: Diffed against ci/test/00_setup_env_mac_cross.sh and only `CONTAINER_NAME` & `HOST` differ by CPU-arch, as expected.
Makes sense to have ARM be the main one going forward and move the old setup to "_intel".
(https://github.com/bitcoin/bitcoin/pull/33549#discussion_r2428360859)
reviewed: Diffed against ci/test/00_setup_env_mac_cross.sh and only `CONTAINER_NAME` & `HOST` differ by CPU-arch, as expected.
Makes sense to have ARM be the main one going forward and move the old setup to "_intel".
🚀 fanquake merged a pull request: "ci: fix buildx gha cache authentication on forks"
(https://github.com/bitcoin/bitcoin/pull/33508)
(https://github.com/bitcoin/bitcoin/pull/33508)