💬 fanquake commented on issue "How to compile the GUI on opensuse tumbleweed with cmake?":
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2434952208)
> -DWITH_QRENCODE=OFF

Is this known to be broken / also didn't work previously?

💬 laanwj commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814763442)
It's only up to 7 iterations (assuming the key size is 8), sure, youre're right.

But ok, yea, i'm a bit divided about relying on specific non-trivial things being optimized out, makes the output very dependent on specific compiler decisions (which may be fickle in some cases).

💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814766636)
Often the simplest code gets optimized most, since it's more predictable.
Would you like me to extend the test or benchmark suite or try something else to make sure we're comfortable with the change?

💬 laanwj commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814766970)
Right, that's trivial for x86_64. Let's also check for architectures that do require alignment for 64-bit reads and writes, like RISC-V.

💬 laanwj commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814773275)
No, it's fine. It just gives me goosebumps seeing code like this, but if it doesn't affect the benchmark and no one else cares then it doesn't matter.

💬 maflcko commented on issue "How to compile the GUI on opensuse tumbleweed with cmake?":
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2434984915)
I never tried libqrencode with autotools.

💬 maflcko commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2435026697)
Is `/Volumes/SSD/Dev/` and `/Users/sjors/dev/` the same path? I don't know why, but it seems the two are mixed up. My recommendation would be to put the source code and the depends dir on the same path (one of the two) and try again.

💬 maflcko commented on pull request "test: use context managers and add file existence checks in feature_fee_estimation.py":
(https://github.com/bitcoin/bitcoin/pull/31030#issuecomment-2435052022)
Are you still working on this?

💬 laanwj commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2435060786)
> It allows for a binary response format, which is not possible with the (JSON-)RPC interface. For large queries, not having the JSON de/serialization round-trip can make a very meaningful performance difference.

Exactly. Univalue was designed to be a simple and easy to review JSON library, it wasn't designed for performance. The encoding/decoding overhead can matter.

> What is the use of REST interface that doesn't have enough endpoints?

That is a more existential question. There are
...

💬 laanwj commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2435085619)
i'm divided on this. There's a use-case, for sure, but this script is mainly used to generate the manpages for a release. And we've had exactly the opposite issue where conditional behavior resulted in incomplete manual pages.
(Prompting me to open #17506)
So not sure it should be the default. As an option, sure.

💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814847538)
Added RISC-V compiler to https://godbolt.org/z/n5rMeYeas - where it seems to my untrained eyes that it uses two separate 32 bit xors to emulate the 64 bit operation:
```asm
xor a4,a4,s0
xor a5,a5,t2
srli t6,a4,8
srli t5,a4,16
srli t4,a4,24
srli a7,a5,8
```

💬 sipa commented on issue "Remove BIP94 from regtest":
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2435106952)
For the first option, the BIP94 timewarp behavior could perhaps be turned into a (versionbits) deployment?

Then mainnet and testnet3 can have it off (by not having a deployment defined), always on on testnet4, and it would be implicitly configurable on regtest using `-vbparams`?

💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814852528)
To get rid of the goosebumps I'm handling the remaining 4 bytes as a single 32 bit xor now, so the final loop (when keys are 8 bytes long, which is mostly the case for us, I think) does 3 iterations at most. So even if it's not optimized away, we should be fine doing 3 divisions by a nice round number like 8.

💬 maflcko commented on issue "Remove BIP94 from regtest":
(https://github.com/bitcoin/bitcoin/issues/31137#issuecomment-2435118435)
Anything is probably fine here. A pre-mined testnet4 chain can possibly be used for the test as well, instead.

💬 laanwj commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2435123562)
We're kind of having the conceptual discussion over in #31065. Probably not a good idea to split it. But in retrospect it should have been done here, and sooner, before someone started working on it.

💬 maflcko commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814863916)
> divisions by a nice round number like 8.

I don't think the compiler knows the number here, so can't use it to optimize the code based on it.

💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814865983)
Is that important for max 3 divisions?

💬 fanquake commented on pull request "build: replace custom `MAC_OSX` macro with existing `__APPLE__`":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2435152474)
Guix Build:
```bash
245051dc0f18c4da7060996df1ce9eb298b449e48b62cbad30017707c94faed2 guix-build-6c6b2442edab/output/aarch64-linux-gnu/SHA256SUMS.part
55b4b8b790069826c0564c1d3372fada3c8e71dc1a172f2c360aa74c7c041ef1 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu-debug.tar.gz
133bdd966b70875c8aa6b62eed27235fc327dfeaa8e048db524ec9ccf6085602 guix-build-6c6b2442edab/output/aarch64-linux-gnu/bitcoin-6c6b2442edab-aarch64-linux-gnu.tar.gz
f34455a7dc5581bb
...

💬 fanquake commented on issue "How to compile the GUI on opensuse tumbleweed with cmake?":
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2435157929)
Even though the package is available, looks like it didn't work there either.

👍 fanquake approved a pull request: "build: replace custom `MAC_OSX` macro with existing `__APPLE__`"
(https://github.com/bitcoin/bitcoin/pull/29450#pullrequestreview-2392337705)
ACK 6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e - at this point it seems unlikely that we'll need to accomodate non-macOS Apple, so consolidating to `__APPLE__` seems ok for now.