π¬ 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
...
(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
(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
...
(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
...
(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
(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
(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.
(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Γ‘
(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
(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()));
```
(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
(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'
...
(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.
(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.
π¬ hebasto commented on pull request "Update `secp256k1` subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2714770025)
> > The latter is required for #31507.
>
> If we are going to start bumping subtrees, can you at least add a PR description, given 31507 [still doesn't have one](https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-2548249261).
My apologies. Fixed.
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2714770025)
> > The latter is required for #31507.
>
> If we are going to start bumping subtrees, can you at least add a PR description, given 31507 [still doesn't have one](https://github.com/bitcoin/bitcoin/pull/31507#issuecomment-2548249261).
My apologies. Fixed.
π¬ ryanofsky commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2714781131)
I don't see a good reason to cause silent breakage here, but if that's what we want to do it seems fine (really) to merge 568fcdddaec2cc8decba5a098257f31729cc1caa anytime. Would just point to https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2644640989 to explain why silent failures are unnecessary and should be easy to avoid.
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2714781131)
I don't see a good reason to cause silent breakage here, but if that's what we want to do it seems fine (really) to merge 568fcdddaec2cc8decba5a098257f31729cc1caa anytime. Would just point to https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2644640989 to explain why silent failures are unnecessary and should be easy to avoid.
π¬ hebasto commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#issuecomment-2714784268)
cc @darosior for sanity check of French (fr) translation.
(https://github.com/bitcoin/bitcoin/pull/32004#issuecomment-2714784268)
cc @darosior for sanity check of French (fr) translation.
π¬ darosior commented on issue "29.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/32026#issuecomment-2714788716)
For pinholing could you encourage people to test against their router at home and share the result along with the model of their router at https://github.com/bitcoin/bitcoin/issues/31663?
(https://github.com/bitcoin/bitcoin/issues/32026#issuecomment-2714788716)
For pinholing could you encourage people to test against their router at home and share the result along with the model of their router at https://github.com/bitcoin/bitcoin/issues/31663?
π¬ glozow commented on pull request "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`":
(https://github.com/bitcoin/bitcoin/pull/32025#discussion_r1989585146)
Interestingly, the effect of inserting by the child's txid is that you get "missing inputs" instead of the slightly more correct "did not spend parent's ephemeral dust" from the `submitpackage` results. There is a result for both wtxid and txid in the map: we put "missing inputs" when we tried the child initially, and then failed to overwrite it (because we're using the wrong key here) the second time. The RPC code copies the result from a query by wtxid.
Here is the diff for `mempool_ephemer
...
(https://github.com/bitcoin/bitcoin/pull/32025#discussion_r1989585146)
Interestingly, the effect of inserting by the child's txid is that you get "missing inputs" instead of the slightly more correct "did not spend parent's ephemeral dust" from the `submitpackage` results. There is a result for both wtxid and txid in the map: we put "missing inputs" when we tried the child initially, and then failed to overwrite it (because we're using the wrong key here) the second time. The RPC code copies the result from a query by wtxid.
Here is the diff for `mempool_ephemer
...
π¬ glozow commented on pull request "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`":
(https://github.com/bitcoin/bitcoin/pull/32025#discussion_r1989591603)
Btw I'm not suggesting you add this exact diff to the PR (I had to comment out a part of the test). But it can be adapted into a regression test later.
(https://github.com/bitcoin/bitcoin/pull/32025#discussion_r1989591603)
Btw I'm not suggesting you add this exact diff to the PR (I had to comment out a part of the test). But it can be adapted into a regression test later.
π¬ janb84 commented on issue "29.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/32026#issuecomment-2714960286)
> For pinholing could you encourage people to test against their router at home and share the result along with the model of their router at [#31663](https://github.com/bitcoin/bitcoin/issues/31663)?
@darosior
Thank you for your feedback! I've made several improvements:
- Added text with a link to the issue mentioned with a request to test and report.
- Implemented the missing successful PCP logging that was mentioned in the issue
- Added a clarifying statement to enable PCP if an attempt fa
...
(https://github.com/bitcoin/bitcoin/issues/32026#issuecomment-2714960286)
> For pinholing could you encourage people to test against their router at home and share the result along with the model of their router at [#31663](https://github.com/bitcoin/bitcoin/issues/31663)?
@darosior
Thank you for your feedback! I've made several improvements:
- Added text with a link to the issue mentioned with a request to test and report.
- Implemented the missing successful PCP logging that was mentioned in the issue
- Added a clarifying statement to enable PCP if an attempt fa
...