💬 achow101 commented on pull request "test: Don't enforce BIP94 on regtest unless specified by arg":
(https://github.com/bitcoin/bitcoin/pull/31156#issuecomment-2448353436)
ACK e60cecc8115d3b28be076792baa5e4ea26d353a6

✅ achow101 closed an issue: "Remove BIP94 from regtest"
(https://github.com/bitcoin/bitcoin/issues/31137)

🚀 achow101 merged a pull request: "test: Don't enforce BIP94 on regtest unless specified by arg"
(https://github.com/bitcoin/bitcoin/pull/31156)

👍 tdb3 approved a pull request: "Update manpage descriptions"
(https://github.com/bitcoin/bitcoin/pull/29686#pullrequestreview-2406316184)
re ACK 47f50c7af5572520fd986b313a63a44a76d3c859

Ran

```
BUILDDIR=$PWD/build contrib/devtools/gen-manpages.py
```

and checked the output of each of the pages with `man -l doc/man/<file>`.
Also displayed "Command-line options" in the GUI.


💬 l0rinc commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#discussion_r1823396896)
As mentioned in the comments, it seems to me that `16` may be a better default value based on the measured IBDs - basically just as fast as `128`, without having to worry about the increase in e.g. `MaxGrandParentOverlapBytes` and `ExpandedCompactionByteSizeLimit` (10x and 25x this value) called e.g. in `IsTrivialMove` with a warning: `"the move could create a parent file that will require a very expensive merge later on"` (or any other such surprise) - which we likely want to avoid:
```sugges
...

🚀 achow101 merged a pull request: "build: have "make test" depend on "make all""
(https://github.com/bitcoin/bitcoin/pull/31015)

💬 achow101 commented on pull request "Fix unsigned integer overflows in interpreter":
(https://github.com/bitcoin/bitcoin/pull/24214#issuecomment-2448430410)
ACK bbbbaa0d9ac9ae9c9b8109503aa30213eed543b9

🚀 achow101 merged a pull request: "Fix unsigned integer overflows in interpreter"
(https://github.com/bitcoin/bitcoin/pull/24214)

⚠️ Sjors opened an issue: "Gracefully handle dropped UPnP support "
(https://github.com/bitcoin-core/gui/issues/843)
https://github.com/bitcoin/bitcoin/pull/31130 dropped UPnP support. It's now recommended that
users use PCP (with NATPMP fallback).

But this is not explained in a friendly manner:

<img width="496" alt="error" src="https://github.com/user-attachments/assets/d35891ac-82e0-4f61-8e9b-671aaa1d0d75">

A user would have to figure out that they need to manually edit settings.json or delete it and redo all their settings.

We should probably automatically delete it from settings.json. And the
...

💬 Sjors commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2448504460)
Done in https://github.com/bitcoin-core/gui/issues/843

💬 Sjors commented on pull request "test: Don't enforce BIP94 on regtest unless specified by arg":
(https://github.com/bitcoin/bitcoin/pull/31156#issuecomment-2448519062)
TIL about the `-test=<option>` hook, that's a nice solution. Post-merge concept ACK.

✅ Sjors closed an issue: "macOS 13.7 depends build can't find qt"
(https://github.com/bitcoin/bitcoin/issues/31050)

💬 Sjors commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2448527957)
That seems plausible. I'll keep an eye on that issue. Once that's resolved and there's still a macOS specific problem remaining, I'll reopen.

🤔 hodlinator reviewed a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2405166236)
> In case you do want a test with different behavior, I think you can use %n specifier which is not supported by tinyformat

Could document non-parity like so (unless you prefer I do it as part of #30933):
```C++
// Non-parity
int n{};
BOOST_CHECK_EXCEPTION(tfm::format(std::string{"%n"}, n), tfm::format_error,
HasReason{"tinyformat: %n conversion spec not supported"});
ConstevalFormatString<1>::Detail_CheckNumFormatSpecifiers("%n");
```

💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822782509)
(One could expect that at least one precision-digit was required after '.' but it is not in tinyformat so this behavior is consistent).

💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1822780992)
Should this be removed?

💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1823481424)
Why not use `FormatStringCheck<sizeof...(Args)>` in `printf` and `printfln` directly below as well?

💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1823488811)
Could also add
```C++
PassFmt<1>("%.f");
```
maybe grouped with the `<1>` calls above. Confirmed tinyformat allows for it, see how the success status returned by `parseWidthOrPrecision` is ignored:
https://github.com/bitcoin/bitcoin/blob/e53829d3952c6ed275507a66e77636aad67d106b/src/tinyformat.h#L778-L788

💬 github12101 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2448576849)
> > In my main machine, is ryzen with 32gb ddr4 with 2 sticks of memory. The other machine is Xeon, with 32gb memory, in quad channel ddr3. So i dont think its memory, because the 2 computers are stable. I can run prime95 in both for 2 hours with no problem.
>
> it's true, I just ran disk diagnostics, no issues. this is a new 4tb ssd. going to run a memory diagnostic... but my system has been stable so I would be very surprised if it was that. I'm also running on ZFS. I keep hitting this, eve
...

👍 ryanofsky approved a pull request: "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests"
(https://github.com/bitcoin/bitcoin/pull/30906#pullrequestreview-2404680812)
Code review ACK 3b5b2457f713014fd5073da34b76a5a8cd796e4a. Nice change that should make the API more safe and also makes tests more complete and easier to maintain.

💬 ryanofsky commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1822513330)
In commit "test: Migrate GetCoinsMapEntry to return MaybeCoin" (e8e70a4ae3cbd7a648629c2df4b5a7f6f0686438)

Would be better to leave this outside of the try loop so it is clearer this is not expected to throw std::logic_error, and if it it did, the test framework would catch it.

Same for CheckWriteCoins test below.