💬 dergoegge commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2435345680)
Looks like switching from Assume to assert in BitSet has a noticeable impact on the `Linearize*` bench tests: https://corecheck.dev/bitcoin/bitcoin/pulls/31093

💬 laanwj commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1815036443)
maybe
"If you want to open a port automatically, consider using the `-natpmp` option instead, which uses PCP or NAT-PMP depending on router support."
(in the GUI, the option is still called NATPMP as well, so no need to mention that seperately i think)

💬 1440000bytes commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2435361835)
Is it possible for core to remove support for 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.

👍 TheCharlatan approved a pull request: "cmake, qt, test: Remove problematic code"
(https://github.com/bitcoin/bitcoin/pull/31147#pullrequestreview-2392642727)
ACK fb46d57d4e7263495c007f4a499a349bff2b21e0

✅ fanquake closed an issue: "How to compile the GUI on opensuse tumbleweed with cmake?"
(https://github.com/bitcoin-core/gui/issues/842)

🚀 fanquake merged a pull request: "cmake, qt, test: Remove problematic code"
(https://github.com/bitcoin/bitcoin/pull/31147)

📝 furszy opened a pull request: "ci: display logs of failed unit tests automatically"
(https://github.com/bitcoin/bitcoin/pull/31148)
Saw it here https://github.com/bitcoin/bitcoin/actions/runs/11488618084/job/31975712362?pr=31000.

The 'test-each-commit' and 'win64' CI jobs currently do not display test logs when an error occurs, making it almost impossible to debug issues that don't happen locally. Fix this by setting the CTest `--output-on-failure` flag (per [README](https://github.com/bitcoin/bitcoin/blob/2f40e453ccdf5f75b599fc7168abd512510686c5/src/test/README.md?plain=1#L130)).

💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1815047765)
Done.

💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1815048051)
Done. The whole dump is already compared a few lines below, but I guess it can't hurt to the detailed address->label entry checks in addition.

💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1815048142)
Done.

💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#issuecomment-2435378650)
Thanks for the review! Rebased on master (as I suspect CI would be very unhappy otherwise) and addressed comments https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813809067, https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813825732 and https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813830674

👋 l0rinc's pull request is ready for review: "optimization: pack util::Xor into 64/32 bit chunks instead of doing it byte-by-byte"
(https://github.com/bitcoin/bitcoin/pull/31144)

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