💬 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

💬 maflcko commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3219797956)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

🤔 151henry151 reviewed a pull request: "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3150945450)
Would it be better to leave the comments there and simply remove the TODO tag? It looks a bit like a comment was cut in half there, but perhaps adding back in the
```The `CMAKE_SKIP_BUILD_RPATH` variable setting has been removed```
Would be better than removing the rest of that information. What do you think?

💬 Sjors commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3219963543)
Holding off on rebasing until #32876 is merged or I have to touch it.

💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#issuecomment-3220029611)
@Prabhat1308 can you update your review to the latest version?

I've rebased it just in case.

💬 Sjors commented on pull request "wallet: support bip388 policy with external signer":
(https://github.com/bitcoin/bitcoin/pull/33008#issuecomment-3220037327)
@pythcoiner thanks, updated the description.

👍 hodlinator approved a pull request: "[IBD] flush UTXO set in batches proportional to `dbcache` size"
(https://github.com/bitcoin/bitcoin/pull/31645#pullrequestreview-3128539265)
ACK 956a6b454704120ba4c482d7157db89c20130e75

Change scales hidden `-dbbatchsize` by the `-dbcache` setting if unset.

Compared IBD until block 910'000 from local peer in 3 variations (both nodes on SSD); PR change for `-dbcache` of 450 vs 45'000, and base commit for PR with 45'000.

* Baseline of `-dbcache=450` averaged 267mins across 3 runs.
* `-dbcache=45000` without this PR averaged 241mins across 3 runs (these were probably the runs where I was doing the least work on the other node)
...

💬 hodlinator commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2282516167)
I would just default to testing at compile time:
* No need to (re)run ctest/test_bitcoin to exercise checks.
* Only run when compilation unit is (re)built. Not re-run when iterating on test code in other compilation units.
* Inconsistency might push other tests towards being converted to compile time, which I would say is a positive effect.

💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2298006417)
I clicked commit suggestion too quickly -- It looks a bit like a comment was cut in half there. Would we want to remove the additional lines of the TODO as well, or rephrase them as comments?

💬 Prabhat1308 commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#issuecomment-3220153666)
re-ACK [`37d2246`](https://github.com/bitcoin/bitcoin/pull/31725/commits/37d22461aa8f62d7fee9b65c8580b7f7a25f3382)