👍 ryanofsky approved a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2163569279)
Code review ACK 606a7ab862470413ced400aa68a94fd37c8ad3d3. Just small formatting, include, and static_assert changes since last review.

I think it would be great to follow up on dergoegge's comment about fuzzing
https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1668437385. It seems like it could make fuzzing output much more useful. I don't think it is critical to do it as part of this PR though, since the fuzz test currently relies on global state and this PR isn't changing that.

✅ glozow closed an issue: "Double lock detected in `Warnings::GetMessages()`"
(https://github.com/bitcoin/bitcoin/issues/30400)

🚀 glozow merged a pull request: "Use `WITH_LOCK` in `Warnings::Set`"
(https://github.com/bitcoin/bitcoin/pull/30404)

💬 mzumsande commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2214416290)
> @instagibbs @sr-gi maybe @mzumsande. This will likely affect you. Any feedback?

Thanks - no objections, I've updated everywhere, so no longer affected by this.

💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2214421760)
> It seems that `libbitcoin_consensus` still depends on `libbitcoin_util` via the `ParseHex()` call in `pubkey.cpp`.
>
> However, the `"consensus util"` dependency is not allowed in `contrib/devtools/check-deps.sh`.

Nice find! This seems to point to a bug in the `check-deps.sh` script for exported template functions. In this case, the fact that pubkey.cpp depends on TryParseHex function is detected in generated `consensus_imports.txt` file:

```
_Z11TryParseHexIhESt8optionalISt6vectorIT
...

🚀 fanquake merged a pull request: "build: Bump clang minimum supported version to 16"
(https://github.com/bitcoin/bitcoin/pull/30263)

💬 ryanofsky commented on pull request "refactor: Make uint256S(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2214432177)
Concept ACK, and would be very nice for this to cover ParseHex. If it did, it seems like it would fix the unexpected consensus library dependency on the util library that hebasto reported in https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2209258843:

https://github.com/bitcoin/bitcoin/blob/a83f050dbe1392fc6b1b6c2a140c7346653b40d3/src/pubkey.cpp#L193

👋 maflcko's pull request is ready for review: "refactor: Remove Span operator==, Use std::ranges::equal"
(https://github.com/bitcoin/bitcoin/pull/29071)

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668526162)
4b1e978b2bafd9da564aa52d2ce64a723cf64036 nit:
```suggestion
// Verify that any graph of transactions has its ancestry correctly computed by DepGraph, and if
```

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1644451932)
maybe mention that caller must ensure inc includes `Ancestors(inc)`

🤔 glozow reviewed a pull request: "cluster mempool: cluster linearization algorithm"
(https://github.com/bitcoin/bitcoin/pull/30126#pullrequestreview-2118340933)
Looked at the first 4 commits so far, just doing code review and trying the algos out in `MiniMiner`.

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1639799588)
In the docs, maybe instead of "best remaining ancestor set", say "highest feerate ancestor set" ?

Side note - I was thinking that this was a replica of the ancestor set algo in `BlockAssembler` but this isn't the minimum of individual and ancestor feerate.

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668545670)
304d3cb23ba9f084b98f9f29b47e3dfbb61ca334
may be helpful context
```suggestion
// Never MarkDone the same transaction more than once as this function
// blindly subtracts the transaction's feerate from m_ancestor_set_feerates
Assume(select.IsSubsetOf(m_todo));
```

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668665030)
For each of the `CandidateFinder`s, would it be helpful to expose a `AllDone()` function? Seems slightly inconvenient for the caller to separately keep track of `todo` for a `while(todo.Any())` loop. `FindCandidateSet()` Assumes that todo isn't empty, so we can't use that to query.

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668716016)
Note to reviewers: pseudocode for the function when first introduced in 5661fcdb15244bc6d602379c294276c7ec686505 can be found [here](https://delvingbitcoin.org/t/how-to-linearize-your-cluster/303#h-21-searching-6)

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1668724062)
And I suppose it's not problematic if it is optimal but didn't claim to be so?

📝 theStack opened a pull request: "rpc: doc: use "output script" terminology consistently in "asm"/"hex" results"
(https://github.com/bitcoin/bitcoin/pull/30408)
The wording "public key script" was likely chosen as a human-readable form of the technical term `scriptPubKey`, but it doesn't seem to be really widespread. Replace it by the more (probably most?) common term "output script" instead. Note that the argument for the `decodescript` RPC is not necessarily an output script (it could e.g. be also a redeem script), so in this case we just stay generic and use "script".

See also the draft BIP "Terminology for Transaction Components" (https://github.
...

💬 maflcko commented on pull request "rpc: doc: use "output script" terminology consistently in "asm"/"hex" results":
(https://github.com/bitcoin/bitcoin/pull/30408#issuecomment-2214477162)
ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b

🚀 ryanofsky merged a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141)

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1668941931)
I updated this to no longer clear flags.