💬 rkrux commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2318999432)
I think this particular error is appropriately specific. The error in `sendtoaddress`, while easier to parse, is thrown because it also tries to sign the transaction, though at an earlier stage in the RPC flow. Perhaps this particular test can avoid testing for this specific error because the different error is conspicuous.

💬 glozow commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3249315224)
Should this be backported?

👍 dergoegge approved a pull request: "p2p: add `DifferenceFormatter` fuzz target and invariant check"
(https://github.com/bitcoin/bitcoin/pull/33252#pullrequestreview-3180811360)
Code review ACK 65a10fc3c52ea09a4794345bcf607dff908c783a

📝 m3dwards opened a pull request: "added qa assets yml"
(https://github.com/bitcoin/bitcoin/pull/33292)

✅ m3dwards closed a pull request: "added qa assets yml"
(https://github.com/bitcoin/bitcoin/pull/33292)

🤔 glozow reviewed a pull request: "doc: truc packages allow sub min feerate transactions"
(https://github.com/bitcoin/bitcoin/pull/33220#pullrequestreview-3180848706)
ACK 7270839af425adddb3ed436a58a41b5bc843dab5

✅ glozow closed an issue: "doc: Mempool Policy documentation Outdated since TRUC"
(https://github.com/bitcoin/bitcoin/issues/32067)

🚀 glozow merged a pull request: "doc: truc packages allow sub min feerate transactions"
(https://github.com/bitcoin/bitcoin/pull/33220)

🤔 glozow reviewed a pull request: "[29.x] finalise v29.1"
(https://github.com/bitcoin/bitcoin/pull/33271#pullrequestreview-3180870194)
ACK 084c95a18c9978c0a047ffe219a9eef8ab327ea6

👍 willcl-ark approved a pull request: "[29.x] finalise v29.1"
(https://github.com/bitcoin/bitcoin/pull/33271#pullrequestreview-3180882773)
ACK 084c95a18c9978c0a047ffe219a9eef8ab327ea6

This all looks correct to me

💬 ryanofsky commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#issuecomment-3249470528)
This is a nice idea and approach seems ok if it works in practice. I think a more general approach might to have the bitcoin build set a variable the libmultiprocess build can use to show better error messages like:

```
set(MP_SUBPROJECT_ERROR "Configure cmake with with -DENABLE_IPC=OFF if you do not need IPC functionality.")
```

It could append this message if it can't find the cap'n proto package, or doesn't find a compatible version (for https://github.com/bitcoin-core/libmultiproces
...

⚠️ maflcko opened an issue: "ci: GHA fallback centos task runs out of space"
(https://github.com/bitcoin/bitcoin/issues/33293)
https://github.com/bitcoin-core/gui/actions/runs/17433074590/job/49495964258?pr=884#step:8:3785

```
...
+ eval 'TEST_RUNNER_EXTRA=()'
++ TEST_RUNNER_EXTRA=()
+ LD_LIBRARY_PATH=/home/runner/work/_temp/depends/x86_64-pc-linux-gnu/lib
+ /home/runner/work/_temp/build/test/functional/test_runner.py --ci -j4 --tmpdirprefix /home/runner/work/_temp/ci/scratch/test_runner/ --ansi --combinedlogslen=99999999 --timeout-factor=40 --quiet --failfast
WARNING! There may be insufficient free space in /home/runn
...

💬 maflcko commented on issue "ci: GHA fallback centos task runs out of space":
(https://github.com/bitcoin/bitcoin/issues/33293#issuecomment-3249499925)
It may be possible to free up some space with something like https://github.com/google/oss-fuzz/blob/14cab84abbc736e24fede3f0267a8e220fb4974e/.github/workflows/project_tests.yml#L65-L74

💬 maflcko commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin-core/gui/pull/884#issuecomment-3249501476)
(ci failure can be ignored, see https://github.com/bitcoin/bitcoin/issues/33293)

💬 willcl-ark commented on issue "ci: GHA fallback centos task runs out of space":
(https://github.com/bitcoin/bitcoin/issues/33293#issuecomment-3249508043)
```
Free disk space:
+ [[ ci_native_centos == \c\i\_\n\a\t\i\v\e\_\a\s\a\n ]]
++ /home/runner/work/_temp/depends/config.guess
Filesystem Size Used Avail Use% Mounted on
overlay 72G 48G 25G 67% /
tmpfs 64M 0 64M 0% /dev
shm 64M 0 64M 0% /dev/shm
/dev/root 72G 48G 25G 67% /etc/hosts
tmpfs 7.9G 0 7.9G 0% /proc/acpi
tmpfs 7.9G 0 7.9G 0% /proc/scsi
tmpfs 7.9G 0 7.9G 0% /sys/firmw
...

🤔 theStack reviewed a pull request: "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys"
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3180916078)
Reviewed up to 38d74136c6552ae746bec6e05b8ec3069cd581ce, left a few non-blocking suggestions below.

I think the order of commits d65c8df972de55ade06557cabb1b8972d4169fb1 ... 38d74136c6552ae746bec6e05b8ec3069cd581ce is currently slightly confusing for reviewers, as it doesn't reflect the protocol flow, i.e. the signature aggregation function `CreateMuSig2AggregateSig` should ideally be introduced _after_ the partial signature creation function `CreateMuSig2PartialSig`.

💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2319159859)
in commit a7901710a6b222cbdeba474bbe8b78788841e58a: it's only a few lines of code, but to deduplicate with the next commit (introducing `CreateMuSig2PartialSig`), could introduce a helper function like `MuSig2SessionId`

💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2319145295)
in commit d65c8df972de55ade06557cabb1b8972d4169fb1: naming nit: `participants` doesn't say much imho, could rename to `participant_pubkeys` (or just `part_pubkeys` / `pubkeys`, if that's too long) to be more specific

💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2319173890)
in 38d74136c6552ae746bec6e05b8ec3069cd581ce: if `our_pubkey` is not contained in the `pubkeys` list, I suppose we want to return early with `std::nullopt`, rather than accessing at index 0?

💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2319170915)
in commit 38d74136c6552ae746bec6e05b8ec3069cd581ce: nit, could rename to something like `our_pubkey_index` to be more expressive