Bitcoin Core Github
44 subscribers
112K links
Download Telegram
πŸ“ fanquake opened a pull request: "guix: strip binaries in libexec"
(https://github.com/bitcoin/bitcoin/pull/33342)
#31679 moved some internal binaries to `libexec/`, but the Guix build wasn't updated to stip these binaries of their debug symbols.
πŸ’¬ rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3266861514)
In 8849f574104d920c1629e756c5495e1d2c523844 "sign: Create MuSig2 signatures for known MuSig2 aggregate keys"

In commit message:
> Lastly, if the partial signatures could be
created, add our own public nonces for the private keys that we know, if
they do not yet exist.

Is is supposed to say "... partial signatures could not be created ..." instead? Based on the tone in the message used.
πŸ€” 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.