💬 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-3407583434)
> Tested, and `--skipreorg` successfully reduces test runtime from `60s to 35s`.
>
> I’m not very sure about this approach — splitting was also suggested here https://github.com/bitcoin/bitcoin/issues/16613#issuecomment-548952103 I think it’s a better long-term solution, but could be done in a follow-up.

I agree on that for the long-term, I could address on a follow-up and keep this as a simple solution for now.

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2433482378)
> Such simple PRs

The thread pool, tests, and fuzz harness are 560 lines. More than half the additions to this PR. I don't think it's fair to say that would be a simple PR.

💬 stickies-v commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2432963713)
This doesn't combine well with `invalidateblock`: e.g. when invalidating the tip, the log just gets spammed with warnings as we cycle through new peers:

```
2025-10-15T14:45:32Z UpdateTip: new best=000000092ca51c152010fd76552e12d9c9c7f410ebd04012dcff49eeecb6cfff height=274019 version=0x20000000 log2_work=42.943635 tx=27881630 date='2025-10-15T14:27:13Z' progress=1.000000 cache=71.8MiB(503926txo)
...
2025-10-15T14:45:50Z UpdateTip: new best=0000000b9e7f0760c48d76ef301616dbaa1421788b29911c02
...

💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3407677785)
Not sure why I get this failure, but when installing a fresh Fedora VM, then podman-docker inside that, and then running this pull rebased, I got:

```
MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh" ./ci/test_run_all.sh

...

100% tests passed, 0 tests failed out of 135

Total Test time (real) = 42.30 sec
+ traffic_monitor_end unit
+ test_name=unit
++ get_interfaces
++ set -o pipefail
++ ifconfig
++ awk -F ':| ' '/^[^[:space:]]/ { if (
...

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-3407722811)
> ### Way forward
>
> Given that this PR has struggled to attract reviewers over the past two years, I think we need a different approach.
This is a risky change that touches critical index infrastructure - let's do it in tiny, focused steps instead.
>
> I'd like to propose keeping this PR open as a draft tracking PR that contains the full end-to-end implementation for reference and discussion. Meanwhile, we can merge the work through a series of smaller, focused PRs:

> * Start with a mi
...

👍 theuni approved a pull request: "Update libmultiprocess subtree in 30.x branch"
(https://github.com/bitcoin/bitcoin/pull/33519#pullrequestreview-3341726802)
ACK ae63cc4bf2d25a5b7ce9d166aa288431e8ca16a7 . Verified that it's the same as what's in master.

I don't see any reason to wait. We can always bump again for v30.1 if necessary.

💬 maflcko commented on pull request "test: add option to skip large re-org test in feature_block":
(https://github.com/bitcoin/bitcoin/pull/33003#issuecomment-3407742302)
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.

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-3407743683)
Now that the CI is happy again. Update regarding the introduced changes:

Before we had queues at two different levels, one at the thread pool level and another one at the index level. The initial rationale behind it was to granularly control the way on which the process interrupts them when needed. Not draining the index's `pending_tasks` queue as soon as the first task fails to process or a shutdown was triggered. But, after talking with mzumsande last week, I realized that this was actually
...

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2433579778)
Yeah, updated. Thanks!

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2433580060)
updated. Thanks!

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2433592734)
> > Such simple PRs
>
> The thread pool, tests, and fuzz harness are 560 lines. More than half the additions to this PR. I don't think it's fair to say that would be a simple PR.

I don't think the fuzz and tests hard to grasp, but fair. I should have said that it is simple in comparison to the other changes, which require further contextual knowledge and not just multi-threading.

💬 instagibbs commented on pull request "policy: don't CheckEphemeralSpends on reorg":
(https://github.com/bitcoin/bitcoin/pull/33616#discussion_r2433595098)
Took feedback in other PR ahead of time https://github.com/bitcoin/bitcoin/pull/33504#discussion_r2389364292

I can definitely put an `Assume()` on it.

📝 MamunC0der opened a pull request: "Update GitHub Action to github-script@v8"
(https://github.com/bitcoin/bitcoin/pull/33634)
Version v8 introduces performance improvements, enhanced JavaScript runtime support, and updated dependencies for greater security and reliability.
Release notes: actions/github-script@v8: https://github.com/actions/github-script/releases/tag/v8

🤔 hodlinator reviewed a pull request: "net: make m_nodes_mutex non-recursive"
(https://github.com/bitcoin/bitcoin/pull/32394#pullrequestreview-3341089959)
Reviewed 4aad3714d66cc80c08cb1c827fb49fba7dcf8d50

All annotations make sense, but left Q regarding 1-2 potentially missing ones.

💬 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)