🚀 ryanofsky merged a pull request: "rpc: Use untranslated error strings in loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/30395)

💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1671033583)
Fixed

💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2218456437)
> I'm not sure if it's the intended behavior, but passing the same `NUM` twice to `importdescriptors` does not fail and ends up with a single _change_ descriptor:

Good catch.

The BIP does not specify either way yet, but I don't think there's any reason to allow duplicates here, so I've implemented that and added a test.

💬 achow101 commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#issuecomment-2218543678)
ACK ad1e3620af88f73b869b4a2dbb74e03f5cd93517

💬 theStack commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2218643656)
The unit test failure in the CI was caused by the [`ConnmanTestMsg::Handshake`](https://github.com/bitcoin/bitcoin/blob/c06b3764fe38de90e13261213dca34d93ec07f18/src/test/util/net.cpp#L20) helper (used in the unit test [`outbound_slow_chain_eviction`](https://github.com/bitcoin/bitcoin/blob/c06b3764fe38de90e13261213dca34d93ec07f18/src/test/denialofservice_tests.cpp#L45)), where a version handshake is done manually by calling the relevant connman/peerman methods. Due to a missing call to `SendMess
...

🚀 ryanofsky merged a pull request: "fuzz: improve utxo_snapshot target"
(https://github.com/bitcoin/bitcoin/pull/30329)

💬 achow101 commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#issuecomment-2218656952)
ACK c9dacd958d7c1e98b08a7083c299d981e4c11193

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1671191613)
https://delvingbitcoin.org/t/introduction-to-cluster-linearization/1032

🤔 ryanofsky reviewed a pull request: "fuzz: improve utxo_snapshot target"
(https://github.com/bitcoin/bitcoin/pull/30329#pullrequestreview-2167379330)
Post-merge code review ACK de71d4dece604907afc4fc26b7788e9c1a4cbecb

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.

So
...

💬 ryanofsky commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#discussion_r1671160670)
In commit "fuzz: improve utxo_snapshot target" (de71d4dece604907afc4fc26b7788e9c1a4cbecb)

Since fuzz test is not deterministic, and doesn't give us an easy way to know hash values here actually allow snapshot to be loaded, it might be good to write a separate regtest generating a an empty chain with height 200 and verifying these hashes.

🚀 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