Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989151765)
Nit for verbosity here and in the partial sigs loop below.

```diff
-for (const auto& [pubkey, pubnonce] : pubnonces) {
+for (const auto& [participant_pubkey, pubnonce] : pubnonces) {
```
πŸ’¬ rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989218447)
Since it is being reused now, maybe a constant or an enum can also be an option.
πŸ’¬ rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989223541)
I would be in favour of using `compressed` terminology here over `plain` like proposed in the BIP change here: https://github.com/bitcoin/bips/pull/1705
πŸ’¬ rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989243401)
`agg_pubkey`?

Using only `key` alludes to the private key though in this case there is no aggregate private key but still to be consistent with the rest of the codebase and to get rid of any doubt in the mind of the reader.
πŸ’¬ rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989209332)
Maybe?

`(2 * CPubKey::COMPRESSED_SIZE + 1)`
`(2 * CPubKey::COMPRESSED_SIZE + CSHA256::OUTPUT_SIZE + 1)`
πŸ’¬ rkrux commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1989306381)
`_pubkeys` instead of `_keys`

In order to be consistent with the codebase
πŸ’¬ l0rinc commented on issue "Fully validated AssumeUTXO starts revalidating after restart":
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2714449973)
Thanks for the context @mzumsande, @Sjors, @fjahr.

> so that a user that Ctrl+C's out of the first check

The first check finished fully (took me several nights), but now if I want to use that node, I still can't because it's validating again.

I didn't cancel it this time, the new validation after restart took *23.5 more minutes* (it's frustrating to wait here, maybe https://github.com/bitcoin/bitcoin/pull/31645 helps), telling me yet again that `has been fully validated`:
```bash
[...]
2025-0
...
πŸ’¬ ryanofsky commented on pull request "build: Enhance Ccache performance across worktrees and build trees":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2714472813)
> > I don't think there are any. Just mentioned this because `-fdebug-prefix-map` seems to be added conditionally in our build.
>
> Specifically, which condition are you referring to?

`-fdebug-prefix-map` is only added if `try_append_cxx_flags` succeeds and if the compilation target explicitly links against `core_interface`. It's reasonable to assume that these things are always true, but since correctness of the output when CCACHE_NOHASHDIR=1 is used seems to depend on them it would be go
...
πŸ’¬ hebasto commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#issuecomment-2714561117)
> It would still be good to point to some place on Transifex where reviewers can see that discussion.

I've posted an announcement, which refers to this PR:
- on the Transifex website -- https://app.transifex.com/bitcoin/communication/d:4ca41e70-aeda-4632-83d1-b20b3bbd0dd9/?q=project%3Abitcoin
- on the ML -- https://groups.google.com/g/bitcoin-translators/c/qam5uo0h7cA
πŸ’¬ ryanofsky commented on issue "build: ccache doesn't hit across build dirs":
(https://github.com/bitcoin/bitcoin/issues/31994#issuecomment-2714578102)
Thanks for writing this up so clearly. It would be good to add this to the documentation.

re: https://github.com/bitcoin/bitcoin/issues/31994#issuecomment-2708846512

> Additionally, ccache docs [state](https://ccache.dev/manual/4.9.1.html#config_hash_dir):
>
> > The CWD will not be included in the hash if [`base_dir`](https://ccache.dev/manual/4.9.1.html#config_base_dir) is set (_and matches the CWD_) and the compiler option `-fdebug-prefix-map` is used.

I still don't understand why setting
...
πŸ’¬ hebasto commented on issue "build: ccache doesn't hit across build dirs":
(https://github.com/bitcoin/bitcoin/issues/31994#issuecomment-2714625797)
> Thanks for writing this up so clearly. It would be good to add this to the documentation.
>
> re: [#31994 (comment)](https://github.com/bitcoin/bitcoin/issues/31994#issuecomment-2708846512)
>
> > Additionally, ccache docs [state](https://ccache.dev/manual/4.9.1.html#config_hash_dir):
> > > The CWD will not be included in the hash if [`base_dir`](https://ccache.dev/manual/4.9.1.html#config_base_dir) is set (_and matches the CWD_) and the compiler option `-fdebug-prefix-map` is used.
>
> I st
...
πŸ‘ Mikaela11 approved a pull request: "Updated MacOS icon to more closely fit Apple's design standards"
(https://github.com/bitcoin-core/gui/pull/852#pullrequestreview-2674918483)
Michaela LΓΆrinczova
πŸ’¬ ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2714686904)
Rebased c2c5a0f492ae2ce54afdd031e8a2f2689a8c4942 -> 7e4b3a6e3c6a1bc34dc6af9130922cb71f1f2670 ([`pr/subtree.20`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.20) -> [`pr/subtree.21`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.21), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.20-rebase..pr/subtree.21)) due to conflict with #31982
πŸ‘ theuni approved a pull request: "cmake: Set top-level target output locations"
(https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2674970756)
ACK 568fcdddaec2cc8decba5a098257f31729cc1caa

+1 sjors.

I understand the arguments for/against the symlink, but don't feel strongly either way myself.
πŸ’¬ Mikaela11 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/fa3b8162406bb21425a6fb5c6a96d17175545a4c#r153562187)
Michaela LΓΆrinczovΓ‘
πŸ€” glozow reviewed a pull request: "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`"
(https://github.com/bitcoin/bitcoin/pull/32025#pullrequestreview-2674965082)
nice catch, concept ACK
πŸ’¬ glozow commented on pull request "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`":
(https://github.com/bitcoin/bitcoin/pull/32025#discussion_r1989504076)
callers typically use txid, so it's best to include both

```suggestion
strprintf("tx %s (wtxid=%s) did not spend parent's ephemeral dust", out_child_txid.ToString(), out_child_wtxid.ToString()));
```
πŸ’¬ instagibbs commented on pull request "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`":
(https://github.com/bitcoin/bitcoin/pull/32025#issuecomment-2714715220)
concept ACK, agree that txid should also be reported to user since that's used quite often by callers
πŸ’¬ ryanofsky commented on issue "build: ccache doesn't hit across build dirs":
(https://github.com/bitcoin/bitcoin/issues/31994#issuecomment-2714742260)
> This interpretation of the Ccache documentation does not describe the actual behaviour on my different systems. I read "base_dir matches the CWD" as "base_dir equals the CWD". However, I might be wrong.

You are probably right but this behavior does not seem to make sense or correspond to documentation of base_dir. It is probably ok for us to force CCACHE_NOHASHDIR, but it seems like it would be safer if ccache just detected conditions it should and shouldn't hash CWD correctly itself. Ccache'
...
πŸ’¬ hebasto commented on pull request "[POC] build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-2714766273)
Rebased on https://github.com/bitcoin/bitcoin/pull/32028.

> > < to be added >
>
> It'd be good if this could actually be filled in, so it's clear what the goals are / what's trying to be acheived here.

Done.