💬 hodlinator commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2433600255)
Shouldn't we also annotate this one with
```C++
EXCLUSIVE_LOCKS_REQUIRED(!connman.m_nodes_mutex)
```
?
...which leads to also annotating `ThreadMessageHandler`.

💬 hodlinator commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2433078269)
nit:
On master, we will be holding on to the `m_nodes_mutex` as we start the lookup and removal for this older node at `lNodesAnnouncingHeaderAndIDs.front()`.
This older node can be removed from `CConnman::m_nodes` just before we execute an outer `ForNode` (while not holding `m_nodes_mutex`).

Don't think there is any risk to how it works on master or in this PR, we `.pop_front()` the old entry regardless of whether we find it or not. But might still be nice to add
```
Assume(lNodesAnn
...

💬 kevkevinpal commented on pull request "node: change a tx-relay on/off flag to enum":
(https://github.com/bitcoin/bitcoin/pull/33567#discussion_r2433652529)
gotcha that works then!

💬 brunoerg commented on pull request "test: add option to skip large re-org test in feature_block":
(https://github.com/bitcoin/bitcoin/pull/33003#issuecomment-3407856655)
> I am not sure about splitting this test. I think there is a benefit to run a reorg on a "dirty" state, so if the reorg test is run, we'd want to fully run it. If the reorg should be skipped, there is the option added in this pull.

Make sense. Given that running a reorg on a "dirty" state is useful, this flag is the best option. Otherwise we would have 2 tests doing similar things since we would have to reproduce a dirty state for the reorg anyway.

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2433653970)
answered below https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-3407722811.

💬 kevkevinpal commented on pull request "node: change a tx-relay on/off flag to enum":
(https://github.com/bitcoin/bitcoin/pull/33567#issuecomment-3407870385)
ACK [07a9264](https://github.com/bitcoin/bitcoin/pull/33567/commits/07a926474b5a6fa1d3d4656362a0117611f6da2f)

💬 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