💬 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
...

💬 stringintech commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2087539901)
Looks like we have an early return in `GetTransactionSigOpCost` for coinbase tx and no use for `coins` span; so maybe we could just pass an empty span here?
```suggestion
nSigOpsCost += GetTransactionSigOpCost<const Coin>(tx, {}, flags);
```

💬 stringintech commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2087577718)
Do we need to update the [documentation](https://github.com/TheCharlatan/bitcoin/blob/16a695fbff4a92eba3eb72ec6aa766e71c52f773/src/consensus/tx_verify.h#L24:L31) for `CheckTxInputs` now that the input existence check is removed from it?