💬 jamesob commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#discussion_r1189039363)
Fixed.

💬 jamesob commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#discussion_r1189039290)
Fixed, and I've dropped the commit in question.

🤔 jamesob reviewed a pull request: "assumeutxo: net_processing changes"
(https://github.com/bitcoin/bitcoin/pull/24008#pullrequestreview-1419316384)
I've udpated this PR to remove the new `ChainstateManager::GetAllForBlockDownload()`, instead adding a parameter `assumed_first` to the existing `GetAll()`. I think this is clearer at the net_processing call site and avoids another unnecessary method.

👍 instagibbs approved a pull request: "p2p: Avoid prematurely clearing download state for other peers"
(https://github.com/bitcoin/bitcoin/pull/27608#pullrequestreview-1419317015)
ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4

💬 jamesob commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1540768765)
> Also, as far as I understand it, AssumeUTXO will mess up the order of the blocks stored on disk - the first blockfiles would have the blocks between the AssumeUTXO block and the tip, and the subsequent blockfiles would have the blocks between Genesis and the AssumeUTXO block. I wonder if that could break or slow down something, have you tried a reindex for example?

It's worth noting that there's a proposed remedy for this in #27596 - see https://github.com/bitcoin/bitcoin/pull/27596/commits
...

💬 st3b1t commented on pull request "rpc: append rpcauth.py hash in config and show pass":
(https://github.com/bitcoin/bitcoin/pull/27588#issuecomment-1540770214)
> Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

done, the resultant commit, only two files:
https://github.com/bitcoin/bitcoin/pull/27588/commits/e1046eca9d637dfd29599d156a57a8a3a7440b23

💬 sr-gi commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1540782565)
Addressed comments by @MarcoFalke and fixed a couple of logs from the tests

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1189064293)
> This is a slightly different approach to -fastprune and others, which are set in ApplyArgsManOptions().
So I wonder what is the criterion for an arg to be set in ApplyArgsManOptions(), as opposed to being initialized directly?

Initializing in the list is done for option members that don't have sane default or empty values. This also has precedent in the `ChainstateManager::Options`.

> And, closely connected, isn't the current approach of not calling ApplyArgsManOptions() in some spots
...

👍 theuni approved a pull request: "add ryanofsky to trusted-keys"
(https://github.com/bitcoin/bitcoin/pull/27604#pullrequestreview-1419398801)
ACK 59ebee3fb4181baf20fab263cf1b587ece1bd5e2.

🤔 fjahr reviewed a pull request: "p2p: Avoid prematurely clearing download state for other peers"
(https://github.com/bitcoin/bitcoin/pull/27608#pullrequestreview-1419469898)
Code review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4

💬 fjahr commented on pull request "p2p: Avoid prematurely clearing download state for other peers":
(https://github.com/bitcoin/bitcoin/pull/27608#discussion_r1189141220)
nit: Could have moved the lock outside the `if..else` and removed the duplicate line in the `else` below.

📝 glozow opened a pull request: "rpc: allow submitpackage to be called outside of regtest"
(https://github.com/bitcoin/bitcoin/pull/27609)
Suggested in https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1510851570

This RPC is safe (no DoS issues or big bugs AFAIK) but still experimental - interface may change, not all features (e.g. package RBF) are implemented, etc. If a miner wants to expose this to people, they can effectively use "package relay." Obviously it should be protected by some kind of rate limit; we don't cache rejections for stuff from RPC.

💬 jamesob commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1541020239)
Concept ACK

🤔 mzumsande reviewed a pull request: "p2p: Avoid prematurely clearing download state for other peers"
(https://github.com/bitcoin/bitcoin/pull/27608#pullrequestreview-1419557634)
Code Review ACK 52e52071e01f4e98d87a47e1d5f3c5c3cc6dbaf4

💬 mzumsande commented on pull request "p2p: Avoid prematurely clearing download state for other peers":
(https://github.com/bitcoin/bitcoin/pull/27608#discussion_r1189204222)
nit: requested **from** another peer

📝 ajtowns opened a pull request: "Improve performance of p2p inv to send queues"
(https://github.com/bitcoin/bitcoin/pull/27610)
Couple of performance improvements when draining the inventory-to-send queue:

* drop txs that have already been evicted from the mempool (or included in a block) immediately, rather than at the end of processing
* marginally increase outgoing trickle rate during spikes in tx volume

💬 ajtowns commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1189250200)
Ah, I see what you mean. That makes sense.

Next point on those lines though: adding many things to `m_recently_announced_invs` can overflow the bloom filter, either causing false positives (leading to privacy bugs), or since it's a rolling bloom filter, causing overflow leading to false negatives (leading to functionality bugs where you can't get the txs from the MEMPOOL response). Wouldn't it be better to make the insertion conditional on `txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY` ?
...

💬 theStack commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1541097841)
For the reporters using [htop](https://htop.dev/) to analyze CPU usage (which is the case for at least @poiuty @gits7r, according to the posted screenshots): could you go to the display settings (reachable via key F2) and activate "tree view" and the global option "show custom thread names"? This would show the CPU usage per thread, which could be interesting for further investigation, e.g.:


(FWI
...

💬 ajtowns commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1541197101)
I think #27610 should improve the non-debug cpu usage in b-msghand. Both commits should cleanly cherry-pick for on top of v24.x; and the first commit (the "CompareDepthAndScore" one) should cleanly cherry-pick for v23.x.

👍 jarolrod approved a pull request: "add ryanofsky to trusted-keys"
(https://github.com/bitcoin/bitcoin/pull/27604#pullrequestreview-1419713726)
ACK https://github.com/bitcoin/bitcoin/commit/59ebee3fb4181baf20fab263cf1b587ece1bd5e2