🤔 marcofleon reviewed a pull request: "[29.x] *san CI backports"
(https://github.com/bitcoin/bitcoin/pull/33294#pullrequestreview-3197068612)
ACK 7c6be9acae5a16956a7f8e53ae3f944a187a6713, looks okay to me

💬 fanquake commented on pull request "cmake: Install internal binaries to <prefix>/libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3266901778)
Followup to strip `libexec/` bins in ##33342.

💬 fanquake commented on pull request "ci: disable cirrus cache in 32bit arm job":
(https://github.com/bitcoin/bitcoin/pull/33302#issuecomment-3266908097)
cc @m3dwards

👍 ryanofsky approved a pull request: "ci, iwyu: Treat warnings as errors for specific directories"
(https://github.com/bitcoin/bitcoin/pull/31308#pullrequestreview-3197074943)
Code review ACK 1d49346465de8edec85bb2d498859b4fbc346935. Makes sense to try IWYU in a few directories like this to see how stable it is and make sure it won't cause problems.

💬 ryanofsky commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2330621699)
In commit "ci: Do not patch `leveldb` to workaround UB in "tidy" CI job" (b495e2dc95db42e3219720f5beeb6c136d067a4a)

Could the comment clarify what the issues are? Even just a hint or pointer to more information would be helpful. if they are IWYU issues, maybe just say "to avoid IWYU issues"

💬 ryanofsky commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2330635225)
In commit "ci, iwyu: Treat warnings as errors for specific directories" (1d49346465de8edec85bb2d498859b4fbc346935)

I think it'd be a little better to called this ENFORCED_IWYU instead of FORCED_IWYU. Both make sense but former seems a little clearer.

💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2330683445)
> Not yet.

Is there anything blocking that? It'd be good if we'd atleast report issues upstream, before adding workarounds (with little-to-no documentation).

💬 ryanofsky commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3266988266)
All ci jobs are passing now that the new `tool_bitcoin.py` test is disabled on windows, so this PR should be ready to review. I'll set up a vm and address the windows test problem in a followup PR, since I think the _wexecvp/stdout issue might not be trivial to fix.

<!-- begin push-24 -->
Updated 0c82805f75438c38f5a7f942df2a96ec3d61d68e -> f9685d6a1389938b0cceb31d9eef201ab3dd2e86 ([`pr/ipc-auto.23`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-auto.23) -> [`pr/ipc-auto.24`](https://gi
...

💬 mzumsande commented on issue "use -loadblock to load blk*****.dat files, but the blocks in it are not recognized":
(https://github.com/bitcoin/bitcoin/issues/33280#issuecomment-3266992579)
I looked at the code, and it appears that `loadblock` currently only works with non-xor'ed block files:
[AutoFile file{fsbridge::fopen(path, "rb")};](https://github.com/bitcoin/bitcoin/blob/3eea9fd39532eeda648e44de365fc4c9112f6fc6/src/node/blockstorage.cpp#L1253C9-L1254C1) doesn't use an obfuscation key.

I don't know if that was discussed when block obfuscation was added (FYI @maflcko).
`loadblock` is probably not a feature that is used very much these days, but unless someone wants to add `
...

🚀 fanquake merged a pull request: "[29.x] *san CI backports"
(https://github.com/bitcoin/bitcoin/pull/33294)

👍 ryanofsky approved a pull request: "clang-tidy: Disable `UndefinedBinaryOperatorResult` check in `src/ipc`"
(https://github.com/bitcoin/bitcoin/pull/33312#pullrequestreview-3197272526)
Code review ACK 589b65f06c3376df4cde3fac5c1d39a2d3254920. Thanks for the fix!

It could make sense to make a similar change in the libmultiprocess repository, so the `src/ipc/libmultiprocess/` directory would be covered and this fix could be moved to `src/ipc/capnp/` instead of `src/ipc/`, but current approach seems simpler and in the long run when https://github.com/capnproto/capnproto/pull/2334 rolls out, or the LLVM isInMainFile check works better, this can be dropped.

Not running clang-
...

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