🚀 ryanofsky merged a pull request: "rpc: move UniValue in blockToJSON"
(https://github.com/bitcoin/bitcoin/pull/30094)

💬 achow101 commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2108732705)
> For frequent pruning flushes there's no need to empty the cache and kill connect block speed. However, simply using `Sync` in place of `Flush` actually slows down a pruned full IBD with a high `dbcache` value. This is because as the cache grows, sync takes longer since every coin in the cache is scanned to check if it's dirty. For frequent prune flushes and a large cache this constant scanning starts to really slow IBD down, and just emptying the cache on every prune becomes faster.
>
> To
...

🚀 achow101 merged a pull request: "validation: don't clear cache on periodic flush: >2x block connection speed"
(https://github.com/bitcoin/bitcoin/pull/28233)

💬 theuni commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2108751842)

> > I would be great to check all UniValue::push_back and UniValue::pushKV invocations in the codebase.
>
> I can check the remainder of the codebase for other occurrences too.

I was already working on this and... wow. You've opened a big can of worms here @willcl-ark :). But that's a good thing because it should reduce memusage and increase performance I'd assume.

See here for my first pass. It's a beast. https://github.com/theuni/bitcoin/commits/less-univalue-copies/

The first co
...

💬 Rob1Ham commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2108752829)
tACK https://github.com/bitcoin/bitcoin/commit/06c2c713c52b60231efc3e00d2c5eb0bf9e345f9

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2108754056)
@achow101 That works, but it misses the point. If you delete the dirty entries on a prune flush, then those entries need to be re-read from disk when they are (likely soon) spent.

💬 achow101 commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2108756050)
> @achow101 That works, but it misses the point. If you delete the dirty entries on a prune flush, then those entries need to be re-read from disk when they are (likely soon) spent.

Sorry, I meant clear the Dirty and spent ones.

💬 jonatack commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1599061492)
Commit 48dc2ff8e7819e810d2436219268f220b39c14f

in fc0a50e92aaaf3b049fcfcd0873dadc7150cfecf I made this one `info` (default) level [here](https://github.com/bitcoin/bitcoin/pull/25203/commits/fc0a50e92aaaf3b049fcfcd0873dadc7150cfecf#diff-8945a03581b372d674f34dcb6a63e537343e376ebcd9d8c7c3d94d4366cd9b9dR379). It seems important, when the user turns `i2p` logging on, to see if the session was successfully created. I look for this logging when launching a node or restarting the I2P router.

💬 theuni commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#discussion_r1599061542)
This will just be forwarded to the copy ctor. `push_back(std::move(txout))` would be more correct (and clear).

💬 jonatack commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1599062889)
same here as https://github.com/bitcoin/bitcoin/pull/29833/files#r1599061492

💬 jonatack commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1599063683)
same comment here as https://github.com/bitcoin/bitcoin/pull/29833/files#r1599061492

💬 jonatack commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1599069512)
(Missing space here?)

```suggestion
LogPrintLevel(BCLog::I2P, BCLog::Level::Error, "Couldn't accept %s: %s\n", disconnect ? " (will close the session)" : "", errmsg);
```

💬 jonatack commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#issuecomment-2108786764)
If the third commit is re-changing lines that are already changed in the first commit, would suggest combining them.

💬 theStack commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1599046868)
nit: seems like the second parameter (`std::error_code{}`) isn't needed (here and in all other instances below)
```suggestion
throw std::ios_base::failure("Invalid UTXO set snapshot magic bytes. Please check if this is indeed a snapshot file or if you are using an outdated snapshot format.");
```

💬 theStack commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1599077443)
nit: could use our fancy `Join` helper here (available in util/string.h)
```suggestion
std::string heights_formatted = Join(available_heights, ", ", [&](const auto& i) { return ToString(i); });
```

🤔 jonatack reviewed a pull request: "i2p: fix and improve logs"
(https://github.com/bitcoin/bitcoin/pull/29833#pullrequestreview-2053826139)
(reposting using the "review changes" button to perhaps appease the bot :)

ACK modulo comments above. If the third commit is re-changing lines that are already changed in the first commit, would suggest combining them.

👍 TheCharlatan approved a pull request: "depends: set AR & RANLIB for CMake"
(https://github.com/bitcoin/bitcoin/pull/30078#pullrequestreview-2053850771)
ACK 019ad7327c397094d7648b55503bf5373b108a57

Guix build (aarch64):
```
bdbb759d06e9766c5aa29a9ee1cea6f021d50618e1abe4732ae0120c4b444829 guix-build-019ad7327c39/output/aarch64-linux-gnu/SHA256SUMS.part
82aa7b4af365dde09b7fe435bd20432aa59168cb448f7a8cb898a8acb6178453 guix-build-019ad7327c39/output/aarch64-linux-gnu/bitcoin-019ad7327c39-aarch64-linux-gnu-debug.tar.gz
93c539e8c42ff767e46fc3f501cf9d543b954202372c930c177ff22eea6037f5 guix-build-019ad7327c39/output/aarch64-linux-gnu/bitcoin-0
...

📝 theStack opened a pull request: "refactor: simplify `FormatSubVersion` using strprintf/Join"
(https://github.com/bitcoin/bitcoin/pull/30098)
Rather than using std::ostringstream and manually joining the comments, use strprintf and our own `Join` helper.

👍 AngusP approved a pull request: "p2p: index TxOrphanage by wtxid, allow entries with same txid"
(https://github.com/bitcoin/bitcoin/pull/30000#pullrequestreview-2053871488)
tACK 16483fee7c6d93722bfb79fce9efbe841ec13d6a with some nits

💬 AngusP commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1599118360)
Nit: given the discussion in the PR, you could extend this with a note on why we might still want to keep multiple same-txid-diff-witness transactions in the orphanage

```suggestion
// It is possible that the transaction in the orphanage has the same txid but a different
// witness (e.g. malleated by an attacker) and we don't want to return false positives,
// nor can we tell which of the same-txid-different-witness transactions in the orphanage
// could be evicted without
...