Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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

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