💬 Eunovo commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2481025343)
https://github.com/bitcoin/bitcoin/pull/33689/commits/e1eb4cd3a5eb192cd6d9ee5d255688c06ab2089a:

Can we enforce this rule using a thread-local variable?

💬 laanwj commented on pull request "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO":
(https://github.com/bitcoin/bitcoin/pull/33702#discussion_r2481313804)
A copyright header change slipped in here 😄

👍 laanwj approved a pull request: "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO"
(https://github.com/bitcoin/bitcoin/pull/33702#pullrequestreview-3404168943)
Code review ACK fa2fd0ba1fbd4085df24a2c5472636957db28521

💬 laanwj commented on pull request "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3473021005)
ACK b0c706795ce6a3a00bf068a81ee99fef2ee9bf7e

Personally, i lost my trust in luke-jr to be responsible with project infrastructure longer ago, when he hijacked the transifex project for Knots (kicking all of the other admins out). Recent developments have done nothing to improve this sentiment.

💬 TheCharlatan commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#discussion_r2481363335)
Why did you think this single test was not enough? Is there additional clarity or coverage from the additional checks?

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#issuecomment-3473040563)
Pushed, thanks for the review, you had a few good points, added you as coauthor.

> the gain of using the spaceship operator (which comes at the cost of readability) is marginal.

the spaceship might not be very familiar to us yet, but it's arguably simpler, having fewer moving parts, and some compilers prefer that over the manual comparisons.

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2481401944)
> Goal is to keep the same synchronization mechanisms we had in the http server code

What is the reason for that, isn't the goal to have a reusable `ThreadPool`?

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2481400847)
k, thanks, please resolve the comment

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2481442031)
Where would we be getting negative numbers from?
If we care about the value being in a certain range, we could validate that instead:
```C++
assert(num_workers > 0 && num_workers <= MAX_WORKER_COUNT);
```

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2481430614)
Can you point me to the test that fails please? I have removed it, ran the tests added in this PR and they all passed.

💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2481432376)
Please see my suggestions above, it's a `size_t` there

💬 maflcko commented on pull request "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO":
(https://github.com/bitcoin/bitcoin/pull/33702#discussion_r2481453065)
Yeah, I try to update them while touching the file. Otherwise, one of the bot accounts will "follow-up" with it. Just some examples from this year: https://github.com/bitcoin/bitcoin/pull/32008,
https://github.com/bitcoin/bitcoin/pull/31642,
https://github.com/bitcoin/bitcoin/pull/32963, ...

Will leave as-is for now, but I am thinking that I may follow-up with something to close this bot and churn issue.

💬 rustaceanrob commented on pull request "test: Format strings in `test_runner`":
(https://github.com/bitcoin/bitcoin/pull/33753#issuecomment-3473150822)
I found the lint in additional files and have updated accordingly in 78d4d36730d44de93690b43f900a9202517291f6

💬 0xB10C commented on pull request "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled":
(https://github.com/bitcoin/bitcoin/pull/33738#issuecomment-3473161056)
Concept ACK

👍 TheCharlatan approved a pull request: "test: Format strings in `test_runner`"
(https://github.com/bitcoin/bitcoin/pull/33753#pullrequestreview-3404408458)
ACK 78d4d36730d44de93690b43f900a9202517291f6

📝 roconnor-blockstream opened a pull request: "Relax standardness rules regarding CHECKMULTISIG"
(https://github.com/bitcoin/bitcoin/pull/33755)
In PR #5247, the STRICTENC standardness rules were tightned with regards to CHECKMULTISIG so that unparsable public keys fail the script when they are encountered. The overall purpose here was to disallow the use of confusing hybrid public keys by policy while remaining keeping policy compatible (i.e. strictly stronger) than consensus rules.

Comments in PR #5247 note that "I don't believe it should affect any system in production", however this believe is/was false. Counterparty was stuffing
...

💬 ryanofsky commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3473238238)
re: https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3472631221
> Can there ever only be one `waitNext()` call on per `BlockTemplate`? If not, does `interruptWait()` just stop all of them?

There can be multiple waitNext() calls and the current implementation of interruptWait() will interrupt exactly one of them arbitrarily. I speculated on how this could be improved in https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3453225085, if there is a use-case for multiple waitNex
...

💬 Sjors commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#issuecomment-3473263150)
I also can't think of a use case, but maybe we should fail if a client (accidentally) tries?

🤔 mzumsande reviewed a pull request: "doc: document fingerprinting risk when operating node on multiple networks"
(https://github.com/bitcoin/bitcoin/pull/33750#pullrequestreview-3404398572)
Concept ACK

I think that fingerprinting attacks are easy enough (and some, such as https://github.com/bitcoin/bitcoin/pull/33498, are hard to fix) that this warning is justified.
Fingerprinting methods have never been researched systematically as far as I know, so I'm sure there are various unknown ones besides the ones publicly and privately known. Newly found fingerprinting methods aren't classified as CVEs but reported and fixed openly. So this is just a reflection of the status quo.

...

💬 mzumsande commented on pull request "doc: document fingerprinting risk when operating node on multiple networks":
(https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2481496048)
maybe add something like "... and therefore helps strengthen the network" to stress that it's also common to be a bridge node for altruistic reasons, even if the node operator isn't particularly concerned about attacks on their own node.