👍 rkrux approved a pull request: "rpc: getorphantxs follow-up"
(https://github.com/bitcoin/bitcoin/pull/31043#pullrequestreview-2397501918)
tACK 0ea84bc362f395fd247623c22942eb5ca3d1b874

👍 itornaza approved a pull request: "rpc: getorphantxs follow-up"
(https://github.com/bitcoin/bitcoin/pull/31043#pullrequestreview-2397502622)
tACK 0ea84bc362f395fd247623c22942eb5ca3d1b874

Tested against all of the standard test and extended harness. Everything looks great!

👍 laanwj approved a pull request: "Drop miniupnp dependency"
(https://github.com/bitcoin/bitcoin/pull/31130#pullrequestreview-2397521287)
Code review ACK 40e5f26a3ff77e50df808f6f850c617aec2df203
i have also checked that:
- no mentions of miniupnp as a dependency remain
- `-natpmp` still works to forward ports on my router after this
- `-upnp`, if still present in the configuration file or command line, gives a custom message and exits

💬 sipa commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818083019)
I think it suffices to have a `bool m_have_changeset GUARDED_BY(cs){false};`, as it's only used for tracking and enforcing the "no two changesets" rule?

💬 sipa commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818083695)
Nit, if it's a member class of `CTxMempool` anyway, maybe a less clunky name like `ChangeSet` suffices?

💬 sipa commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818086982)
Style: use braces/newlines/indentation when the entire `if` statement doesn't fit on a single line.

📝 andrewtoth converted_to_draft a pull request: "validation: fetch block inputs on parallel threads ~17% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/31132)
When fetching inputs in ConnectBlock, each input is fetched from the cache in series. A cache miss means a round trip to the disk db to fetch the outpoint and insert it into the cache. Since the db is locked from being written during ConnectTip, we can fetch all block inputs missing from the cache on parallel threads before entering ConnectBlock. Using this strategy resulted in a ~17% faster IBD (or master was ~21% slower).

Doing IBD with 16 vcores from a local peer with default settings, sto
...

💬 sipa commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818101205)
I do find this function a bit confusing; it seems like `m_ancestors` is effectively a cache for the result of `m_pool->CalculateMempoolAncestors()`, but then I'd expect this function to also act like a cache?

How about:

```c++
util::Result<CTxMemPool::setEntries> CalculateMemPoolAncestors(TxHandle tx, const Limits& limits)
{
LOCK(m_pool->cs);
auto it = m_ancestors.find(tx);
util::Result<CTxMemPool::setEntries> ret;
if (it == m_ancestors.end()) {
ret = m_pool-
...

📝 theStack opened a pull request: "key: clear out secret data in `DecodeExtKey`"
(https://github.com/bitcoin/bitcoin/pull/31166)
Same as in `DecodeSecret`, we should also clear out the secret data from the vector resulting from the Base58Check parsing for xprv keys. Note that the if condition is needed in order to avoid UB, see #14242 (commit d855e4cac8303ad4e34ac31cfa7634286589ce99).

👍 i-am-yuvi approved a pull request: "Drop miniupnp dependency"
(https://github.com/bitcoin/bitcoin/pull/31130#pullrequestreview-2397573025)
Tested ACK 40e5f26a3ff77e50df808f6f850c617aec2df203

- -upnp gives custom messages and exits
- -natpmp works

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r1818140297)
I tried this and it causes memory errors, since the remaining futures will have a dangling ref to `m_condition`.

💬 mzumsande commented on issue "Stop at header":
(https://github.com/bitcoin/bitcoin/issues/31162#issuecomment-2440142364)
For what would you need to use this kind of feature? This probably wouldn't be as simple to implement as current `-stopatheight` (one would need to keep downloading blocks after the last header was received but not accept any more headers), so I think that there should be a serious use case.

💬 kevkevinpal commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1818164107)
Thanks!

Updated in d7fd766feb2f579bdba0e778bacdeb13103e8282

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818172232)
Done.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818172279)
Done.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818172329)
No longer relevant after rewriting this function.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818173833)
I reworked your suggestion to capture the caching idea, and now invoke this function in `Apply`. However, there's still an ugly special case in `Apply()` in that we can only used a cached ancestor set calculation for the first transaction in a package (because the calculation is no longer correct once transactions start getting added to the mempool).

Note that in the cluster mempool implementation, this ugliness in `Apply()` goes away, because we will no longer need to have ancestor sets cal
...

👋 l0rinc's pull request is ready for review: "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`"
(https://github.com/bitcoin/bitcoin/pull/31144)

⚠️ Bahramgho opened an issue: "BTC"
(https://github.com/bitcoin/bitcoin/issues/31170)
1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa

✅ pinheadmz closed an issue: "BTC"
(https://github.com/bitcoin/bitcoin/issues/31170)