🚀 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?

📝 maflcko opened a pull request: "fuzz: Properly setup wallet in wallet_fees target"
(https://github.com/bitcoin/bitcoin/pull/32488)
`g_wallet_ptr` is destructed after the `testing_setup`. This is not supported and will lead to issues such as https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2863875857 or https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2855259932.

This could be fixed by fixing the initialization order.

However, the global wallet is also modified in the fuzz target, which is bad fuzzing practise.

So instead fix it by constructing a fresh wallet for each fuzz iteration.

🤔 hodlinator reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2838013470)
Code Review 4e1aae19512df82af584a064640c2143c5c5fa4f

Seems like a slight fix would be good in the NSI script, see inline comment.

Nice simplification of `ExecVp` since [previous review](https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2821205668).

Tested on NixOS.

### Tested redirection of stderr/stdout

```
./build/bin/bitcoin node > foo
./build/bin/bitcoin node --nonexistentarg 2> err
```
"foo" stdout file contains expected log output. Confirmed by changing bit
...

💬 hodlinator commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087562615)
Would be nice to merge exec.h/cpp into subprocess.h due to similar functionality. But I don't know what happens to copyright in that case, so might be better to avoid for that reason.

💬 hodlinator commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087618429)
nit: Could document requirements in header:
```C++
//! Cross-platform wrapper for POSIX execvp function.
//! @param argv Needs to end with nullptr element, in line with execvp.
int ExecVp(const char* file, char* const argv[]);
```

💬 hodlinator commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087636740)
It appears we should also be deleting this in the uninstall section further down, right after:
https://github.com/bitcoin/bitcoin/blob/3eb0d327153f61ef2121a4b261b024f6ee1f3398/share/setup.nsi.in#L133

💬 hodlinator commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087585166)
Should this include util/exec.h?

💬 hodlinator commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087629089)
Might be nice to add a rationale for `-named` in the commit message of fc47f776fbafaaf9062e6b4dc29ba6e42e390d2a as it isn't discussed in the linked issue.

💬 hodlinator commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087583779)
Might be clearer to return `std::nullopt` upon failure? Feels a bit sloppy currently, but maybe okay for a glorified shell script as you wrote elsewhere: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1920888966

✅ l0rinc closed a pull request: "doc: document workaround and fallback for macOS fuzzing"
(https://github.com/bitcoin/bitcoin/pull/32084)