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

💬 fanquake commented on 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#issuecomment-2435611527)
cc @ryanofsky

💬 maflcko commented on pull request "ci: display logs of failed unit tests automatically":
(https://github.com/bitcoin/bitcoin/pull/31148#issuecomment-2435612120)
CI failure is (obviously) unrelated and can be ignored: https://github.com/bitcoin/bitcoin/issues/31151

👍 pablomartin4btc approved a pull request: "wallet: optimize migration process, batch db transactions"
(https://github.com/bitcoin/bitcoin/pull/28574#pullrequestreview-2393060142)
re-ACK c98fc36d094a08d44f3c95431db2c5f34a96cc73

(no diff since my previous review, just rebased)

💬 fanquake commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#discussion_r1815248326)
In b75d532ca4703e26f59c7d33a882a6c1295784ed: The `CLIENT_TARNAME` re-naming is incomplete.