💬 Sjors commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2097201740)
(note: I'll push this in a few hours, just in case you're up early...)

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2097218973)
Well that's what confused me. My first thought was: why does this work if we're not checking the witness?

But maybe the sigops related comment needs to go above the "only the non-witness" portion.

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2097226712)
I don't think the 3 character savings is worth making people jump to the documentation. `MAX_TX_` vs `MAX_BLOCK_` seems like a good convention to keep.

💬 Sjors commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#issuecomment-2893307461)
I was thinking ahead to when BIP54 is activated, but yes, "tightening".

💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.4":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2893315569)
> I have a local manifest which builds 0.16.3 lief: https://gist.github.com/willcl-ark/3ac07722025b696910adba256d813cb6

I was using basically this manifest taken from your branch (https://github.com/willcl-ark/bitcoin/tree/bump-lief), but it seems some build changes were made in lief `0.16.5` that prevent this from working, I've rebased, bumping to `0.16.4`

👋 davidgumberg's pull request is ready for review: "deps: Bump lief to 0.16.4"
(https://github.com/bitcoin/bitcoin/pull/32431)

💬 laanwj commented on pull request "Set minimum supported Windows version to 1903 (May 2019 Update)":
(https://github.com/bitcoin/bitcoin/pull/32537#discussion_r2097253630)
Imo, no need to do this as a separate step. Might as well roll the update to the release notes template into #32380.

💬 laanwj commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2097259314)
Can we maybe check the codepage early at startup (e.g. `SetupEnvironment()`) and fail if it isn't correct? This seems more thorough than any version check.

👍 fanquake approved a pull request: "lint: Check for missing trailing newline"
(https://github.com/bitcoin/bitcoin/pull/32477#pullrequestreview-2853191950)
ACK fa9198af55df74b0c19c9125d256ad4df83cf005

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