💬 Sjors commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#issuecomment-2815888994)
If they don't exist yet, can you also add a test to show `MAX_SCRIPT_SIZE` doesn't "apply" to `OP_RETURN`?

💬 instagibbs commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#issuecomment-2815920324)
@Sjors can you detail what the test would be? OP_RETURNs are already not entered into the utxo set at any length

👋 ryanofsky's pull request is ready for review: "bitcoin-cli: Add -ipcconnect option"
(https://github.com/bitcoin/bitcoin/pull/32297)

💬 sipa commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r2050956877)
Nitty nit: I think both the original and the suggestion are wrong.

"The compressed aggregate public key ~for~ which this pubnonce is for."

💬 stickies-v commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2815963984)
re-ACK 65fcfbb2b38bef20a58daa6c828c51890180611d - no changes except addressing style nits and adding missing includes.

nit: some of the new `util/transaction_identifier.h` includes, such as in `interfaces/wallet.h` could have been a forward declaration instead to minize compile time

💬 pablomartin4btc commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#issuecomment-2815995857)
`-legacy` argument shouldn't be removed from `bitcoin-wallet`/ `wallettool.cpp`? (leaving `-descriptors` `true` by default):

```
/build/bin/bitcoin-wallet -legacy -wallet="pepe" create
Topping up keypool...
Wallet info
===========
Name: pepe
Format: sqlite
Descriptors: no
Encrypted: no
HD (hd seed available): yes
Keypool Size: 2000
Transactions: 0
Address Book: 0

```

💬 wicherfree commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#discussion_r2050990904)
Bitcoin

💬 wicherfree commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#discussion_r2050994379)
1

💬 jonatack commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2051045020)
In commit 0a6547d26b570625922478e7e993ad1c97ec3ab4 maybe reference from root (feel free to ignore)

```suggestion
You can find installation instructions in the `/doc/build-*.md` file for your platform, or self-compile
```

🤔 jonatack reviewed a pull request: "doc: Improve `dependencies.md`"
(https://github.com/bitcoin/bitcoin/pull/31895#pullrequestreview-2779295634)
Swung by here a couple times previously and this pull is looking better now.

ACK fefe8fc3c38701bdc3a3b1c6e4d0c8885f94e75f modulo question below

💬 jonatack commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2051041929)
In commit fefe8fc3c38701bdc3a3b1c6e4d0c8885f94e75f ISTM the CMake entry shouldn't change in this commit, apart from ordering alphabetically (why does it change here)

🚀 glozow merged a pull request: "psbt: MuSig2 Fields"
(https://github.com/bitcoin/bitcoin/pull/31247)

💬 achow101 commented on pull request "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`":
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r2051114781)
Prior to this PR, the multisig descriptors would not insert any pubkeys int `out.pubkeys` inside of `MakeScripts`. The pubkeys that are inserted to that `FlatSigningProvider` are the ones inserted into the keypool.

💬 jonatack commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2051136269)
> Other help texts include \n, maybe do that here as well?

I'm agnostic as to that.

A few text compaction ideas, feel free to pick/choose/ignore:

s/The CPU time (user + system)/Total CPU time/

s/spent processing/processing/

s/duration. Will be omitted on platforms that do not support this or if still not measured./duration, if supported by the platform and measured./

s/Note that high CPU time is not necessarily a bad thing - new valid transactions and blocks require CPU time t
...

💬 jonatack commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#issuecomment-2816220345)
Latest changes since my last review LGTM. I plan to review the code more closely.

Empirically, running a node on this branch, it looks like over time most peers settle into less than 0.5 per milles, only a few are higher, with maybe one peer around 4.

📝 theStack opened a pull request: "test: add test for decoding PSBT with MuSig2 PSBT key types (BIP 373)"
(https://github.com/bitcoin/bitcoin/pull/32305)
This PR is a follow-up to #31247 (see https://github.com/bitcoin/bitcoin/pull/31247#pullrequestreview-2427834909) and adds a functional test for decoding PSBTs (using the `decodepsbt` RPC) with MuSig2 per-input and per-output types. The first commit adds the new MuSig2 key types to the test frameworks and extends the PSBT serialization to cope with lists of bytestrings.

📝 hebasto opened a pull request: "ci: Temporarily disable `WalletMigration` benchmark"
(https://github.com/bitcoin/bitcoin/pull/32306)
The `WalletMigration` benchmark is currently failing on CI.

This PR temporarily disables it until the issue is resolved.

An alternative to https://github.com/bitcoin/bitcoin/pull/32302.

💬 achow101 commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#issuecomment-2816244694)
> `-legacy` argument shouldn't be removed from `bitcoin-wallet`/ `wallettool.cpp`? (leaving `-descriptors` `true` by default):

Good catch! Added a commit to disable legacy wallet creation from the wallet tool.

💬 hebasto commented on pull request "ci: Temporarily revert "Drop bench -priority-level in win cross CI"":
(https://github.com/bitcoin/bitcoin/pull/32302#issuecomment-2816244798)
> Can we add a different change...

An alternative has been suggested in https://github.com/bitcoin/bitcoin/pull/32306.

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2051118275)
Let me know if you think the final commit in the latest push is an improvement.