👍 pinheadmz approved a pull request: "net: Allow -proxy=[::1] on nodes with IPV6 lo only"
(https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2178514340)
ACK 23333b7ed243071c9b4e4f04c727556d8065acbb

Reviewed updated code change and tested in linux VM with local ipv6 only.

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 23333b7ed243071c9b4e4f04c727556d8065acbb
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmaVfcgACgkQ5+KYS2KJ
yTorBw/7BFoooqdMN+Zb000m/qKsUoMhW36vZBdGTuxt3xkWrRHlggm8ym/fQfen
jt9VcWYl6vmZFnU+nUvJbdN+GLTdf+fDT8vdJdYKjKSxmflEIGLRnf5Pktf2E
...

💬 pinheadmz commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1678310386)
If you need to retouch, you might consider using a local `addrinfo` variable for the second call since it serves a specific purpose only inside this if condition. It doesn't matter because it's not like it gets used again later, just something I noticed.

💬 hebasto commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2229281988)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/263.

💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1678338506)
Don't have strong feeling either way but there is an argument that doing it with one variable means any changes to flags passed to the first call get automatically added to the second call which is I think better default behaviour.

🤔 mzumsande reviewed a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2178557577)
Concept ACK - don't know the txrequest logic very well though.

The PR description should be updated (`UpdatedBlockTipSync` was renamed).

💬 mzumsande commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1678336506)
I think you meant `m_tx_download_mutex`

👍 ryanofsky approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2178579385)
Light code review ACK 408c11e9bced08c51a7645a2de2c39c18e4360d9. This looks good, but I didn't review the getCoinbaseMerklePath implementation closely to check that it is computing the right thing.

💬 ryanofsky commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1678349206)
In commit "Have createNewBlock return BlockTemplate interface" (4af3d9a483e4107301f0f0a42da781dfa79acb82)

Should be possible to avoid getBlockHeader calls by just using the `block` variable:

```diff
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -941,7 +941,7 @@ static RPCHelpMan getblocktemplate()
result.pushKV("vbavailable", std::move(vbavailable));
result.pushKV("vbrequired", int(0));

- result.pushKV("previousblockhash", block_template->getBlockHeader().hashP
...

💬 ryanofsky commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1678361713)
In commit "Add getCoinbaseMerklePath() to Mining interface" (1f5f16d166f8944d532aadc382b30f68a06720e2)

Instead of implementing this function in `node/interfaces.cpp` file it would be better to add a standalone `GetCoinBaseMerklePath(const CBlock&block)` function or a new `CBlock` method that the interface method could call. This would keep the `interfaces.cpp` file simple, and let it just contain glue code instead of more complicated functionality. It would also make the merkle path functiona
...

💬 achow101 commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2229460209)
c192b30156ae41638291010b40b874479ea1943c "clusterlin: add algorithms for connectedness/connected components" seems to be only relevant for the optimizations in #30286, so could be dropped from here?

👍 tdb3 approved a pull request: "test: Fix intermittent failure in p2p_v2_misbehaving.py"
(https://github.com/bitcoin/bitcoin/pull/30420#pullrequestreview-2178783087)
cr and t ACK c6d43367a1ec47c004991143f031417c4e4b8095
Repeated test in https://github.com/bitcoin/bitcoin/pull/30420#pullrequestreview-2168907582

💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2229618185)
Another update: After again seeing no data corruption for a while (since the last update above), five of the ten servers again got data corruption over a two day period. This reinforces my beliefs that (a) this data corruption depends on the contents of the blocks received by the bitcoind, and that whatever triggers it is currently relatively infrequent (about once every two weeks); and (2) the data corruption happens at some earlier time and is not detected by bitcoind until sometime later when
...

🤔 furszy reviewed a pull request: "index: Check all necessary block data is available before starting to sync"
(https://github.com/bitcoin/bitcoin/pull/29770#pullrequestreview-2179012886)
It would be nice to implement 81638f5d42b841 differently, without adding the `<chain.h>` dependency to all index headers. This dependency provides access to node internal types that indexes running on other processes (in the future) will not know about.

It seems we all implemented the same "index X requires block data" and "index Y requires block undo data" in slightly different ways. I did it in #26966 some time ago, and ryanofsky also did it in #24230.

My preference would be to follow #2
...

💬 furszy commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#discussion_r1678634382)
In 81638f5d42b841f:

This include isn't needed.

💬 www222fff commented on issue "scriptPubKey no address":
(https://github.com/bitcoin/bitcoin/issues/30450#issuecomment-2229827248)
yes, only output need scriptPubKeys

👍 tdb3 approved a pull request: "kernel: De-globalize static validation variables"
(https://github.com/bitcoin/bitcoin/pull/30425#pullrequestreview-2179090811)
code review ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d
Nice cleanup/prep.

💬 maflcko commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#issuecomment-2230122996)
ACK c6d43367a1ec47c004991143f031417c4e4b8095

💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1678927084)
I only used `getBlockHeader()` here to test the implementation, but it's indeed not really useful in this context.

✅ fanquake closed an issue: "ci: failure in `p2p_v2_misbehaving.py`"
(https://github.com/bitcoin/bitcoin/issues/30419)

🚀 fanquake merged a pull request: "test: Fix intermittent failure in p2p_v2_misbehaving.py"
(https://github.com/bitcoin/bitcoin/pull/30420)