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

💬 rkrux commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#issuecomment-3219290236)
> I would prefer to not split into multiple PR's since most of the commits here are refactors that are setting-up for splitting wallet creation and loading, the first PR(s) in the series would have very little motivation, but if you insist I am open to splitting it up.

No insistence at the moment, I will do a full review of the PR soon.

💬 Sjors commented on pull request "Update libmultiprocess subtree to fix build issues":
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3219371219)
ACK 323b3fd27283282f2f8eb1096f56f23d8230f2d6

💬 Sjors commented on issue "integer sanitizer warning, when running with -natpmp=1 enabled":
(https://github.com/bitcoin/bitcoin/issues/33245#issuecomment-3219376514)
cc @laanwj

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

💬 frankomosh commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3219462042)
> 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.
>
> A
...

📝 frankomosh 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
...

💬 frankomosh commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3219472358)
@maflcko sorry for not being able to reply soon, that was because I was away from my computer. And no, it is not LLM generated code, I have been working on it for a while now.

💬 frankomosh commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3219475989)
> 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.
>
> A
...

💬 frankomosh commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2297637212)
Ok. I'll change this

📝 ajtowns opened a pull request: "Revert compact block cache inefficiencies"
(https://github.com/bitcoin/bitcoin/pull/33253)
Reconstructing compact blocks is on the hot path for block relay, so revert changes from #28391 and #29752 that made it slower. Also add a benchmark to validate reconstruction performance, and a comment giving some background as to the approach.

💬 frankomosh commented on pull request "fuzz: add target for `DifferenceFormatter` and invariant check in `net_processing.cpp`":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2297683020)
thought it better to use this format because it is more focused toward testing the differential encoding logic. perhaps a comment(as you suggested) would do?

💬 ajtowns commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3219632528)
On my dev machine with a release build, I see a 64% performance gain on the benchmark after the reversion patches are applied.

cc @sipa @gmaxwell