💬 willcl-ark commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3355296742)
One possible "tightening" could be to only export the [exact variables needed](https://github.com/bitcoin/bitcoin/actions/runs/18126966670/job/51584371561?pr=33508#step:6:333):
```
Exporting ACTIONS_RUNNER_HOOK_JOB_STARTED # <-- likely not required for buildx gha cache
Exporting ACTIONS_RUNTIME_URL
Exporting ACTIONS_RUNTIME_TOKEN
Exporting ACTIONS_CACHE_URL
Exporting ACTIONS_RESULTS_URL
Exporting ACTIONS_CACHE_SERVICE_V2
```
But IMO exporting all variables prefixed with `ACTIONS_` t
...
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3355296742)
One possible "tightening" could be to only export the [exact variables needed](https://github.com/bitcoin/bitcoin/actions/runs/18126966670/job/51584371561?pr=33508#step:6:333):
```
Exporting ACTIONS_RUNNER_HOOK_JOB_STARTED # <-- likely not required for buildx gha cache
Exporting ACTIONS_RUNTIME_URL
Exporting ACTIONS_RUNTIME_TOKEN
Exporting ACTIONS_CACHE_URL
Exporting ACTIONS_RESULTS_URL
Exporting ACTIONS_CACHE_SERVICE_V2
```
But IMO exporting all variables prefixed with `ACTIONS_` t
...
💬 brangnu commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-3355366242)
Concept NACK.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-3355366242)
Concept NACK.
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2393958445)
> I don't think multisig inputs tend to be mixed with non-multisig inputs.
Ah, I should have rephrased the sentence to highlight spending multiple MuSig2 unspents in the PSBT instead of highlighting the other one.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2393958445)
> I don't think multisig inputs tend to be mixed with non-multisig inputs.
Ah, I should have rephrased the sentence to highlight spending multiple MuSig2 unspents in the PSBT instead of highlighting the other one.
💬 hodlinator commented on pull request "ci: Switch to dynamic library linkage in native Windows job":
(https://github.com/bitcoin/bitcoin/pull/32182#issuecomment-3355516795)
Noticed the build artifact `x86_64-w64-mingw32-executables-<run>` is 916MB (https://github.com/bitcoin/bitcoin/actions/runs/18155350909?pr=31723 / https://github.com/bitcoin/bitcoin/actions/runs/18155350909/job/51673850541?pr=31723#step:9:161).
Artifact contents:
```shell
$ unzip -l x86_64-w64-mingw32-executables-18155350909.zip
Archive: x86_64-w64-mingw32-executables-18155350909.zip
Length Date Time Name
--------- ---------- ----- ----
617942101 10-01-2025 07:57 bi
...
(https://github.com/bitcoin/bitcoin/pull/32182#issuecomment-3355516795)
Noticed the build artifact `x86_64-w64-mingw32-executables-<run>` is 916MB (https://github.com/bitcoin/bitcoin/actions/runs/18155350909?pr=31723 / https://github.com/bitcoin/bitcoin/actions/runs/18155350909/job/51673850541?pr=31723#step:9:161).
Artifact contents:
```shell
$ unzip -l x86_64-w64-mingw32-executables-18155350909.zip
Archive: x86_64-w64-mingw32-executables-18155350909.zip
Length Date Time Name
--------- ---------- ----- ----
617942101 10-01-2025 07:57 bi
...
👍 rkrux approved a pull request: "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys"
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3288310595)
> receive and spend from imported musig(0 descriptors.
Is this a typo in the PR description?
----
This was a big PR that required me to split the review into multiple parts in which I tried to think of scenarios that could cause the flow could break or be unsafe.
Few points from my understanding of this PR:
1. Appropriate steps are taken to secure the `MuSig2SecNonce`.
- It is stored only in memory - never backed up on disk, or serialised. Okay with the tradeoff of having to
...
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3288310595)
> receive and spend from imported musig(0 descriptors.
Is this a typo in the PR description?
----
This was a big PR that required me to split the review into multiple parts in which I tried to think of scenarios that could cause the flow could break or be unsafe.
Few points from my understanding of this PR:
1. Appropriate steps are taken to secure the `MuSig2SecNonce`.
- It is stored only in memory - never backed up on disk, or serialised. Okay with the tradeoff of having to
...
✅ vasild closed a pull request: "fuzz: set the output argument of FuzzedSock::Accept()"
(https://github.com/bitcoin/bitcoin/pull/31316)
(https://github.com/bitcoin/bitcoin/pull/31316)
💬 vasild commented on pull request "fuzz: set the output argument of FuzzedSock::Accept()":
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-3355567303)
Closing this because https://github.com/bitcoin/bitcoin/pull/28584 was merged which includes the commit in this PR.
This PR: 83199523c90591d57cd5046212c878a4d54d621d
#28584 contains: e883b37768812d96feec207a37202c7d1b603c1f
```sh
$ git range-diff e883b37768~..e883b37768 83199523c9~..83199523c9
1: e883b37768 = 1: 83199523c9 fuzz: set the output argument of FuzzedSock::Accept()
$
```
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-3355567303)
Closing this because https://github.com/bitcoin/bitcoin/pull/28584 was merged which includes the commit in this PR.
This PR: 83199523c90591d57cd5046212c878a4d54d621d
#28584 contains: e883b37768812d96feec207a37202c7d1b603c1f
```sh
$ git range-diff e883b37768~..e883b37768 83199523c9~..83199523c9
1: e883b37768 = 1: 83199523c9 fuzz: set the output argument of FuzzedSock::Accept()
$
```
💬 stickies-v commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3355697586)
re-ACK 1aaaaa078bb2efed126e3f41ecf7c81ccf005818
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3355697586)
re-ACK 1aaaaa078bb2efed126e3f41ecf7c81ccf005818
👍 vasild approved a pull request: "test: fix p2p_leak_tx.py"
(https://github.com/bitcoin/bitcoin/pull/33121#pullrequestreview-3288480537)
ACK 14ae71f323dd011c6d51470ea15cf00750970f65
(https://github.com/bitcoin/bitcoin/pull/33121#pullrequestreview-3288480537)
ACK 14ae71f323dd011c6d51470ea15cf00750970f65
👍 hebasto approved a pull request: "ci: Remove bash -c from cmake invocation using eval"
(https://github.com/bitcoin/bitcoin/pull/33401#pullrequestreview-3288490644)
ACK 50194029e7c2581b751931080f5999785a39929f.
(https://github.com/bitcoin/bitcoin/pull/33401#pullrequestreview-3288490644)
ACK 50194029e7c2581b751931080f5999785a39929f.
🚀 hebasto merged a pull request: "ci: Remove bash -c from cmake invocation using eval"
(https://github.com/bitcoin/bitcoin/pull/33401)
(https://github.com/bitcoin/bitcoin/pull/33401)
💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394150885)
done.
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394150885)
done.
💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394151442)
ah I see how it could be confusing, clarified it to "since the previous commit removes BLOCK_FAILED_CHILD usage in InvalidateBlock"
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394151442)
ah I see how it could be confusing, clarified it to "since the previous commit removes BLOCK_FAILED_CHILD usage in InvalidateBlock"
💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394152350)
good point. I've removed it since `BLOCK_FAILED_MASK = 96` is never written to disk but only used while reading BlockStatus values. if 96 exists on disk, it is the responsibility of `BLOCK_FAILED_VALID` and `BLOCK_FAILED_CHILD` and we have dealt with those cases.
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394152350)
good point. I've removed it since `BLOCK_FAILED_MASK = 96` is never written to disk but only used while reading BlockStatus values. if 96 exists on disk, it is the responsibility of `BLOCK_FAILED_VALID` and `BLOCK_FAILED_CHILD` and we have dealt with those cases.
💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394152594)
done.
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394152594)
done.
💬 trevarj commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3355880463)
> Hopefully Guix makes a release tag again one day.
This is coming soon, there have been talks of it.
In the meantime, you can simply `guix pull` and receive the freshest version (from master). If you don't want master, you can edit your Guix channels (`~/.config/guix/channels.scm`) and pin the commit to something "stable".
I'm not certain that would cause an issue though. To me it just sounds like unstable substitute servers. For context, if you try installing Guix System it could tak
...
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3355880463)
> Hopefully Guix makes a release tag again one day.
This is coming soon, there have been talks of it.
In the meantime, you can simply `guix pull` and receive the freshest version (from master). If you don't want master, you can edit your Guix channels (`~/.config/guix/channels.scm`) and pin the commit to something "stable".
I'm not certain that would cause an issue though. To me it just sounds like unstable substitute servers. For context, if you try installing Guix System it could tak
...
👍 stickies-v approved a pull request: "[28.x] backports + 28.3rc1"
(https://github.com/bitcoin/bitcoin/pull/33476#pullrequestreview-3288706062)
re-ACK 9968b15937cbc2071c3474bd83f9c9fb4bb3a970
Getting the same bitcoin.conf example.
(https://github.com/bitcoin/bitcoin/pull/33476#pullrequestreview-3288706062)
re-ACK 9968b15937cbc2071c3474bd83f9c9fb4bb3a970
Getting the same bitcoin.conf example.
💬 stickies-v commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2394279590)
Oh I see. The fact that test functions are slightly renamed combined with this patch of code being ~duplicated tripped me up a bit. Thanks for the clarification, can be marked as resolved.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2394279590)
Oh I see. The fact that test functions are slightly renamed combined with this patch of code being ~duplicated tripped me up a bit. Thanks for the clarification, can be marked as resolved.
🤔 sipa reviewed a pull request: "validation: remove BLOCK_FAILED_CHILD"
(https://github.com/bitcoin/bitcoin/pull/32950#pullrequestreview-3288768556)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32950#pullrequestreview-3288768556)
Concept ACK
⚠️ venpisey12 opened an issue: "0968812058"
(https://github.com/bitcoin/bitcoin/issues/33513)
(https://github.com/bitcoin/bitcoin/issues/33513)