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

💬 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_r1837281249)
[My bad](https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1837275100).

💬 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_r1837293159)
I ended up with only `// Help the compiler specialize 1, 2 and 4 byte cases`, since the rest was just speculation

💬 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_r1837293203)
Now I have to rename the PR :D

💬 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_r1837293258)
Done

💬 gmart7t2 commented on issue "importdescriptors always rescans":
(https://github.com/bitcoin/bitcoin/issues/31263#issuecomment-2469280887)
Even when the earliest timestamp specified is a week in the future it still rescans blocks from 2 hours ago.

That goes against what the documentation string says.

💬 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#issuecomment-2469297214)
> > 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 histogram still representative. Do you have a better idea?

So `raw_histogram` really is the raw data?

Added:
```C++
uint64_t raw_total_bytes{0};
for (size_t i{0}; i
...

💬 achow101 commented on issue "importdescriptors always rescans":
(https://github.com/bitcoin/bitcoin/issues/31263#issuecomment-2469315807)
It doesn't make sense for a user to import descriptors with a future timestamp. If the timestamp is greater than "now", then the timestamp for rescanning will be "now".

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1837356425)
What do you mean by "accurate"? Is the concern that someone scales this by <0.5 and then the test might fail?