✅ ryanofsky closed a pull request: "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning"
(https://github.com/bitcoin/bitcoin/pull/33281)

💬 ryanofsky commented on pull request "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning":
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3267075492)
Closing in favor of #33312, which actually fixes the the error output, unlike this PR

💬 D33r-Gee commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#discussion_r2330771778)
Thanks @Sjors for providing feedback and suggesting to add test coverage via the RPC...

Looking into it now and will update shortly

👍 ryanofsky approved a pull request: "guix: strip binaries in libexec"
(https://github.com/bitcoin/bitcoin/pull/33342#pullrequestreview-3197287990)
Code review ACK 3cceda9f4855ac1f5df349f42efdbf5058d08f48. Good catch and thanks for the fix

💬 hebasto commented on pull request "guix: strip binaries in libexec":
(https://github.com/bitcoin/bitcoin/pull/33342#issuecomment-3267088084)
Concept ACK.

Building.

💬 D33r-Gee commented on pull request "[DRAFT] Expose AssumeUTXO Load Snapshot Functionality To The GUI":
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-3267106706)
> I'm a bit confused about the relation between this and [bitcoin/bitcoin#33117](https://github.com/bitcoin/bitcoin/pull/33117). I would suggest rebasing [60f676a](https://github.com/bitcoin-core/gui/commit/60f676abf54a454361ae9f5dd17509ec929acbac) on that branch for clarity.
>

Will rebase shortly with the changes from [bitcoin/bitcoin#33117](https://github.com/bitcoin/bitcoin/pull/33117)

> Managed to load the most recent testnet4 snapshot at height 90,000! (after I rebased)

WooHoo!

...

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2330818442)
Thanks! I’ve updated the comment to note that the tidy task uses `git diff`, which would be affected by the patch if it weren’t skipped.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2330819058)
Thanks! Taken.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3267157378)
The feedback from @ryanofsky has been addressed.

💬 ryanofsky commented on pull request "draft: CMake Goals and Guidelines":
(https://github.com/bitcoin/bitcoin/pull/33317#issuecomment-3267190797)
Concept ACK. Seems nice to document goals of cmake build. To avoid confusion, I hope the document will be expanded to say which of the guidelines are currently being met, which are more aspirational, and what needs to change to meet the goals. i think it would also be helpful to link to PRs/issues where these topics have been discussed. Other ideas (not sure they are good but just in case):

- Maybe add this to the doc/design/ folder instead of the doc/ folder, especially if you think if will
...

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2330865174)
An upstream issue: https://github.com/include-what-you-use/include-what-you-use/issues/1809.

💬 l0rinc commented on pull request "chore: compare against keys.size() in writeObject comma check":
(https://github.com/bitcoin/bitcoin/pull/33337#issuecomment-3267229143)
NACK, the existing value makes more sense to me, it's not solving any actual problem

👍 ryanofsky approved a pull request: "ci, iwyu: Treat warnings as errors for specific directories"
(https://github.com/bitcoin/bitcoin/pull/31308#pullrequestreview-3197403345)
Code review ACK d03d861cb6d81f90f3b4add2db3dbdc0bf95a0ef. Just fixing up comment and changing variable name since last review. Thanks for the updates!

💬 ryanofsky commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2330883192)
re: https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2330818442

In commit "ci: Do not patch `leveldb` to workaround UB in "tidy" CI job" (b7248ee2462a26418f0ef1e0a0f04950269d37aa)

Thanks, with new comment I think I understand why this is needed now. This is fine as is, but in case you do want to update it, I think a change like following would make the link between `git diff` and the UB detector clearer:

```diff
--- a/ci/test/03_test_script.sh
+++ b/ci/test/03_test_script.sh
...

💬 l0rinc commented on pull request "Fix benchmark CSV output":
(https://github.com/bitcoin/bitcoin/pull/33340#discussion_r2330890677)
We should likely update the arm version as well: https://github.com/bitcoin/bitcoin/pull/33340/files#diff-687f22b3470aa3123586a32b91f6aaead5bd364298e6b91954cd9a2ef3f00763L683

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2330899302)
Thanks! The comments have been reworked per your suggestion.

💬 purpleKarrot commented on pull request "draft: CMake Goals and Guidelines":
(https://github.com/bitcoin/bitcoin/pull/33317#issuecomment-3267270664)
Thanks for the review, @ryanofsky.

I wanted to write down the goals first, and then create issues/PRs to make changes towards the goals bottom up: that means starting from libmultiprocess, minisketch, secp256k1. bitcoin-core will serve as an example how those libraries are consumed as subprojects.

In the next step, bitcoin-core should follow the same goals and guidelines, because external clients may want to consume bitcoinkernel as a subproject, or because bitcoin-gui is extracted as anot
...

🤔 enirox001 reviewed a pull request: "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet"
(https://github.com/bitcoin/bitcoin/pull/33268#pullrequestreview-3197482104)
Tested ACK 113a422. Ran the full functional test suite including `wallet_anchor.py`; all tests passed. Fix for 0 value anchor detection and sendall size errors looks good. LGTM.

💬 hebasto commented on pull request "Fix benchmark CSV output":
(https://github.com/bitcoin/bitcoin/pull/33340#discussion_r2330912591)
Thanks! Fixed.

📝 HowHsu opened a pull request: "help: enrich help text for `-loadblock`"
(https://github.com/bitcoin/bitcoin/pull/33343)
`-loadblock` doesn't support XOR-ed files, mention it in its help text to avoid troubles for users.

👍 l0rinc approved a pull request: "Fix benchmark CSV output"
(https://github.com/bitcoin/bitcoin/pull/33340#pullrequestreview-3197485791)
Code review ACK 790b440197bde322432a5bab161f1869b667e681