💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2097450731)
In 556fe4bc71e29ca695122adbfd8dd6c9aa0f9a9e "descriptor: Add unused(KEY) descriptor": can you add a comment to elucidate the magic here?

💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2097509718)
In 213103fdc1c9ebcabbe01e1fc87345409a90dee2 "wallet: Add addhdkey RPC": I guess we don't need this anymore

💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2097484975)
In https://github.com/bitcoin/bitcoin/commit/556fe4bc71e29ca695122adbfd8dd6c9aa0f9a9e "descriptor: Add unused(KEY) descriptor": this might be a good time to document what `key_exp_index` is actually for.

No test breaks if I do this:

```diff
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index ff0782d769..ff694155e9 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -2093,14 +2093,12 @@ std::vector<std::unique_ptr<DescriptorImpl>> ParseScript(
...

💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2097436848)
In c6be6ba76493892d42bb3ddc671cac9582f7d4b3 "test: Test for addhdkey": it would be nice to distinguish between garbage and an xpub, and handle the latter more clearly, e.g. "Extended public key (xpub) provided, but private key (xpriv) is required."

Also nit: could squash this test into the previous commit, since there's no useful way test before and after when introducing a new RPC method.

💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2097507121)
In 7e9557572d2f2a958d740b6dd93e8f4b92141e67 "test: Simple test for importing unused(KEY)": can you add a test to verify that you can't import the tpub?

This passes (when put above the xprv import):

```py
self.test_importdesc({"timestamp": "now", "desc": descsum_create(f"unused({xpub})")},
success=False,
wallet=wallet)
```

💬 polespinasa commented on pull request "rpc: generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2893727699)
> Perhaps we can make the second argument of `generateblock` optional, and if there is no set of txs provided then mine the mempool.

This is implemented here 8b5dd8a5f135ce1aaf80d8c28943f503f4ee19d4
With a small difference, if no set of txs is provided then we mine the mempool, if an empty set is provided we mine an empty block (original behaviour of the RPC).

Note: for the moment fee collector is not implemented if a set of tx is set manually

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2893760250)
`32488cfd6c...fb12a062b8`: rebase due to conflicts

💬 Sjors commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#issuecomment-2893793850)
Trivial rebase after #32562, dropped changes in RPC code and added a comment in the test.

💬 hebasto commented on pull request "cmake: Remove `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}` from `bitcoin-build-config.h`":
(https://github.com/bitcoin/bitcoin/pull/32551#issuecomment-2893800893)
Rebased to resolve a conflict with the merged bitcoin/bitcoin#32477.

🤔 polespinasa reviewed a pull request: "rpc: Round verificationprogress to 1 for a recent tip"
(https://github.com/bitcoin/bitcoin/pull/32528#pullrequestreview-2853652269)
tACK fa53098472521de9d5b3cc42983043c240b7c321

Left a small comment

💬 polespinasa commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2097622471)
I guess `GetConsensus().nPowTargetSpacing` returns 10min?

If so, could we use this even if the block being validated is not within the 2h range? I think we could just never rely on miner-set timestamps.

👍 willcl-ark approved a pull request: "doc: add missing copyright headers"
(https://github.com/bitcoin/bitcoin/pull/31864#pullrequestreview-2853699729)
ACK c7c3bfadfc6e294547cbc0077a175845c0633906

This changeset:

- Adds copyright headers to files where they're missing
- Updates copyright headers which are missing "group" info (i.e. to whom the copyright is granted)

It does **not** update all headers with "-present". This is left for a future change.

I did a cursory check on git files to see if any had been missed using:

```bash
❯ fd -e cpp -e h -e py -E src/secp256k1 -E src/leveldb -E src/minisketch -E src/crc32 | xargs rg --pc
...

📝 hebasto opened a pull request: "subprocess: Backport upstream changes"
(https://github.com/bitcoin/bitcoin/pull/32567)
A list of the backported PRs:
- https://github.com/arun11299/cpp-subprocess/pull/119

The following PRs were skipped for backporting:
- https://github.com/arun11299/cpp-subprocess/pull/118 because there is no changes in the header code.

Required for https://github.com/bitcoin/bitcoin/pull/32566.

💬 hebasto commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32567#issuecomment-2893909163)
cc @laanwj

💬 hebasto commented on pull request "Use subprocess library for notifications":
(https://github.com/bitcoin/bitcoin/pull/32566#discussion_r2097661935)
Backported from upstream in https://github.com/bitcoin/bitcoin/pull/32567.

💬 fanquake commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32567#issuecomment-2893928418)
Can't this just go in with #32566? Why does it need it's own PR?

🤔 rkrux reviewed a pull request: "wallet: Fix logging of wallet version"
(https://github.com/bitcoin/bitcoin/pull/32553#pullrequestreview-2853743852)
ACK 4b2cd0b41ff4800c8801f2c44883eaec60a035fa

I can see the difference in the logs, certainly better to have.

**Master**
```
2025-05-20T10:31:39Z init message: Loading wallet…
2025-05-20T10:31:39Z [test] Wallet file version = 10500, last client version = 299900
2025-05-20T10:31:40Z [test] Descriptors: 13, Descriptor Keys: 1 plaintext, 0 encrypted, 1 total.
```

**PR**
```
2025-05-20T10:22:15Z init message: Loading wallet…
2025-05-20T10:22:15Z [test] Last client version = 299900
2
...

💬 hebasto commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32567#issuecomment-2893947117)
> Can't this just go in with #32566? Why does it need it's own PR?

It would be easier to track backports.

⚠️ maflcko reopened an issue: "intermittent issue in rpc_signer.py (enumeratesigners timeout)"
(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.

💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2893966294)
It even happens on gcc-14, it seems:

https://cirrus-ci.com/task/5476449961902080?logs=ci#L5506