💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1814701175)
I don't see it. I still see `else if (g_fuzzing && !val) abort();`. Thus, I don't see any `||g_fuzzing`. Also, the `std::abort` wasn't removed nor the "duplicate" val check.

💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1814702785)
If you want to keep them for some reason, it would be good to explain, and then also add the missing includes.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814706398)
Agreed, I have restored this argument where `LimitMempoolSize()` is invoked in `AcceptSingleTransaction()`, and I pulled this change out into its own commit so that it's easier to review and less tangled with the other changes.

More thoughts on this [here](https://github.com/bitcoin/bitcoin/pull/28251#issuecomment-2433501732). It would be nice to have a test that packages which involve a parent transaction that would be just at the bottom of the mempool without their children are able to mak
...

📝 maflcko opened a pull request: "ci: Temporary workaround for old CCACHE_DIR cirrus env"
(https://github.com/bitcoin/bitcoin/pull/31146)
On a CI re-run, the historic env vars and CI config is used from Cirrus. However, the most recent CI config and CI scripts from this repo are used. This may lead to issues.

For example, `CCACHE_DIR` in the old location may be missing on new CI workers and lead to errors.

Fix it, by falling back to the old logic when the old `CCACHE_DIR` was detected.

💬 dergoegge commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1814722191)
I misunderstood what you meant, slow start today😴 should be fixed now!

📝 hebasto opened a pull request: "cmake, qt, test: Remove problematic code"
(https://github.com/bitcoin/bitcoin/pull/31147)
The removed code aimed to make Qt's minimal integration plugin DLL available for `test_bitcoin-qt.exe` on Windows.

However, there are two issues:
1. The code is broken because the destination directory must end with a trailing slash (`/`).
2. It is unnecessary because Qt's minimal integration plugin is not used on Windows. For more details, please refer to the following code:https://github.com/bitcoin/bitcoin/blob/fb46d57d4e7263495c007f4a499a349bff2b21e0/src/qt/test/CMakeLists.txt#L38-L44

...

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1814722910)
Please see https://github.com/bitcoin/bitcoin/pull/31147.

💬 l0rinc commented on pull request "build: replace custom `MAC_OSX` macro with existing `__APPLE__`":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2434907274)
> @l0rinc can you rebase this?

Rebased, checked for leftover `MAC_OSX` (found none)

✅ fanquake closed an issue: "Support transaction broadcast in REST interface"
(https://github.com/bitcoin/bitcoin/issues/31017)

💬 fanquake commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2434913714)
Closing for now, given the PR was closed. Feel free to discuss further. cc @laanwj @0xB10C

💬 l0rinc commented on pull request "doc: Extend developer-notes with file-name-only debugging fix":
(https://github.com/bitcoin/bitcoin/pull/30670#issuecomment-2434915226)
Rebased to avoid unrelated build failure

💬 maflcko commented on pull request "doc: Extend developer-notes with file-name-only debugging fix":
(https://github.com/bitcoin/bitcoin/pull/30670#issuecomment-2434920707)
It is also possible to simply re-run the CI, which wouldn't invalidate review.

💬 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_r1814734979)
i have a hard time believing this will make a large difference, especially with the two memcpys involved
On modern CPUs, ALU operations (especially bitwise ones) are so fast compared to any kind of memory access.
And this isn't some advanced crypto math, it's *one* Xor operation per word with a fixed key.

Could avoid the memcpys if the code takes memory alignment into account, but that makes it even more complex. Not sure the pros/cons work out here.

💬 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_r1814736038)
Please avoid % (especially with a dynamic value) in an inner loop. It's essentially a division operation and those are not cheap.

💬 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_r1814739738)
Thanks for the hint, I deliberately removed that optimization (please check the commit messages for details), since these are optimized away.
Also, this is just the leftover part, so for key of length 8 (the version used in most places) this will have 7 iterations at most.
Can you see any difference with any of the benchmarks?

💬 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_r1814741753)
The speedup comes from the vectorized operations, i.e. doing 64 bit xor instead of byte-by-byte xor (memcpy seems to be eliminated on 64 bit architectures successfully), see https://godbolt.org/z/Koscjconz

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