💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2104022638)
Being able to generate a (derived) descriptor without parsing is still on my wish list: https://github.com/bitcoin/bitcoin/issues/24003

But even in that case it could generate, convert to string and then re-parse as a sanity check.

💬 Sjors commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#issuecomment-2903590736)
> Split the LockCoin commit into https://github.com/bitcoin/bitcoin/pull/3259,

#32593

⚠️ fanquake reopened an issue: "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode"
(https://github.com/bitcoin/bitcoin/issues/32524)
https://cirrus-ci.com/task/5094568045051904?logs=ci#L4419

```
[18:26:32.573] test_framework.authproxy.JSONRPCException: 'enumeratesigners' RPC took longer than 1200.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344)
```

Looking at the full log, the test is just idle and then times out. However, there is no indication why the RPC does not proceed.

💬 vasild commented on pull request "tests: Expand HTTP coverage to assert libevent behavior":
(https://github.com/bitcoin/bitcoin/pull/32408#discussion_r2104068784)
Ok, it could be that libevent behaves in an unexpected way. In https://github.com/bitcoin/bitcoin/pull/32061 we could tighten up the test to:

* connect
* start timer in the test (A)
* send, server timer starts after last packet received from the client (B)
* try to recv, connection will be closed by the server due to server timeout (C)
* stop the timer in the test (D)

💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2903656040)
Another data point: I haven't see a failure so far in my G++ snapshot DEBUG=1 nightly run: https://github.com/maflcko/b-c-nightly/actions/runs/15153825944/job/42604725571

💬 mrberlinorg commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2104140401)
Removing the check for nDataOut > 1 would allow multiple OP_RETURN outputs in a single transaction, which goes against the standard behavior of the Bitcoin protocol. This could introduce several issues:

Non-standard transactions: Multiple OP_RETURN outputs in a transaction are considered non-standard. Allowing them could lead to network inconsistencies, as some nodes might not be able to properly process or relay these transactions.

Network congestion: More OP_RETURN outputs would increase
...

💬 cdecker commented on pull request "chainparams: Re-add seed.bitcoinstats.com":
(https://github.com/bitcoin/bitcoin/pull/31086#issuecomment-2903836922)
Sure, I just don't want something that isn't progressing wasting too much time. I will eventually pick this up, stabilize and reopen, but for now my head is on other things, hence the retraction for now.

💬 willcl-ark commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2104201681)
A gentle reminder not to post AI-generated comments like this again here to avoid being banned.

> multiple OP_RETURN outputs in a single transaction, which goes against the standard behavior of the Bitcoin protocol.

There are no protocol rules defining how many OP_RETURN outputs can be included in a transaction.

> Non-standard transactions: Multiple OP_RETURN outputs in a transaction are considered non-standard. Allowing them could lead to network inconsistencies, as some nodes might no
...

💬 willcl-ark commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2903870430)
The windows job fails as currently we activate the dependency provider automatically based on whether a(ny) toolchain is in use.

This works fine for depends, but doesn't make sense on windows where the vcpkg toolchain is used, without depends.

The idea for doing it this way was to try and avoid users having to pass _two_ flags in order to build from depends -- a toolchain and a dependency-provider -- but perhaps this is necessary after all...

I do recall reading somewhere in cmake docs
...

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