💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1815097941)
At least locally I can drop all other changes now as well and it compiles:


```diff
diff --git a/src/util/bitset.h b/src/util/bitset.h
index 7db6f2424f..6f9e808c37 100644
--- a/src/util/bitset.h
+++ b/src/util/bitset.h
@@ -101,7 +101,7 @@ class IntBitSet
/** Progress to the next 1 bit (only if != IteratorEnd). */
constexpr Iterator& operator++() noexcept
{
- assert(m_val != 0);
+ Assume(m_val != 0);
m_val &= m_val - I{1
...

📝 stickies-v opened a pull request: "tinyformat: enforce compile-time checks for format string literals"
(https://github.com/bitcoin/bitcoin/pull/31149)
_Carved out of #30928 - this PR doesn't include the controversial change of suppressing `tinyformat::format_error` throwing behaviour, and just adds enforcement of compile-time checks for format string literals._

Reduce unexpected run-time crashes from string formatting by enforcing compile-time format string checks for most* `tfm::format` and `strprintf` usage.

This PR should introduce no behaviour change.
The main changes are:
- remove the `const std::string&` `tfm::format` overload si
...

📝 stickies-v converted_to_draft a pull request: "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error"
(https://github.com/bitcoin/bitcoin/pull/30928)
Avoid unexpected run-time crashes from string formatting by enforcing compile-time format string checks for most* `tfm::format` and `strprintf` usage, and by returning an error string instead of throwing for a run-time tfm::format_error so callsites don't need to implement this error handling.

This PR should introduce no behaviour change.
The main changes are:
- remove the `const std::string&` `tfm::format` overload since it's not necessary. Usage of this overload is removed by one of:
-
...

💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2435469092)
You can just turn the Assume to assert at compile-time, eagerly, if possible (decided by the compiler) at no downside, and the benefit that it will be evaluated and checked at compile-time:

<details><summary>possible diff</summary>


```diff
diff --git a/src/util/bitset.h b/src/util/bitset.h
index 7db6f2424f..6f9e808c37 100644
--- a/src/util/bitset.h
+++ b/src/util/bitset.h
@@ -101,7 +101,7 @@ class IntBitSet
/** Progress to the next 1 bit (only if != IteratorEnd). */

...

💬 dergoegge commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2435476872)
I'll try reverting the bitset changes, I remember that it failed to compile without but let's see...

💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2435488019)
You can push the diff as a dummy commit for now, if you want. I'll create a separate pull for it, because the Assume change at compile time possibly makes sense on its own. Then you can rebase on that and squash everything down to one commit again.

💬 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-2435489968)
I've carved out the first 3 commits into https://github.com/bitcoin/bitcoin/pull/31149, since other PRs (e.g. #31061 and #31072) somewhat rely on this, and everyone seems in agreement that adding compile-time checks is good. Existing ACKs should be trivially transferable.

Converting this PR to draft until #31149 is merged to keep the controversial error suppression discussion until later.

💬 maflcko commented on pull request "ci: display logs of failed unit tests automatically":
(https://github.com/bitcoin/bitcoin/pull/31148#issuecomment-2435505056)
Lgtm. Nice find.


I wish this was enabled by default. There should never be a case to be shy when an error occurs and autotools didn't do that either.

💬 plebhash commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2435505351)
> Is it possible for core to remove testnet to avoid maintenance burden?
>
> Regtest and Signet could work fine for testing. Maybe testnet could be supported by some fork if there is enough demand.

`testnet4` is being used for testing Stratum V2 in development, where a network with realistic mining dynamics is important

neither `regtest` nor `signet` allow for this

💬 stickies-v commented on pull request "refactor: Clean up messy strformat and bilingual_str usages":
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1815162280)
> This commit (message) only becomes true after commit [4ce60d8](https://github.com/bitcoin/bitcoin/commit/4ce60d82a0c1e21ee7ecba69e4c8926720829429) from #30928. So I wonder if [4ce60d8](https://github.com/bitcoin/bitcoin/commit/4ce60d82a0c1e21ee7ecba69e4c8926720829429)+faa34ad0a7f5271958e5313e7aae2baaed41587e could be split up into a first pull request, as everything else is based on it?

This is now ~done in https://github.com/bitcoin/bitcoin/pull/31149

💬 TheCharlatan commented on pull request "build: replace custom `MAC_OSX` macro with existing `__APPLE__`":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2435539032)
Post-merge and Guix build reproduced ACK 6c6b2442edabe9e3f77d29aacd6681bc3fa16d9e

💬 darosior commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1815180724)
Took the suggestion, thanks.

💬 darosior commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2435551513)
Updated the release note to something clearer and fixed the `check-doc.py` lint error by using `AddArg()` directly for the hidden `-upnp` arg.

📝 maflcko opened a pull request: "util: Treat Assume as Assert when evaluating at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31150)
There is no downside or cost of treating an `Assume` at compile-time as an `Assert` and it may even help to find bugs while compiling without `ABORT_ON_FAILED_ASSUME`.

This is also required for https://github.com/bitcoin/bitcoin/pull/31093

💬 maflcko commented on pull request "util: Treat Assume as Assert when evaluating at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31150#issuecomment-2435561479)
Can be tested by adding a failing `Assume` at compile-time and observing that it passes on master, but fails to compile with this commit:

```diff
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index ba4ba6c3b6..4f8815e179 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -99,6 +99,7 @@ void TxOrphanage::EraseForPeer(NodeId peer)

void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
{
+constexpr int FOO{Assume(0)};
unsigned int
...

👍 hebasto approved a pull request: "ci: display logs of failed unit tests automatically"
(https://github.com/bitcoin/bitcoin/pull/31148#pullrequestreview-2393005380)
ACK 8523d8c0fc85c5511311628d47c78fa380b99f6e, I have reviewed the code and it looks OK.

👍 dergoegge approved a pull request: "util: Treat Assume as Assert when evaluating at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31150#pullrequestreview-2393006448)
ACK fa824ea44c7b1d55f20e4941d78641cd129e1ea3

📝 dergoegge converted_to_draft a pull request: "Introduce `g_fuzzing` global for fuzzing checks"
(https://github.com/bitcoin/bitcoin/pull/31093)
This PR introduces a global `g_fuzzing` that indicates if we are fuzzing.

If `g_fuzzing` is `true` then:

* Assume checks are enabled
* Special fuzzing paths are taken (e.g. pow check is reduced to one bit)

Closes #30950 #31059

💬 maflcko commented on pull request "util: Treat Assume as Assert when evaluating at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31150#issuecomment-2435603367)
Force pushed to fix a true warning that didn't show locally.

⚠️ maflcko opened an issue: "ci: intermittent issue in rpc_misc.py node0 stderr terminate called after throwing an instance of 'kj::ExceptionImpl' [15:12:14.943] what(): mp/proxy.cpp:242: disconnected: write(m_post_fd, &buffer, 1): Broken pipe [15:12:14.943] stack: 657ed083 657ed86e f7ac7d20 f7713156 f77a7e07 "
(https://github.com/bitcoin/bitcoin/issues/31151)
https://cirrus-ci.com/task/5442110629871616?logs=ci#L3430

```
[15:12:14.943] node0 2024-10-24T15:12:14.578291Z [shutoff] [src/logging/timer.h:58] [Log] [bench] FlushStateToDisk: write block and undo data to disk started
[15:12:14.943] node0 2024-10-24T15:12:14.578357Z [shutoff] [src/logging/timer.h:58] [Log] [bench] FlushStateToDisk: write block and undo data to disk completed (0.06ms)
[15:12:14.943] node0 2024-10-24T15:12:14.578365Z [shutoff] [src/logging/timer.h:58] [Log] [bench] Fl
...