💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756569850)
The goal is not to create a full POSIX-validator here, so there will always be an example where something is "invalid". The goal is to catch tinyformat-invalid *and* POSIX-invalid concerning the argument count.

Apart from that, I don't think it makes sense to spend a massive amount of time on format strings that are POSIX-invalid, but currently happen to be tinyformat-valid. No one should be using them, but if someone uses them it won't cause a runtime error but a harmless formatting error (o
...

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756570344)
Closing for now

🚀 fanquake merged a pull request: "test: Drop no longer needed workarounds"
(https://github.com/bitcoin/bitcoin/pull/30847)

🚀 fanquake merged a pull request: "security-check: test for `_FORTIFY_SOURCE` usage in release binaries"
(https://github.com/bitcoin/bitcoin/pull/27038)

🚀 fanquake merged a pull request: "build: Add more cmake presets"
(https://github.com/bitcoin/bitcoin/pull/30871)

💬 maflcko commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756594931)
Thanks, adjusted commit message.

💬 maflcko commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756595040)
Thanks, adjusted commit message.

💬 maflcko commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756595333)
Yes.

I do not know. I presume this is just the initial value that was set when there were no debug builds or builds with sanitizers (which are larger).

💬 maflcko commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756595452)
Nah, it was just copy-pasted from above. Fixed the style here.

💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#issuecomment-2345968106)
utACK 2222b44d35c092b5aaefed12bc5ca608e6e78760

💬 fanquake commented on pull request "test: remove unused src_dir param from run_tests after CMake migration":
(https://github.com/bitcoin/bitcoin/pull/30733#issuecomment-2345997775)
@davidgumberg fyi you've ack'd the previous commit has here.

🚀 fanquake merged a pull request: "test: remove unused src_dir param from run_tests after CMake migration"
(https://github.com/bitcoin/bitcoin/pull/30733)

💬 fanquake commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2346016603)
Could you rebase this, now that guix builds are reproducible again.

💬 l0rinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1756663411)
It seems to me that production code never actually calls `AddFlags` with `0` value - and we're always setting `DIRTY`, even when `FRESH` is set.
So it would make sense to change `AddFlags` from having a custom `uint8_t flags` parameter which is always `DIRTY` (and sometimes `FRESH`) in reality (and is only called with `0` flag in tests) to have a freshness parameter like `inline void SetDirtyAndFresh(CoinsCachePair& self, CoinsCachePair& sentinel, bool is_fresh = false) noexcept` instead.
This
...

💬 hebasto commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1754576656)
> I think I'm missing something here. Isn't it a bug that us already setting `SECP256K1_BUILD_CTIME_TESTS=OFF` is sometimes overridden later on? Otherwise, this change is unclear, because the ctime tests (along with the other secp256k1 tests) are already supposedly controlled by `BUILD_TESTS`, but now the ctime tests are being wrapped in another `BUILD_TESTS` conditional? (adding a comment would be good, otherwise I assume anyone reading this code will assume the second conditional is redundant)
...

💬 willcl-ark commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2346027116)
> There are a bunch of places where `CCACHE_MAXSIZE` is hardcoded, making it impossible to set a larger size. This needs to be fixed first, so I did that in #30869 (among other stuff).

Awesome!

> Assuming .5GB of caches per task, 10 tasks per push, 100 pushes per day, and a cache duration of 20 days, means that 10TB of storage should be sufficient.

Not sure I understand this; the idea is to have "one ccache per task" surely, not one per push?

I was thinking we should set up with:

...

💬 hebasto commented on issue "GUI event loop should be block free":
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2346033798)
> What is the status of the QML GUI? IIRC it was put on hold, because the Android APK in depends couldn't be updated for some reason, but I may be misremembering.

The development continues, but the code base is not synced with this repository until re-introducing ability to build for Android.

💬 fanquake commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756673070)
I'm not sure, because in the case being fixed here (`-DBUILD_FOR_FUZZING`), `BUILD_TESTS=OFF`, so we aren't forcing it to `ON`?

It also doesn't explain why our setting (of `OFF`) is being overriden later. The same thing doesn't happen if you pass `SECP256K1_BUILD_CTIME_TESTS=OFF` when invoking cmake.

💬 theStack commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2346043306)
> Could you rebase this, now that guix builds are reproducible again.

Sure, done.

💬 hebasto commented on pull request "build: Skip secp256k1 ctime tests when tests are not being built":
(https://github.com/bitcoin/bitcoin/pull/30865#discussion_r1756681136)
> I'm not sure, because in the case being fixed here (`-DBUILD_FOR_FUZZING`), `BUILD_TESTS=OFF`, so we aren't forcing it to `ON`?

We cannot force it to `ON` because the build will requires the installed Valgrind, which is not always the case.

> It also doesn't explain why our setting (of `OFF`) is being overriden later.

When?