💬 maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2596372569)
contains database -> contains a database [missing article makes the sentence ungrammatical / harder to parse]

💬 maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2596330616)
nit https://github.com/bitcoin/bitcoin/commit/f85ff75dad4ce8712fe65a636cf36d57b4066785: Any reason to revert contains to count?

💬 maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2597935686)
0cb19b7: Please don't add lock annotations to the cpp file. They won't be visible anyway. You'll have to add them to the .h file.

💬 maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2598294698)
I think this is just a refactor, and if a nullptr existed here, it would already exist on current master?

💬 maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2598298692)
I think the current name is fine, but if we are allowed to bike-shed, then I'd replace `Detect` With `Add`, or `Load`, because the function does more than just read-only detection: `LoadAssumeutxoChainstate()`.

💬 maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2598302815)
> `TipChainstate`

Not sure this is clearer. All chainstates have a tip, each. So it would be unclear, which tip-chainstate this refers to :sweat_smile:

💬 maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2598307984)
> It's not validated until later and the maybe refers to `std::optional` afaict.

I think the name is fine, because the stats (if they exist), are validated, because they are derived from the validated chainstate. But not strong opinion.

💬 vasild commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#discussion_r2597405891)
This is somewhat of a misuse of `p2p_port()`. It works, but looking at it I wonder if it can be done better. Maybe introduce `proxy_port(n)`? At some point if we implement also SAM proxy (for hijacking I2P connections), then we might need e.g. `proxy_port(0)` for the SOCKS5 proxy and `proxy_port(1)` for the SAM proxy.

💬 vasild commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#discussion_r2597394343)
Would it make sense to introduce `auto_listener_port(n)`?

💬 vasild commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#discussion_r2598416420)
If this is changed to:

```diff
- # Create listener with default P2PInterface
- listener = P2PInterface()
- actual_to_addr, actual_to_port = self._create_auto_outbound_listener(listener, i)
+ if self.decide_auto_outbound_where_to:
+ actual_to_addr, actual_to_port, listener = self.decide_auto_outbound_where_to(i)
+ else:
+ # Create listener with default P2PInterface
+
...

💬 adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2598446881)
switched to tx number (1-based indexing) for user reporting

💬 0xB10C commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#issuecomment-3626793175)
- rebased to include https://github.com/bitcoin/bitcoin/pull/29641. Now using `LogInfo` instead of `LogPrintf`. Updated the commit message an PR description accordingly.
- Switched to AJ's suggestion from https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2596296831 to only have minimal code duplication. This logs all new outbound connections with `LogInfo()`, which can potentially get rate-limited, and logs inbound connections with `LogDebug(BCLog::NET, ...)` which is only active with `
...

👋 0xB10C's pull request is ready for review: "log: don't rate-limit "new peer" with -debug=net"
(https://github.com/bitcoin/bitcoin/pull/34008)

📝 albert4719 opened a pull request: "Improve Chinese translations for peer connection types"
(https://github.com/bitcoin-core/gui/pull/917)
This PR improves several Chinese translations related to peer connection types in the
Bitcoin Core Qt GUI. The existing translations contain a mix of literal interpretations
and colloquial wording that can be misleading or unclear in a technical networking UI.

Specifically:

- "Manual" was translated as "手册", which commonly refers to documentation in Chinese.
In Bitcoin Core, "Manual" indicates a peer connection that was manually established.
This PR updates the translation to "手动连接
...

💬 fanquake commented on pull request "Improve Chinese translations for peer connection types":
(https://github.com/bitcoin-core/gui/pull/917#issuecomment-3626809753)
Thanks, however translations are handled via Transifex. Please contribute any changes there.

✅ fanquake closed a pull request: "Improve Chinese translations for peer connection types"
(https://github.com/bitcoin-core/gui/pull/917)

📝 fanquake opened a pull request: "netbase: Remove "tor" as a network specification"
(https://github.com/bitcoin/bitcoin/pull/34031)
"tor" as a network specification was deprecated in 60dc8e4208 in favor of "onion"
and this commit removes it and updates the relevant test.

Previously #16029. This has been warning as being deprecated since `v0.17.0`.

💬 fanquake commented on pull request "netbase: Remove "tor" as a network specification":
(https://github.com/bitcoin/bitcoin/pull/16029#issuecomment-3627007328)
Picked up in #34031.

💬 maflcko commented on pull request "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO":
(https://github.com/bitcoin/bitcoin/pull/33702#discussion_r2598713637)
Heh, I wish all of this was written in Rust, so that types are checked at compile-time :sweat_smile:

💬 adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2598734691)
these were great suggestions, thank you - added