💬 janb84 commented on pull request "refactor: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/33116#discussion_r2252256558)
NIT; is the variable name still descriptive ?
```suggestion
for (const Txid& txid : replaced_txids) replaced_list.push_back(txid.ToString());
```

💬 darosior commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2252275892)
The latest version now uses a `SpendsNonAnchorWitnessProg` helper in policy.h.

💬 darosior commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2252276108)
Done.

💬 darosior commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2252276375)
Taken.

💬 darosior commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#issuecomment-3151900931)
I fixed the bug @Crypt-iQ found (thanks!) and added a functional test that explicitly tests for witness-stripping detection for both native and p2sh-wrapped outputs. This test would have failed without the fix. In addition, i made the segwit-spend detection into its own helper function for which i added a unit test to sanity check detection against all defined output types.

💬 sipa commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3151919059)
Do we have numbers on how often the rejection filter catches things by txid? I suspect that in practice that will be just due to fetches of orphan parents which are (policy) invalid, due to non-witness reasons?

💬 glozow commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3151926755)
> Do we have numbers on how often the rejection filter catches things by txid?

I didn't see any in a 2-week period 😅

💬 Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2252317767)
should be fixed in 8dd5c0a0447bbb6fe597aabfa725b397276da166

💬 glozow commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2252309272)
Transactions received through RPC aren't checked against the reject filter, so I think this comment can be confusing

💬 glozow commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2252317643)
Is it right to return false early? You need to check all inputs, no?

💬 Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2252319235)
Should be fixed in 8dd5c0a0447bbb6fe597aabfa725b397276da166

💬 Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2252321022)
Per offline discussion, removed the `#ifdef DEBUG_LOCKCONTENTION` since the category masks are now properly reset between iterations (thanks!).

💬 Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#issuecomment-3151977573)
Latest push 31ef2b6 -> 8dd5c0a:
- adds a DEBUG_ONLY flag to disable the rate limiting. This was pointed out [here](https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2246301270) that some of the functional tests were having their logs suppressed during normal operation.
- resets logging categories between tests without relying on disabling the `LOCK` category.

Should the DEBUG_ONLY flag get a release note?

💬 glozow commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2252329056)
It might be a good idea to have the unit tests cover transactions with multiple input types

🤔 mzumsande reviewed a pull request: "refactor, index: Remove member variables in coinstatsindex"
(https://github.com/bitcoin/bitcoin/pull/33134#pullrequestreview-3085407112)
Concept ACK

🤔 furszy reviewed a pull request: "refactor, index: Remove member variables in coinstatsindex"
(https://github.com/bitcoin/bitcoin/pull/33134#pullrequestreview-3085424612)
I would only remove the per-block values from the class and leave the `total_*` ones there. Mainly because these values effectively cache the last state, which will lets us remove the previous record disk read that we do for every connected block (reading from disk is slower than caching a few accumulators at the class level).

💬 darosior commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2252378427)
How is this related to RPC?

💬 darosior commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2252384313)
> Is it right to return false early? You need to check all inputs, no?

It is intentional to conserve behaviour. Currently we check scripts sequentially from first input to last. If input N is consensus-invalid and input N+1 is witness stripped we will return a consensus-invalid error. In my opinion it would be better to `continue` here but i think it's better to not change behaviour in this PR.

> It might be a good idea to have the unit tests cover transactions with multiple input types

...

💬 glozow commented on pull request "validation: detect witness stripping early on":
(https://github.com/bitcoin/bitcoin/pull/33105#discussion_r2252385326)
Oh woops, I thought these were being submitted through RPC because of the err strings. Nevermind.

👍 ismaelsadeeq approved a pull request: "util: detect and warn when using exFAT on MacOS"
(https://github.com/bitcoin/bitcoin/pull/31453#pullrequestreview-3085477284)
reACK eb68bb48ab2bb163f20dc7430d0401d88ca43f4a

Also retested on both `bitcoin-qt` and `bitcoind` binaries release build and I get the same warnings when datadir/blocksdir or both are on exFat drive format