💬 andrewtoth commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470109463)
I see, so the goal is to stop the thread pool but in a non-blocking way. Could the same be done via a bool parameter to `Stop` to skip joining?
Also, could we modify `Start` in that case to not assert on number of threads, and instead join them if `m_interrupt` is true and we still have outstanding threads?

📝 SatsAndSports opened a pull request: "Remove unnecessary seed from chainparams.cpp"
(https://github.com/bitcoin/bitcoin/pull/33723)
New Bitcoin nodes don't gain anything by connecting to `dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us.` any more

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user e
...

💬 glozow commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3457210211)
As mentioned previously, I don't think we should make people rebase for this change, particularly if it's just for style. Maybe reduce the conflicts or close the PR?

📝 maflcko opened a pull request: " refactor: Return uint64_t from GetSerializeSize "
(https://github.com/bitcoin/bitcoin/pull/33724)
Consensus code should arrive at the same conclusion, regardless of the architecture it runs on. Using architecture-specific types such as `size_t` can lead to issues, such as the low-severity [CVE-2025-46597](https://bitcoincore.org/en/2025/10/24/disclose-cve-2025-46597/).

The CVE was already worked around, but it may be good to still fix the underlying issue.

Fixes https://github.com/bitcoin/bitcoin/issues/33709 with a few refactors to use explicit fixed-sized integer types in serializati
...

💬 m3dwards commented on issue "ci: Where to run heavy and fragile CI tasks?":
(https://github.com/bitcoin/bitcoin/issues/33668#issuecomment-3457222086)
> They could be run on just the master branch for every merge

I like this idea for things that are expensive (beyond what can be solved with more CPU / Memory). As for fragile, I'm not sure I like the idea of just running fragile things less over running them regularly to get more data on why they are fragile and hopefully fixing that. That said, fixing fragile CI jobs often takes significant development effort and you can often be left wondering if it was worth the investment over disabling th
...

🚀 glozow merged a pull request: "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator.{h,cpp}`"
(https://github.com/bitcoin/bitcoin/pull/33218)

🚀 glozow merged a pull request: "test: add functional test for `TestShell` (matching doc example)"
(https://github.com/bitcoin/bitcoin/pull/33546)

💬 davidgumberg commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3457245344)
> one race condition we might hit is a compact block being sent to our node before that peer receives the message un-selecting them as a hb peer.

In this case we probably are OK to drop, even though it wastes a little bandwidth, and for the time that our `SENDCMPCT` is on the wire to our new high-bandwidth peer, we are down to only 2 HB peers.

We'll only unselect a peer as high bandwidth when [selecting another](https://github.com/bitcoin/bitcoin/blob/24434c1284b84e2c33cf3240ca677d77cad298
...

💬 andrewtoth commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470161972)
I see. Then perhaps more directly:
```suggestion
// Note: m_interrupt must be guarded by m_mutex, and cannot be replaced by an unguarded atomic bool.
```

✅ achow101 closed an issue: "Cannot import descriptors with label and internal:false"
(https://github.com/bitcoin/bitcoin/issues/32376)

🤔 ryanofsky reviewed a pull request: "refactor: Add util::Result failure values, multiple error and warning messages"
(https://github.com/bitcoin/bitcoin/pull/25665#pullrequestreview-3388968746)
<!-- begin push-67 -->
Updated 8b892d41fdeb5756fd83f6050f27a170338d260a -> 90b6a005d20ee6375beea1e685c35f265f6829c1 ([`pr/bresult2.66`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.66) -> [`pr/bresult2.67`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.67), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/bresult2.66..pr/bresult2.67))<!-- end --> fixing typos and reverting unneeded wallet change

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r2469805979)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r2454523296

> nit: Does this change to the wallet belong in this commit?

Nice catch. Dropped this change. I assume this change was necessary at some point but it no longer seems to be. (For reference this change was in commit f142cc48c7e67a2414f30487e2b9c58e868e2b23)

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r2469713813)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r2454538331

This is an interesting idea because you are right that basically everywhere the `Update()` method is used here and in follow-up PRs (#25722 and #29700), result state rarely goes from failure->success.

There is one place it happens in this PR though: when a failed LoadChainstate operation is [retried](https://github.com/ryanofsky/bitcoin/blob/8b892d41fdeb5756fd83f6050f27a170338d260a/src/init.cpp#L1753-L1758).

We cou
...

💬 dergoegge commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457355840)
ACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e

Don't think a lot of damage can be done by controlling just one of the seeds but it's just a matter of time before Luke pulls some random shenanigans.

💬 pinheadmz commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457377979)
> Don't think a lot of damage can be done by controlling just one of the seeds but it's just a matter of time before Luke pulls some random shenanigans.

This is going to be a hard thread to moderate because personal attacks are forbidden, but this is one of the very few lines of code that literally point to a specific central entity.

The PR author is also a first-time contributor to the project and I'm worried this PR is going to get brigaded by people susceptible to social media influence
...

👍 stickies-v approved a pull request: "Remove unnecessary seed from chainparams.cpp"
(https://github.com/bitcoin/bitcoin/pull/33723#pullrequestreview-3389712658)
ACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e

Operators of DNS seeds have a (limited) ability to be harmful to users, e.g. by filtering results or logging requests. Luke has shown multiple cases of hostile and unpredictable behaviour towards the Bitcoin Core project, so removing this seed seems like the responsible thing to do, even if we can't guarantee the reliability of other DNS seeds.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470247644)
Originally we flushed, so if we still want to, we may want to extend this to assert that behavior. Or avoid flushing and simplify the bench. Or add the flushing behavior above to a test, etc.

💬 pinheadmz commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457396673)
I think it is far more mature to wait until the service becomes unreliable or violates a stated policy rule before removing. Otherwise the project is merely reacting preemptively to someone's tweets.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470254148)
Even consensus invalid ones? Or did I misunderstand the context here?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470260510)
Yeah, but we want to understand where the differences are coming from, otherwise we'd have the "faster-on-my-machine" syndrome. If you disagree, just resolve the issue.