forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 0
add block obfuscation warning #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Filter netlink responses to only consider default routes by checking the destination prefix length (rtm_dst_len == 0). Previously, we selected the first route with an RTA_GATEWAY attribute, which for IPv6 often resulted in choosing a non-default route instead of the actual default. This caused occasional PCP port mapping failures because a gateway for a non-default route was selected.
This shouldn't usually be hit, but is a good belt-and-braces.
Unified the deprecation warning for the recently deprecated datacarrier[size] options to match the phrasing of release-notes-32406.md.
Currently a `BUILD_FOR_FUZZING` build will failure to configure, with missing `capnp`.
The assumption was removed in commit 3eab8b7.
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).
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.
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.
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.
Reverts: e87429a This was added in PR bitcoin#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: bitcoin#31545
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>
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.
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.
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
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.
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Co-authored-by: Max Edwards <youwontforgetthis@gmail.com>
Removed as unused.
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.
Print the ccache hit-rate for the job using a GitHub annotation if it was below 75%.
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.
ci/lint_run.sh: Only used in .cirrus.yml. Refer to test/lint/README.md on how to run locally. ci/lint_run_all.sh: Only used in .cirrus.yml for stale re-runs of old pull request tasks.
fa3f682 test: Fixup fill_mempool docstring (MarcoFalke) Pull request description: The assumption was removed in commit 3eab8b7. ACKs for top commit: l0rinc: ACK fa3f682 enirox001: ACK bitcoin@fa3f682 - docstring cleanup, looks good. pablomartin4btc: ACK fa3f682 naiyoma: ACK fa3f682 Tree-SHA512: 5b1fc003b5bea4f98c1b79376b06f5565ccddaf6b4969e1b16f1225755e1c68a5fde3ccafa86d2923a92aa8a8f9c47dfd048be2ff705d7269664188cf11da3f4
…e notes 2885bd0 doc: unify `datacarriersize` warning with release notes (Lőrinc) Pull request description: Follow-up to bitcoin#32406 --- The [release notes](https://github.com/bitcoin/bitcoin/blob/a189d636184b1c28fa4a325b56c1fab8f44527b1/doc/release-notes-32406.md#L1) claim > [...] marked as deprecated and are expected to be removed in a future release but the [warning itself](https://github.com/bitcoin/bitcoin/blob/2885bd0e1c4fc863a7f28ff0fd353f5cffb03442/src/init.cpp#L907) claims > [...] marked as deprecated. They **will** be removed in a future version. To be less aggressive (since some have objected against this version online) - and to unify the deprecation warning with the release notes - I have changed the warning to communicate our expectation in a friendlier way. ACKs for top commit: cedwies: ACK 2885bd0 ryanofsky: Code review ACK 2885bd0. I don't think it is good for the release notes and the runtime warning message to say two different things. I'd also be happy if release notes were updated to match the runtime warning, instead of vice versa. Whatever is more accurate is better. ajtowns: ACK 2885bd0 kevkevinpal: ACK [2885bd0](bitcoin@2885bd0) achow101: ACK 2885bd0 janb84: ACK 2885bd0 Zero-1729: crACK 2885bd0 jonatack: ACK 2885bd0 hodlinator: ACK 2885bd0 w0xlt: ACK bitcoin@2885bd0 optout21: ACK 2885bd0 Tree-SHA512: a9d2a64ab96b3dd7f3a1a29622930054fd5c56e573bc96330f4ef3327dc024b21b3fbc8a698d17aea7c76f57f0c2ccd6403b2df344ae2f69c645ceb8b6fa54a5
755152a kernel: add testnet4 assumeutxo param at height 90'000 (fanquake) a651268 kernel: add mainnet assumeutxo param at height 910'000 (fanquake) 943de66 kernel: update headersync params (fanquake) 66fb962 kernel: update chainTxData (fanquake) c3cb26e kernel: update assumevalid and minimumChainWork (fanquake) b4adae7 kernel: update assumed blockchain & chainstate sizes (fanquake) Pull request description: Also adds assumeutxo params for mainnet at `910'000` & testnet4 & `90'000`. ACKs for top commit: Sjors: re-ACK 755152a achow101: ACK 755152a w0xlt: ACK bitcoin@755152a hodlinator: ACK 755152a Tree-SHA512: d26b023b1fdbb72e81c3879aa274e3900742f015d77c1bb3cde00b3b882642c6b35a6268c6ccf35fa9243b58b5a1ebe6eba3eb7f58678eee5bd8a6d6e7518a6f
3c5da69 ci: remove un-needed lint_run*.sh files (willcl-ark) 2aa288e ci: fix annoying docker warning (will) dd1c590 ci: add ccache hit-rate warning when < 75% (will) f427284 doc: Detail configuration of hosted CI runners (will) 3f339e9 ci: dynamically match makejobs with cores (will) 4393ffd ci: remove .cirrus.yml (will) bc41848 ci: port lint (will) d290a8e ci: port msan-depends (will) 9bbae61 ci: port tsan-depends (will) bf7d536 ci: port tidy (will) 549074b ci: port centos-depends-gui (will) 58e38c3 ci: port previous-releases-depends-debug (will) 341196d ci: port fuzzer-address-undefined-integer-nodepends (will) f2068f2 ci: port no-IPC-i686-DEBUG (will) 2a00b12 ci: port nowallet-libbitcoinkernel (will) 9c2514d ci: port mac-cross-gui-notests (will) 2c990d8 ci: force reinstall of kernel headers in asan (will) 8842514 ci: update asan-lsan-ubsan (will) f253031 ci: port arm 32-bit job (will) 04e7bfb ci: update windows-cross job (will) cc1735d ci: add job to determine runner type (will) 020069e ci: add Cirrus cache host (will) 9c2b96e ci: have base install run in right dir (will) 18f6be0 ci: use docker build cache arg directly (will) 94a0932 ci: use buildx in ci (will) fdf64e5 ci: add configure-docker action (will) 33ba073 ci: add REPO_USE_CIRRUS_RUNNERS (will) b232b0f ci: add caching actions (will) b8fcc9f ci: add configure environment action (will) Pull request description: This changeset migrates all current self-hosted CI jobs over to hosted [Cirrus Runners](https://cirrus-runners.app/). These runners cost a flat rate of $150/month, and we qualify for an open source discount of 50%. Therefore they are $75/month/runner. One "runner" should more accurately be thought of in terms of the number of vCPU you are purchasing: https://cirrus-runners.app/pricing/ or in terms of "concurrency", where 1 runners gets you 1.0 concurrency. e.g. a Linux x86 Runner gets you 16 vCPU (1.0 concurrency) and 64GB RAM to be provisioned as you choose, amongst one or more jobs. Cirrus Runners currently only support Linux (x86 and Arm64) and MacOS (Arm64). This changeset does **not** move the existing Github Actions native MacOS runners away from being run on Github's infrastructure. This could be a follow up optimisation. Runs from this changeset using Cirrus Runners can be found at: https://github.com/testing-cirrus-runners/bitcoin2/actions which shows an uncached run on master ([CI#1](https://github.com/testing-cirrus-runners/bitcoin2/actions/runs/16298637161)), an outside pull request ([CI#3](https://github.com/testing-cirrus-runners/bitcoin2/actions/runs/16303305483?pr=1)) and an updated push to master ([CI#4](https://github.com/testing-cirrus-runners/bitcoin2/actions/runs/16304182527)). These workflows were run on 10 runners, and we would recommend purchasing a similar number for our CI in this repo to achieve the speed and concurrency we expect. We include some optional performance commits, but these could be split out and made into followups or dropped entirely. ## Benefits ### Maintenance As we are not self-hosting, nobody needs to maintain servers, disks etc. ### Bus factor Currently we have a very small number of people with the know-how working on server setup and maintenance. This setup fixes that so that "anyone" familiar with GitHub-style CI systems can work on it. ### Scaling These do _not_ "auto-scale"/have "unlimited concurrency" like some solutions, but if we want more workers/cpu to increase parallism or increase the runner size of certain jobs for a speed-up we can simply buy more concurrency using the web interface. ### Speed Runtimes aproximate current runtimes pretty well, with some jobs being faster. Caching improvements on pull request (re-runs) are left as future optimisations from the current changeset (see below). ### GitHub workflow syntax With a migration to the more-commonly-used GitHub workflow syntax, migration to other providers in the future is often as simple as a one-line change (and installing a new GitHub app to the repo). If we decide to self-host again, then we can also self-host GitHub runners (using https://github.com/actions/runner) and maintain new GH-style CI syntax. ### Reporting GitHub workflows provide nicer built-in reporting directly on the "Checks" page of a pr. This includes more-detailed action reporting, and a host of pretty nice integrated features, such as [Workflow Commands](https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions) for creating annotations that can print messages during runs. See for example at the bottom of this window where we report `ccache` hitrate, if it was below 90%: https://github.com/testing-cirrus-runners/bitcoin/actions/runs/16163449125?pr=1 These could be added conditionally into our CI scripts to report interesting or other information. ## Costs ### Financial Relative to competitors Cirrus runners are cheap for the hosted CI-world. However these are likely more expensive than our current setup, or a well-configured (new) self-hosted setup. If we started with 10 runners to be shared amongst all migrated jobs, this would total $750/mo = $9000/yr. Note that we are not trying to comptete here on cost directly. ### Dependencies We would be dependent on Cirrus infra. ## Forks - Forks should be able to run CI without paid Cirrus runners. This behaviour is achieved through a rather verbose `runs-on:` directive. - This directive hardcodes the main repo (unfortunately you cannot use the `env` github context in this field in particular, for some reason). - This directive also allows for a fork to patch the `runs-on:` field in the ci.yml file if they want to use Cirrus Runners too. - The workflow otherwise will fallback to the GitHub free runners on forks. - This cirrus cache action transparently falls back to github actions cache when not running on cirrus, so forks will get some free github caching (10GB per repo). All jobs work on forks, but will run (slowly) on GitHub native free hosted runners, instead of Cirrus runners. They will also suffer from poor cache hit-rates, but there's nothing that can be done about that, and the situtation is an improvement on today. ## Migration process The main org should also, in addition to pulling code changes: 1. Permit the actions `docker/setup-buildx-action@v3` and `docker/login-action@v3` to be run in this repo. ## Caching For the number of CI jobs we have, cache usage on GitHub would be an issue as GH only provides 10GB of cache space, **per repo**. However cirrus provides [10 GB per runner](https://cirrus-runners.app/setup/#speeding-up-the-cache), which scales better with the number of runners. The `cirruslabs/action/[restore|save]` action we use here redirects this to Cirrus' own cache and is both faster and larger. In the case that user is running CI on a fork, the cirrus cache falls back transparently to GitHub default cache without error. ### ccache, depends-sources, built-depends - Cached as blobs via `cirruslabs/actions/cache` action. - Current implementation: - On `push`: restores and saves caches. - On `pull_request`: restores but does **not** save caches. This means a new pull request should hit a _pretty relevant_ cache. Old pull requests **which are not being rebased on master** may suffer from lower cache hit-rate. If we save caches on all pull request runs we run the risk of evicting recent (and more relevant) cache blobs. It may be possible in a future optimisation to widen this to save on pull request runs too, but it will also depend on how many runners we provision and what cache churn rates are like in the main repo. ### Docker build layer caching - Cached using the `gha` cache backend - These cache blobs compete for space with `ccache`, `depends-sources` and `depends-built` caches - `gha` cache allows `--cache-from` to be used from pull requests, which does not work using a registry cache type (technically we could use a public read-only token to get this working, but that feels wrong) This backend does network i/o and so are marginally slower than our current disk i/o cache. ## But what about... `x`? We have tested many other providers, including [Runs-on](https://runs-on.com/), [Buildjet](https://buildjet.com/), [WarpBuild](https://www.warpbuild.com/), and GitHub hosted runners (and investigated even more). But they all fall short in one-way or another. - Runs-On and Buildjet (and others) require installing GH apps with much too-liberal permissions (e.g. `Administration: Read|Write`) for our use-case. - GitHub hosted runners suffer from all of high costs, lower speed, small cache, and the requirement for a GitHub Teams subscription. - WarpBuild seems to be simply too expensive. ## TODO: To complete migration from self-hosted to hosted for this repo, the backport branches `27.x`, `28.x` and `29.x` would also need their CI ported, but these are left for followups to this change (and pending review/changes here first). ----- Work and experimentation undertaken with m3dwards ACKs for top commit: maflcko: re-ACK 3c5da69 🏗 m3dwards: ACK 3c5da69 achow101: ACK 3c5da69 janb84: re ACK 3c5da69 Tree-SHA512: 9f7f2dddf1a5eebc56b4101663283d4219d189cda6054dba760f1288bed9e6ed3f2fa029a5caedc76c31b1271ea0a0cb0967a796086360d8f5be8277379b6397
…tions 7270839 doc: truc packages allow sub min feerate transactions (Pol Espinasa) Pull request description: Fixes bitcoin#32067 Some policy documentation is outdated since TRUC. This PR aims to update the documentation to the actual policy state. ACKs for top commit: w0xlt: reACK bitcoin@7270839 glozow: ACK 7270839 Tree-SHA512: 1272e7acc76c76d7e145cdd07827ece31253dba4b99b9a22fc986fcd538830e46392fda877736cb496f3e53a0abcb9d8403d439bb1da63b88da7f8b6f17b6c8b
Handle multi-part netlink responses to prevent truncated results from large routing tables. Previously, we only made a single recv call, which led to incomplete results when the kernel split the message into multiple responses (which happens frequently with NLM_F_DUMP). Also guard against a potential hanging issue where the code would indefinitely wait for NLMSG_DONE for non-multi-part responses by detecting the NLM_F_MULTI flag and only continue waiting when necessary.
af4156a build: set ENABLE_IPC to OFF when fuzzing (fanquake) Pull request description: A `BUILD_FOR_FUZZING` build will currently failure to configure, with missing `capnp`. ACKs for top commit: Crypt-iQ: tACK af4156a marcofleon: ACK af4156a dergoegge: utACK af4156a janb84: ACK af4156a Tree-SHA512: e3c5238cb5823116a958502eab84ee72a94cac0853fc3908ef97b6b6dc037db27806be0726f321d70ab706c37924dec526b46a3a46ea3f3f3684ce48da46a803
… for default route while querying default gateway 88db09b net: handle multi-part netlink responses (willcl-ark) 42e99ad net: skip non-route netlink responses (willcl-ark) 57ce645 net: filter for default routes in netlink responses (willcl-ark) Pull request description: ...for default route in pcp pinholing. Currently we only make a single recv call, which trucates results from large routing tables, or in the case the kernel may split the message into multiple responses (which may happen with `NLM_F_DUMP`). We also do not filter on the default route. For IPv6, this led to selecting the first route with an `RTA_GATEWAY` attribute, often a non-default route instead of the actual default. This caused PCP port mapping failures because the wrong gateway was used. Fix both issues by adding multi-part handling of responses and filter for the default route. Limit responses to ~ 1MB to prevent any router-based DoS. ACKs for top commit: achow101: ACK 88db09b davidgumberg: Code Review re-ACK 88db09b Sjors: re-utACK 88db09b Tree-SHA512: ea5948edebfad5896a487a61737aa5af99f529fad3cf3da68dced456266948238a7143383847e79a7bb90134e023eb173c25116d8eb80ff57fa4c4a0377ca1ed
fae610d ci: Remove redundant RUN_UNIT_TESTS_SEQUENTIAL (MarcoFalke) Pull request description: `RUN_UNIT_TESTS_SEQUENTIAL` is useful to detect cases where global state is left dirty in the test process and leads to subsequent unit test cases failing. However, one CI task is sufficient to catch this. As there already is one, add docs there and remove this env var (and extra logic). ACKs for top commit: fanquake: ACK fae610d Tree-SHA512: b7ace1257d039f144cb0acb08d5d19d641028464517e6a2468e248ed79b2511512dc904867dacd66157b7483ec8041c95cce00f8ce3c89f3a2c3bb47939d7ff9
9b76eef ci: cd into BASE_BUILD_DIR for GetCMakeLogFiles (Sjors Provoost) Pull request description: When a bug is introduced in cmake, we render its logs, which was broken: bitcoin#33290 (comment) ACKs for top commit: maflcko: lgtm ACK 9b76eef 💽 hebasto: ACK 9b76eef, [obviously](bitcoin#33290 (comment)). Tree-SHA512: 97a440efcaa058af1144d66257ac0157d499b66fd9a00fa01e0eaa013267af1643cb062ec6c74366fab88cb706c9cf9d5fb0316195e79582679c7ae8d39ec200
When the router doesn't support natpmp and PCP, one'd normally expect the UDP packet to be ignored, and hit a time out. This logs a warning that is already in the debug category. However, there's also the case in which sending an UDP packet causes a ICMP response. This is returned to user space as "connection refused" (despite UDP having no concept of connections). Move the warnings from `Send` and `Recv` to debug level too, to reduce log spam in that case. Closes bitcoin#33301.
…ort natpmp/pcp 4f1a4cb net: Quiet down logging when router doesn't support natpmp/pcp (laanwj) Pull request description: When the router doesn't support natpmp and PCP, one'd normally expect the UDP packet to be ignored, and hit a time out. This logs a message that is already in the debug category. However, there's also the case in which sending an UDP packet causes a ICMP response (type 3, code 3 "port unreachable"). This is returned to user space as "connection refused" (despite UDP having no concept of connections). Move the warnings from `Send` and `Recv` to debug level too, to reduce log spam in that case. Closes bitcoin#33301. ACKs for top commit: willcl-ark: utACK 4f1a4cb sipa: utACK 4f1a4cb davidgumberg: Tested ACK bitcoin@4f1a4cb achow101: ACK 4f1a4cb darosior: utACK 4f1a4cb mzumsande: utACK 4f1a4cb Tree-SHA512: 2c99a5679720482ece47af33616b6b207509fb58ba1962a1c2d30f8d0e68554f8f5ef25224313d93f4c5a1cc702183fcf8e6119abc411209c9884119ef680aad
-blocksxor ?
e0ae99a to
15c66fd
Compare
l0rinc
pushed a commit
that referenced
this pull request
Jan 9, 2026
Bench (AArch64 Cortex-A76, -O2, taskset core 2, `bench_bitcoin -filter='CHACHA20_.*' -min-time=1000`, 5 runs; median [min-max] ns/byte):
- clang 22: 1MB 1.80 [1.79-1.80], 256B 1.63 [1.63-1.64], 64B 2.59 [2.57-2.60]
- gcc 14: 1MB 5.37 [5.37-5.38], 256B 5.14 [5.13-5.15], 64B 2.70 [2.70-2.75]
The speedup/slowdown only shows up once we hit the multi-block path (1MB/256B).
Single-block (64B) remains scalar and stays ~unchanged.
Assembly highlights (AArch64):
- clang emits NEON-friendly rotates/shuffles (`shl`+`usra` and `ext`) with a small stack frame.
- gcc emits a very large stack frame and scalar pack/unpack sequences around shuffles.
Example prologue (gcc):
mov x13, #0x9160
sub sp, sp, x13
Example inner-sequence (gcc):
fmov x18, d18
bfxil x10, x18, #0, #32
Example inner-sequence (clang):
usra v25.4s, v16.4s, #25
ext v22.16b, v10.16b, v10.16b, #4
Delta vs previous commit:
- clang: ~18% faster at 1MB (2.20 -> 1.80 ns/B)
- gcc: ~2.2x slower at 1MB (2.45 -> 5.37 ns/B) due to poor multi-state codegen.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TODO