💬 dergoegge commented on pull request "fuzz: fix bug in p2p_headers_presync harness":
(https://github.com/bitcoin/bitcoin/pull/30980#issuecomment-2382599033)
> According to https://oss-fuzz.com/fuzzer-stats?group_by=by-day&fuzzer=afl_bitcoin-core_p2p_headers_presync&project=bitcoin-core the exec/s is 0 for afl.

This is because oss-fuzz doesn't use our afl++ harness but rather the [afl libfuzzer driver](https://github.com/AFLplusplus/AFLplusplus/tree/stable/utils/aflpp_driver). When using this driver, afl++ will fork after the initialization code is run (https://github.com/AFLplusplus/AFLplusplus/blob/d21fb1a558b25c4f46692fa999c0028dfe0eecc0/utils/
...

💬 dergoegge commented on pull request "fuzz: fix bug in p2p_headers_presync harness":
(https://github.com/bitcoin/bitcoin/pull/30980#discussion_r1780745018)
nit: You could get rid of `total_work` as a variable and just do:

```suggestion
assert(CalculateClaimedHeadersWork(all_headers) < chainman.MinimumChainWork());
```

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

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

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

...

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

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

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

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

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

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

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

💬 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

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

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

💬 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

💬 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

💬 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

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

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