✅ hebasto closed a pull request: "Set minimum supported Windows version to 1903 (May 2019 Update)"
(https://github.com/bitcoin/bitcoin/pull/32537)

👍 stickies-v approved a pull request: "doc: remove // for ... comments"
(https://github.com/bitcoin/bitcoin/pull/32562#pullrequestreview-2851071402)
ACK e4aee0b3a23a9609e16bd7c8eda618dcf0a8b1db

The new includes seem arbitrary, they usually still don't represent the full include list (according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)), and could be dropped, but I verified them against the IWYU run and mostly seem fine (I added comments for the ones that seem incorrect.


Found a couple more include comments in `util/fs_helpers.cpp`

💬 stickies-v commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095904411)
nit: `#include <limits.h>` can be removed according to IWYU I suspect because we also `#include <limits>`

💬 stickies-v commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095909033)
nit: `chrono` and `string` are not required according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)

💬 stickies-v commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095917052)
nit: `cstring` and `string` includes can just be dropped according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)

💬 stickies-v commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#discussion_r2095910935)
nit: the `span.h` include can now be removed according to [IWYU](https://api.cirrus-ci.com/v1/task/4921334028828672/logs/ci.log)

👋 l0rinc's pull request is ready for review: "script: short-circuit `GetLegacySigOpCount` for known scripts"
(https://github.com/bitcoin/bitcoin/pull/32532)

💬 maflcko commented on pull request "ci: Move DEBUG=1 to centos task":
(https://github.com/bitcoin/bitcoin/pull/32560#discussion_r2095935838)
No reason. I'd say anything is fine here.

Initially this was set in 14788fbadac3aa352eb51da81ecdfa01208ca33c, but we have a larger cache now.

👋 hebasto's pull request is ready for review: "Modernize use of UTF-8 in Windows code"
(https://github.com/bitcoin/bitcoin/pull/32380)

💬 maflcko commented on pull request "Use WitnessV0KeyHash in TestAddAddressesToSendBook":
(https://github.com/bitcoin-core/gui/pull/875#issuecomment-2891405513)
can someone pls assign 30.0 milestone?

💬 maflcko commented on pull request "Remove legacy `Parse(U)Int*`":
(https://github.com/bitcoin/bitcoin/pull/32520#discussion_r2095953566)
thx. I don't want to check any explicit values, so I pushed something else.

🤔 l0rinc reviewed a pull request: "node: cap `-maxmempool` and `-dbcache` values for 32-bit"
(https://github.com/bitcoin/bitcoin/pull/32530#pullrequestreview-2851134975)
I thought we're [sunsetting 32 bit](https://github.com/bitcoin/bitcoin/issues/32375) support slowly (it's why I was originally confused by the PR description).
I agree with others that the limits are arbitrary, we should only warn if the values obviously don't fit into a `size_t`.

💬 l0rinc commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2095947213)
Same, can be constexpr and could use more descriptive bitness check:
```suggestion
constexpr auto max_db_cache{SIZE_MAX == UINT32_MAX ? MAX_32BIT_DBCACHE : std::numeric_limits<size_t>::max()};
```

💬 l0rinc commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2095948822)
Agree with @Sjors, which would also simplify `max_db_cache` to just `std::numeric_limits<size_t>::max()`

💬 l0rinc commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2095942544)
we could make it an `if constexpr` as well and use a more descriptive way of checking for 32 bitness:
```suggestion
if constexpr (SIZE_MAX == UINT32_MAX && *mb > MAX_32BIT_MEMPOOL_MB) {
```

💬 l0rinc commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#discussion_r2095953423)
... and I think it should be a `size_t` instead

💬 maflcko commented on pull request "kernel: Remove dependency on clientversion":
(https://github.com/bitcoin/bitcoin/pull/32543#discussion_r2096016006)
> Then again, some people are just forking the project on master and tag some past version on it. That could be equally confusing, no?

Hopefully that is rare. Though, just including the full commit hash in bug reports shouldn't hurt.

💬 fanquake commented on pull request "Use subprocess library for notifications":
(https://github.com/bitcoin/bitcoin/pull/32566#issuecomment-2891504575)
Concept ACK. Seems like there is some overlap here with #32380, but that this PR should go first, otherwise #32380 is going to be refactoring code, that is being deleted here.

💬 stickies-v commented on pull request "doc: remove // for ... comments":
(https://github.com/bitcoin/bitcoin/pull/32562#issuecomment-2891504937)
re-ACK 7193245cd66791216d4e586ca09302b26d4b7528

💬 fanquake commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2096035158)
Is anything enforcing this, or is the assumption that any Windows 10 system is this at least version? What happens if it isn't?