💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2469713772)
It should be simpler now. I'm not sure if this comment is still valid though with the current approach. I agree if we already had a ThreadPool this would be much cleaner.

💬 fanquake commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3456681905)
Rebased this for #33185.

⚠️ Humancyyborg opened an issue: "⚠️ General optimization suggestion"
(https://github.com/bitcoin/bitcoin/issues/33722)


> ### ⚠️ Apology & Context
>
> Hey everyone — I sincerely apologize if this post feels off-topic or promotional. That’s not my intention.
>
> This is a **desperate but genuine attempt** to bring visibility to a project we believe can make a real difference.
> We’re a small, decentralized research group trying to merge **blockchain transparency** with **neuroscience and AI** to fight **epilepsy**.
>
> If this isn’t the right place for it, I completely understand — but if you resonate wi
...

✅ pinheadmz closed an issue: "⚠️ General optimization suggestion"
(https://github.com/bitcoin/bitcoin/issues/33722)

💬 willcl-ark commented on issue "Failure to bind to ZMQ addresses is swallowed in debug logs":
(https://github.com/bitcoin/bitcoin/issues/33715#issuecomment-3456782171)
I agree on both counts. If a user has configured zmq but it fails to bind I think we should log this at a higher level than debug, and I agree that the node aborting startup is most likely the correct action to take.

💬 waketraindev commented on pull request "addrman, net: filter during address selection via AddrPolicy to avoid underfill":
(https://github.com/bitcoin/bitcoin/pull/33663#discussion_r2469805737)
Thanks for running mutation testing and for review!

I'm thinking:
```cpp
const std::set<CAddress> vExpectedAddresses = {addr1, addr3, addr4, addr5};
BOOST_CHECK_EQUAL(vAddrPolicyPort.size(), vExpectedAddresses.size());
BOOST_CHECK(std::all_of(vAddrPolicyPort.begin(), vAddrPolicyPort.end(), [&](const CAddress& a){ return vExpectedAddresses.contains(a); }));
BOOST_CHECK(std::none_of(vAddrPolicyPort.begin(), vAddrPolicyPort.end(), policyPortPolicy));
```

💬 waketraindev commented on pull request "addrman, net: filter during address selection via AddrPolicy to avoid underfill":
(https://github.com/bitcoin/bitcoin/pull/33663#issuecomment-3456841808)
Added test verifying only expected addresses are returned and filtered ones excluded.

💬 andrewtoth commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2469840049)
Isn't this comment already implied by `GUARDED_BY(m_mutex)`? Also it's written by the control thread and read by worker threads, so of course it must only be modified while holding the mutex. This comment doesn't add anything IMO.

💬 andrewtoth commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2469885604)
I'm unclear what the purpose of this function is. It interrupts the workers, like `Stop`, but doesn't join them or wait for them to finish.
If the idea is to clear the workqueue and park the threads without being able to add more tasks, it does not accomplish that since the threads will exit their loops as well and not be parked on the cv.

👍 brunoerg approved a pull request: "test: Avoid shutdown race in NetworkThread"
(https://github.com/bitcoin/bitcoin/pull/33140#pullrequestreview-3389226941)
code review ACK fa6db79302d28e6b31b07b24580430614ffcd0e8

I could not reproduce the issue intermittently via the provided script, but I could see the race when adding a sleep before calling `run_forever()`. Anyway, regardless of it, waiting until the thread is actually running is a best practice.

💬 brunoerg commented on pull request "test: Avoid shutdown race in NetworkThread":
(https://github.com/bitcoin/bitcoin/pull/33140#discussion_r2469916322)
Not related but why we don't call `set_event_loop` when running it?

```py
class NetworkThread(threading.Thread):
network_event_loop = None

def __init__(self):
super().__init__(name="NetworkThread")
# There is only one event loop and no more than one thread must be created
assert not self.network_event_loop

NetworkThread.listeners = {}
NetworkThread.protos = {}
if platform.system() == 'Windows':
asyncio.set_event
...

💬 TheCharlatan commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#issuecomment-3456941699)
Re-ACK 5be5a45847f710451a74aed10f6e4efcbace4b45

✅ hebasto closed a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/33445)

📝 hebasto reopened a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/33445)
This PR:

1. Updates to [IWYU 0.25](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.25), which is compatible with Clang 21.

2. Fixes new "modernize-use-default-member-init" warnings. The warning in `interpreter.cpp` is a [false positive](https://github.com/llvm/llvm-project/issues/160394), so it has been suppressed.

🤔 glozow reviewed a pull request: "Cluster mempool"
(https://github.com/bitcoin/bitcoin/pull/33629#pullrequestreview-3388723571)
The empty commits can be dropped. You could prefix some commits with "prep" or "cleanup," but I think it's pretty clear already.

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2469541272)
In 4c359dabd2a7ca5a5d7469070f55e66a88196291

Is this benchmark supposed to be deleted? It seems like it was written a long time ago.

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2469577973)
in 83c8753abf9c2af75cfbacd5488605332da588a4, I think this is missing a few words?

```suggestion
/** How many linearization iterations required for TxGraph clusters to have "acceptable" quality, if they cannot be optimally linearized with fewer iterations. */
```

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2469628917)
2812471c2bfce6215d6d58c1defd39c7a050edbd: Maybe deserves a comment

```suggestion
// PrioritiseTransaction calls stack on previous ones. Set the new transaction fee to be current modified fee + feedelta.
m_txgraph->SetTransactionFee(*it, it->GetModifiedFee() + nFeeDelta);
```

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2469865825)
in 0e25736d6e0

It would help the fuzzer run faster to just call `check` once at the end, instead of after each round of potential removals.

💬 glozow commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2469950583)
If not squashed, perhaps move it earlier so that the test can have the correct string when introduced?