💬 maflcko commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402132872)
I presume this is semi-related to the "Anti"-virus software. It is possible that the software adds load on the system, which makes data corruption more likely, due to increased heat production. You could try to limit the power consumption for the machine, or limit the compute that Bitcoin Core consumes and see if it helps. However, I am not sure how to do that on Windows.
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402132872)
I presume this is semi-related to the "Anti"-virus software. It is possible that the software adds load on the system, which makes data corruption more likely, due to increased heat production. You could try to limit the power consumption for the machine, or limit the compute that Bitcoin Core consumes and see if it helps. However, I am not sure how to do that on Windows.
💬 maflcko commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2402146900)
> EDIT: I also think the new uses of .c_str() are gross, though I didn't look into why those are necessary and that is not my main objection to this PR.
I removed `tfm::format_raw(fmt.original.c_str(), ...` in https://github.com/bitcoin/bitcoin/pull/31061, where it remains just `tfm::format(fmt.original, ...`. Obviously the two pulls conflict a bit, but the conflicts are trivial to solve either way.
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2402146900)
> EDIT: I also think the new uses of .c_str() are gross, though I didn't look into why those are necessary and that is not my main objection to this PR.
I removed `tfm::format_raw(fmt.original.c_str(), ...` in https://github.com/bitcoin/bitcoin/pull/31061, where it remains just `tfm::format(fmt.original, ...`. Obviously the two pulls conflict a bit, but the conflicts are trivial to solve either way.
📝 fanquake locked a pull request: "build: Disallow building fuzz binary without -DBUILD_FOR_FUZZING"
(https://github.com/bitcoin/bitcoin/pull/31059)
This pull request addresses [issue #31057](https://github.com/bitcoin/bitcoin/issues/31057).
Build System:
- Modified `CMakeLists.txt` to prevent building the fuzz binary unless `BUILD_FOR_FUZZING` is enabled.
- Added a fatal error message if an attempt is made to build the fuzz binary without `BUILD_FOR_FUZZING`.
Building the fuzz binary without `-DBUILD_FOR_FUZZING` results in a binary that is less effective for testing because it won't crash on `Assume` statements and won't bypass
...
(https://github.com/bitcoin/bitcoin/pull/31059)
This pull request addresses [issue #31057](https://github.com/bitcoin/bitcoin/issues/31057).
Build System:
- Modified `CMakeLists.txt` to prevent building the fuzz binary unless `BUILD_FOR_FUZZING` is enabled.
- Added a fatal error message if an attempt is made to build the fuzz binary without `BUILD_FOR_FUZZING`.
Building the fuzz binary without `-DBUILD_FOR_FUZZING` results in a binary that is less effective for testing because it won't crash on `Assume` statements and won't bypass
...
📝 fanquake locked a pull request: "ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING"
(https://github.com/bitcoin/bitcoin/pull/31060)
Certainly! Here is a title and description for your pull request that you can copy and paste:
---
**Title:**
ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING
---
**Pull Request Description:**
### ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING
This pull request addresses [issue #31057](https://github.com/bitcoin/bitcoin/issues/31057) by updating the Continuous Integration (CI) workflow to disallow building the fuzz binary without `-DBUILD_FOR_FUZZING`
...
(https://github.com/bitcoin/bitcoin/pull/31060)
Certainly! Here is a title and description for your pull request that you can copy and paste:
---
**Title:**
ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING
---
**Pull Request Description:**
### ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING
This pull request addresses [issue #31057](https://github.com/bitcoin/bitcoin/issues/31057) by updating the Continuous Integration (CI) workflow to disallow building the fuzz binary without `-DBUILD_FOR_FUZZING`
...
⚠️ virtu opened an issue: "Distribute darknet node addresses via DNS seeds using AAAA records"
(https://github.com/bitcoin/bitcoin/issues/31062)
### Please describe the feature you'd like to see added.
Right now, Bitcoin Core can only receive IPv4 and IPv6 node addresses from DNS seeds. Adding support for darknet addresses would bring the advantages of using DNS seeds for node discovery to darknet nodes.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
Before TorV2 addresses were deprecated, [Bitcoin Core](https://github.com/bitcoin/bitcoin/blob/5fe6878b5f7c1c97
...
(https://github.com/bitcoin/bitcoin/issues/31062)
### Please describe the feature you'd like to see added.
Right now, Bitcoin Core can only receive IPv4 and IPv6 node addresses from DNS seeds. Adding support for darknet addresses would bring the advantages of using DNS seeds for node discovery to darknet nodes.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
Before TorV2 addresses were deprecated, [Bitcoin Core](https://github.com/bitcoin/bitcoin/blob/5fe6878b5f7c1c97
...
👍 ryanofsky approved a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2356947303)
Code review ACK fabb28f6c8f772d38917a23dfbda706292070ba1. This looks great, and implementation is very clean. 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. More details in comments below.
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2356947303)
Code review ACK fabb28f6c8f772d38917a23dfbda706292070ba1. This looks great, and implementation is very clean. 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. More details in comments below.
💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793416404)
In commit "refactor: Mark run-time Untranslated() c_str with std::string" (fa213aa88dfe9354c314a855c72ea33b3966c5c0)
I think this change is unfortunate and doesn't make sense. 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.
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793416404)
In commit "refactor: Mark run-time Untranslated() c_str with std::string" (fa213aa88dfe9354c314a855c72ea33b3966c5c0)
I think this change is unfortunate and doesn't make sense. 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.
💬 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`
(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
...
(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>)`
(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("...", ...))`
(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
...
(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.
(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
...
(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
...
(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.
(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
...
(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)
(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.
(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.
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793544349)
Thanks, added unrelated change to fix comment.