💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3354814334)
Updated 21b0503c2f19f5e4662cea1ceecb425b8460967b -> f3ca1d65b0444097fd0e2f059a01eb5b29c950c1 ([kernelApi_67](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_67) -> [kernelApi_68](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_68), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_67..kernelApi_68))

* Improved docstrings for processing blocks.
* Added three more validation interface callbacks, which should make it a bit easier to use when implementing a blocks
...

💬 maflcko commented on pull request "ci: Remove bash -c from cmake invocation using eval":
(https://github.com/bitcoin/bitcoin/pull/33401#issuecomment-3354869230)
lgtm ACK 50194029e7c2581b751931080f5999785a39929f

💬 brainx commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#discussion_r2393557892)
Bitcoin ❤️

💬 BrandonOdiwuor commented on pull request "ci: Remove bash -c from cmake invocation using eval":
(https://github.com/bitcoin/bitcoin/pull/33401#discussion_r2393578871)
fixed

👍 hodlinator approved a pull request: "build: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3287946924)
re-ACK 1aaaaa078bb2efed126e3f41ecf7c81ccf005818

Bumped version in 3 more places and dropped fuzz-workaround.

```
a9cdca5dc929d3c6c937e0a9069f2607552283087c51c4fac7921531a4b97bbf guix-build-1aaaaa078bb2/output/arm64-apple-darwin/SHA256SUMS.part
673dcca25defb6d09d5ffb9aa9bec86884d445bc15c94d191bb81b06bc13b0a1 guix-build-1aaaaa078bb2/output/arm64-apple-darwin/bitcoin-1aaaaa078bb2-arm64-apple-darwin-codesigning.tar.gz
674cfe724d57aeb2afaff18195347c90b347565d800ede2842d42ca04b1a9023 guix-
...

💬 hodlinator commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2393728354)
nit: This change to ci.yml in this PR should probably be broken out to a separate commit or mentioned in the commit message as now it's just "Drop support for EOL macOS 13".

Cheers, will check out 33509.

💬 Sjors commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3355157444)
> @Sjors Is there something else I should check to try to reproduce the issue you had?

I have no idea. One factor might have been my guix version:

```
guix (GNU Guix) 20dbf225f332ccc707578263ed710dcf2a8fb78e
Copyright (C) 2024 the Guix authors
```

🤔 hodlinator reviewed a pull request: "node: Add --debug_runs/-waitfordebugger + --debug_cmd"
(https://github.com/bitcoin/bitcoin/pull/31723#pullrequestreview-3282841954)
Latest push adds a functional test and breaks apart commits somewhat.

💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2392681962)
Added a test for Linux (the only OS where we explicitly *enable* attaching) in the latest push.

💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2392687192)
New test includes `assert_start_raises_init_error()` at the end for this precisely.

💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2390087376)
That's a clever way of doing it, did something very close to it now (hopefully it's small enough that source code license issues don't apply).

💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2390038435)
Did something similar to https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2389662377

💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2392650479)
To me there's a silent, implied, "Enables ..." or "Provides ..." at the beginning of the description for these options that would get repetitive if spelled out, which may make "support" more natural.

Note that simply enabling the option does not pause startup, one also has to start with `-waitfordebugger`.

Curious to hear what other reviewers think.

🤔 hodlinator reviewed a pull request: "ci: Check macos-cross executables on macOS"
(https://github.com/bitcoin/bitcoin/pull/33509#pullrequestreview-3288065733)
Concept ACK d40e61e87cfb18f75742c2efa6e705f38e3b36c9

It feels more complete to test something closer to what the end user will be running (although we are still not using Guix builds).

Since we don't have high hopes of this actually detecting any issues that the rest of the CI would not have found, seems to make more sense as a nightly job?

Runs fine:
https://github.com/bitcoin/bitcoin/actions/runs/18132888148/job/51606766678?pr=33509

💬 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
...

💬 brangnu commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(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.

💬 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
...

👍 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
...

✅ vasild closed a pull request: "fuzz: set the output argument of FuzzedSock::Accept()"
(https://github.com/bitcoin/bitcoin/pull/31316)