💬 Sjors commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1545737922)
Partial utACK for the changes in `CompareDepthAndScore`. Took me a while to wrap my head around it.

💬 instagibbs commented on pull request "test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595)":
(https://github.com/bitcoin/bitcoin/pull/27631#issuecomment-1545744936)
IIUC the function of that data push is just to "randomize" the sighash, so I think dropping size 1 from the range is reasonable, maybe leave a comment as such if my intuition is correct?

📝 MarcoFalke opened a pull request: "test: Return dict in MiniWallet::send_to"
(https://github.com/bitcoin/bitcoin/pull/27640)
Returning a tuple has many issues:

* If only one value is needed, it can not be indexed by name
* If another value is added to the return value, all call sites need to be updated

Byte the bullet now and update all call sites to fix the above issues.

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1192388557)
done

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1545766429)
I'm clean up history later when we're closer to ACKs

💬 Sjors commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1192393157)
> "should be considered" for what

For processing, which begins by dropping a transaction from `m_tx_inventory_to_send` before deciding whether to send it. Since we stop processing after having sent `broadcast_max` transactions, it makes sense to do the cleaning first.

A return value of `true` means hasha is lower, i.e. would be at the bottom of the heap and processed last. But because we swap a and b when calling this, it works. That was introduced in the second commit of #7840.


We co
...

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1192406738)
This "warmup" should really only be an issue after we've seen new blocks from 3+ peers since we don't persist hb peers across restarts and we have to wait for `MaybeSetPeerAsAnnouncingHeaderAndIDs` to be called on multiple unique peers. I now suspect the warmup period handles itself? My logs show 4+ blocks per node with only one partial block.

👍 hebasto approved a pull request: "wallet: fix deadlock in bdb read write operation"
(https://github.com/bitcoin/bitcoin/pull/27556#pullrequestreview-1424569448)
ACK de202f860b3f79774d5235dec59a140fa5c82fa7, tested on Ubutnu 23.04.

> To replicate it, need bdb support and drop the first commit. The test will run forever.

The test is deadlocked with the second commit dropped as well.

💬 sangaman commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1545805593)
So far my results with `MALLOC_ARENA_MAX=1` seem to be good, I've been comfortably under 2GB RAM every time I've checked. I added it as an env var in my systemd configuration which I copied and modified from the one in this repo, I figure it ought to have that env var as well. I'll open a PR to add it.

📝 sangaman opened a pull request: "init: add MALLOC_ARENA_MAX=1 to systemd"
(https://github.com/bitcoin/bitcoin/pull/27641)
This adds the `MALLOC_ARENA_MAX=1` environment variable as suggested in https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-memory.md#linux-specific to the systemd service file definition.

Without this env var, memory usage can grow significantly especially when `rpcthreads` is increased above its default value.

Closes #24542.

I have tested this change myself with positive results after dealing with memory consumption issues for a long time using systemd on a 8GB RAM raspi4. I fig
...

💬 theStack commented on pull request "test: Return dict in MiniWallet::send_to":
(https://github.com/bitcoin/bitcoin/pull/27640#issuecomment-1545824983)
Concept ACK

💬 willcl-ark commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1545830080)
I think setting this to 1 will give significantly worse performance for most users though?

Seems like it would be better to add it to the documentation if anything...

👍 brunoerg approved a pull request: "test: Return dict in MiniWallet::send_to"
(https://github.com/bitcoin/bitcoin/pull/27640#pullrequestreview-1424649982)
crACK faf4315c88d8c81c2ff84870bc81aef3cf719816

💬 furszy commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1545855845)
Is it me or DrahtBot is typing as a human?

💬 RandyMcMillan commented on pull request "p2p: skip netgroup diversity follow-up":
(https://github.com/bitcoin/bitcoin/pull/27467#discussion_r1192468422)
`to be two`

https://github.com/jonatack/bitcoin/pull/1

💬 jamesob commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1545856135)
I've done a number of runs here with some combination of indexing and pruning, and I think this is in about as good a shape as it can be, with the following caveat:

When using assumeutxo with pruning and indexing, the snapshot chainstate cannot be pruned because of how we handle indexing during bg sync. Since the snapshot chainstate isn't indexed until after the background sync completes (in order to avoid out-of-order indexation), we aren't able to prune the snapshot chainstate based upon th
...

✅ sangaman closed a pull request: "init: add MALLOC_ARENA_MAX=1 to systemd"
(https://github.com/bitcoin/bitcoin/pull/27641)

💬 jonatack commented on pull request "p2p: skip netgroup diversity follow-up":
(https://github.com/bitcoin/bitcoin/pull/27467#discussion_r1192498239)
Thank you for looking -- "to" (as in "toward") is correct here.

💬 mzumsande commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1192502249)
> Any reason not to send `TX` directly and avoid the `INV`, like below?

It has been suggested to stop processing unrequested TXs messages, see #21224. While that didn't make it, the idea is still floating around. If bitcoin core itself started relying on unrequested msgs being processed, that would probably kill it once for all.
Maybe that's ok, but we should make sure that there aren't alternative clients that would ignore unrequested TXs.

📝 sangaman opened a pull request: "init: add MALLOC_ARENA_MAX=1 to systemd"
(https://github.com/bitcoin/bitcoin/pull/27642)
This adds the MALLOC_ARENA_MAX=1 environment variable as suggested in https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-memory.md#linux-specific to the systemd service file definition.

Without this env var, memory usage can grow significantly especially when rpcthreads is increased above its default value.

Closes https://github.com/bitcoin/bitcoin/issues/24542.

I have tested this change myself with positive results after dealing with memory consumption issues for a long time usi
...