💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1801063717)
Any reason to use a different branch here? Removing the constexpr above and adding `||g_fuzzing` to the if should achieve the same? There shouldn't be a downside, because the `g_fuzzing` will always be checked anyway?
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1801063717)
Any reason to use a different branch here? Removing the constexpr above and adding `||g_fuzzing` to the if should achieve the same? There shouldn't be a downside, because the `g_fuzzing` will always be checked anyway?
✅ fanquake closed an issue: "docs: "Fuzzing the Bitcoin Core P2P layer using Honggfuzz NetDriver" is outdated"
(https://github.com/bitcoin/bitcoin/issues/30957)
(https://github.com/bitcoin/bitcoin/issues/30957)
💬 maflcko commented on pull request "refactor: Clean up messy strformat and bilingual_str usages":
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1801085975)
nit in faa34ad0a7f5271958e5313e7aae2baaed41587e: Since you include the commit here, but just as a note: The commit message isn't 100% correct. Should there be an error in the format string, the `ConstevalFormatString` fails substitution and leads to the `std::string` one being selected.
This commit (message) only becomes true after commit 4ce60d82a0c1e21ee7ecba69e4c8926720829429 from https://github.com/bitcoin/bitcoin/pull/30928. So I wonder if 4ce60d82a0c1e21ee7ecba69e4c8926720829429+faa34ad
...
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1801085975)
nit in faa34ad0a7f5271958e5313e7aae2baaed41587e: Since you include the commit here, but just as a note: The commit message isn't 100% correct. Should there be an error in the format string, the `ConstevalFormatString` fails substitution and leads to the `std::string` one being selected.
This commit (message) only becomes true after commit 4ce60d82a0c1e21ee7ecba69e4c8926720829429 from https://github.com/bitcoin/bitcoin/pull/30928. So I wonder if 4ce60d82a0c1e21ee7ecba69e4c8926720829429+faa34ad
...
💬 maflcko commented on pull request "refactor: Clean up messy strformat and bilingual_str usages":
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1801093580)
nit in 6fb39655c5cd2bebad902d271d1edae64b817d2b: Are you sure this change is correct? I'd presume this was intentionally written to denote an Untranslated string that would otherwise be translated. Alternatively, it seems like an oversight that should be translated?
My preference would be to leave this as-is for now.
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1801093580)
nit in 6fb39655c5cd2bebad902d271d1edae64b817d2b: Are you sure this change is correct? I'd presume this was intentionally written to denote an Untranslated string that would otherwise be translated. Alternatively, it seems like an oversight that should be translated?
My preference would be to leave this as-is for now.
👍 maflcko approved a pull request: "refactor: Clean up messy strformat and bilingual_str usages"
(https://github.com/bitcoin/bitcoin/pull/31072#pullrequestreview-2369229188)
lgtm. Left some comments for context.
(https://github.com/bitcoin/bitcoin/pull/31072#pullrequestreview-2369229188)
lgtm. Left some comments for context.
💬 maflcko commented on pull request "refactor: Clean up messy strformat and bilingual_str usages":
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1801097275)
fd188650f0aefced09fdb5c3d21c960791104774: Just for reference, everything in this call graph is untranslated, so if there was a translation, it would already be a bug, and this patch wouldn't fix that bug.
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1801097275)
fd188650f0aefced09fdb5c3d21c960791104774: Just for reference, everything in this call graph is untranslated, so if there was a translation, it would already be a bug, and this patch wouldn't fix that bug.
💬 maflcko commented on pull request "refactor: Clean up messy strformat and bilingual_str usages":
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1801118756)
fdd1992e44c4eb49b802aad30c12638cf92fcf8e: I am thinking that `strprintf(Untranslated("something"), ...)` is something that may be used normally. Right now, it may not be used extensively, or only used to concatenate two blobs. However, I can imagine that in the future someone may want to write `strprintf(Untranslated("<a href="...">%s</a><br>%s<br>"), ...)` to format some bilingual strings into html, which would be a bit more verbose by doing with plenty of `+`.
Not saying that this is someth
...
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1801118756)
fdd1992e44c4eb49b802aad30c12638cf92fcf8e: I am thinking that `strprintf(Untranslated("something"), ...)` is something that may be used normally. Right now, it may not be used extensively, or only used to concatenate two blobs. However, I can imagine that in the future someone may want to write `strprintf(Untranslated("<a href="...">%s</a><br>%s<br>"), ...)` to format some bilingual strings into html, which would be a bit more verbose by doing with plenty of `+`.
Not saying that this is someth
...
💬 dergoegge commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1801145478)
Will change on next push
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1801145478)
Will change on next push
💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#discussion_r1801153793)
Thanks, in case I touch it again I can address it.
(https://github.com/bitcoin/bitcoin/pull/29936#discussion_r1801153793)
Thanks, in case I touch it again I can address it.
💬 theuni commented on pull request "CI: Add label to scripted-diffs":
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2414031525)
Agree with @maflcko that this isn't adding much as-is. My suggestion was for @DrahtBot to add the label only once it had actually run the scripted-diff successfully.
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2414031525)
Agree with @maflcko that this isn't adding much as-is. My suggestion was for @DrahtBot to add the label only once it had actually run the scripted-diff successfully.
💬 achow101 commented on pull request "rpc: getdescriptorinfo also returns normalized descriptor":
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-2414064252)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-2414064252)
Are you still working on this?
✅ achow101 closed a pull request: "refactor: extract CCheckQueue's data handling into a separate container "Bag""
(https://github.com/bitcoin/bitcoin/pull/27331)
(https://github.com/bitcoin/bitcoin/pull/27331)
💬 achow101 commented on pull request "refactor: extract CCheckQueue's data handling into a separate container "Bag"":
(https://github.com/bitcoin/bitcoin/pull/27331#issuecomment-2414068322)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin/bitcoin/pull/27331#issuecomment-2414068322)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
💬 laanwj commented on pull request "chainparams: Re-add seed.bitcoinstats.com":
(https://github.com/bitcoin/bitcoin/pull/31086#issuecomment-2414068733)
It currently doesn't give me any results for `x9`, IIRC the most common filter:
```
host x9.seed.bitcoinstats.com.
```
(https://github.com/bitcoin/bitcoin/pull/31086#issuecomment-2414068733)
It currently doesn't give me any results for `x9`, IIRC the most common filter:
```
host x9.seed.bitcoinstats.com.
```
💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1801282821)
> I quickly glanced over this PR only, do I understand it correctly that we're treating benchmarks as tests here (I'm not sure I agree with that concept)
I'm not sure this is the best place for the conversation, but generally speaking, benchmarks are a form of testing. The difference is that they evaluate performance against previous runs rather than behavior.
> adding parameters to the benchmark setup that only apply to a smaller subset of the benchmarks?
This applies to every benchmar
...
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1801282821)
> I quickly glanced over this PR only, do I understand it correctly that we're treating benchmarks as tests here (I'm not sure I agree with that concept)
I'm not sure this is the best place for the conversation, but generally speaking, benchmarks are a form of testing. The difference is that they evaluate performance against previous runs rather than behavior.
> adding parameters to the benchmark setup that only apply to a smaller subset of the benchmarks?
This applies to every benchmar
...
✅ achow101 closed a pull request: "RFC: Instanced logs"
(https://github.com/bitcoin/bitcoin/pull/30338)
(https://github.com/bitcoin/bitcoin/pull/30338)
✅ achow101 closed a pull request: "wallet: Deniability API (Unilateral Transaction Meta-Privacy)"
(https://github.com/bitcoin/bitcoin/pull/27792)
(https://github.com/bitcoin/bitcoin/pull/27792)
💬 achow101 commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-2414103373)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-2414103373)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
✅ achow101 closed a pull request: "Deniability - a tool to automatically improve coin ownership privacy"
(https://github.com/bitcoin-core/gui/pull/733)
(https://github.com/bitcoin-core/gui/pull/733)
💬 achow101 commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-2414104356)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-2414104356)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
✅ achow101 closed a pull request: "validation: Replace MinBIP9WarningHeight with MinBIP9WarningStartTime"
(https://github.com/bitcoin/bitcoin/pull/27427)
(https://github.com/bitcoin/bitcoin/pull/27427)