Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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?
📝 Talej opened a pull request: "doc: corrected listunspent rpc quoting"
(https://github.com/bitcoin/bitcoin/pull/31275)
bitcoin-cli help lockunspent RPC example shows:

```
As a JSON-RPC call
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "lockunspent", "params": [false, "[{\"txid\":\"a08e6907dbbd3d809776dbfc5d82e371b764ed838b5655e72f463568df1aadf0\",\"vout\":1}]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
```
Including quotes around the transactions argument. Using the RPC in this way fails:

```
curl --data-binary '{"jsonrpc": "1.0", "id": "curltest
...
👋 Talej's pull request is ready for review: "doc: corrected listunspent rpc quoting"
(https://github.com/bitcoin/bitcoin/pull/31275)