👍 hodlinator approved a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2852896828)
ACK 989537ff4026d7c3fa5ba99701e0a4b134d950f7

Optimizing by operating on whole-word units instead of byte-for-byte makes for more efficient use of common hardware. Extracting the optimization into the `Obfuscation` abstraction which caches whole-word "rotations" (the XOR-key pre-rotated at all possible byte-offsets) makes sense.

Clear speedup in benchmarks. Even if the least improved of those measured (`ReadBlockBench`) would be the most representative of real-world use, it is sped up by ~1
...

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2097124059)
nit: Could update Godbolt-link in PR description to a newer version of `Obfuscation` with cached rotations?

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2097134481)
nit: This is more of a sanity-check only necessary for debug builds, right?
```suggestion
Assume(size <= target.size());
```
See also: #32100

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2097271922)
informational: Tried to optimize for aligned writes:
```suggestion
uint64_t rot_key{m_rotations[key_offset_bytes % SIZE_BYTES]}; // Continue obfuscation from where we left off
const size_t alignment_remaining{std::min(SIZE_BYTES - (reinterpret_cast<ptrdiff_t>(target.data()) % SIZE_BYTES), target.size())};
Xor(target, rot_key, alignment_remaining);
target = target.subspan(alignment_remaining);
rot_key = m_rotations[(key_offset_bytes + alignment_remain
...

🚀 fanquake merged a pull request: "lint: Check for missing trailing newline"
(https://github.com/bitcoin/bitcoin/pull/32477)

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