💬 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)
💬 fanquake commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3400800275)
Backported to `30.x` in #33609.
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3400800275)
Backported to `30.x` in #33609.
💬 maflcko commented on pull request "ci: Add macOS cross task for arm64-apple-darwin":
(https://github.com/bitcoin/bitcoin/pull/33549#discussion_r2428422264)
It is not used right now, but can be in the future. This is a separate commit. I'll adjust the commit message the next time I have to re-touch to clarify this further.
(https://github.com/bitcoin/bitcoin/pull/33549#discussion_r2428422264)
It is not used right now, but can be in the future. This is a separate commit. I'll adjust the commit message the next time I have to re-touch to clarify this further.
💬 fanquake commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3400827432)
Backported to `29.x` in #33611.
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3400827432)
Backported to `29.x` in #33611.
💬 willcl-ark commented on pull request "ci: Build ci_native_base image layer":
(https://github.com/bitcoin/bitcoin/pull/33620#issuecomment-3400842646)
I think if we want something like this to work it could be tricky without using a registry to host the base image which all other jobs pulled from (on a cache hit).
It may be possible without one, but as each job is `scope`d to itself, which is what keeps the caches seperate,: https://github.com/bitcoin/bitcoin/actions/runs/18489967967/job/52681101678?pr=33620#step:9:151 ... I think current approach would end up with one base_image per job, which is not quite what we are after here.
One po
...
(https://github.com/bitcoin/bitcoin/pull/33620#issuecomment-3400842646)
I think if we want something like this to work it could be tricky without using a registry to host the base image which all other jobs pulled from (on a cache hit).
It may be possible without one, but as each job is `scope`d to itself, which is what keeps the caches seperate,: https://github.com/bitcoin/bitcoin/actions/runs/18489967967/job/52681101678?pr=33620#step:9:151 ... I think current approach would end up with one base_image per job, which is not quite what we are after here.
One po
...