💬 theuni commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#issuecomment-2877835364)
Hmm, not relevant to what's being addressed here, but couldn't the per-tx `cache->hashPrevouts` and `cache->hashSequence` be replaced with a similar midstate cache trick?

📝 l0rinc opened a pull request: "blocks: avoid recomputing block header hash in `ReadBlock`"
(https://github.com/bitcoin/bitcoin/pull/32487)
Eliminate one a block header hash calculation per block read by reusing the hash for:
* proof‑of‑work verification;
* (optional) integrity check against the supplied hash.

This part of the code wasn't covered by tests either, so the first commit exercises this part first, before pushing the validation to the delegate method.

💬 achow101 commented on pull request "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors":
(https://github.com/bitcoin/bitcoin/pull/32475#issuecomment-2877842291)
CI should be fixed

💬 glozow commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2877848547)
reACK 8673e8f0191

🤔 glozow reviewed a pull request: "cluster mempool: add txgraph diagrams/mining/eviction"
(https://github.com/bitcoin/bitcoin/pull/31444#pullrequestreview-2838031475)
reACK 8673e8f0191

🚀 glozow merged a pull request: "cluster mempool: add txgraph diagrams/mining/eviction"
(https://github.com/bitcoin/bitcoin/pull/31444)

🚀 glozow merged a pull request: "test: Fix feature_pruning test after nTime typo fix"
(https://github.com/bitcoin/bitcoin/pull/32312)

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-2877875187)
Now ready for review, with #31444 merged.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2087588925)
As I understand it it returns different values across restarts (which suffices here) but I may be wrong. The doc is confusing. Anyway, your solution doesn't hurt.

🤔 musaHaruna reviewed a pull request: "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`"
(https://github.com/bitcoin/bitcoin/pull/32432#pullrequestreview-2838067718)
tACK [2fee8f4z](https://github.com/bitcoin/bitcoin/pull/32432/commits/2fee8f4886e1421099dfb8ba959b9d0c7e4b0aa8)

Compiled Successfully without errors
Test the code functionality using:
`build/bin/bitcoin-cli -regtest help getnewaddress `
Result:
`Arguments:
.....
2. address_type (string, optional, default=set by -addresstype) The address type to use. Options are "legacy", "p2sh-segwit", "bech32", "bech32m".`
.....

🤔 glozow reviewed a pull request: "mining: rename gbt_force and gbt_force_name"
(https://github.com/bitcoin/bitcoin/pull/32386#pullrequestreview-2838070577)
ACK 0750249289c092fc8e2e29669fec73a58b873767, seems sensible

🚀 glozow merged a pull request: "mining: rename gbt_force and gbt_force_name"
(https://github.com/bitcoin/bitcoin/pull/32386)

💬 theuni commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2087601815)
Yeah, I had hoped for the same :(

The problem here (as usual with these annotations) is aliasing.

As a simplified example:
```c++
CCheckQueueControl<CScriptCheck> control(m_chainman.GetCheckQueue());
CCheckQueueControl<CScriptCheck> control2(m_chainman.GetCheckQueue());
```

Here, clang has no idea that `&control.m_lock.m_control_mutex` == `&control2.m_lock.m_control_mutex`.

The only alternative, really, would be something like:

```diff
diff --git a/src/checkqueue.h b/src/chec
...

🤔 glozow reviewed a pull request: "fees: document non-monotonic estimation edge case"
(https://github.com/bitcoin/bitcoin/pull/31080#pullrequestreview-2838089123)
ACK 1e0de7a6ba926487c8a075856b74af2a3a0eb8ef

✅ glozow closed an issue: "Bitcoin is returning higher fees for 36 block window than 2 block window (on testnet)"
(https://github.com/bitcoin/bitcoin/issues/11800)

🚀 glozow merged a pull request: "fees: document non-monotonic estimation edge case"
(https://github.com/bitcoin/bitcoin/pull/31080)

💬 theuni commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2087617462)
Done.

💬 theuni commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#issuecomment-2877918269)
Updated to address @TheCharlatan's nit.

🤔 stringintech reviewed a pull request: "kernel: Separate UTXO set access from validation functions"
(https://github.com/bitcoin/bitcoin/pull/32317#pullrequestreview-2837977865)
I had a few more comments (sorry for the scattered feedback; didn't have the chance to review everything earlier).

💬 stringintech commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2087608225)
In commit 5908680, the description mentions: "By returning early in case of a BIP30 validation failure, an additional failure check for an invalid block reward in case of its failure is left out."

To validate my understanding: there are also other checks in `ConnectBlock` before the `!state.IsValid()` check that we may have skipped because of the early return when `SpendBlock` fails - including transaction input validation, script verification, fee range checks, ...; Is that correct?

It ma
...