🚀 achow101 merged a pull request: "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate"
(https://github.com/bitcoin/bitcoin/pull/30636)

📝 pablomartin4btc opened a pull request: "devtools, utxo-snapshot: Fix block height out of range in script"
(https://github.com/bitcoin/bitcoin/pull/30690)
<details>
<summary>Fixing a <a href="https://github.com/bitcoin/bitcoin/pull/28553#pullrequestreview-2251032570">bug</a> in <code>utxo_snapshot.sh</code>.</summary>

```
/contrib/devtools/utxo_snapshot.sh 840000 snapshot2.dat ./src/bitcoin-cli -datadir=${AU_DATADIR}
Do you want to disable network activity (setnetworkactive false) before running invalidateblock? (Y/n):
Disabling network activity
false
error code: -8
error message:
Block height out of range
```

And the user will see
...

💬 brunoerg commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1725475551)
Cool, thank you.

👍 theuni approved a pull request: "depends: Fix CMake-generated `libzmq.pc` file"
(https://github.com/bitcoin/bitcoin/pull/30508#pullrequestreview-2251549063)
Nice. UtACK f1e39c4594295b4ce5976145ee665e38f2f8e5f1

📝 achow101 opened a pull request: "Fix maybe-uninitialized warning in IsSpentKey"
(https://github.com/bitcoin/bitcoin/pull/30691)
After 6ed424f2db609f9f39ec1d1da2077c7616f3a0c2, I started seeing a maybe-unitialized warning in `CWallet::IsSpentKey`:

```
In destructor ‘constexpr std::_Vector_base<_Tp, _Alloc>::~_Vector_base() [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’,
inlined from ‘constexpr std::vector<_Tp, _Alloc>::~vector() [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/bits/stl_vector.h:738:7,
inlined from
...

👍 instagibbs approved a pull request: "cluster mempool: optimized candidate search"
(https://github.com/bitcoin/bitcoin/pull/30286#pullrequestreview-2251100080)
code review ACK 1b8f143fb791fe5ba40ce6610cd4e8e04b951495

Thanks for dropping those couple commits.

> I think this will need analysis with real-world clusters to see actual impact, which may be easier once the cluster mempool project is further along. I've dropped it for this reason.

On that note I think we'll need some diagram-quality style benchmarks to make informed decisions in the future, otherwise it makes ongoing changes difficult to justify.

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1725236618)
a12ca7b8d99d868ff49e90d33b5e90c169849ccc

may want to consider something like `m_sorted_depgraph`

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1725257738)
same check already being done in split_fn, but I also like it here on creation
```Suggestion
inc(std::move(i)), und(std::move(u)), pot_feerate(std::move(p_f)) {
Assume(pot_feerate.IsEmpty() == inc.feerate.IsEmpty());
}
```

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725534553)
new name is better and makes flow easier to understand,thanks

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725548581)
:facepalm: I was missing the `static` in front of `prevout_hash`. This is making a lot more sense now.

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725548968)
might be worth a comment!

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1725555941)
Done.

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1725556045)
Done.

💬 sipa commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2302743849)
> On that note I think we'll need some diagram-quality style benchmarks to make informed decisions in the future, otherwise it makes ongoing changes difficult to justify.

FWIW, the two dropped commits are pure optimizations (they should have zero effect on the outcome, or the number of iterations performed; just on the amount of time spent per iteration).

I'll add an overview of the expected impact of each commit in this PR description.

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2302746644)
@sipa sure, but if we're weighing a change to this, it still matters as it may modify how many iterations we're willing to do

🤔 instagibbs reviewed a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2251681805)
reviewed through bbec8e261467637ce26ecf551b528d9061cf4ffe

via `git range-diff master 388c2f5d7ee5f38cbefaff0c85cf298083127299 bbec8e261467637ce26ecf551b528d9061cf4ffe`

LGTM aside from waiting on fuzzer coverage results + fixes

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1725592055)
nit: just assert it's in orphanage instead

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2302806417)
reACK 12760a57b3a6cd1aeb3b7532311f648de2b45aa4

via `git range-diff master 1b8f143fb791fe5ba40ce6610cd4e8e04b951495 12760a57b3a6cd1aeb3b7532311f648de2b45aa4`

💬 fjahr commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#issuecomment-2302873162)
ACK 59ff17e5af4e382cbe16f183767beef1bdcd9131

Reviewed the code and verified that the newly added tests do actually test the change in behavior.

💬 maflcko commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1725663019)
Not sure about this. To catch the seed, logging should be enabled (and it is enabled in all CI tasks). (This isn't changed in this pull request, so the comment seems tangential)

If the burden is put on each test that uses any kind of randomness to capture and memorize the seed, and print it on failure, it will be spotty at best and confusing, verbose and inconsistent in general.

If you are worried that the seed could be missed, I share your concern, but a better or more general fix would b
...