💬 maflcko commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1670449484)
```suggestion
self.wait_until(lambda: len(node.getpeerinfo()) == 0)
```

nit in the last commit: I am not a fan of using `assert_debug_log` to sync the test logic. Otherwise the test may fail on slow machines.

If you want to keep it, please make the sync explicit by increasing the timeout of `assert_debug_log`. Otherwise, you can do a proper wait (as suggested) by moving the `wait_until` into the inner scope.

💬 maflcko commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1670450225)
```suggestion
peer->m_initial_outbound_version_message_sent = true;
```

nit: Could clarify the name

💬 maflcko commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2217564380)
ACK fb587ce36afcc8789214af45a36082c4f5238e50

💬 dergoegge commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#discussion_r1670484095)
This should nuke the whole parent tmp dir that was created for the fuzz test, not just `init_datadir`. I'm seeing tons of left over directories otherwise.

💬 dergoegge commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#discussion_r1670126790)
Trying to choose probabilities like this won't really work with modern fuzzing engines (because the inputs given to the harness are not uniformly random) and is probably equivalent to the following:

```suggestion
const bool is_random{fuzzed_data_provider.ConsumeBool()};
```

📝 glozow opened a pull request: "MiniMiner: use FeeFrac in AncestorFeerateComparator"
(https://github.com/bitcoin/bitcoin/pull/30412)
Fixes crash in #30367. See https://github.com/bitcoin/bitcoin/issues/30367#issuecomment-2217459257

Previously, we were only comparing feerates up to 1/1000 precision, since CFeeRate comparison just looks at their respective nSatoshisPerK. This could lead to MiniMiner selecting packages in the wrong order (i.e. by txid) if their feerates were less than 0.001sat/vB different. Fix this by creating + comparing `FeeFrac`s instead.

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