💬 sipa commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2450100260)
@ryanofsky Yeah, the context here is exactly `Assume()` calls where the conditions is easily shown to be side-effect free by the compiler. I have been using those fairly liberally (inside production code), because it means free extra checks in fuzzing tests that have no runtime cost, but are simultaneously definitely not going to result in different behavior between fuzz tests and production code (because it's the compiler deciding the condition is side-effect free).
I think these are very us
...
  (https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2450100260)
@ryanofsky Yeah, the context here is exactly `Assume()` calls where the conditions is easily shown to be side-effect free by the compiler. I have been using those fairly liberally (inside production code), because it means free extra checks in fuzzing tests that have no runtime cost, but are simultaneously definitely not going to result in different behavior between fuzz tests and production code (because it's the compiler deciding the condition is side-effect free).
I think these are very us
...
💬 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_r1824627942)
I've added the fills to make sure we're not using them after conversion anymore.
What would be the advantage of the static asserts?
I don't mind removing these failsafes if you think they're redundant or noisy.
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824627942)
I've added the fills to make sure we're not using them after conversion anymore.
What would be the advantage of the static asserts?
I don't mind removing these failsafes if you think they're redundant or noisy.
💬 polespinasa commented on issue "Warn about / refuse unsupported clang version":
(https://github.com/bitcoin/bitcoin/issues/30947#issuecomment-2450106148)
I'm not able to run fuzzing tests also because of this:
```
$ cmake --build build_fuzz
....
/bitcoin/src/uint256.h:133:19: error: call to consteval function 'util::ConstevalHexDigit' is not a constant expression
auto lo = util::ConstevalHexDigit(*(str_it++));
```
Which clang version are we suposed to use?
```
$ clang --version
Ubuntu clang version 14.0.0-1ubuntu1.1
```
  (https://github.com/bitcoin/bitcoin/issues/30947#issuecomment-2450106148)
I'm not able to run fuzzing tests also because of this:
```
$ cmake --build build_fuzz
....
/bitcoin/src/uint256.h:133:19: error: call to consteval function 'util::ConstevalHexDigit' is not a constant expression
auto lo = util::ConstevalHexDigit(*(str_it++));
```
Which clang version are we suposed to use?
```
$ clang --version
Ubuntu clang version 14.0.0-1ubuntu1.1
```
💬 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_r1824630954)
yes
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824630954)
yes
💬 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_r1824631355)
it's removed in the next commit
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824631355)
it's removed in the next commit
💬 fanquake commented on issue "Warn about / refuse unsupported clang version":
(https://github.com/bitcoin/bitcoin/issues/30947#issuecomment-2450109402)
> Which clang version are we suposed to use?
https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md
  (https://github.com/bitcoin/bitcoin/issues/30947#issuecomment-2450109402)
> Which clang version are we suposed to use?
https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md
💬 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_r1824645305)
> This pattern has been [used before](https://github.com/bitcoin/bitcoin/blob/master/src/coins.cpp#L64)
In the example you give, the variable is used in a more "positive" sense. Here we test the negated value, which is part of what makes it jarring.
One could maybe improve readability by moving the negation and renaming:
```suggestion
if (bool unknown_key = !Read(OBFUSCATE_KEY_KEY, obfuscate_key_vector); unknown_key && params.obfuscate && IsEmpty()) {
```
> I meant to narrow the
...
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824645305)
> This pattern has been [used before](https://github.com/bitcoin/bitcoin/blob/master/src/coins.cpp#L64)
In the example you give, the variable is used in a more "positive" sense. Here we test the negated value, which is part of what makes it jarring.
One could maybe improve readability by moving the negation and renaming:
```suggestion
if (bool unknown_key = !Read(OBFUSCATE_KEY_KEY, obfuscate_key_vector); unknown_key && params.obfuscate && IsEmpty()) {
```
> I meant to narrow the
...
💬 maflcko commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2450130161)
> so unless the compiler knows `<check expression>` does not have side effects, must execute it and can't optimize it out?
Correct, but the that hasn't been changed by `g_fuzzing` at all. It is not the execution of `<check expression>`, but the execution of the negation of it, and the corresponding expensive branch in hot paths which has to be emitted for the unlikely case that the negation evaluates to true.
  (https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2450130161)
> so unless the compiler knows `<check expression>` does not have side effects, must execute it and can't optimize it out?
Correct, but the that hasn't been changed by `g_fuzzing` at all. It is not the execution of `<check expression>`, but the execution of the negation of it, and the corresponding expensive branch in hot paths which has to be emitted for the unlikely case that the negation evaluates to true.
💬 fanquake commented on pull request "depends: For mingw cross compile use -gcc-posix to prevent library conflict":
(https://github.com/bitcoin/bitcoin/pull/31013#issuecomment-2450135287)
Backporting to 28.x in #31104 for consistency.
  (https://github.com/bitcoin/bitcoin/pull/31013#issuecomment-2450135287)
Backporting to 28.x in #31104 for consistency.
💬 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_r1824651933)
You're right, I've used `xor_key` in the same case before
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824651933)
You're right, I've used `xor_key` in the same case before
💬 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_r1824652007)
But done anyway
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824652007)
But done anyway
💬 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_r1824652156)
Removed, thanks
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824652156)
Removed, thanks
💬 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_r1824652216)
Simplified, thanks
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824652216)
Simplified, thanks
💬 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_r1824652442)
Done, thanks
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824652442)
Done, thanks
💬 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_r1824655597)
Removed in the end, not that important
  (https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1824655597)
Removed in the end, not that important
💬 sipa commented on issue "bench: `linearizeoptimallyexample11` benchmark now running 4x slow than previously":
(https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2450144705)
> Correct, but the that hasn't been changed by `g_fuzzing` at all.
Ah, right, it's possible that the compiler can turn `Assume(<expr>);` into `if (g_fuzzing) { bool x = <expr>; if (!x) { ... } }`, where `<expr>` is not evaluated at all if `g_fuzzing` is false. I was operating under the assumption that with the emitted branch, `<expr>` would always be evaluated even if side-effect free, but that is indeed not the case.
Still, just the fact that the `g_fuzzing` condition is there means a con
...
  (https://github.com/bitcoin/bitcoin/issues/31178#issuecomment-2450144705)
> Correct, but the that hasn't been changed by `g_fuzzing` at all.
Ah, right, it's possible that the compiler can turn `Assume(<expr>);` into `if (g_fuzzing) { bool x = <expr>; if (!x) { ... } }`, where `<expr>` is not evaluated at all if `g_fuzzing` is false. I was operating under the assumption that with the emitted branch, `<expr>` would always be evaluated even if side-effect free, but that is indeed not the case.
Still, just the fact that the `g_fuzzing` condition is there means a con
...
⚠️ darosior opened an issue: "(Past issue) On Windows, pruned nodes could crash while deleting a block file"
(https://github.com/bitcoin/bitcoin/issues/31193)
This was reported by @andrewtoth to the security list on 2022-12-20 and fixed in #26533. The security team at the time determined it was indeed a bug but not a vulnerability.
I'm opening this issue in public in order to leave a trace of this bug, which as far as i know wasn't public yet.
Original email from Andrew Toth on 2022-12-20:
> Hello,
>
> I have discovered a bug that can cause pruned nodes running on Windows to crash. Keeping an open file handle to a blk*.dat file on Windows c
...
  (https://github.com/bitcoin/bitcoin/issues/31193)
This was reported by @andrewtoth to the security list on 2022-12-20 and fixed in #26533. The security team at the time determined it was indeed a bug but not a vulnerability.
I'm opening this issue in public in order to leave a trace of this bug, which as far as i know wasn't public yet.
Original email from Andrew Toth on 2022-12-20:
> Hello,
>
> I have discovered a bug that can cause pruned nodes running on Windows to crash. Keeping an open file handle to a blk*.dat file on Windows c
...
✅ darosior closed an issue: "(Past issue) On Windows, pruned nodes could crash while deleting a block file"
(https://github.com/bitcoin/bitcoin/issues/31193)
  (https://github.com/bitcoin/bitcoin/issues/31193)
💬 TheCharlatan commented on pull request "kernel, refactor: return error status on all fatal errors":
(https://github.com/bitcoin/bitcoin/pull/29700#issuecomment-2450150408)
Can you rebase this?
  (https://github.com/bitcoin/bitcoin/pull/29700#issuecomment-2450150408)
Can you rebase this?
⚠️ pinheadmz opened an issue: "Remove libevent as a dependency (HTTP / cli / torcontrol)"
(https://github.com/bitcoin/bitcoin/issues/31194)
## Immediate advantages
### UNIX domain sockets for JSON-RPC
- Old feature request: https://github.com/bitcoin/bitcoin/issues/5029
- UNIX sockets have been added to [ZMQ](https://github.com/bitcoin/bitcoin/pull/27679) and [Tor proxy](https://github.com/bitcoin/bitcoin/pull/27375) already
- UNIX sockets with libevent [doesn't work](https://github.com/bitcoin/bitcoin/issues/5029#issuecomment-2203469108)
### Remove a dependency
- libevent hasn't had a release since 2020
- The [xz utils back
...
  (https://github.com/bitcoin/bitcoin/issues/31194)
## Immediate advantages
### UNIX domain sockets for JSON-RPC
- Old feature request: https://github.com/bitcoin/bitcoin/issues/5029
- UNIX sockets have been added to [ZMQ](https://github.com/bitcoin/bitcoin/pull/27679) and [Tor proxy](https://github.com/bitcoin/bitcoin/pull/27375) already
- UNIX sockets with libevent [doesn't work](https://github.com/bitcoin/bitcoin/issues/5029#issuecomment-2203469108)
### Remove a dependency
- libevent hasn't had a release since 2020
- The [xz utils back
...
