💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2827002303)
> Have you ever encountered IBD failures for code that passes CI? Seems like missing test coverage in that case?

yes, extended one of the tests now, hoping that will cover it next time.

> then chided me in public

Definitely wasn't my intention to scold you in any way, just didn't (and still don't) understand what you're objecting to or suggesting in that part of the code. Pushed some changes, if it's still not clear, let's discuss in person.

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