💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1794962653)
thanks, fixed

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1794969088)
> in [aaaa4fb](https://github.com/bitcoin/bitcoin/commit/aaaa4fb20156b4375d92e1eca4acc90a425a1896): seems more straightforward to just extend `ConstevalStringLiteral` instead of creating a new `util::Original`? This would 1) minimize the diff and 2) I find `ConstevalStringLiteral` a much more helpful name than `Original`.

Are you sure it would minimize the diff? `Original` needs to deal with the concept of translations, which seems out-of-scope for `ConstevalStringLiteral`. Having two separat
...

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1794970457)
I've renamed the type to `Translatable<bool _>`, which should clear this up without adding an enum?

The type is only supposed to be used internally, so I can also wrap it into a `detail::` if you think this is helpful?

💬 maflcko commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2404457507)
Looking at https://github.com/hodlinator/bitcoin/actions it doesn't look it happened yet. Maybe you can crank up the cron to once an hour instead of every three hours?

💬 TheCharlatan commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2404460342)
I've been reading over the changes here again and I am not sure what the purpose of using the new `processNewBlock` interface function in `submitblock` is. It just wraps the exact same call to the chainman and the chainman is still invoked directly from it. Is it ok for other external users of `processNewBlock` to skip over the pre-checks and optimizations that are normally done in `submitblock`?

Reading through this again, because the way I'd like to see most of these RPCs evolving in the f
...

💬 hodlinator commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2404460847)
Sure, will do! Thanks again for spotting the remaining mempool filter.

💬 fanquake commented on issue "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`":
(https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2404473034)
> From the issue description: "I would like to propose that we split the CI jobs into separate fuzz-only jobs that use -DBUILD_FOR_FUZZING"

I think regardless of other changes we make with any build options, we should do this in any, and have a fuzz job per Linux/Windows/macOS.

💬 maflcko commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2404482511)
What is the status/expectation of this? I presume at some point the switch will happen either way and right now it is just waiting for a good enough reason?

According to https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402990346, it may be required for qt6.8

Also, in the title "builds" should say "build"?

💬 fjahr commented on pull request "init: Correct coins db cache size setting":
(https://github.com/bitcoin/bitcoin/pull/31064#discussion_r1794993575)
The comment above needs to be edited. At least "Conservative value which is arbitrarily chosen" should make clear that it's referring to 0.2. And the reference to `MaybeRebalanceCache` should probably also be changed.

💬 fjahr commented on pull request "init: Correct coins db cache size setting":
(https://github.com/bitcoin/bitcoin/pull/31064#issuecomment-2404487485)
Concept ACK

💬 TheCharlatan commented on pull request "init: Correct coins db cache size setting":
(https://github.com/bitcoin/bitcoin/pull/31064#discussion_r1795000040)
Seemed clear to me that the comment refers to the smaller factor, but reading it again, I think you are right. Will re-word.

💬 fanquake commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2404501882)
> What is the status/expectation of this? I presume at some point the switch will happen either way and right now it is just waiting for a good enough reason?

The status is mostly that. It has been useful to push occasionally and see what breaks. If there are some new compelling reasons to switch we could consider it. I'll take it out of draft, and if anyone wants to leave (conceptual) review they can.

💬 fanquake commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2404502351)
cc @maflcko. Do you have an opinion here?

👋 fanquake's pull request is ready for review: "guix: use GCC 13 to build releases"
(https://github.com/bitcoin/bitcoin/pull/29881)

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1795043415)
> Suggested fix to address both:

Good catch. Though, I don't think the fix fixes all issues. There was a third that the `translatable` member field was unused. I've pushed a fix to fix all three issues.

💬 Sjors commented on pull request "ci: Add missing -DWERROR=ON to test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/31045#issuecomment-2404573726)
It only impacts CI, so is fine by me.

💬 fanquake commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2404575815)
> could we add a safety harness to make sure it's never being used accidentally for a live node?
> Maybe somewhere in init.cpp or bitcoind.cpp:
> Edit: I realize that bitcoind should be disabled in that case. This would be belt-and-suspenders to make sure that always holds.

Just want to note that putting something in either of these files wouldn't be enough to prevent someone producing a kernel lib with the fuzzing code.

💬 fanquake commented on pull request "build: scripted-diff: drop config/ subdir for bitcoin-config.h":
(https://github.com/bitcoin/bitcoin/pull/30937#issuecomment-2404578174)
Guix Build
```bash
a78d6d030a78972bf2ebab6d3599a3ce8b325e5422e192ab97d65966f3ee0ae4 guix-build-09b0161c4a25/output/aarch64-linux-gnu/SHA256SUMS.part
a3ff2330715466bef18e536f5ef4c6c00c9a359e11f896b3afade7364b4e40eb guix-build-09b0161c4a25/output/aarch64-linux-gnu/bitcoin-09b0161c4a25-aarch64-linux-gnu-debug.tar.gz
dcedb6cea8f7799d34d64b9a73deda40452887898a44e6b5a2048b7ebf304924 guix-build-09b0161c4a25/output/aarch64-linux-gnu/bitcoin-09b0161c4a25-aarch64-linux-gnu.tar.gz
aa2b1c6e8fc747872
...

💬 fanquake commented on pull request "build: scripted-diff: drop config/ subdir for bitcoin-config.h":
(https://github.com/bitcoin/bitcoin/pull/30937#issuecomment-2404581733)
```bash
bitcoin/src/common/netif.cpp:5:10: fatal error: 'config/bitcoin-config.h' file not found
5 | #include <config/bitcoin-config.h> // IWYU pragma: keep
| ^~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
```

💬 TheCharlatan commented on pull request "init: Correct coins db cache size setting":
(https://github.com/bitcoin/bitcoin/pull/31064#issuecomment-2404584380)
Updated e24783efe5cc17c17349b8ed96ef852e73ff8309 -> 3a4a788ee0db83d20607f14801dbed2ee932943c ([patchCoinsDBCacheSizeInit_0](https://github.com/TheCharlatan/bitcoin/tree/patchCoinsDBCacheSizeInit_0) -> [patchCoinsDBCacheSizeInit_1](https://github.com/TheCharlatan/bitcoin/tree/patchCoinsDBCacheSizeInit_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/patchCoinsDBCacheSizeInit_0..patchCoinsDBCacheSizeInit_1))

* Addressed @fjahr's [comment](https://github.com/bitcoin/bitcoin/pull/310
...