💬 kevkevinpal commented on pull request "test: [move-only] binary utils to utils.py":
(https://github.com/bitcoin/bitcoin/pull/33633#discussion_r2433669132)
is there a reason why you moved this logic above this line of code?

`self.options.cachedir = os.path.abspath(self.options.cachedir)`

💬 kevkevinpal commented on pull request "test: [move-only] binary utils to utils.py":
(https://github.com/bitcoin/bitcoin/pull/33633#issuecomment-3407903290)
ACK [fa75ef4](https://github.com/bitcoin/bitcoin/pull/33633/commits/fa75ef4328f638221bcf85fcbefa885122084622)

Looks straightforward to me and makes sense to move those functions outside of the test framework to be used externally

💬 l0rinc commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2433680213)
The review effort would be the same for 2 or nproc threads, but the implications would be limited with the former - we cannot have e.g. complete deadlock if at most two threads are stuck on indexing. And the recent failures on CI indicate that deadlock concerns aren't unfounded.

⚠️ maflcko opened an issue: "Intermittent issue in feature_assumeutxo.py", line 629, in run_test assert not self.has_blockfile(n1, "00002"), "too many blockfiles""
(https://github.com/bitcoin/bitcoin/issues/33635)
Seen locally in https://drahtbot.space/temp_scratch/feature_assumeutxo_265.tar.zstd:

commit should be roughly 40e7d4cd0d7f1d922b92b0c640d3d89eef059411

```
node1 2025-10-15T18:56:48.763939Z [shutoff] [txdb.cpp:151] [virtual bool CCoinsViewDB::BatchWrite(CoinsViewCacheCursor &, const uint256 &)] [coindb] Writing final batch of 0.00 MiB
node1 2025-10-15T18:56:48.763948Z [shutoff] [txdb.cpp:153] [virtual bool CCoinsViewDB::BatchWrite(CoinsViewCacheCursor &, const uint256 &)] [coindb] Committed
...

💬 maflcko commented on pull request "test: [move-only] binary utils to utils.py":
(https://github.com/bitcoin/bitcoin/pull/33633#discussion_r2433693756)
i don't think the order matters, because the two lines are completely unrelated and independent

💬 kevkevinpal commented on pull request "test: [move-only] binary utils to utils.py":
(https://github.com/bitcoin/bitcoin/pull/33633#discussion_r2433700549)
No problem was just checking to see if there was a reason for it

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2433717356)
> The review effort would be the same for 2 or nproc threads, but the implications would be limited with the former - we cannot have e.g. complete deadlock if at most two threads are stuck on indexing. And the recent failures on CI indicate that deadlock concerns aren't unfounded.

That's not what happened at all. Tasks are not depending on each other (they actually never were), they run in isolation, so threads cannot deadlock between each other. They are not waiting for any resource from oth
...

💬 ryanofsky commented on pull request "doc: document capnproto and libmultiprocess deps in 29.x":
(https://github.com/bitcoin/bitcoin/pull/33623#discussion_r2433856235)
I went ahead and created a v5.0 [tag](https://github.com/bitcoin-core/libmultiprocess/tags) pointing at this revision, as well as tags for versions used by other bitcoin core releases, described in https://github.com/bitcoin-core/libmultiprocess/pull/228.

So it might make sense to replace 1954f7f65661d49e700c344eae0fc8092decf975 with 5.0 here, or maybe just add (5.0) after if we think it is important to specify the exact hash.

It'd also be possible to update the depends file:

```diff
diff --g
...

👍 ryanofsky approved a pull request: "doc: document capnproto and libmultiprocess deps in 29.x"
(https://github.com/bitcoin/bitcoin/pull/33623#pullrequestreview-3342137913)
Code review ACK ad73a8e2540005f0d81310e10b61c4a61897b217. Thanks for the documentation! This change looks good as-is but i think it could also make sense to reference a version number instead of a commit hash so I left a suggestion below.

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2433669609)
Wrote up some tests for cluster count and size limits in RBFs, package RBFs, package submission, at different configs of `-limitclustercount` and `-limitclustersize`, etc:

```diff
diff --git a/test/functional/mempool_cluster.py b/test/functional/mempool_cluster.py
index 3da8b477a2f..c75eb22bf9b 100755
--- a/test/functional/mempool_cluster.py
+++ b/test/functional/mempool_cluster.py
@@ -4,59 +4,322 @@
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test cluste
...

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2433686891)
It would be nice to expose these through the `getmempoolinfo` RPC.

```diff
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 492693c9cef..1b62c74f0b3 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -812,6 +812,8 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool)
ret.pushKV("fullrbf", true);
ret.pushKV("permitbaremultisig", pool.m_opts.permit_bare_multisig);
ret.pushKV("maxdatacarriersize", pool.m_opts.max_datacarrier_bytes.value_or(0));
+
...

💬 TheCharlatan commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2434014043)
I don't know why we sometimes return a reference and sometimes just copy it. But I think I'll try to return a view here where possible.

🤔 mzumsande reviewed a pull request: "p2p: Allow block downloads from peers without snapshot block after assumeutxo validation"
(https://github.com/bitcoin/bitcoin/pull/33604#pullrequestreview-3342475616)
Concept ACK

🤔 instagibbs reviewed a pull request: "Cluster mempool"
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3340512731)
reviewed through 8cdd7bb11adeaeb4709c670bef4b57362cfbebcb

focusing on logic this time around, only glanced at tests

💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432836478)
6c73e4744837a7dc138a9177df3a48f30a1ba6c1

Had to double-check the claim on space usage, seems to hold

https://www.boost.org/doc/libs/1_73_0/libs/multi_index/doc/performance.html#:~:text=On%20the%20other%20hand%2C%20the,N%2D1)p%2C%20where

💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2432642810)
51430680ecb722e1d4ee4a26dac5724050f41c9e

The why of this is unclear to me through 8cdd7bb11adeaeb4709c670bef4b57362cfbebcb , worth reordering or expanding commit message?

💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2433860250)
ae1ac5410383e57ed16867580a5fd355a46953de

Seems more self-contained to start/stop the builder inside `addChunks`

💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2433884234)
ae1ac5410383e57ed16867580a5fd355a46953de

if we're essentially only using this, why not inline `chunk_feerate` instead?

💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434013019)
2aad9f01e514b77538eb780138a8e52484eaf5a8

is this right? If so this warrants a test (note to self and others)
```Suggestion
// If we can't calculate a feerate, it's because the cluster size limits were hit, and we may want to try package RBF.
```

💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2433318113)
5a388c0d595b2318fea4b1dce977e2d5ff1abc48

```Suggestion
// Check if the transactions would exceed the cluster size limit.
```