📝 MarcoFalke opened a pull request: "ci: New ccache and docker image for arm task"
(https://github.com/bitcoin/bitcoin/pull/27880)
https://github.com/bitcoin/bitcoin/issues/27879
(https://github.com/bitcoin/bitcoin/pull/27880)
https://github.com/bitcoin/bitcoin/issues/27879
💬 ajtowns commented on issue "arm64 CI failure":
(https://github.com/bitcoin/bitcoin/issues/27879#issuecomment-1590608062)
> I think you can check the output of `merge_base` to see what was used? For example, https://cirrus-ci.com/task/6628961686388736?logs=merge_base#L18
I looked at that and somehow didn't see it. Weird.
(https://github.com/bitcoin/bitcoin/issues/27879#issuecomment-1590608062)
> I think you can check the output of `merge_base` to see what was used? For example, https://cirrus-ci.com/task/6628961686388736?logs=merge_base#L18
I looked at that and somehow didn't see it. Weird.
💬 MarcoFalke commented on issue "arm64 CI failure":
(https://github.com/bitcoin/bitcoin/issues/27879#issuecomment-1590612318)
There is certainly something sketchy going on with the upstream hardware/software. I've had to reset the pre-build of the image thrice: https://cirrus-ci.com/task/4729934984249344. Also, once that passes, the actual build fails half the time with `curl: (35) OpenSSL SSL_connect: Connection reset by peer in connection to raw.githubusercontent.com:443 ` https://cirrus-ci.com/task/5920692756021248?logs=ci#L208
(https://github.com/bitcoin/bitcoin/issues/27879#issuecomment-1590612318)
There is certainly something sketchy going on with the upstream hardware/software. I've had to reset the pre-build of the image thrice: https://cirrus-ci.com/task/4729934984249344. Also, once that passes, the actual build fails half the time with `curl: (35) OpenSSL SSL_connect: Connection reset by peer in connection to raw.githubusercontent.com:443 ` https://cirrus-ci.com/task/5920692756021248?logs=ci#L208
💬 hebasto commented on issue "arm64 CI failure":
(https://github.com/bitcoin/bitcoin/issues/27879#issuecomment-1590618772)
> There is certainly something sketchy going on with the upstream hardware/software. I've had to reset the pre-build of the image thrice: https://cirrus-ci.com/task/4729934984249344. Also, once that passes, the actual build fails half the time with `curl: (35) OpenSSL SSL_connect: Connection reset by peer in connection to raw.githubusercontent.com:443 ` https://cirrus-ci.com/task/5920692756021248?logs=ci#L208
>
>
>
>
@fkorotkov What do you think?
(https://github.com/bitcoin/bitcoin/issues/27879#issuecomment-1590618772)
> There is certainly something sketchy going on with the upstream hardware/software. I've had to reset the pre-build of the image thrice: https://cirrus-ci.com/task/4729934984249344. Also, once that passes, the actual build fails half the time with `curl: (35) OpenSSL SSL_connect: Connection reset by peer in connection to raw.githubusercontent.com:443 ` https://cirrus-ci.com/task/5920692756021248?logs=ci#L208
>
>
>
>
@fkorotkov What do you think?
💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1229134617)
Great catch! Should we also set if for watch only wallets? On line 4190?
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1229134617)
Great catch! Should we also set if for watch only wallets? On line 4190?
💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1590624734)
reACK 24b3ee7ee9b4fd1fb6f07bb03676379b5e58d3ad
Two new commits added since last review to ensure `WALLET_FLAG_GLOBAL_HD_KEY` is set for all new and migrated descriptor wallets.
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1590624734)
reACK 24b3ee7ee9b4fd1fb6f07bb03676379b5e58d3ad
Two new commits added since last review to ensure `WALLET_FLAG_GLOBAL_HD_KEY` is set for all new and migrated descriptor wallets.
💬 S3RK commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1590636794)
reACK cdba23db353a1beff831ff4fc83d01ed64e8c2a9
Appreciate the perseverance in addressing all the comments and arriving at consensus. Thank for working on this.
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1590636794)
reACK cdba23db353a1beff831ff4fc83d01ed64e8c2a9
Appreciate the perseverance in addressing all the comments and arriving at consensus. Thank for working on this.
💬 MarcoFalke commented on pull request "Raise on invalid -debug and -loglevel config options":
(https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1229151830)
Any reason to remove the trailing dot? This will invalidate the translations and is inconsistent with the other Error messages in this pull?
(https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1229151830)
Any reason to remove the trailing dot? This will invalidate the translations and is inconsistent with the other Error messages in this pull?
📝 hebasto opened a pull request: "ci: Use latest `macos-ventura-xcode:14.3.1` image"
(https://github.com/bitcoin/bitcoin/pull/27881)
As documented: https://github.com/bitcoin/bitcoin/blob/427853ab49f610e971b73ea4cc1d5366747e52b1/.cirrus.yml#L339
Last time, the image was updated in https://github.com/bitcoin/bitcoin/pull/26388.
The `check_clang` script diff:
```diff
--- master
+++ pr
@@ -1,5 +1,5 @@
clang --version
-Apple clang version 14.0.0 (clang-1400.0.29.202)
-Target: arm64-apple-darwin22.1.0
+Apple clang version 14.0.3 (clang-1403.0.22.14.1)
+Target: arm64-apple-darwin22.5.0
Thread model: posix
-Instal
...
(https://github.com/bitcoin/bitcoin/pull/27881)
As documented: https://github.com/bitcoin/bitcoin/blob/427853ab49f610e971b73ea4cc1d5366747e52b1/.cirrus.yml#L339
Last time, the image was updated in https://github.com/bitcoin/bitcoin/pull/26388.
The `check_clang` script diff:
```diff
--- master
+++ pr
@@ -1,5 +1,5 @@
clang --version
-Apple clang version 14.0.0 (clang-1400.0.29.202)
-Target: arm64-apple-darwin22.1.0
+Apple clang version 14.0.3 (clang-1403.0.22.14.1)
+Target: arm64-apple-darwin22.5.0
Thread model: posix
-Instal
...
📝 MarcoFalke opened a pull request: "ci: Bump ARM task to debian:bookworm"
(https://github.com/bitcoin/bitcoin/pull/27882)
The task currently uses Debian 11 Bullseye, which will be EOL in ~2024-07, according to https://wiki.debian.org/DebianReleases#Production_Releases
So it would be good to bump either now or latest in 6 months.
Ideally the ARM CI task uses the version of GCC that guix uses to cross compile, which is currently GCC 10 and Debian Bookworm ships with GCC 12, so I'll leave this in draft for now. See also https://packages.debian.org/bookworm/gcc-arm-linux-gnueabihf
(https://github.com/bitcoin/bitcoin/pull/27882)
The task currently uses Debian 11 Bullseye, which will be EOL in ~2024-07, according to https://wiki.debian.org/DebianReleases#Production_Releases
So it would be good to bump either now or latest in 6 months.
Ideally the ARM CI task uses the version of GCC that guix uses to cross compile, which is currently GCC 10 and Debian Bookworm ships with GCC 12, so I'll leave this in draft for now. See also https://packages.debian.org/bookworm/gcc-arm-linux-gnueabihf
📝 MarcoFalke opened a pull request: "ci: Bump macOS cross task to ubuntu:jammy"
(https://github.com/bitcoin/bitcoin/pull/27883)
It shouldn't matter what underlying image is used for the task, because the compiler is fully provided by `./depends/`.
So just use the latest Ubuntu LTS, which is also most likely the OS that is used by people cross-compiling, if there are any at all.
(https://github.com/bitcoin/bitcoin/pull/27883)
It shouldn't matter what underlying image is used for the task, because the compiler is fully provided by `./depends/`.
So just use the latest Ubuntu LTS, which is also most likely the OS that is used by people cross-compiling, if there are any at all.
💬 hebasto commented on pull request "ci: Bump macOS cross task to ubuntu:jammy":
(https://github.com/bitcoin/bitcoin/pull/27883#issuecomment-1590773020)
Concept ACK. I see no compelling reasons for _"Check that Focal can cross-compile to macos"_.
(https://github.com/bitcoin/bitcoin/pull/27883#issuecomment-1590773020)
Concept ACK. I see no compelling reasons for _"Check that Focal can cross-compile to macos"_.
💬 MarcoFalke commented on issue "arm64 CI failure":
(https://github.com/bitcoin/bitcoin/issues/27879#issuecomment-1590773880)
Aparently AWS was down yesterday? Maybe someone needs to turn a server off and on again to fix it?
(https://github.com/bitcoin/bitcoin/issues/27879#issuecomment-1590773880)
Aparently AWS was down yesterday? Maybe someone needs to turn a server off and on again to fix it?
💬 MarcoFalke commented on pull request "ci: Bump macOS cross task to ubuntu:jammy":
(https://github.com/bitcoin/bitcoin/pull/27883#issuecomment-1590779213)
> Concept ACK. I see no compelling reasons for "Check that Focal can cross-compile to macos".
It may have been a leftover from Gitian, before it was removed? See 5baff2b31840bdbc465f55b875aa6e9480288215
(https://github.com/bitcoin/bitcoin/pull/27883#issuecomment-1590779213)
> Concept ACK. I see no compelling reasons for "Check that Focal can cross-compile to macos".
It may have been a leftover from Gitian, before it was removed? See 5baff2b31840bdbc465f55b875aa6e9480288215
💬 hebasto commented on pull request "Disable and uncheck blank when private keys are disabled":
(https://github.com/bitcoin-core/gui/pull/739#issuecomment-1590829482)
Concept ACK.
(https://github.com/bitcoin-core/gui/pull/739#issuecomment-1590829482)
Concept ACK.
💬 hebasto commented on pull request "Disable and uncheck blank when private keys are disabled":
(https://github.com/bitcoin-core/gui/pull/739#issuecomment-1590830348)
cc @furszy @ryanofsky @S3RK
(https://github.com/bitcoin-core/gui/pull/739#issuecomment-1590830348)
cc @furszy @ryanofsky @S3RK
💬 hebasto commented on pull request "Add menu option to migrate a wallet":
(https://github.com/bitcoin-core/gui/pull/738#issuecomment-1590837892)
Usually, we submit changes to non-`qt` subdirectory, i.e., the first commit of this PR, to the main repo, no?
(https://github.com/bitcoin-core/gui/pull/738#issuecomment-1590837892)
Usually, we submit changes to non-`qt` subdirectory, i.e., the first commit of this PR, to the main repo, no?
💬 hebasto commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1590840776)
@hernanmarino Are you still working on this PR?
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1590840776)
@hernanmarino Are you still working on this PR?
💬 MarcoFalke commented on pull request "Add menu option to migrate a wallet":
(https://github.com/bitcoin-core/gui/pull/738#issuecomment-1590849795)
> Usually, we submit changes to non-qt subdirectory, i.e., the first commit of this PR, to the main repo, no?
I don't think this rule applies to interface changes that only affect the gui. Otherwise, it just seems like extra work for reviewers and authors to artificially rip stuff apart for no good reason, with the extra risk of ending up merging just one ripped-off half, but not the other?
(https://github.com/bitcoin-core/gui/pull/738#issuecomment-1590849795)
> Usually, we submit changes to non-qt subdirectory, i.e., the first commit of this PR, to the main repo, no?
I don't think this rule applies to interface changes that only affect the gui. Otherwise, it just seems like extra work for reviewers and authors to artificially rip stuff apart for no good reason, with the extra risk of ending up merging just one ripped-off half, but not the other?
💬 hebasto commented on pull request "Add menu option to migrate a wallet":
(https://github.com/bitcoin-core/gui/pull/738#issuecomment-1590853447)
> > Usually, we submit changes to non-qt subdirectory, i.e., the first commit of this PR, to the main repo, no?
>
> I don't think this rule applies to interface changes that only affect the gui. Otherwise, it just seems like extra work for reviewers and authors to artificially rip stuff apart for no good reason, with the extra risk of ending up merging just one ripped-off half, but not the other?
I agree. It would be better to just document this case for further references.
(https://github.com/bitcoin-core/gui/pull/738#issuecomment-1590853447)
> > Usually, we submit changes to non-qt subdirectory, i.e., the first commit of this PR, to the main repo, no?
>
> I don't think this rule applies to interface changes that only affect the gui. Otherwise, it just seems like extra work for reviewers and authors to artificially rip stuff apart for no good reason, with the extra risk of ending up merging just one ripped-off half, but not the other?
I agree. It would be better to just document this case for further references.
💬 hebasto commented on pull request "Add menu option to migrate a wallet":
(https://github.com/bitcoin-core/gui/pull/738#issuecomment-1590853884)
Concept ACK.
(https://github.com/bitcoin-core/gui/pull/738#issuecomment-1590853884)
Concept ACK.