Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 itornaza commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757372497)
The argument list is really difficult to read due to the nature of the arguments. However, I understand that it can be no better than that anyway!
👍 maflcko approved a pull request: "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake"
(https://github.com/bitcoin/bitcoin/pull/30888#pullrequestreview-2301100325)
Concept ACK, but it would be more consistent and easier to maintain to also transform the json one.

Seems a bit absurd that the regex replacment is faster than appending to a string, so it would be good to test this on Windows and Linux with the minimum and maximum supported cmake version to make sure this is really the last time this needs to be touched.
💬 maflcko commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757374195)
I think it is fine to drop comment around self-explanatory and self-documenting code
💬 maflcko commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757374567)
Same, etc.
💬 maflcko commented on issue "Closing a wallet using the fa46088440 28.x QT client segfaults":
(https://github.com/bitcoin/bitcoin/issues/30887#issuecomment-2346976210)
Looks like this may have been changed recently in https://github.com/bitcoin-core/gui/commit/e682e7db7e399ce888e492eab8f99c389aae05b5. However, I do not know if that change is related.
💬 l0rinc commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2346985310)
> Seems a bit absurd that the regex replacment is faster than appending to a string

I did some simple profiling and this regex version likely works with some mutable stringbuilder in the background, while the previous string solution was likely quadratic since it's immutable, so it was recreated after each byte.
And the file write is likely faster than that since it uses some buffering.

> also transform the json one

I'm still working on that one, but given the concept ACK, I'll do that
...
📝 maflcko opened a pull request: " log: Use ConstevalFormatString "
(https://github.com/bitcoin/bitcoin/pull/30889)
This changes all logging (including the wallet logging) to produce a
`ConstevalFormatString` at compile time, so that the format string can be
validated at compile-time.
💬 achow101 commented on pull request "test: Wait for local services to update in feature_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/30880#issuecomment-2346986883)
ACK 19f4a7c95a99162122068d4badffeea240967a65
💬 brunoerg commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#discussion_r1757397347)
Done!
💬 brunoerg commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#issuecomment-2347004914)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/30445#discussion_r1756250067
achow101 closed an issue: "ci: failure in feature_assumeutxo.py"
(https://github.com/bitcoin/bitcoin/issues/30878)
🚀 achow101 merged a pull request: "test: Wait for local services to update in feature_assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/30880)
💬 hebasto commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2347015700)
Concept ACK on increasing performance.
💬 l0rinc commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757406024)
Done
💬 l0rinc commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757406164)
Done
💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2347020039)
I recreated a CI timeout and the machine's CPU was idle flat at 0% for most of the part for 2 hours, but I didn't look where it spent so much time "doing nothing".
📝 jonatack opened a pull request: "doc: unit test runner help fixup"
(https://github.com/bitcoin/bitcoin/pull/30890)
Running `test_bitcoin --help` prints the list of arguments that may be passed, not the list of tests, so fix that.

An optional second commit improves the clarity and organization of the "Running individual tests" section.
💬 l0rinc commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2347026457)
> it would be good to test this on Windows and Linux with the minimum and maximum supported cmake version to make sure this is really the last time this needs to be touched.

While the speed difference is algorithmic, shouldn't be OS dependent, I would appreciate if someone would do that.

> would be more consistent and easier to maintain to also transform the json one

pushed
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757414433)
> The argument list is really difficult to read due to the nature of the arguments. However, I understand that it can be no better than that anyway!

To be fair, I think this could be significantly simpler now that we are using c++20, which supports [concepts](https://en.cppreference.com/w/cpp/language/constraints). In general there should be no reason to use std::enable_if anymore now that concepts are available. There's a lot of older code like this that could be switched to use concepts, an
...