💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2058288419)
So this:

<details>
<summary>[patch] rename</summary>

```diff
--- i/src/net.h
+++ w/src/net.h
@@ -1355,24 +1355,24 @@ private:
/**
* Determine whether we're already connected to a given "address:port".
* Note that for inbound connections, the peer is likely using a random outbound
* port on their side, so this will likely not match any inbound connections.
* @param[in] str_addr String of the form "address:port", e.g. "1.2.3.4:8333".
* @return tru
...

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2058295285)
https://github.com/bitcoin/bitcoin/pull/31144/checks?check_run_id=41075184292
Was able to repro with GCC using:
```
cmake -B build -DCMAKE_BUILD_TYPE=Debug -DAPPEND_CPPFLAGS="-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC"
```
We both forgot to limit the span:
```C++
const bool all_zeros{!obfuscation || std::ranges::all_of(std::span{key_bytes}.first(min(write_size, key_bytes.size())), [](auto b) { return b == std::byte{0}; })};
```

💬 laanwj commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#issuecomment-2827463844)
Looks like a follow-up to #32212 (first thought it was a duplicate).

💬 VolodymyrBg commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#issuecomment-2827469885)
> Looks like a follow-up to #32212 (first thought it was a duplicate).
But issue is opened

💬 VolodymyrBg commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#issuecomment-2827478307)
> Looks like a follow-up to #32212 (first thought it was a duplicate).

#32212 was merged 3 weeks ago and the author is maflcko, but he left a comment that this test can be deleted 2 weeks ago

💬 maflcko commented on issue "ci: failures in cross-built Windows CI":
(https://github.com/bitcoin/bitcoin/issues/32197#issuecomment-2827478590)
Looks like the failure did not happen again for the last two weeks, so far?

👍 laanwj approved a pull request: "Fix failing util_time_GetTime test on Windows"
(https://github.com/bitcoin/bitcoin/pull/32318#pullrequestreview-2791014170)
Code review ACK 51e4c5b42260855e5b5306d677ad34560976b9fd, timing-based tests are brittle, and i don't think this is a very important thing to test as part of our test suite.

💬 maflcko commented on issue "test: bip324_tests & net_tests failure with `-O3 -flto`":
(https://github.com/bitcoin/bitcoin/issues/32337#issuecomment-2827497286)
It would be good to reduce the test case and report it upstream. Though, similar to https://github.com/bitcoin/bitcoin/issues/32276, it seems tedious to reduce.

💬 maflcko commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#discussion_r2058378961)
not sure we need to keep a comment about tests that were removed. What is the goal here? If the goal is to prevent re-introducing the test, it seems weak, because the comment can be missed easily. Also, the remainder of the code here in this function is concerned about mocktime, so it would be better to remove the comment and rename the test name to clarify it is about mocktime only.

💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2058384454)
but, but, but... :) this function does care to compare the port as it compares `node->m_addr_name` to `addr.ToStringAddrPort()`. If the argument is changed from `CAddress` to `CNetAddr`, then it would need another argument - the string "addr:port". This oddness is the same in `master`. Now I wonder if this condition:

```cpp
node->addr == net_addr || node->m_addr_name == str_addr

// or in master, which is the same:
// FindNode(static_cast<CNetAddr>(addr)) || FindNode(addr.ToStringAddrPort
...

💬 jesterhodl commented on issue "gui: translation spam?":
(https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2827592786)
> #### Erroneous translation:
> ```
> <source>Warning: If you encrypt your wallet and lose your passphrase, you will &lt;b&gt;LOSE ALL OF YOUR BITCOINS&lt;/b&gt;!</source>
> <translation>hasłoOstrzeżenie: Jeśli zaszyfrujesz swój portfel i zgubisz hasło - &lt;b&gt;STRACISZ WSZYSTKIE SWOJE BITCONY&lt;/b&gt;!</translation>
> ```
Transifex Updated

> #### Erroneous translation:
> ```
> <source>Wallet to be encrypted</source>
> <translation>Portfel do zaszyfrowania </translation>
> ``
...

💬 VolodymyrBg commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#discussion_r2058428705)
Done

👍 maflcko approved a pull request: "Fix failing util_time_GetTime test on Windows"
(https://github.com/bitcoin/bitcoin/pull/32318#pullrequestreview-2791196960)
lgtm

💬 maflcko commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#discussion_r2058447384)
```suggestion
SetMockTime(111s);
```

nit: Could clarify while touching

💬 hodlinator commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2058445224)
These are the best I could come up with:
* `IsConnectedToStrAddr`
* {`IsConnectedToService` | `IsConnectedToAddrPort`}
* `IsConnectedToAddress`

`Is`-prefix == short way to signify boolean query function.

💬 laanwj commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#issuecomment-2827632336)
Would make sense to squash this to one commit imo, no need to introduce the comment just to remove it again.

💬 jesterhodl commented on issue "gui: translation spam?":
(https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2827634501)
> I've pushed an update to check more broadly. The new issues (for just Polish) include some false positives around (change/reszta). Though, the others largely apply as far as I can see:

I went through all the listed findings in polish translation and corrected most of them. Some of the findings are actually invalid, eg. "reszta" really means "change".

I'm happy we give it another whirl after Transifex changes go in.

NOTE: I also updated some other labels I stumbled upon which need. In genera
...

💬 VolodymyrBg commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#issuecomment-2827658840)
> Would make sense to squash this to one commit imo, no need to introduce the comment just to remove it again.

squashed

💬 hodlinator commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2058477569)
Inconsistency seems to have been introduced when we started checking for string-version in 9bab521df895c149579b9e64931405c56b008afb in April of 2012. Not on purpose I would wager.

👍 laanwj approved a pull request: "Fix failing util_time_GetTime test on Windows"
(https://github.com/bitcoin/bitcoin/pull/32318#pullrequestreview-2791268166)
re-ACK 3dbd50a576be55941cb4b5034dc2171c03afb07c

Changed since last: renamed test to `util_mocktime`, added seconds to `SetMockTime` arg, and faulty test is removed instead of replaced by a comment.