💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2296806927)
nit:
```suggestion
auto it = std::ranges::remove_if(onion_binds,
```

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2296866701)
do we really need to recreate this or can we reuse `seen_whitebinds` or even better, just iterate `vWhiteBinds` values directly?

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2296866208)
as mentioned before, I think we can safely check `vBinds` directly - it's simpelr and likely even faster than this

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2296873010)
nit: in first case `increment` means +1, here it's `+1` - but more generally, the code is already, it's enough if we just document the intent:
```suggestion
port += 2 # avoid conflicts since we've used two ports before
```

----

or even better, why are we even reusing the same variable, why not create a port iterator and instead of the list a lambda accepting two ports something like (untested, since this test is skipped on a Mac):
```python
def run_test(self):
lp = addr_t
...

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2296807196)
Since we don't expect this to happen often, I guess it's enough to stop at the first error instead of listing all 👍

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2297089974)
I find it a bit inconsistent that we're not erring here - we don't know why they provided the same thing twice, seems like a mistake, not sure we should attempt to correct it silently. Would also simplify the code if we unified the way we handle these

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2297095727)
nit: would make sense to invert this if wee keep this format, it makes a bit more sense to use `it` in the condition where we're checking it (would also enable embedding it in the condition).
Not sure this applies if you remove the reundant deduplication code and use iteration instead of sets. I think that would simplify all of this considerably.

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2297097850)
nit: if it's a regex, we need to escape some chars

💬 maflcko commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3218891081)
@frankomosh Is this LLM generated?

✅ maflcko closed a pull request: "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`"
(https://github.com/bitcoin/bitcoin/pull/33252)

💬 maflcko commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3219006626)
Thanks, but the value of purely LLM generated "vibe" pull requests is limited, because:

* The "author" does not understand the changes and can not reply to code review feedback.
* There are more than 300 pull requests (most written by real humans) waiting for review.
* Incentive to review purely LLM generated changes is limited, because the feedback likely does not help to train a real human planning to contribute in the future, but is likely only passed on to an LLM api.

Also, specific
...

📝 maflcko reopened a pull request: "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`"
(https://github.com/bitcoin/bitcoin/pull/33252)
Adds a fuzz test for the [`DifferenceFormatter`](https://github.com/bitcoin/bitcoin/blob/e3f416dbf7633b2fb19c933e5508bd231cc7e9cf/src/blockencodings.h#L22-L42) (used in [`BlockTransactionsRequest`](https://github.com/bitcoin/bitcoin/blob/master/src/blockencodings.h#L44-L54), [BIP 152](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki)). The DifferenceFormatter class implements differential encoding for compact block transactions (BIP 152). This PR ensures that its strictly-monotonic
...

💬 maflcko commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3219107162)
Re-opening, based on request by @dergoegge

🤔 maflcko reviewed a pull request: "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`"
(https://github.com/bitcoin/bitcoin/pull/33252#pullrequestreview-3150210173)
(left my previous comment as in-line review comments)

💬 maflcko commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2297310457)
in 358ad3f7df44e2715b69e80fc973e4f55517a67c: This is not around "validation", the commit title should start with "p2p" and the pull request title as well. (This code is run in production, not only during fuzz tests)

💬 maflcko commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2297313734)
nit: should have a comment to be based on `BlockTransactionsRequest`?

💬 maflcko commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2297318084)
an alternative would be to directly use `BlockTransactionsRequest` and provide a dummy uint256 in fuzz code (filled into the bytes)

💬 maflcko commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2297310690)
nit: pls sort

💬 maflcko commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2297315951)
nit: There is no need to serialize single bytes. You can just pass the vector into the `DataStream` constructor

💬 janb84 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2297383833)
```suggestion

```

If the TODO is solved than there is no more TO DO. Please remove the todo or reword it what is still left to do.