✅ maflcko closed a pull request: "Comment Changes"
(https://github.com/bitcoin/bitcoin/pull/32555)

💬 maflcko commented on pull request "Comment Changes":
(https://github.com/bitcoin/bitcoin/pull/32555#issuecomment-2889687264)
Thanks for your interest in contributing! However, since there are hundreds of pending pull requests, I am closing this to focus review on the others. If you wish to contribute in the future, please focus on creating high-quality, original content that demonstrates a clear understanding of the project's requirements and goals. Also, see the [contributing guidelines](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring).

💬 maflcko commented on pull request "RFC: script: short-circuit `GetLegacySigOpCount` for known scripts":
(https://github.com/bitcoin/bitcoin/pull/32532#issuecomment-2889730573)
I can't tell from the pull description, but is there a end-user visible performance difference? If yes, how much?

💬 maflcko commented on pull request "refactor: Remove workaround for resolved MSVC bug":
(https://github.com/bitcoin/bitcoin/pull/32552#issuecomment-2889757878)
It would be good to adjust the URL to avoid confusion and duplicate work and pull requests

💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2889766830)
Whatever the issue is, it may be fixed in gcc-14. At least https://cirrus-ci.com/task/5477938151292928 passed 25 times. (The prior commit failed at least once: https://cirrus-ci.com/task/5645419964792832?logs=ci#L4122)

📝 meghasurya07 opened a pull request: "bench: Add WalletMigration benchmark for legacy-to-descriptor wallet upgrade"
(https://github.com/bitcoin/bitcoin/pull/32556)

Adds a benchmark to measure performance of migrating a legacy wallet
with multiple watch-only and key entries to the descriptor wallet format.

✅ meghasurya07 closed a pull request: "bench: Add WalletMigration benchmark for legacy-to-descriptor wallet upgrade"
(https://github.com/bitcoin/bitcoin/pull/32556)

📝 meghasurya07 opened a pull request: "bench: Add WalletMigration benchmark for legacy-to-descriptor wallet upgrade"
(https://github.com/bitcoin/bitcoin/pull/32557)
Adds a benchmark to measure performance of migrating a legacy wallet
with multiple watch-only and key entries to the descriptor wallet format.

✅ achow101 closed a pull request: "bench: Add WalletMigration benchmark for legacy-to-descriptor wallet upgrade"
(https://github.com/bitcoin/bitcoin/pull/32557)

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2095000787)
Ah, it's a verb, then it makes sense.

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2095003386)
In that case it's less confusing if you don't pass this argument (as the default is false).

💬 hebasto commented on pull request "refactor: Remove workaround for resolved MSVC bug":
(https://github.com/bitcoin/bitcoin/pull/32552#issuecomment-2889886048)
> It would be good to adjust the URL to avoid confusion and duplicate work and pull requests

Sure. I'll wait until it's been properly triaged.

💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2889893521)
Backtrace with file names and lines:

```
Thread 33 (Thread 0x7f9d93e006c0 (LWP 18294) "b-httpworker.2"):
#0 0x00007f9da1dfca9a in read () from /lib/x86_64-linux-gnu/libc.so.6
#1 0x000055f3d4633e22 in read (__nbytes=1024, __buf=0x7f9d93dfb030, __fd=30) at /usr/include/x86_64-linux-gnu/bits/unistd.h:28
#2 subprocess::util::read_atmost_n (read_upto=1024, buf=0x7f9d93dfb030 "", fp=0x7f9d8c0036c0) at ./util/subprocess.h:437
#3 subprocess::Popen::execute_process (this=this@entry=0x7f9d93dfb8b0) a
...

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2095044674)
Update: I see you already did that in 048c95ea55.

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2889977973)
Rebased after #32452.

🤔 hebasto reviewed a pull request: "qt: drop unused watch-only functionality"
(https://github.com/bitcoin/bitcoin/pull/32459#pullrequestreview-2849775166)
In `src/qt/forms/overviewpage.ui`, after removing all items from `row="0"` of the `gridLayout` element, all subsequent rows should be renumbered as follows: 1 -> 0, 2 -> 1, 3 -> 2, 4 -> 3, 5 -> 4.

💬 Sjors commented on pull request "Mining: Avoid copying template CBlocks":
(https://github.com/bitcoin/bitcoin/pull/32547#issuecomment-2890004759)
Concept ACK on adding a `TemplateToJSON` helper, as well as the new alias for `BlockAssembler::Options`.

This breaks the multiprocess build somehow, cc @ryanofsky, e.g.

```
[06:00:12.877] /ci_container_base/src/interfaces/mining.h:54:34: note: unimplemented pure virtual method 'getCoinbaseMerklePath' in 'ProxyClient'
[06:00:12.877] 54 | virtual std::vector<uint256> getCoinbaseMerklePath() const = 0;
```

💬 Sjors commented on pull request "scripted-diff: test: remove 'descriptors=True' argument for `createwallet` calls":
(https://github.com/bitcoin/bitcoin/pull/32544#issuecomment-2890031213)
ACK 86de8c1668005304b2c630ca2ad4a8ca8e348e90

> The argument `descriptors` could also be removed from `createwallet` RPC.

Unfortunately not, as long as we support positional arguments. We can ignore it though.

⚠️ maflcko opened an issue: "intermittent issue in qt addressBookTests: (table_view->model()->rowCount()): [3 != 2] Loc: [./qt/test/addressbooktests.cpp(183)]"
(https://github.com/bitcoin/bitcoin/issues/32558)
Seen on the test-each-commit task, as well as on task `CentOS, depends, gui`: https://github.com/bitcoin/bitcoin/runs/42411027184

```
[12:40:35.901] ********* Finished testing of WalletTests *********
[12:40:35.901] ********* Start testing of AddressBookTests *********
[12:40:35.901] Config: Using QtTest library 6.7.3, Qt 6.7.3 (x86_64-little_endian-lp64 static debug build; by GCC 14.2.1 20250110 (Red Hat 14.2.1-7)), centos 10
[12:40:35.901] PASS : AddressBookTests::initTestCase()
[12:
...

💬 Sjors commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2890058758)
Rebased just in case. Renumbered columns in `overviewpage.ui`.