🚀 achow101 merged a pull request: "streams: cache file position within AutoFile"
(https://github.com/bitcoin/bitcoin/pull/30884)
(https://github.com/bitcoin/bitcoin/pull/30884)
👋 achow101's pull request is ready for review: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827)
(https://github.com/bitcoin/bitcoin/pull/30827)
💬 achow101 commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2354420266)
Backported in #30827
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2354420266)
Backported in #30827
💬 maflcko commented on pull request "AssumeUTXO: Don't Assume m_chain_tx_count in GuessVerificationProgress":
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2354677700)
> However, as the test uncovered, `GuessVerificationProgress` is called with snapshot blocks that have `m_chaint_tx_count = 0` when the assumeutxo background sync is in progress.
Can you explain this a bit better? IIRC the snapshot base block itself has the count set, as well as any validated blocks after it. See `src/node/blockstorage.cpp: base->m_chain_tx_count = au_data.m_chain_tx_count;
`. So I think calling `GuessVerificationProgress` in this case shouldn't lead to issues. Wherea
...
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2354677700)
> However, as the test uncovered, `GuessVerificationProgress` is called with snapshot blocks that have `m_chaint_tx_count = 0` when the assumeutxo background sync is in progress.
Can you explain this a bit better? IIRC the snapshot base block itself has the count set, as well as any validated blocks after it. See `src/node/blockstorage.cpp: base->m_chain_tx_count = au_data.m_chain_tx_count;
`. So I think calling `GuessVerificationProgress` in this case shouldn't lead to issues. Wherea
...
💬 maflcko commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2354696900)
> I don't see a suggestion in the previous conversation to skip these.
All but a few linters apply to all files (this includes doc-only, or markdown-only files). See for example the subtree check: https://github.com/bitcoin/bitcoin/runs/30239893248
```
src/crc32c in HEAD currently refers to tree 81b971cac88189a11a3d7491d8e57437f534d195
src/crc32c in HEAD was last updated in commit 5d45552fd4303f8d668ffbc50cce1053485aeead (tree 454691a9b89ee8b9e1f71a48a7398edba49c3805)
diff --git a/REA
...
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2354696900)
> I don't see a suggestion in the previous conversation to skip these.
All but a few linters apply to all files (this includes doc-only, or markdown-only files). See for example the subtree check: https://github.com/bitcoin/bitcoin/runs/30239893248
```
src/crc32c in HEAD currently refers to tree 81b971cac88189a11a3d7491d8e57437f534d195
src/crc32c in HEAD was last updated in commit 5d45552fd4303f8d668ffbc50cce1053485aeead (tree 454691a9b89ee8b9e1f71a48a7398edba49c3805)
diff --git a/REA
...
💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2354720389)
I did a macOS 13 GHA run (https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/10886978505/job/30208236022) and it failed with: `2024-09-16T15:55:29.8666110Z [0;34m node0 stderr Error: A fatal internal error occurred, see debug.log for details: Corrupt block found indicating potential hardware failure. [0m` in `feature_block.py` on ~master.
So possibly Github could be facing hardware issues on macOS, leading to other issues down the line?
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2354720389)
I did a macOS 13 GHA run (https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/10886978505/job/30208236022) and it failed with: `2024-09-16T15:55:29.8666110Z [0;34m node0 stderr Error: A fatal internal error occurred, see debug.log for details: Corrupt block found indicating potential hardware failure. [0m` in `feature_block.py` on ~master.
So possibly Github could be facing hardware issues on macOS, leading to other issues down the line?
✅ Sjors closed a pull request: "ci: honor ci bypass prefix in test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/30898)
(https://github.com/bitcoin/bitcoin/pull/30898)
💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2354725686)
The only use case I have in mind is a large refactor where, e.g. to make the diff easier to follow, an intermediate commit can't build.
I'm not seeing much excitement for that functionality. Such PR can always cherry-pick this commit, so closing for now.
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2354725686)
The only use case I have in mind is a large refactor where, e.g. to make the diff easier to follow, an intermediate commit can't build.
I'm not seeing much excitement for that functionality. Such PR can always cherry-pick this commit, so closing for now.
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2354761241)
Rebased after #30440. Just some `#include` and `using` conflicts since that PR also touched the mining interface.
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2354761241)
Rebased after #30440. Just some `#include` and `using` conflicts since that PR also touched the mining interface.
📝 maflcko opened a pull request: "ci: Use macos-14 GHA image"
(https://github.com/bitcoin/bitcoin/pull/30913)
There shouldn't be any downside, because XCode remains pinned to the same version.
However, builds are expected to be a bit faster with M1, which seems nice.
(https://github.com/bitcoin/bitcoin/pull/30913)
There shouldn't be any downside, because XCode remains pinned to the same version.
However, builds are expected to be a bit faster with M1, which seems nice.
💬 Sjors commented on pull request "Remove Taproot activation height":
(https://github.com/bitcoin/bitcoin/pull/26201#issuecomment-2354780061)
#27433 needed the rebase here
(https://github.com/bitcoin/bitcoin/pull/26201#issuecomment-2354780061)
#27433 needed the rebase here
💬 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?