💬 achow101 commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#issuecomment-2302595244)
ACK 917e70a6206c62c4c492fa922425fc8e00d3f328

💬 l0rinc commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2302599944)
ACK c139a788e0052ece8f6c5689e4cd04b406032875

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