🤔 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)

💬 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()
$
```

💬 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

👍 vasild approved a pull request: "test: fix p2p_leak_tx.py"
(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.

🚀 hebasto merged a pull request: "ci: Remove bash -c from cmake invocation using eval"
(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.

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