💬 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.

💬 maflcko commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#issuecomment-2827719802)
lgtm ACK 3dbd50a576be55941cb4b5034dc2171c03afb07c

💬 laanwj commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#issuecomment-2827725228)
LGTM
```
pruneblockchain height

Deletes block and undo data up to a specified height or timestamp.
Requires `-prune` to be enabled at startup; this action is irreversible.

Arguments:
1. height (numeric, required) The block height to prune up to. May be set to a discrete height, or to a UNIX epoch time
to prune blocks whose block time is at least 2 hours older than the provided timestamp.

Result:
n (numeric) Height of the last block pruned

Examples:
> bitcoin
...

💬 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_r2058523521)
Good that we advertise our listening port for outbound peers! Seems kind of unnecessary to tell inbound peers about the address they report as seeing us as (`CService{node.GetAddrLocal()}`):
https://github.com/bitcoin/bitcoin/blob/458720e5e98c6e9103aea1fdfcd39bafc26c27bb/src/net.cpp#L246-L266
But I guess it simplifies the addrman-logic a little bit, not having to populate/refresh it from the local outbound connections.

💬 maflcko commented on issue "gui: translation spam?":
(https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2827758811)
> 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".


Thanks. I was aware that reszta is correct. It was missing from the context, so I've added it in https://github.com/maflcko/DrahtBot/commit/54e4a219e74dbf8064edbe60c0cc6d741381a928

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

Yeah, I am happy to run the script again, but I do wonder if there is a bett
...

💬 fanquake commented on pull request "test: Suppress upstream `-Wduplicate-decl-specifier` in bpfcc":
(https://github.com/bitcoin/bitcoin/pull/32336#issuecomment-2827779344)
Backported to 29.x in #32292.

📝 vasild opened a pull request: "net: remove unnecessary check from AlreadyConnectedToAddress()"
(https://github.com/bitcoin/bitcoin/pull/32338)
`CConnman::AlreadyConnectedToAddress()` searches the existent nodes by address or by address-and-port:

```cpp
FindNode(static_cast<CNetAddr>(addr)) || FindNode(addr.ToStringAddrPort())
```

but:

* if there is a match by just the address, then the address-and-port search will not be evaluated and the whole condition will be `true`
* if the there is no node with the same address, then the second search by address-and-port will not find a match either.

The search by address-and-por
...

💬 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_r2058610798)
Opened https://github.com/bitcoin/bitcoin/pull/32338 with this single change, dropping the `|| FindNode(addr.ToStringAddrPort())` condition.

👍 laanwj approved a pull request: "Crash fix, disconnect numBlocksChanged() signal during shutdown"
(https://github.com/bitcoin-core/gui/pull/864#pullrequestreview-2791496832)
Code review ACK 0ee5376fb302ebb21fb6192ed708800ee8035eb5
Nice solution, also handles the (hypothethical) case where a client model is replaced with a new one.

💬 theStack commented on issue "intermittent issue in feature_bip68_sequence.py: sequence_value = utxos[j]["confirmations"] IndexError: list index out of range":
(https://github.com/bitcoin/bitcoin/issues/32334#issuecomment-2827953910)
I can reproduce this issue locally by passing the randomseed as shown in the CI logs, i.e.
```
$ ./build/test/functional/feature_bip68_sequence.py --randomseed 6169832640268785903
```
Seems that the number of UTXOs generated in the `test_sequence_lock_confirmed_inputs` sub-test (currently 200) is not enough in some cases. A simple minimal-diff fix might be to simply increase that number in this line:

https://github.com/bitcoin/bitcoin/blob/458720e5e98c6e9103aea1fdfcd39bafc26c27bb/test/functiona
...

🤔 furszy reviewed a pull request: "test: Test that migration automatically repairs corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2791552950)
utACK bfcf5c5ab1fdba46d492aa0872c3d5f9a4a87633

📝 maflcko opened a pull request: "ci: Merge fuzz task for macOS and Windows"
(https://github.com/bitcoin/bitcoin/pull/32339)
Currently macOS and Windows eat 4 CI tasks. This is fine, but not needed and duplicates compilation time and space requirements.

Slim down the tasks into just two to reduce the ccache footprint, as well as total compilation time.

💬 dergoegge commented on pull request "ci: Merge fuzz task for macOS and Windows":
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2828129019)
* Don't these jobs run in parallel? i.e. how does this save time?
* This requires changing the build type to `Debug`, do we really want that?

💬 pablomartin4btc commented on pull request "wallet: migration, avoid loading legacy wallet after failure when BDB isn't compiled":
(https://github.com/bitcoin/bitcoin/pull/31451#issuecomment-2828134729)
I think there's an issue with this fix which is that when we call the `restorewallet` RPC, before this PR the wallet was loaded and the "`load_on_startup`" argument was set, this is no longer the case unless we pass `load_after_restore=true`. Having said that, if that gets fixed, since we are disabling loading legacy wallets (#31250) if someone calls `restorewallet` RPC with `load_on_startup=true` on a legacy wallet, next time the node or `QT` starts it would fail and the app will be [closed](ht
...

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2058783987)
Upstreamed in https://github.com/arun11299/cpp-subprocess/pull/113.

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2058786523)
Upstreamed in https://github.com/arun11299/cpp-subprocess/pull/112.