💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2354789767)
This probably has a silent merge conflict with #30440.
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2354789767)
This probably has a silent merge conflict with #30440.
💬 Sjors commented on pull request "getblocktemplate improvements for segwit and sigops":
(https://github.com/bitcoin/bitcoin/pull/27433#issuecomment-2354795677)
Simple rebase after #30440.
(https://github.com/bitcoin/bitcoin/pull/27433#issuecomment-2354795677)
Simple rebase after #30440.
💬 Sjors commented on pull request "doc: update NeedsRedownload() and nStatus comment":
(https://github.com/bitcoin/bitcoin/pull/29624#issuecomment-2354805581)
Just the bi-annual rebase the doctor asked for :-)
(https://github.com/bitcoin/bitcoin/pull/29624#issuecomment-2354805581)
Just the bi-annual rebase the doctor asked for :-)
💬 Sjors commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2354820876)
> I can't seem to have contrib/signet/miner fail on master, am I doing something wrong?
It's possible that this changed in some recent rebase. Instead of failing it produces an empty witness and sticks that in the signet commitment.
I updated the PR description.
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2354820876)
> I can't seem to have contrib/signet/miner fail on master, am I doing something wrong?
It's possible that this changed in some recent rebase. Instead of failing it produces an empty witness and sticks that in the signet commitment.
I updated the PR description.
💬 maflcko commented on pull request "ci: Use macos-14 GHA image":
(https://github.com/bitcoin/bitcoin/pull/30913#issuecomment-2354837423)
### Master
* https://github.com/bitcoin/bitcoin/actions/runs/10895856163/job/30234757205#step:3:12
```
Apple clang version 15.0.0 (clang-1500.0.40.1)
Target: x86_64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
```
* https://github.com/bitcoin/bitcoin/actions/runs/10895856163/job/30234757205#step:7:2006: (43 minutes with 21% ccache hit rate)
### Pull
* https://github.com/bitc
...
(https://github.com/bitcoin/bitcoin/pull/30913#issuecomment-2354837423)
### Master
* https://github.com/bitcoin/bitcoin/actions/runs/10895856163/job/30234757205#step:3:12
```
Apple clang version 15.0.0 (clang-1500.0.40.1)
Target: x86_64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
```
* https://github.com/bitcoin/bitcoin/actions/runs/10895856163/job/30234757205#step:7:2006: (43 minutes with 21% ccache hit rate)
### Pull
* https://github.com/bitc
...
💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2354840124)
> (Unrelated to this issue, one could consider moving to the M1 runners, given that they may speed up the build. Is the removal of `sudo xcode-select --switch /Applications/Xcode_15.0.app` required? Personally I find it useful to have the minimum supported xcode version documented and checked by CI.)
Proposed alternative in https://github.com/bitcoin/bitcoin/pull/30913
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2354840124)
> (Unrelated to this issue, one could consider moving to the M1 runners, given that they may speed up the build. Is the removal of `sudo xcode-select --switch /Applications/Xcode_15.0.app` required? Personally I find it useful to have the minimum supported xcode version documented and checked by CI.)
Proposed alternative in https://github.com/bitcoin/bitcoin/pull/30913
💬 maflcko commented on pull request "doc: update NeedsRedownload() and nStatus comment":
(https://github.com/bitcoin/bitcoin/pull/29624#issuecomment-2354854297)
re-ACK af9f9878934f88036423021c70ef523b6c9e1c90
(https://github.com/bitcoin/bitcoin/pull/29624#issuecomment-2354854297)
re-ACK af9f9878934f88036423021c70ef523b6c9e1c90
🤔 willcl-ark reviewed a pull request: "ci: Use macos-14 GHA image"
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2308747588)
Concept ACK.
Very much in favour of speeding these jobs up! Changes look good to me so far, just one Q on the jobname with this change in architecture.
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2308747588)
Concept ACK.
Very much in favour of speeding these jobs up! Changes look good to me so far, just one Q on the jobname with this change in architecture.
💬 willcl-ark commented on pull request "ci: Use macos-14 GHA image":
(https://github.com/bitcoin/bitcoin/pull/30913#discussion_r1762621194)
This is still building using x86_64-apple-darwin as the host (from 00_setup_env_mac_native.sh). I wonder if the "native" here is therefore confusing?
(https://github.com/bitcoin/bitcoin/pull/30913#discussion_r1762621194)
This is still building using x86_64-apple-darwin as the host (from 00_setup_env_mac_native.sh). I wonder if the "native" here is therefore confusing?
💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2354875962)
I am seeing a 2h timeout on running `clang --version`, even before any CI-specific code is run: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/10897268811/job/30238399156
So I fail to see how this could be anything other than a GitHub hardware, software, or data corruption bug.
I don't think this can be fixed on our side, apart from removing the task.
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2354875962)
I am seeing a 2h timeout on running `clang --version`, even before any CI-specific code is run: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/10897268811/job/30238399156
So I fail to see how this could be anything other than a GitHub hardware, software, or data corruption bug.
I don't think this can be fixed on our side, apart from removing the task.
💬 TheCharlatan commented on pull request "depends: sqlite 3.46.1":
(https://github.com/bitcoin/bitcoin/pull/29991#issuecomment-2354884459)
Guix build (aarch64):
```
773a4d0dbc05ceb91439802ce3374b252f45d2a4c69c3fa5930d9450d8c1f340 guix-build-def6dd0c597f/output/aarch64-linux-gnu/SHA256SUMS.part
c7719110515db339e6ca87f8255338eb21ed2df2fa89e2ae72a10860a6fff2a5 guix-build-def6dd0c597f/output/aarch64-linux-gnu/bitcoin-def6dd0c597f-aarch64-linux-gnu-debug.tar.gz
83a75cb1242f725f234e16799791890d0aebd2efab1632ac3db300b73ad664a9 guix-build-def6dd0c597f/output/aarch64-linux-gnu/bitcoin-def6dd0c597f-aarch64-linux-gnu.tar.gz
f6d09f3839
...
(https://github.com/bitcoin/bitcoin/pull/29991#issuecomment-2354884459)
Guix build (aarch64):
```
773a4d0dbc05ceb91439802ce3374b252f45d2a4c69c3fa5930d9450d8c1f340 guix-build-def6dd0c597f/output/aarch64-linux-gnu/SHA256SUMS.part
c7719110515db339e6ca87f8255338eb21ed2df2fa89e2ae72a10860a6fff2a5 guix-build-def6dd0c597f/output/aarch64-linux-gnu/bitcoin-def6dd0c597f-aarch64-linux-gnu-debug.tar.gz
83a75cb1242f725f234e16799791890d0aebd2efab1632ac3db300b73ad664a9 guix-build-def6dd0c597f/output/aarch64-linux-gnu/bitcoin-def6dd0c597f-aarch64-linux-gnu.tar.gz
f6d09f3839
...
💬 maflcko commented on pull request "ci: Use macos-14 GHA image":
(https://github.com/bitcoin/bitcoin/pull/30913#discussion_r1762647527)
That sounds like a bug in the CI config. However, it should be harmless, because `HOST` is unused, because depends isn't used here.
I'll add an unrelated bugfix commit to remove the wrong config value.
(https://github.com/bitcoin/bitcoin/pull/30913#discussion_r1762647527)
That sounds like a bug in the CI config. However, it should be harmless, because `HOST` is unused, because depends isn't used here.
I'll add an unrelated bugfix commit to remove the wrong config value.
💬 stratospher commented on pull request "addrman: change internal id counting to int64_t":
(https://github.com/bitcoin/bitcoin/pull/30568#discussion_r1762661233)
I'm fine with the version on the branch now because `Serialise()` and `Unserialise()` are symmetrical. `nIndex` is [written as int](https://github.com/bitcoin/bitcoin/blob/51f7668d31e2624e41c7ce77fe33162802808f3f/src/addrman.cpp#L220) into peers.dat in `Serialise()` and [read as int](https://github.com/bitcoin/bitcoin/blob/51f7668d31e2624e41c7ce77fe33162802808f3f/src/addrman.cpp#L325) from peers.dat in `Unserialise()`.
(https://github.com/bitcoin/bitcoin/pull/30568#discussion_r1762661233)
I'm fine with the version on the branch now because `Serialise()` and `Unserialise()` are symmetrical. `nIndex` is [written as int](https://github.com/bitcoin/bitcoin/blob/51f7668d31e2624e41c7ce77fe33162802808f3f/src/addrman.cpp#L220) into peers.dat in `Serialise()` and [read as int](https://github.com/bitcoin/bitcoin/blob/51f7668d31e2624e41c7ce77fe33162802808f3f/src/addrman.cpp#L325) from peers.dat in `Unserialise()`.
💬 stratospher commented on pull request "addrman: change internal id counting to int64_t":
(https://github.com/bitcoin/bitcoin/pull/30568#issuecomment-2354910571)
ACK 51f7668d31e2624e41c7ce77fe33162802808f3f. I think it's a good change to make the nId space large(64 bits) so that the nId values are distinct.
(https://github.com/bitcoin/bitcoin/pull/30568#issuecomment-2354910571)
ACK 51f7668d31e2624e41c7ce77fe33162802808f3f. I think it's a good change to make the nId space large(64 bits) so that the nId values are distinct.
🤔 hebasto reviewed a pull request: "build: speed up by flattening the dependency graph"
(https://github.com/bitcoin/bitcoin/pull/30911#pullrequestreview-2308818404)
Concept ACK.
a39a915c226775396f6505efe77a4aba4d0ae3ad
Did you consider setting `CMAKE_ADD_CUSTOM_COMMAND_DEPENDS_EXPLICIT_ONLY` globally?
aaf30ef89465c18228b4e46fd1378b9087274cbe
> turns out this triggers an [upstream bug](https://gitlab.kitware.com/cmake/cmake/-/issues/24058), which I guess can be considered an edge-case until fixed in CMake. I've added 2 per-lib opt-outs as a result.
This bug may also affect the `crc32c` library after [switching](https://github.com/bitcoin/bitcoin/p
...
(https://github.com/bitcoin/bitcoin/pull/30911#pullrequestreview-2308818404)
Concept ACK.
a39a915c226775396f6505efe77a4aba4d0ae3ad
Did you consider setting `CMAKE_ADD_CUSTOM_COMMAND_DEPENDS_EXPLICIT_ONLY` globally?
aaf30ef89465c18228b4e46fd1378b9087274cbe
> turns out this triggers an [upstream bug](https://gitlab.kitware.com/cmake/cmake/-/issues/24058), which I guess can be considered an edge-case until fixed in CMake. I've added 2 per-lib opt-outs as a result.
This bug may also affect the `crc32c` library after [switching](https://github.com/bitcoin/bitcoin/p
...
💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1762668345)
02c6a1ed58867fa14b1e0c42f476ef45e64ea7ea `-> (result: BlockTemplate);`
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1762668345)
02c6a1ed58867fa14b1e0c42f476ef45e64ea7ea `-> (result: BlockTemplate);`
🚀 fanquake merged a pull request: "guix: (explicitly) build Linux GCC with `--enable-cet`"
(https://github.com/bitcoin/bitcoin/pull/30438)
(https://github.com/bitcoin/bitcoin/pull/30438)
💬 maflcko commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2354922939)
> ... though currently only Ninja benefits from this being set, and only CMake >= 3.27 understands it.
I wonder if one CI task should be using Ninja (and cmake >= 3.27), if it isn't too hard to implement. Otherwise this config will remain untested and errors may sneak in to the master branch, only being detected after merge.
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2354922939)
> ... though currently only Ninja benefits from this being set, and only CMake >= 3.27 understands it.
I wonder if one CI task should be using Ninja (and cmake >= 3.27), if it isn't too hard to implement. Otherwise this config will remain untested and errors may sneak in to the master branch, only being detected after merge.
💬 hebasto commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#discussion_r1762679548)
A note for reviewers. The Guix builds are using CMake 3.24.2 (the `cmake-minimal` package).
(https://github.com/bitcoin/bitcoin/pull/30911#discussion_r1762679548)
A note for reviewers. The Guix builds are using CMake 3.24.2 (the `cmake-minimal` package).
🤔 hebasto reviewed a pull request: "ci: Use macos-14 GHA image"
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2308880257)
Concept ACK.
Perhaps explicitly mention the switch from the `x86_64` architecture to `arm64`?
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2308880257)
Concept ACK.
Perhaps explicitly mention the switch from the `x86_64` architecture to `arm64`?