💬 l0rinc commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2261573911)
Done

💬 l0rinc commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2261578300)
Thanks, done

💬 l0rinc commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2261571418)
Not after the fact, but that the commited code contained the added comments already explaining - not important, it's done :)

💬 l0rinc commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2261577984)
Done

💬 l0rinc commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2261493776)
Reworded the commit message, thanks

💬 l0rinc commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2261477492)
Done

💬 l0rinc commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2261477694)
Brought the change one commit earlier to avoid modifying the same thing twice.

💬 l0rinc commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2261570975)
If you say you find it more readable, I don't mind - changed

👋 l0rinc's pull request is ready for review: "test: use local `CBlockIndex` in block read hash mismatch check"
(https://github.com/bitcoin/bitcoin/pull/33154)

💬 sipa commented on pull request "validation: detect witness stripping without re-running Script checks":
(https://github.com/bitcoin/bitcoin/pull/33105#issuecomment-3166037370)
ACK ffd82f3783d1af4f94f7dbd0af3a80d4591eb185

While discussing possible alternatives to this with @darosior and @glozow, I feel I came to an understanding about the problem I didn't have before.

Consider an approach where there are completely separate txid and wtxid rejection caches. The txid cache is used when one has a txid, the wtxid cache is used when one has a wtxid. Ideally then, we insert failed transactions:
* Always into the wtxid cache (even when TX_WITNESS_STRIPPED, because it w
...

🤔 w0xlt reviewed a pull request: "test: revive test verifying that `GetCoinsCacheSizeState` switches from OK→LARGE→CRITICAL"
(https://github.com/bitcoin/bitcoin/pull/33021#pullrequestreview-3098989496)
ACK https://github.com/bitcoin/bitcoin/pull/33021/commits/554befd8738ea993b3b555e7366558a9c32c915c

Some observations:
* Most of the master branch `src/test/validation_flush_tests.cpp` also ends early in my setup.
* This PR revives (and simplifies) that test, making it robust across all platforms.
* The `MAX_ATTEMPTS ` is good safeguard to keep CI from hanging due to initial map sizes (libc++/libstdc++/arch differences).

🤔 l0rinc requested changes to a pull request: "util: detect and warn when using exFAT on MacOS"
(https://github.com/bitcoin/bitcoin/pull/31453#pullrequestreview-3099049023)
Seeing this planned for [V30](https://github.com/bitcoin/bitcoin/milestone/72) I reviewed it a bit more thoroughly.
My Concept ACK remains, but I think we can document and simplify and modernize it a bit more.

💬 l0rinc commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2261640773)
Line seems to state the same thing multiple times.
The previous line already states that some should be avoided:

```suggestion
## Filesystem recommendations

When choosing a filesystem for the data directory (`datadir`) or blocks directory (`blocksdir`), some filesystems should be avoided:

- **macOS**: The exFAT filesystem should not be used. There have been multiple reports of database corruption and data loss when using this filesystem with Bitcoin Core due to filesystem-level compat
...

💬 l0rinc commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2261642351)
In [other cases](https://github.com/bitcoin/bitcoin/pull/33040) we've just removed the whole TOC - looks like this was kept since it's not technically called like that, but consider removing the whole thing as an alternative.

💬 l0rinc commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2261653793)
This is either redundant or incomplete: we should either delete it or mention the `error_paths` check as well

💬 l0rinc commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2261669615)
First, while I believe this to be correct, could you provide a documentation that shows the possible values? I'm wondering if it's always written like this or if it's sometimes exFAT (like you refer to it) or `[ExFAT](https://support.apple.com/guide/disk-utility/file-system-formats-dsku19ed921c/mac)` (like the apple doc does sometimes) - or even if it would/should work with something like https://github.com/relan/exfat (found this accidentally, ignore if not important)

Second: nit, we don't h
...

💬 l0rinc commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2261656071)
We already have `using util::Join;` at the top:
```suggestion
LogInfo("Failed to detect filesystem type for: %s", Join(error_paths, ", "));
```

💬 l0rinc commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2261680365)
nit: "are on exFAT" sounds weird to me + missing comma:
```suggestion
InitWarning(strprintf(_("The following paths are using exFAT, which is known to have intermittent corruption problems on macOS: %s. "
```

💬 l0rinc commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2261681174)
nit: non-standard formatting

💬 l0rinc commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r2261654589)
Weird negation + repetition:
```suggestion
"Move these directories to a different filesystem to avoid data loss."),
```