1.15.0 dev improve random upstream cherry picks #3487
Draft
Conversation
This file contains 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
These are available in sandboxes without access to files or devices. Also [they are safer and more straightforward](https://en.wikipedia.org/wiki/Entropy-supplying_system_calls) to use than `/dev/urandom` as reading from a file has quite a few edge cases: - Linux: `getrandom(buf, buflen, 0)`. [getrandom(2)](http://man7.org/linux/man-pages/man2/getrandom.2.html) was introduced in version 3.17 of the Linux kernel. - OpenBSD: `getentropy(buf, buflen)`. The [getentropy(2)](http://man.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man2/getentropy.2) function appeared in OpenBSD 5.6. - FreeBSD and NetBSD: `sysctl(KERN_ARND)`. Not sure when this was added but it has existed for quite a while. Alternatives: - Linux has sysctl `CTL_KERN` / `KERN_RANDOM` / `RANDOM_UUID` which gives 16 bytes of randomness. This may be available on older kernels, however [sysctl is deprecated on Linux](https://lwn.net/Articles/605392/) and even removed in some distros so we shouldn't use it. Add tests for `GetOSRand()`: - Test that no error happens (otherwise `RandFailure()` which aborts) - Test that all 32 bytes are overwritten (initialize with zeros, try multiple times) Discussion: - When to use these? Currently they are always used when available. Another option would be to use them only when `/dev/urandom` is not available. But this would mean these code paths receive less testing, and I'm not sure there is any reason to prefer `/dev/urandom`. Closes: #9676
Move the OS random test to a sanity check function that is called every time bitcoind is initialized. Keep `src/test/random_tests.cpp` for the case that later random tests are added, and keep a rudimentary test that just calls the sanity check.
If the code was compiled with newer (>=3.17) kernel headers but executed on a system without the system call, every use of random would crash the program. Add a fallback for that case.
|
I benched this on both an x86_64 Intel I7 and an arm64 Apple M1. Results: On i7-970Current FastRandomContext (at 8d4e0bd)New FastRandomContext (at 9c33a27)On Apple M1Current FastRandomContextNew FastRandomContext |
|
I picked the following on top for a ~10% performance increase on the 32-bit FastRandomContext call:
There are probably more enhancements that are worth picking, but this (on the same i7-970 as above) at least consistently gave me: |
|
Everything is looking good so far. Tested successfully on x86_64-pc-linux-gnu jammy. |
This was referenced Mar 27, 2024
|
I can't seem to reproduce a clean cherry-pick like you mention, @patricklodder . Did you reproduce from 9c33a2 or is there another commit? I get conflicts around |
@chromatic This one is a bit further down the road but in comparison here are my results with those cherry-picks @patricklodder mentioned: |
|
Running on a i5-10300H* |
This was referenced Apr 4, 2024
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.
This is a work in progress to cherry-pick some upstream randomization improvements (see #3229 for context).