💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1794934340)
> Special treatment is needed for every native package that uses CMake. For now, this applies only to Qt.

So this will also apply to libmultiprocess. I guess it's currently not a issue?, because we don't build it in Guix, however, that may change soon, #30975, so rather than adding Qt workarounds here, and then having to either add the same workarounds for libmultiprocess, or fix things in a more fundamental way later, it may still be better to make those changes now.

> By default, CMake
...

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1794944263)
Thanks, replaced my commit with your 71d382650b5a0bfb21085e8f6bd22152d2148459

💬 fanquake commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2404417309)
https://github.com/bitcoin/bitcoin/actions/runs/11265108219/job/31326367933#step:12:228

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1794955960)
> But I think those cases are basically broken because they are substituting non-english string fragments inside english strings.

I am not sure I agree. `Untranslated("%s:%i ")` doesn't look English to me. To me, it looks like `Untranslated()` is sometimes used to annotate format strings that are valid in any language and do not need to be translated.

And your fix doesn't prevent non-English fragments in English strings anyway, because you are using `operator+` to concatenate the English/`
...

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1794960822)
Ok, renamed to `Translatable<bool _>`. I don't think `bilingual_*` makes sense, because it is only an internally used type to hold the hand of the compiler. The type is supposed to convert into a real `bilingual_str` either directly or via `bilingual_fmt` and tfm.

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1794962369)
> Yes I do think that's better and basically made the same change in my branch, though without the str() method since it seems it makes it a little less clear what return type is.

Ok, pushed my diff above, with the added `str()` removed again.

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