💬 Sjors commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#issuecomment-2827005396)
re-ACK 17bb63f9f9b08e6af60c089234fe878657dbc88e

💬 hebasto commented on issue "gui: translation spam?":
(https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2827015633)
I'm currently seeking a volunteer for the position of Polish Translation Coordinator:
- https://x.com/hhebasto/status/1915340627031556112
- https://app.transifex.com/bitcoin/communication/d:f5c2059b-fba0-4546-8b5f-0f7b978bee23/?q=project%3Abitcoin

🤔 pablomartin4btc reviewed a pull request: "wallet: Disable creating and loading legacy wallets"
(https://github.com/bitcoin/bitcoin/pull/31250#pullrequestreview-2790487905)
re-ACK 17bb63f9f9b08e6af60c089234fe878657dbc88e

Since my last review:
- > Added a commit to disable legacy wallet creation from the wallet tool.
- Addresed @davidgumberg's feedback.

👍 hodlinator approved a pull request: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-2790161149)
ACK cc8f239663a0874c307efa30815b40d9ef40badb

Fixes thread-unsafe pattern (originally?) raised by me here: https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1988127685
(Even though current master is not susceptible to the found `CNode` being deleted randomly on another thread).

💬 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_r2057808141)
nit: It is somewhat opaque what is going on here. Would be good to add comment:
```suggestion
// Downcast to CNetAddr as we don't care to match on port or other parts of CAddress.
const CNetAddr& net_addr{addr};
```
An even clearer alternative would be to take `CNetAddr` as the function argument and instead clarify this behavior in the comment of the header.

💬 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_r2057829305)
nit: Should avoid deprecated macro:
```suggestion
LogInfo("Failed to open new connection to %s, already connected", addrConnect.ToStringAddrPort());
```

💬 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_r2057856277)
Interesting aspect. Agree current name makes sense - inbound nodes, if they are already connected to us would *not* be using their listening port (even if behind NAT/router). So we could be attempting to establish an outbound connection to a *different* node behind the same IP.

But even if `ConnectNode` here distinguishes between ports, the only caller may override and filter out connections without regard for port, as pointed out above (https://github.com/bitcoin/bitcoin/pull/32326#discussio
...

✅ maflcko closed a pull request: "ci: Temporarily disable failing bpf checks"
(https://github.com/bitcoin/bitcoin/pull/32335)

✅ hebasto closed an issue: "ci: failure in interface_usdt_coinselection.py"
(https://github.com/bitcoin/bitcoin/issues/32322)

🚀 hebasto merged a pull request: "test: Suppress upstream `-Wduplicate-decl-specifier` in bpfcc"
(https://github.com/bitcoin/bitcoin/pull/32336)

💬 0xB10C commented on pull request "test: Suppress upstream `-Wduplicate-decl-specifier` in bpfcc":
(https://github.com/bitcoin/bitcoin/pull/32336#issuecomment-2827076150)
Post merge ACK

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2058025067)
Thanks! (If you re-touch: `std::span{key_bytes}`)

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2058041033)
Ah, I was interpreting "Needed for unobfuscated Read" as applying to `Read`-calls *after* the ctor, hadn't realized it was for the call 2 lines below. :man_facepalming: That's why I didn't think it was relevant to include in https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2057656754. Sorry for this detour. `assert` is good.

Maybe comment could spell out "Needed for unobfuscated Read *directly below*" for similar readers to me, but hopefully there aren't too many of them.

📝 laanwj opened a pull request: "qt: Replace stray tfm::format to cerr with qWarning"
(https://github.com/bitcoin-core/gui/pull/868)
GUI warnings should go to the log, not to the console (which may not be connected at all).

👍 laanwj approved a pull request: "wallet: Disable creating and loading legacy wallets"
(https://github.com/bitcoin/bitcoin/pull/31250#pullrequestreview-2790646401)
re-ACK 17bb63f9f9b08e6af60c089234fe878657dbc88e
git range-diff b2bb27f40c7ab7b51720e41d136dcc9077a525b3..e39118ab0bf08fdaf68b16c86ad304b9133b43c7 9a4c92eb9ac29204df3d826f5ae526ab16b8ad65..17bb63f9f9b08e6af60c089234fe878657dbc88e
- `test: wallet_transactiontime_rescan importdescriptors always rescans`: commit removed (test change not necessary here)
- `test: Remove legacy wallet tests from wallet_backwards_compatibility.py`: comment change typo reverted
- `wallet: Disallow legacy wallet creat
...

💬 maflcko commented on issue "gui: translation spam?":
(https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2827238571)
Pushed another update to check for the format spec, but those are quicker and easier to find via:


```
$ git grep -l '1%1' src/qt/locale/
src/qt/locale/bitcoin_bg.ts
src/qt/locale/bitcoin_bn.ts
src/qt/locale/bitcoin_da.ts
src/qt/locale/bitcoin_es.ts
src/qt/locale/bitcoin_es_CL.ts
src/qt/locale/bitcoin_es_CO.ts
src/qt/locale/bitcoin_es_DO.ts
src/qt/locale/bitcoin_es_SV.ts
src/qt/locale/bitcoin_es_VE.ts
src/qt/locale/bitcoin_it.ts
src/qt/locale/bitcoin_ko.ts
src/qt/locale/bitcoin_pl.ts
src/qt/loc
...

🤔 shahsb reviewed a pull request: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-2790763507)
Concept ACK

💬 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_r2058207242)
> I assume addr-gossip only contains address+port from _outbound_ peers ...

The origin of the gossip is the self-announcement done by each node:

https://github.com/bitcoin/bitcoin/blob/458720e5e98c6e9103aea1fdfcd39bafc26c27bb/src/net_processing.cpp#L5286-L5300

then as this address is received by a node, it is further relayed to other nodes. Eventually everybody stores it in their addrman. Then [`GETADDR` requests](https://developer.bitcoin.org/reference/p2p_networking.html#getaddr) are
...

💬 laanwj commented on pull request "gui: crash fix, disconnect numBlocksChanged() signal during shutdown":
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2827386509)
My preference would be, instead of this fairly fragile check, to symmetrically disconnect the `numBlocksChanged` signal in `setClientModel` when the clientModel is unset.

💬 laanwj commented on pull request "bitcoin-cli: Add -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/32297#issuecomment-2827429481)
> The schema could be unwrapped further, or the data could be sent in some binary json format depending on what your goal is but that interface was the simplest way I could think of implementing this feature.

Right, Say, representing the entire RPC protocol as cap'n'proto structures directly would be interesting (anologous to how c-lightning has a gRPC version of their RPC), but this would be a huge project for which the first step would be the formal definition of the RPC protocol (including
...