💬 kehl-gopher commented on something "":
(https://github.com/bitcoin/bitcoin/commit/edffb50b984ff1c6a4dfdc4ebdb84ca776eb0666#commitcomment-155808830)
wow

💬 TheCharlatan commented on pull request "test: fix pushdata scripts":
(https://github.com/bitcoin/bitcoin/pull/32270#discussion_r2057941914)
Maybe call this just `2byte_push`, since there is no pushdata going on here?

👍 hebasto approved a pull request: "test: Suppress upstream `-Wduplicate-decl-specifier` in bpfcc"
(https://github.com/bitcoin/bitcoin/pull/32336#pullrequestreview-2790397659)
ACK facb9b327b9da39ce1e09ed56199be9efb19b5b8, I have reviewed the code and it looks OK.

💬 hebasto commented on pull request "ci: Temporarily disable failing bpf checks":
(https://github.com/bitcoin/bitcoin/pull/32335#issuecomment-2826954992)
It seems this PR can be closed in favour of https://github.com/bitcoin/bitcoin/pull/32336.

📝 dergoegge converted_to_draft a pull request: "fuzz: Expand script verification flag testing to segwit v0 and tapscript"
(https://github.com/bitcoin/bitcoin/pull/31460)
The `script_flags` harness aims to test that our script verification flags are soft-forks (i.e. applying flags can only tighten the verification rules and not widen them). `SigVersion::WITNESS_V0` and `SigVersion::TAPSCRIPT` scripts are currently not covered by this test, as fuzzers are blocked from e.g. creating a valid taproot script path spend commitment.

This PR:

* Moves the taproot commitment and witness script hash checks to `BaseSignatureChecker` (real impl in `GenericTransactionSi
...

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2057968526)
Oops!…I Did It Again

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2057968721)
Ok, sure, changed it to something similar:
```C++
const bool all_zeros{!obfuscation || std::ranges::all_of(std::span(key_bytes).first(write_size), [](auto b) { return b == std::byte{0}; })};
```

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2057968867)
Sure, done

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2057969172)
`assert(m_obfuscation == 0)` is needed to document (and to make sure) that the obfuscation key is already written with obfuscation turned on, so we have to make sure the obfuscation key is 0 to turn it off for this write only.

https://github.com/bitcoin/bitcoin/blob/master/src/test/dbwrapper_tests.cpp#L29-L46 demonstrates the usage of this call - I've updated it to verify that reopening the database will read the obfuscation key correctly.

So without `assert(m_obfuscation == 0)` we might n
...

🤔 i-am-yuvi reviewed a pull request: "test: Add missing check for empty stderr in util tester"
(https://github.com/bitcoin/bitcoin/pull/32327#pullrequestreview-2790454279)
tACK fadf12a56c294696052c4cb6ee5284030ada7498

I've reviewed the changes and tested on my system

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