💬 willcl-ark commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2097461781)
Crikey that's an old hangover....

I ported initial requirements from my current [Dockerfile](https://github.com/willcl-ark/bitcoin-core-docker/blob/d3676df64a7e5c4b816dc5a6ca542098e5d498f8/master/alpine/Dockerfile#L15)

I foolishly assumed this was just the case of Alpine splitting packages more granularly that e.g. Ubuntu, but taking a closer look now suspect this is a hangover from pre 0.12 where we supported SSL on RPC?

Removed in 6b6819e75f6371b3dbfcc3ba8379379b6637fe31

💬 willcl-ark commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2097462455)
Thanks, dropped `libtool` and added `pkgconf`

💬 laanwj commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2097464820)
Yes, subprocess doesn't execute processes in a shell, so it has to be added explicitly if you want that.

👍 laanwj approved a pull request: "Reintroduce external signer support for Windows"
(https://github.com/bitcoin/bitcoin/pull/29868#pullrequestreview-2853417502)
Code review and lightly tested ACK 3a18075aedd7cff6f06b5fe10966d618b6378701

💬 stickies-v commented on pull request "Remove legacy `Parse(U)Int*`":
(https://github.com/bitcoin/bitcoin/pull/32520#issuecomment-2893615325)
re-ACK faf55fc80b

🤔 Sjors reviewed a pull request: "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor"
(https://github.com/bitcoin/bitcoin/pull/29136#pullrequestreview-2853333285)
Mostly happy with d457faf91e31c33063e45ee050f7480436506635

💬 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_r2097454665)
In 556fe4bc71e29ca695122adbfd8dd6c9aa0f9a9e "descriptor: Add unused(KEY) descriptor": could add a check for empty `unused()`:

```cpp
CheckUnparsable("unused()", "unused()", "No key provided");
```

💬 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_r2097409688)
In 556fe4bc71e29ca695122adbfd8dd6c9aa0f9a9e "descriptor: Add unused(KEY) descriptor": do `unused(KEY)` key descriptors have labels? If not, then it would be more clear to check this condition next to `!desc.descriptor->IsRange()`, and update the comment.

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