💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793403826)
In commit "move-only: Detail_CheckNumFormatSpecifiers and G_TRANSLATION_FUN" (fa6cef7cf762a33755697dff20b75a9f235e96a9)

I think it would make more sense to call this function `util::detail::CheckNumFormatSpecifiers` instead of `util::Detail_CheckNumFormatSpecifiers`

💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793398674)
In commit "refactor: Pick translated string after format" (fa22616839903369757069d50aa8f55e246b2b6c)

Commit message says this change is "required for a future commit" but that makes it sound this is because of some implementation detail, when I think the actual reason is that strprintf can't check format string at compile time if it is passed a runtime std::string value.

Unless I'm missing something would be clearer if commit message just said this passes return value of the underscore fun
...

💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793408147)
In commit "move-only: Detail_CheckNumFormatSpecifiers and G_TRANSLATION_FUN" (fa6cef7cf762a33755697dff20b75a9f235e96a9)

Is this comment accurate? If so it seems like if is a more obvious approach to avoiding -Wunused errors would be to write `(void)fmt;` or just drop the `fmt` variable and declare the function as `void PassFmt(util::ConstevalFormatString<NumArgs>)`

💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793462300)
In commit "refactor: Delay translation of Untranslated() or _() literals" (aaaa4fb20156b4375d92e1eca4acc90a425a1896)

Should be no need for this if take earlier suggestion to replace `strprintf(Untranslated("..."), ...)` with `Untranslated(strprintf("...", ...))`

💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793459318)
In commit "refactor: Delay translation of Untranslated() or _() literals" (aaaa4fb20156b4375d92e1eca4acc90a425a1896)

Name `Original` is vague. Would suggest renaming `Original` something like `bilingual_literal` or `bilingual_const` to be consistent with `bilingual_str` and `bilingual_fmt` since it represents a bilingual literal or constant string which is the return value of the underscore function.

I also think this can just be a normal non-template struct, and `bool translatable` templa
...

💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793440780)
In commit "refactor: Delay translation of Untranslated() or _() literals" (aaaa4fb20156b4375d92e1eca4acc90a425a1896)

Looks like these changes would also not be necessary if using earlier suggestion to replace `strprintf(Untranslated("..."), ...)` with `Untranslated(strprintf("...", ...))` so `Untranslated` definition does not have to change.

💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793473728)
In commit "refactor: Delay translation of Untranslated() or _() literals" (aaaa4fb20156b4375d92e1eca4acc90a425a1896)

I think it confuses and complicates the design for `bilingual_str` to be aware of `Original` since `bilingual_str` is just a simple, low-level pair of strings that shouldn't know about formatting.

I think we do need return value of underscore function to be implicitly convertible to `bilingual_str` but that would be more cleanly implemented by adding an `operator bilingual_s
...

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#issuecomment-2402335504)
> However, I think it could be simplified significantly if there were a scripted diff commit to replace `strprintf(Untranslated("..."), ...)` with `Untranslated(strprintf("...", ...))`, which would be a good change on its own for clarity and consistency.

Are you sure? The two are not equivalent, because args can also be a `bilingual_str`, in which case the scripted-diff does not compile. In any case, it seems unrelated to this pull request and I don't see how it "simplifies" stuff and it touc
...

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793530260)
I don't think this is possible with a simple diff touching only lines that are already touched in this pull request. If you disagree, I am happy to accept any commit that compiles. I am also happy to review any pull request that achieves this and is split up from this pull request.

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793533834)
> It seems like it could be a avoided with a small scripted diff to replace `strprintf(Untranslated("..."), ...)` with `Untranslated(strprintf("...", ...))` and that replacement would make the code clearer and more consistent anyway.

Are you sure? The two are not equivalent, because args can also be a `bilingual_str`, in which case the scripted-diff does not compile. In any case, it seems unrelated to this pull request and I don't see how it "simplifies" stuff and it touches different lines a
...

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

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

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793544349)
Thanks, added unrelated change to fix comment.

💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2402403167)
review ping @maflcko

💬 Xaspr commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402454601)
Ah yes, I'll look into whether the lower load does anything.

Just to be complete on what happened. The IBD continued after restarting Core again, but after an eventual third shutdown and restart of Core to test what would happen, the block database was corrupted again.

```
2024-10-09T13:52:42Z UpdateTip: new best=00000000000000000878cd8de3dccda91cdcf6ef0e6e8536120c65f487dd0197 height=390296 version=0x00000004 log2_work=83.803743 tx=100238753 date='2015-12-26T14:25:06Z' progress=0.09181
...

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402462147)
@maflcko

How do I clean depends sources cache on the Cirrus CI for this PR?

💬 ajtowns commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#discussion_r1793618993)
Added to the inquisition PR in https://github.com/bitcoin-inquisition/bitcoin/pull/68

💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402496447)
They'd have to be cleared on all machines. Let me known if you want that to happen. If you are just looking for a temporary workaround for testing, you can pick a different qt version, like `6.7.1`. Or you can temporarily use a new sources volume, similar to the diff in https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2337816419.

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793642022)
> Also (not sure about this, but) it might be nice to add an explicit conversion method to Original class, so we could write something like `_("Error opening block database").str()` instead of `bilingual_str{_("Error opening block database")}`

Sure, I am happy to add `bilingual_str str()` and `operator bilingual_str()` to the `Original` type and remove the constructors from `bilingual_str`. However, this means that calling `bilingual_str{Original{}}` is now forbidden and one would have to use
...

👍 maflcko approved a pull request: "fuzz: wallet: add target for `CreateTransaction`"
(https://github.com/bitcoin/bitcoin/pull/29936#pullrequestreview-2357490000)
ACK c495731a316d9c97ee05a08cf5087c5535f84bd4 🏻

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK c495731a316d9c97ee05a08cf5
...