💬 dergoegge commented on issue "docs: "Fuzzing the Bitcoin Core P2P layer using Honggfuzz NetDriver" is outdated":
(https://github.com/bitcoin/bitcoin/issues/30957#issuecomment-2382608414)
How about using `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` for these checks? Then we wouldn't have to maintain a patch in the docs.
(https://github.com/bitcoin/bitcoin/issues/30957#issuecomment-2382608414)
How about using `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` for these checks? Then we wouldn't have to maintain a patch in the docs.
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2382630708)
Force push: auto-squashed the fixup commit.
> In this PR, natpmp=1 controls both PCP and NAT-PMP.
Yes, after initial discussion we decided to add no new command line, or UI options. PCP is regarded a newer version of NATPMP (which is strictly true) that has some extra mapping features as well as IPv6 support.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2382630708)
Force push: auto-squashed the fixup commit.
> In this PR, natpmp=1 controls both PCP and NAT-PMP.
Yes, after initial discussion we decided to add no new command line, or UI options. PCP is regarded a newer version of NATPMP (which is strictly true) that has some extra mapping features as well as IPv6 support.
💬 stickies-v 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#discussion_r1780764321)
I think `BOOST_TEST_CONTEXT` would simplify this whole block quite nicely, but it's out of scope for this PR so I'm going not going to push that here. I'll update l184 with a `strprintf` as per your suggestion.
<details>
<summary>git diff on 2d4f82d590</summary>
```diff
diff --git a/src/test/txrequest_tests.cpp b/src/test/txrequest_tests.cpp
index a6c4a3613d..e70d9aa826 100644
--- a/src/test/txrequest_tests.cpp
+++ b/src/test/txrequest_tests.cpp
@@ -164,7 +164,6 @@ public:
...
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780764321)
I think `BOOST_TEST_CONTEXT` would simplify this whole block quite nicely, but it's out of scope for this PR so I'm going not going to push that here. I'll update l184 with a `strprintf` as per your suggestion.
<details>
<summary>git diff on 2d4f82d590</summary>
```diff
diff --git a/src/test/txrequest_tests.cpp b/src/test/txrequest_tests.cpp
index a6c4a3613d..e70d9aa826 100644
--- a/src/test/txrequest_tests.cpp
+++ b/src/test/txrequest_tests.cpp
@@ -164,7 +164,6 @@ public:
...
💬 maflcko commented on pull request "log: Enforce trailing newline at compile time":
(https://github.com/bitcoin/bitcoin/pull/30929#issuecomment-2382647693)
Force pushed some trivial style-only test-only fixups.
(https://github.com/bitcoin/bitcoin/pull/30929#issuecomment-2382647693)
Force pushed some trivial style-only test-only fixups.
💬 stickies-v 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#discussion_r1780781699)
I don't think this is an improvement, I find it less clear about the distinction between the compile-time format string and the run-time string parameters, so I'm gong to leave this as is for now.
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780781699)
I don't think this is an improvement, I find it less clear about the distinction between the compile-time format string and the run-time string parameters, so I'm gong to leave this as is for now.
💬 stickies-v 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#discussion_r1780784357)
That seems unrelated to this PR though?
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780784357)
That seems unrelated to this PR though?
💬 stickies-v 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#discussion_r1780791324)
Looks like this is already covered in #30999, resolving this.
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780791324)
Looks like this is already covered in #30999, resolving this.
💬 stickies-v 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#discussion_r1780792138)
Resolving since it has its own PR now.
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780792138)
Resolving since it has its own PR now.
💬 stickies-v 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#discussion_r1780793262)
I suppose we don't, but that seems unrelated to this PR so going to leave that as is.
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780793262)
I suppose we don't, but that seems unrelated to this PR so going to leave that as is.
💬 stickies-v 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#discussion_r1780796251)
Addressed in #30999, resolving.
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780796251)
Addressed in #30999, resolving.
💬 maflcko commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1780802968)
Added commit fa3ad1da70 bugfix: Mark m_tip_block_cv as guarded by m_tip_block_mutex
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1780802968)
Added commit fa3ad1da70 bugfix: Mark m_tip_block_cv as guarded by m_tip_block_mutex
💬 maflcko commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1780804362)
> documentation
Updated the `m_tip_block` docs, but let me know if this should be moved to the `waitTipChanged` docs instead.
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1780804362)
> documentation
Updated the `m_tip_block` docs, but let me know if this should be moved to the `waitTipChanged` docs instead.
💬 stickies-v 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-2382717634)
Force-pushed to address two nits from @l0rinc - thanks for the review!
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2382717634)
Force-pushed to address two nits from @l0rinc - thanks for the review!
💬 l0rinc 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#discussion_r1780827733)
The comment states:
```
// Avoid triggering the following crash bug:
// * strprintf("%.1s", (char*)nullptr);
```
but we're not calling `strprintf` directly anymore, hence my comment
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780827733)
The comment states:
```
// Avoid triggering the following crash bug:
// * strprintf("%.1s", (char*)nullptr);
```
but we're not calling `strprintf` directly anymore, hence my comment
💬 l0rinc 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#discussion_r1780832127)
wasn't aware of `BOOST_TEST_CONTEXT`, would indeed be an improvement here
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780832127)
wasn't aware of `BOOST_TEST_CONTEXT`, would indeed be an improvement here
💬 l0rinc 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#discussion_r1780833680)
Not sure why it's less clear than before, but it's just a nit from my part anyway
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780833680)
Not sure why it's less clear than before, but it's just a nit from my part anyway
💬 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#discussion_r1780834719)
> but we're not calling `strprintf` directly anymore, hence my comment
`strprintf` is an alias for `tfm::format`, introduced by and for Bitcoin Core. This pull request does not touch the alias and does not change it. It exists before an after this pull request.
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780834719)
> but we're not calling `strprintf` directly anymore, hence my comment
`strprintf` is an alias for `tfm::format`, introduced by and for Bitcoin Core. This pull request does not touch the alias and does not change it. It exists before an after this pull request.
💬 l0rinc 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#discussion_r1780837147)
Can you expand on that? We've changed other `strprintf` instance here to enable compile-time validation, how come this one didn't make the cut?
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780837147)
Can you expand on that? We've changed other `strprintf` instance here to enable compile-time validation, how come this one didn't make the cut?
💬 l0rinc 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#discussion_r1780838413)
Ok, it's not that important
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780838413)
Ok, it's not that important
💬 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-2382746995)
Only changes are some minor style-only test-only fixups.
re-ACK 2e33dbadcf69838c8ea591dccdc666f809c3134a 🔰
<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+krxU1A3Yux4bpw
...
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2382746995)
Only changes are some minor style-only test-only fixups.
re-ACK 2e33dbadcf69838c8ea591dccdc666f809c3134a 🔰
<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+krxU1A3Yux4bpw
...