👍 hodlinator approved a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2863857334)
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e

Concept: https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2821205668

`git range-diff master 4e1aae1 a5ac43d` shows only minor fixups since previous review (https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2838013470).

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2903947412)
Rebased e8ac6d29273c48328f1f77886c3c3f653e6cc58e -> e604a57dccac1c4e6802a3ecc7f4cb4c3229c8ed ([spendblock_2](https://github.com/TheCharlatan/bitcoin/tree/spendblock_2) -> [spendblock_3](https://github.com/TheCharlatan/bitcoin/tree/spendblock_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_2..spendblock_3))

* Fixed conflict with #32575

💬 Sjors commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#issuecomment-2903999864)
Concept ACK

In general normalized descriptors are more useful, because you can't derive addresses from e.g. `xpub/1h/*`. So it makes sense to use them for the parent.

ACK 107517ea7d29d959e7a0b94d8217dcb894680547

I checked that the tests fail if I revert the first commit.

nit: "prividing" -> "providing"

🤔 Sjors reviewed a pull request: "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC"
(https://github.com/bitcoin/bitcoin/pull/32593#pullrequestreview-2863994337)
Concept ACK

💬 Sjors commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2104321500)
`/*peristed=*/bool` ?

💬 Sjors commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2104331800)
This may be a good time to document this function. In particular why it unlocks coins. I'm guessing because we don't want to pollute the wallet db with locks for things that can't be spend anyway? And if there's a reorg we want them to be available.

💬 Sjors commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2104340229)
The original documentation is also a bit unclear.

> A Coin may be locked if it has already been used to fund a transaction that hasn't confirmed yet.

Is this referring to transactions that were generated either:
1. outside the current wallet and not in our mempool
2. by our wallet but without broadcast, e.g. PSBT workflow with `lock_unspents`

💬 Sjors commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2104348210)
11d0513379d9cbb375f5f6e8877743872225a0ff: why don't we persist this?

Could be a separate commit if we want to change the behaviour.

(ditto for the other RPC methods)

💬 Sjors commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2104326011)
Could document this to point out that this will update the wallet database if the coin is persisted.

💬 willcl-ark commented on pull request "depends: use "mkdir -p" when installing xproto":
(https://github.com/bitcoin/bitcoin/pull/32568#issuecomment-2904140202)
OK a second full guix build got the same hashes as my first:

```
src/core/worktree-alpine-MKDIRPROG on  alpine_MKDIRPROG:refs/pull/32568/head [$] via △ v3.31.6 via 🐍 v3.12.10 via ❄️ impure (nix-shell-env) took 1h31m29s
❯ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
cb7dbe6112cdd38db9757af64cd4e617cdced8a6525db8a1469f3f35d29de9f6 guix-build-df9ebbf659d5/output/aarch64-linux-gnu/SHA256SUMS.part
a0b2b6c3fd30ae9aa766
...

💬 fanquake commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2904185367)
Guix Build:
```bash
47df12bafda434889c080ec544817f1058d95dda7d79889ea59bcc42084d73e5 guix-build-24e5fd3bedce/output/aarch64-linux-gnu/SHA256SUMS.part
b0cd4b29b7d3c3d3858aeb3496e7f7c0b0910f6bcaa3c76d537053a2f6e96f2f guix-build-24e5fd3bedce/output/aarch64-linux-gnu/bitcoin-24e5fd3bedce-aarch64-linux-gnu-debug.tar.gz
3d3228c719e4a7df4cd83c2b2d46c6ffd072d331b83cf5f70b938cc976908b91 guix-build-24e5fd3bedce/output/aarch64-linux-gnu/bitcoin-24e5fd3bedce-aarch64-linux-gnu.tar.gz
90ed4ba3126cc7f1
...

💬 willcl-ark commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2904253640)
Pushed an update which now only activates the dependency provider when using the depends toolchain.

This will hopefully appease the windows CI job, as well as making more sense in general and being more expected behaviour.

🤔 janb84 reviewed a pull request: "wallet, rpc: Return normalized descriptor in parent_descs"
(https://github.com/bitcoin/bitcoin/pull/32594#pullrequestreview-2864259747)
ACK 107517ea7d29d959e7a0b94d8217dcb894680547

Before change listtransactions (and others) shows un-normalized descriptors, after the descriptors are normalized


<details>
master:

```
{
"address": "bcrt1qgymj02gx6dzrkh49d9sj0cj8rxfcrut7dyxz7r",
"parent_descs": [
"wpkh(tpubD6NzVbkrYhZ4YdzwuYwQLEypddJ64KVEG9tyZibLxLKMxyMCaru1c7RFX2S8NEQ7dZuNccdzk5qikw51rxzXYWdkeuREgxgdqbJy3ty68qy/84h/1h/0h/0/*)#tz7s9af2"
],
"category": "immature",
"amount": 50.00000000,
...

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2104498772)
Thanks! Fixed in https://github.com/bitcoin/bitcoin/pull/32540/commits/9d7e23e2f505ce6cbe830fc607cf203b1a48ba0d.

📝 hebasto opened a pull request: "test: Fix `system_tests/run_command` test"
(https://github.com/bitcoin/bitcoin/pull/32601)
This PR addresses two issues related to the "Return non-zero exit code, with error message for stderr" check in the `system_tests/run_command` test:
1. The test now checks the exact exit code, which must be 1.
2. The process invocation string is removed from the exception message before checking the stderr output, as the latter is a substring of the former.

This is an alternative to https://github.com/bitcoin/bitcoin/pull/32577 without introducing a shell wrapper and altering quotation in `
...

✅ hebasto closed a pull request: "subprocess: Let shell parse command on non-Windows systems"
(https://github.com/bitcoin/bitcoin/pull/32577)

💬 hebasto commented on pull request "subprocess: Let shell parse command on non-Windows systems":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2904327201)
Closing in favour of https://github.com/bitcoin/bitcoin/pull/32601.

💬 hebasto commented on pull request "subprocess: Let shell parse command on non-Windows systems":
(https://github.com/bitcoin/bitcoin/pull/32577#discussion_r2104533575)
https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2902382891:

> - I don't think the added quotes in external_signer.cpp are doing anything, or it isn't clear what they are trying to do.

Descriptors contain the `(` and `)` characters, which may cause issue with shell:
```
$ echo pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)
bash: syntax error near unexpected token `('
```

💬 maflcko commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104545752)
This will just re-introduce the bugs fixed in https://github.com/bitcoin/bitcoin/pull/31542, no?

💬 maflcko commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104546796)
A better option would be to use the test's temp dir as scratch space

💬 hebasto commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104550766)
> This will just re-introduce the bugs fixed in #31542, no?

Oops, forgot about it :facepalm: