👍 Abdulkbk approved a pull request: "doc: correct typos"
(https://github.com/bitcoin/bitcoin/pull/31271#pullrequestreview-2428220989)
ACK

💬 m3dwards commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2469105561)
Pretty sure this is what you are looking for:

```diff
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index cc1d8dd905..131f350111 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -85,7 +85,7 @@ jobs:
# one for the branch push and one for the pull request.
# This can be avoided by setting SKIP_BRANCH_PUSH=true as a custom env variable
# in Github repository settings.
- if: ${{ vars.SKIP_BRANCH_PUSH != 'true' }} || github.event
...

🤔 hodlinator reviewed a pull request: "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2428114068)
Reviewed a1232973189126cfc9526713011461709685fcc8

`git range-diff master a5cad72 a123297`

The awk script in the comment in c671dd17dced0d51845dc8d2148f288c4c44ecb2 doesn't add the `'` separators...?

Care to explain the `scaling_factor` value?

`XorHistogram` claims to use 8 GB of RAM. Could be a bit much if we want to be able to also run benchmarks on low-end devices.

```diff
diff --git a/src/bench/xor.cpp b/src/bench/xor.cpp
index 2ba8b17f08..bb193c7fcf 100644
--- a/src/bench/x
...

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837215954)
```suggestion
obfuscate_key = 0; // Needed for unobfuscated Read
```

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837227061)
Here and on line 185:
```suggestion
std::vector<std::byte> xor_key_vector{8};
```

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837156020)
Let's not add bloat for this case.
```suggestion
ds.Xor(0xffffffffffffffff);
```

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837161349)
Should work regardless of endianness.
```suggestion
ds.Xor(0xff0fff0fff0fff0f);
```

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837163332)
```suggestion
const uint64_t xor_pat{0xff00ff00ff00ff00};
```

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837235546)
Care to elaborate why we don't just have the `default` statement?

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837249531)
It doesn't (at least not for all test cases), I'm deliberately testing generating vectors and converting them instead of generating 64 bit values since that's what happens in production code (this should address multiple of your comments)

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837249755)
Please see: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837249531

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837251362)
"Specify the optimization categories" means that the compiler will be able to optimize the cases where one of the parameters (the size) is a constant separately from each other. The default statement would work, but would be very slow, since the 1, 2 and 4 byte versions won't be specialized.

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837251698)
Please see: https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837249531

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2469174132)
> The awk script in the comment in https://github.com/bitcoin/bitcoin/commit/c671dd17dced0d51845dc8d2148f288c4c44ecb2 doesn't add the ' separators...?

No, I've added those manually.

> Care to explain the scaling_factor value?

The values in the histogram (i.e. total bytes streamed through xor) add up to ~92gb, but 1 byte values occupy half of that.
I had to scale down the histogram such that the lower values, having a few hundred occurrences aren't flattened out completely - to make the
...

👍 Abdulkbk approved a pull request: "doc: Fix grammatical errors in multisig-tutorial.md"
(https://github.com/bitcoin/bitcoin/pull/31225#pullrequestreview-2428262674)
ACK ac286e0d1bd4c8dbe6d21cbe8c0a131ad8ba1854

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837269742)
All ones (binary) is not endian-sensitive. IMO it's okay for the tests to look slightly different to reduce this kind of noise.

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837268749)
Dummy

💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837270655)
I want to test the setup we have in production, these tests were very useful in revealing endianness problems.

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837275100)
Feel like I'm imitating ChatGPT:
Ah, yes, I get now how the `memcpy()` interaction with endianness preserves the functioning of this specific test.

💬 hodlinator commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837279112)
How about
```suggestion
// Help optimizers along by sending constant parameter values into the inlined function,
// resulting in more efficient substitutions of memcpy() -> native pow-2 copy instructions.
switch (write.size()) {
case 0: break;
case 1: XorInt(write, key, 1); break;
case 2: XorInt(write, key, 2); break;
case 4: XorInt(write, key, 4); break;
default: XorInt(write, key, write.size());
```