🚀 fanquake merged a pull request: "doc: remove // for ... comments"
(https://github.com/bitcoin/bitcoin/pull/32562)

👍 fanquake approved a pull request: "ci: Move DEBUG=1 to centos task"
(https://github.com/bitcoin/bitcoin/pull/32560#pullrequestreview-2853211572)
ACK fa58d6cdab000df288501db4a71487804b08ba4b

✅ fanquake closed an issue: "intermittent issue in rpc_signer.py (enumeratesigners timeout)"
(https://github.com/bitcoin/bitcoin/issues/32524)

🚀 fanquake merged a pull request: "ci: Move DEBUG=1 to centos task"
(https://github.com/bitcoin/bitcoin/pull/32560)

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2097334035)
One more thing - while I was testing this out, I couldn't find a unit test that tested obfuscating a target buffer starting at different offsets in memory. Might be good to add one?

💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2893474523)
i think `_GNU_SOURCE` is a bit of a red herring here. The point here is that modern versions of g++/glibc on Linux set `_POSIX_SOURCE` high enough by default to be able to use `posix_fallocate` out of the box, so there's no more reason to set it explicitly.

💬 hebasto commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2893508935)
> i think `_GNU_SOURCE` is a bit of a red herring here. The point here is that modern versions of g++/glibc on Linux set `_POSIX_SOURCE` high enough by default to be able to use `posix_fallocate` out of the box, so there's no more reason to set it explicitly.

Then maybe remove `_GNU_SOURCE` mentioning from the PR description?

💬 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)
```