🚀 achow101 merged a pull request: "test/BIP324: disconnection scenarios during v2 handshake"
(https://github.com/bitcoin/bitcoin/pull/29431)

💬 furszy commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2218737619)
> When Bitcoin Core makes an outgoing connection after this patch, it won't send out the version message immediately, so it could receive a version message before sending one. In that case, I think that ProcessMessage could send out other messages (WTXIDRELAY etc.) before sending the version message, which would be a protocol violation.

Can we get a test for this? Fixes are great but tests are awesome to not break it again in the future.

💬 mzumsande commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#issuecomment-2218739280)
> This seems like a good change in principle, but I'm not actually sure what additional test coverage it provides. The only valid snapshot that can be loaded seems to be one that is hardcoded for a chain with height 200 and no transactions except for coinbases. After the snapshot is generated there is only a single `ConsumeBool` call determining whether or not headers for the blocks are added before loading the snapshot.

Note that loading a valid snapshot is not the only goal of this PR. It a
...

💬 mzumsande commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2218749965)
> Can we get a test for this? Fixes are great but tests are awesome to not break it again in the future.

Our own test framework was behaving that way before #29353, so maybe the functional tests would have caught it? I thought about reverting c340503b67585b631909584f1acaa327f5af25d3 locally to find out, but didn't have the time yet.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1671260580)
I don't think I should touch this here, it's unrelated.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1671260720)
Done, thanks!

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1671261223)
I don't think it will add anything to have a test for that, no.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1671261364)
Done.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2218763464)
@paplorinc thank you again for your review! I addressed all your nits I think. I don't think that benchmark is related, could just be noise.

💬 achow101 commented on pull request "random: add benchmarks and drop unnecessary Shuffle function":
(https://github.com/bitcoin/bitcoin/pull/30396#issuecomment-2218778250)
ACK 6ecda04fefad980872c72fba89844393f5581120

💬 ryanofsky commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#issuecomment-2218790880)
Rebased 179a0715cdcb3f20f71bf757c37046b0675dc8a5 -> d34b529ed104a3a7e11c7d264703e61d84b1aeb3 ([`pr/alert.1`](https://github.com/ryanofsky/bitcoin/commits/pr/alert.1) -> [`pr/alert.2`](https://github.com/ryanofsky/bitcoin/commits/pr/alert.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/alert.1-rebase..pr/alert.2)) due to silent conflict with #29625

🚀 achow101 merged a pull request: "random: add benchmarks and drop unnecessary Shuffle function"
(https://github.com/bitcoin/bitcoin/pull/30396)

💬 dergoegge commented on pull request "fuzz: Version handshake":
(https://github.com/bitcoin/bitcoin/pull/30413#issuecomment-2218807473)
http://bitcoind-fuzz.dergoegge.de:8000/bitcoin-30413/harnesses/p2p_handshake/coverage_report/coverage/workdir/bitcoin/src/net_processing.cpp.html#L3702

💬 mzumsande commented on pull request "rpc, rest: Improve getblock error when only header is available":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1671297840)
done. Performance doesn't seem that important, but I also think it's a bit clearer that way.

🤔 tdb3 reviewed a pull request: "net: Allow -proxy=[::1] on nodes with IPV6 lo only"
(https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2167594504)
I think this is really close. Left one more comment.

💬 tdb3 commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1671300188)
Unless I'm overlooking something, might be able to simplify/shorten the change by calling `getaddrinfo()` again when `n_err` is non-zero. Would avoid the lambda function and might also get rid of the standalone `ai_flags`.

Something like:

```c++
addrinfo* ai_res{nullptr};
const int n_err{getaddrinfo(name.c_str(), nullptr, &ai_hint, &ai_res)};
if (n_err != 0 && (ai_hint.ai_flags & AI_ADDRCONFIG) == AI_ADDRCONFIG) {
// AI_ADDRCONFIG on some systems may exclude loopback-only addresse
...

📝 furszy opened a pull request: "assumeUTXO: snapshot load, update VERSION msg height"
(https://github.com/bitcoin/bitcoin/pull/30418)
Due to a missing height update; the node sends its pre-snapshot height to
any peer connecting after the snapshot load but before the node receive
a new tip block.

This does not make any difference for us because we are not using the
`VERSION` msg height field but it does not seem consistent plus other
software might be using the field.

💬 mzumsande commented on pull request "rpc, rest: Improve getblock error when only header is available":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2218826297)
> However, you forgot GetBlockChecked (and all the other functions that can fail in the same way)?

Updated to change all functions I could find. I was only looking at `getblock` before, not at other RPCs.

>My preference to fix it would be to move the checks to the inside of the block manager.

> However, this basically requires a full rewrite of the blockmanager read-interface, so I haven't completed it yet. (Happy to pick it up again, if you think it makes sense as well)

Makes sense
...

💬 ryanofsky commented on pull request "kernel, logging: Pass Logger instances to kernel objects":
(https://github.com/bitcoin/bitcoin/pull/30342#issuecomment-2218833239)
Rebased db1b9f7696af80036de739408cd9eae5d06481ec -> 42d0e06d1a4343c2bf3257f26cf18b79072c6b0d ([`pr/gklog.2`](https://github.com/ryanofsky/bitcoin/commits/pr/gklog.2) -> [`pr/gklog.3`](https://github.com/ryanofsky/bitcoin/commits/pr/gklog.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/gklog.2-rebase..pr/gklog.3)) to fix conflict with #30141. Also added commit to let kernel applications direct low-level util output to custom log instances.

💬 achow101 commented on issue "Send RPC calls performance":
(https://github.com/bitcoin/bitcoin/issues/30416#issuecomment-2218834962)
> #26008 may relate? I am not sure if it was included in 27.0 release

That went into 27.0, and seems like a probable candidate for the performance increase. However, I am a little bit skeptical since it should primarily benefit wallets with a ton of descriptors, and yours only has 24.

Maybe it has a less noticeable benefit for descriptors since the cache is an `std::unsorted_map` while each descriptor has a `std::map` for the scripts.

***

I would have expected that the number of tran
...