💬 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.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814887974)
No I think this one is just "belt" -- if you look at lines 146-148 above, we're not always modifying the "child" transaction here, so sometimes it'll be a duplicate of something we already have.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814895952)
Yep you're right. I swapped the order of those two commits to avoid this problem, and I also added a test for package rbf with priority (which fails on https://github.com/bitcoin/bitcoin/commit/1c33e133db5f876f2ba6eeb28877a5dcf3f6ac04, but passes by the end of this branch).

💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435184924)
Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435190097)
> Could remove "To run release binaries on macOS, [Monterey 12](https://support.apple.com/en-us/103260) or later is required." from the pull description, now that macOS 13 is required and this is no longer relevant?

Removed.

💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435192271)
Add the https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2428995399 requirement at the same time?

🚀 fanquake merged a pull request: "build: replace custom `MAC_OSX` macro with existing `__APPLE__`"
(https://github.com/bitcoin/bitcoin/pull/29450)

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435213571)
> Add the [#31042 (comment)](https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2428995399) requirement at the same time?

Rebased on https://github.com/bitcoin/bitcoin/pull/31042.

Please review https://github.com/bitcoin/bitcoin/pull/31042 and https://github.com/bitcoin/bitcoin/pull/31147 first.

💬 hebasto commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#issuecomment-2435214195)
> > It is definitely broken in #30997.
>
> Shouldn't #30997 be based on this PR then (or at least mention in the PR description it's expected to be broken)?

[Done](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2435213571).