🤔 glozow reviewed a pull request: "refactor: use existing RNG object in ProcessGetBlockData"
(https://github.com/bitcoin/bitcoin/pull/30393#pullrequestreview-2166326876)
ACK fa2e74879a3f7423c8d01aa1376b2bd9ccf5658d

🤔 furszy reviewed a pull request: "net: fix race condition in self-connect detection"
(https://github.com/bitcoin/bitcoin/pull/30394#pullrequestreview-2166329252)
Code review ACK fb587ce36afcc8789214af45a36082c4f5238e50

Nice finding.

🚀 glozow merged a pull request: "refactor: use existing RNG object in ProcessGetBlockData"
(https://github.com/bitcoin/bitcoin/pull/30393)

📝 dergoegge opened a pull request: "fuzz: Version handshake"
(https://github.com/bitcoin/bitcoin/pull/30413)
This adds a fuzzing harness dedicated to the version handshake. To avoid determinism issues, the harness creates necessary components each iteration (addrman, peerman, etc). A harness like this would have caught easily https://bitcoincore.org/en/2024/07/03/disclose-timestamp-overflow/.

As a performance optimization, this PR includes a change to `PeerManager` to lazily initialize various filters (to avoid large unnecessary memory allocations each iteration).

👍 dergoegge approved a pull request: "test: [refactor] Pass TestOpts"
(https://github.com/bitcoin/bitcoin/pull/30407#pullrequestreview-2166443256)
utACK fa690c8e532672f7ab53be6f7a0bb3070858745e

💬 ismaelsadeeq commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1670578710)
Why pass `finalize=false`? the test still passes without it.

🤔 ismaelsadeeq reviewed a pull request: "Fix cases of calls to `FillPSBT` errantly returning `complete=true`"
(https://github.com/bitcoin/bitcoin/pull/30357#pullrequestreview-2166405948)
Tested ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902

I've verified that this PR fixes the issue #30077, `complete=false` is returned instead of throwing an error.

Just a single test question and a non-blocking nit.

💬 ismaelsadeeq commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1670582419)
nit
```suggestion
assert_equal(signed_psbt_incomplete["complete"], False)

```

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670609997)
Hmm yes because we no longer erase in `BatchWrite`, since we don't pass the `coinsCache` map anymore.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670611362)
I can expand to `m_current` if that's more readable? Or do you have a better suggestion?

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670612269)
I am deliberately matching the style used where I am making changes, yes.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670613036)
Tests will fail if they are not all cleared, try it :).

💬 maflcko commented on pull request "fuzz: Version handshake":
(https://github.com/bitcoin/bitcoin/pull/30413#issuecomment-2217865662)
Concept ACK, but would be good to explain how this is different from the existing approach, introduced in "fuzz: version handshake" https://github.com/bitcoin/bitcoin/pull/20370, which was a replacement for "fuzz: Version handshake" https://github.com/bitcoin/bitcoin/pull/20097 .


Now that the timedata bug is fixed, and the module and related globals removed completely, it is a bit hard(er) to re-introduce it for testing, I guess. IIRC I did check it at some point and found that the existing
...

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670615264)
The developer guides say to prefer using the recommended style over mimicking the surrounding style.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670617785)
I kept this here intentionally to make the diff easier to review. We don't have to consider whether it breaks anything if we move it up.

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670623673)
If you decide to keep it, yes. Otherwise removing it seems like a better option to me.

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670623930)
Thanks for checking!

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670635101)
nit: you haven't touched this line, but Sonar complains that: `"std::move" should not be called on an object of a type with neither move constructor nor move-assignment operator.` - ignore it, if it's unrelated (the rest from https://corecheck.dev/bitcoin/bitcoin/pulls/28280 are also not very useful, but I can't judge the severity of this one)

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1670639971)
Hmm but `CCoinsCacheEntry` does have a move constructor https://github.com/bitcoin/bitcoin/pull/28280/files#diff-095ce1081a930998a10b37358fae5499ac47f8cb6f25f5df5d88e920a54e0341R156.

💬 dergoegge commented on pull request "fuzz: Version handshake":
(https://github.com/bitcoin/bitcoin/pull/30413#issuecomment-2217899733)
> would be good to explain how this is different from the existing approach, introduced in "fuzz: version handshake" https://github.com/bitcoin/bitcoin/pull/20370, which was a replacement for "fuzz: Version handshake" https://github.com/bitcoin/bitcoin/pull/20097 .

The main difference is that this harness focuses on the version handshake, where a modification to the existing `process_messages` harness would further widen the space of reachable code. It could make sense to have both but I woul
...