Docker currently warns that we are missing a default value.
Set this to scratch which will error if an appropriate image tag is not
passed in to silence the warning.
Previously jobs were running on a large multi-core server where 10 jobs
as default made sense (or may even have been on the low side).
Using hosted runners with fixed (and lower) numbers of vCPUs we should
adapt compilation to match the number of cpus we have dynamically.
This is cross-platform compatible with macos and linux only.
When using hosted runners in combination with cached docker images,
there is the possibility that the host runner image is updated,
rendering the linux-headers package (stored in the cached docker image)
incompatible.
Fix this by doing a re-install of the headers package in
03_test_script.sh.
If the underlying runner kernel has not changed thie has no effect, but
prevents the job from failing if it has.
To remove multiple occurances of the respository name, against which we
compare `${{ github.repository }}` to check if we should use Cirrus
Runners, introduce a helper job which can check a single environment
variable and output this as an input to subsequent jobs.
Forks can maintain a trivial patch of their repo name against the
`REPO_USE_CIRRUS_RUNNERS` variable in ci.yml if they have Cirrus Runners
of their own, which will then enable cache actions and docker build
cache to use Cirrus Cache.
It's not possible to use `${{ env.USE_CIRRUS_RUNNERS }}` in the
`runs-on:` directive as the context is not supported by GitHub.
If it was, this job would no longer be necessary.
Whilst the action cirruslabs/actions/cache will automatically set this
host, the docker `gha` build cache backend will not be aware of it.
Set the value here, which will later be used in the docker build args to
enable docker build cache on the cirrus cache.
This sets the build dir at build time so that Apple SDK gets installed
in the correct/expected location for the runtime to find it.
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Reverts: e87429a2d0f23eb59526d335844fa5ff5b50b21f
This was added in PR #31545 with the intention that self-hosted runners
might use it to save build cache.
As we are not using hosted runners with a registry build cache, the bulk
of this commit can be reverted, simply using the value of
$DOCKER_BUILD_CACHE_ARG in the script.
link: https://github.com/bitcoin/bitcoin/pull/31545
Using buildx is required to properly load the correct driver, for use
with registry caching. Neither build, nor BUILDKIT=1 currently do this
properly.
Use of `docker buildx build` is compatible with podman.
Another action to reduce boilerplate in the main ci.yml file.
This action will set up a docker builder compatible with caching build
layers to a container registry using the `gha` build driver.
It will then configure the docker build cache args.
If set, Cirrus runners will be used on pushes to, and pull requests
against, this repository.
Forks can set this if they have their own cirrus runners.
Add "Restore" and "Save" caching actions.
These actions reduce boilerplate in the main ci.yml configuration file.
These actions are implemented so that caches will be saved on `push`
only.
When a pull request is opened it will cache hit on the caches from the
lastest push, or in the case of depends will hit on any matching depends
hash, falling back to partial matches.
Depends caches are hashed using
`$(git ls-tree HEAD depends "ci/test/$FILE_ENV" | sha256sum | cut -d' ' -f1)`
and this hash is passed in as an input to the actions. This means we
direct cache hit in cases where depends would not be re-built, otherwise
falling back to a partial match.
Previous releases cache is hashed similarly to depends, but using the
test/get_previous_releases.py file.
The cirruslabs cache action will fallback transparently to GitHub's
cache in the case that the job is not being run on a Cirrus Runner,
making these compatible with running on forks (on free GH hardware).
b7b249d3adfbd3c7b0c4d0499a86300f57982972 Revert "[refactor] rewrite vTxHashes as a vector of CTransactionRef" (Anthony Towns)
b9300d8d0a74b15a220a2ce529d5157d109c7ed3 Revert "refactor: Simplify `extra_txn` to be a vec of CTransactionRef instead of a vec of pair<Wtxid, CTransactionRef>" (Anthony Towns)
df5a50e5de204013ba56e1c4967a249ca07ba086 bench/blockencodings: add compact block reconstruction benchmark (Anthony Towns)
Pull request description:
Reconstructing compact blocks is on the hot path for block relay, so revert changes from #28391 and #29752 that made it slower. Also add a benchmark to validate reconstruction performance, and a comment giving some background as to the approach.
ACKs for top commit:
achow101:
ACK b7b249d3adfbd3c7b0c4d0499a86300f57982972
polespinasa:
lgtm code review and tested ACK b7b249d3adfbd3c7b0c4d0499a86300f57982972
cedwies:
code-review ACK b7b249d
davidgumberg:
crACK b7b249d3adfbd3c
instagibbs:
ACK b7b249d3adfbd3c7b0c4d0499a86300f57982972
Tree-SHA512: dc266e7ac08281a5899fb1d8d0ad43eb4085f8ec42606833832800a568f4a43e3931f942d4dc53cf680af620b7e893e80c9fe9220f83894b4609184b1b3b3b42
493ba0f688311280056491986068cd4a9ad2be66 threading: reduce the scope of lock in getblocktemplate (kevkevinpal)
Pull request description:
This change was motivated by https://github.com/bitcoin/bitcoin/pull/32592#discussion_r2294770722
It does exactly what is said in the comment. Reducing the scope of the lock by a bit before it is needed
ACKs for top commit:
stickies-v:
re-ACK 493ba0f688311280056491986068cd4a9ad2be66
maflcko:
lgtm ACK 493ba0f688311280056491986068cd4a9ad2be66
Tree-SHA512: aa3a21ef3da6be6c0af78aa2dda61ee21c3f6d4d9c897413dba9e7d7d2a91e9e069bbc6b6684b45aadaa28d8603dd310f2c2d2e58c31bb4d864204e468fefaf1
509ffea40abbc706ef8b8fc449b7de8677fc5096 ci: return to using dash in CentOS job (fanquake)
Pull request description:
`dash` is available again: https://bugzilla.redhat.com/show_bug.cgi?id=2335416.
ACKs for top commit:
maflcko:
lgtm ACK 509ffea40abbc706ef8b8fc449b7de8677fc5096
davidgumberg:
ACK 509ffea40a
janb84:
crACK 509ffea40abbc706ef8b8fc449b7de8677fc5096
Tree-SHA512: c57194b6158f6453cadb2487be232af5e37aa2234852f04a76fc80909fbfa48c7f8dd30e7be41be67dedb7ec4886930e165fdbaf746d358bb94c6ccc49d6bde6
46ca7712cb5fcf759cfc9f4f32d74215c8c83763 threading: remove unused template instantiations (Cory Fields)
b537a6a6dbd3b2a6725ccfafd57c9cc50cd617b0 threading: remove obsolete critsect macros (Cory Fields)
0d0e0a39b4a58fc48d2f0107e8c08bece58130bc threading: use a reverse lock rather than manual critsect macros (Cory Fields)
3ddd554d318110e8270498760340854929c59405 tests: Add Assertions in reverse_lock tests to exercise thread-safety annotations (Cory Fields)
c88b1cbf57a333261dbb8cf2eae91cf76e056d96 tests: get rid of remaining manual critsect usage (Cory Fields)
Pull request description:
Now that #32467 is merged, the only remaining usage of our old `CRITICAL_SECTION` macros (other than tests) is in `getblocktemplate()` and it can safely be replaced with a `REVERSE_LOCK`.
This PR makes that replacement, replaces the old `CRITICAL_SECTION` macro usage in tests, then deletes the macros themselves.
~While testing this a few weeks ago, I noticed that `REVERSE_LOCK` does not currently work properly with our thread-safety annotations as after the `REVERSE_LOCK` is acquired, clang still believes that the mutex is locked. #32465 fixes this problem. Without that fix, this PR would potentially allow a false-negative if code were added in the future to this chunk of `getblocktemplate` which required `cs_main` to be locked.~
~I added a test for the reverse lock here in the form of a compiler warning in `reverselock_tests.cpp` to simulate that possibility. This PR will therefore cause a new warning (and should fail a warnings-as-errors ci check) until #32465 is merged and this is rebased on top of it.~
Edit: Rebased on top of #32465, so this should now pass tests.
ACKs for top commit:
maflcko:
review ACK 46ca7712cb5fcf759cfc9f4f32d74215c8c83763 📌
fjahr:
Code review ACK 46ca7712cb5fcf759cfc9f4f32d74215c8c83763
TheCharlatan:
ACK 46ca7712cb5fcf759cfc9f4f32d74215c8c83763
furszy:
ACK 46ca7712cb5fcf759cfc9f4f32d74215c8c83763
Tree-SHA512: 5e423c8539ed5ddd784f5c3657bbd63be509d54942c25149f04e3764bcdf897bebf655553338d5af7b8c4f546fc1d4dd4176c2bce6f4683e76ae4bb91ba2ec80
a602f6fb7bf5f9e57299f4d6e246c82379fad8d2 test: index with an unclean restart after a reorg (Martin Zumsande)
01b95ac6f496e24e525b2fc9d69ee8b543da65ff index: don't commit state in BaseIndex::Rewind (Martin Zumsande)
Pull request description:
The committed state of an index should never be ahead of the flushed chainstate.
Otherwise, in the case of an unclean shutdown, the blocks necessary to revert
from the prematurely committed state are not be available, which would corrupt the coinstatsindex in particular.
Instead, the index state will be committed with the next ChainStateFlushed notification.
Fixes#33208
ACKs for top commit:
achow101:
ACK a602f6fb7bf5f9e57299f4d6e246c82379fad8d2
stickies-v:
re-ACK a602f6fb7bf5f9e57299f4d6e246c82379fad8d2
Tree-SHA512: 2559ea3fe066caf746a54ad7daac5031332f3976848e937c3dc8b35fa2ce925674115d8742458bf3703b3916f04f851c26523b6b94aeb1da651ba5a1b167a419
1c3db0ed8e6f51d66facf4820a9225f6c617ac53 doc: use new block_to_connect parameter name (stickies-v)
Pull request description:
The parameter name was previously changed from `pblock` to `block_to_connect` in 9ba1fff29e4794615c599e59ef453848a9bdb880, without updating the documentation.
Addresses https://github.com/bitcoin/bitcoin/pull/33078#discussion_r2279914775.
ACKs for top commit:
purpleKarrot:
ACK 1c3db0ed8e6f51d66facf4820a9225f6c617ac53
janb84:
ACK 1c3db0ed8e6f51d66facf4820a9225f6c617ac53
musaHaruna:
ACK [1c3db0e](1c3db0ed8e)
Tree-SHA512: 8b12243f1d9e5586e487dd705dc5b40ff12025bb5539eb4195f7fde4df38a9fe8eb0a9570a72f9463a2420f7307358409804fcb23bb73e32ff691ac4ef5bc35a
