random: mark RandAddPeriodic and SeedPeriodic as noexcept #17507

fanquake · 2019-11-18T15:27:31Z

The usage of MilliSleep() in SeedPeriodic (previously SeedSleep) was
removed in #17270, meaning it, and its users can now be marked noexcept.

This also corrects the docs in random.h for some of the changes in #17270.

The usage of MilliSleep() in SeedPeriodic (previously SeedSleep) was removed in bitcoin#17270, meaning it, and its users can now be marked noexcept.

laanwj · 2019-11-18T15:35:42Z

src/random.cpp

- *
- * TODO: After moving away from interruptible boost-based thread management,
- * everything can become noexcept here.
+ * None of the RNG code should ever throw any exception.


Very good 👍

practicalswift · 2019-11-18T15:49:34Z

ACK 55b2cb1

Very nice!

Somewhat related: it would be nice to have the same explicit guarantee for LogPrint/LogPrintf. That would allow compilers and static analyzers to treat a larger parts of our code base as "non-throwing" instead of "potentially throwing" :)

laanwj · 2019-11-18T16:32:56Z

Somewhat related: it would be nice to have the same explicit guarantee for LogPrint/LogPrintf.

Wouldn't that be easy to do? LogPrintf already catches the only relevant exception, tinyformat::format_error, internally. I don't think any exceptions are supposed to leak out of it.

practicalswift · 2019-11-18T16:42:28Z

@laanwj Yes, exactly! It is only a matter of formalizing that with a noexcept :)

maflcko · 2019-11-18T17:26:30Z

@practicalswift Wouldn't upstream be a better place to propose this change?

practicalswift · 2019-11-18T17:37:53Z

@MarcoFalke Upstream throws tinyformat::format_error which we catch: thus noexcept only makes sense on our end of things, no? :)

maflcko · 2019-11-18T17:55:38Z

Fine, but LogPrintStr is also called, which might throw exceptions. In either case, this discussion should happen elsewhere.

sipa

ACK 55b2cb1

laanwj · 2019-11-19T07:50:57Z

I don't think LogPrintStr is supposed to ever raise an exception, but sure, that'd need to be checked. Agree this is off topic here.

ACK 55b2cb1

practicalswift · 2019-11-19T09:30:23Z

@laanwj @MarcoFalke Please note that SeedPeriodic which is annotated noexcept in this PR calls LogPrintf:

bitcoin/src/random.cpp

Lines 480 to 491 in 2065ef6

    
           static void SeedPeriodic(CSHA512& hasher, RNGState& rng) 
        
           { 
        
               // Everything that the 'fast' seeder includes 
        
               SeedFast(hasher); 
        
               // High-precision timestamp 
        
               SeedTimestamp(hasher); 
        
               // Dynamic environment data (performance monitoring, ...) 
        
               auto old_size = hasher.Size(); 
        
               RandAddDynamicEnv(hasher); 
        
               LogPrintf("Feeding %i bytes of dynamic environment data into RNG\n", hasher.Size() - old_size);

Thus the question of a noexcept annotation for LogPrintf is not entirely off topic here :)

laanwj · 2019-11-19T10:07:51Z

Thus the question of a noexcept annotation for LogPrintf is not entirely off topic here :)

Sure. I'm going to work on a PR to make logging noexcept.

DrahtBot · 2019-12-05T06:10:45Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#17670 (Move events_hasher into RNGState() by sipa)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

sipa · 2019-12-05T08:41:06Z

ReACK. I accidentally redid part of this in #17670. The documentation improvements here should be done anyway, so let's do this PR first.

55b2cb1 random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake) 461e547 doc: correct random.h docs after #17270 (fanquake) Pull request description: The usage of `MilliSleep()` in SeedPeriodic (previously SeedSleep) was [removed](d61f2bb) in #17270, meaning it, and its users can now be marked `noexcept`. This also corrects the docs in random.h for some of the changes in #17270. ACKs for top commit: practicalswift: ACK 55b2cb1 laanwj: ACK 55b2cb1 sipa: ACK 55b2cb1 Tree-SHA512: 672d369796e7c4f9b4d98dc545e5454999fa1bef373871994a26041d6163c58909e2255e4f820d3ef011679aa3392754eb57477306a89f5fd3d57e2bd7f0811a

… noexcept 55b2cb1 random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake) 461e547 doc: correct random.h docs after bitcoin#17270 (fanquake) Pull request description: The usage of `MilliSleep()` in SeedPeriodic (previously SeedSleep) was [removed](bitcoin@d61f2bb) in bitcoin#17270, meaning it, and its users can now be marked `noexcept`. This also corrects the docs in random.h for some of the changes in bitcoin#17270. ACKs for top commit: practicalswift: ACK 55b2cb1 laanwj: ACK 55b2cb1 sipa: ACK 55b2cb1 Tree-SHA512: 672d369796e7c4f9b4d98dc545e5454999fa1bef373871994a26041d6163c58909e2255e4f820d3ef011679aa3392754eb57477306a89f5fd3d57e2bd7f0811a

Summary: ``` The usage of MilliSleep() in SeedPeriodic (previously SeedSleep) was removed in #17270, meaning it, and its users can now be marked noexcept. This also corrects the docs in random.h for some of the changes in #17270. ``` Backport of core [[bitcoin/bitcoin#17507 | PR17507]]. Depends on D6209. Test Plan: ninja all check-all Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6211

… noexcept 55b2cb1 random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake) 461e547 doc: correct random.h docs after bitcoin#17270 (fanquake) Pull request description: The usage of `MilliSleep()` in SeedPeriodic (previously SeedSleep) was [removed](bitcoin@d61f2bb) in bitcoin#17270, meaning it, and its users can now be marked `noexcept`. This also corrects the docs in random.h for some of the changes in bitcoin#17270. ACKs for top commit: practicalswift: ACK 55b2cb1 laanwj: ACK 55b2cb1 sipa: ACK 55b2cb1 Tree-SHA512: 672d369796e7c4f9b4d98dc545e5454999fa1bef373871994a26041d6163c58909e2255e4f820d3ef011679aa3392754eb57477306a89f5fd3d57e2bd7f0811a

cecbf6c Use secure.h header for secure allocators (Fuzzbawls) d9f67da net: add ifaddrs.h include (fanquake) e906436 build: check if -lsocket is required with *ifaddrs (fanquake) 414f405 rand: only try and use freeifaddrs if available (fanquake) 3a039d6 build: avoid getifaddrs when unavailable (Cory Fields) 77bddd7 Use GetStrongRandBytes in gmp bignum initialization (Fuzzbawls) b70b26f Fix typo in comment in randomenv.cpp (Fuzzbawls) fec460c Put bounds on the number of CPUID leaves explored (Pieter Wuille) 41ab1ff Fix CPUID subleaf iteration (Pieter Wuille) 8a9bbb1 Move events_hasher into RNGState() (Pieter Wuille) 88c2ae5 random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake) 81d382f doc: correct random.h docs after bitcoin#17270 (fanquake) f363ea9 Seed RNG with precision timestamps on receipt of net messages. (Matt Corallo) 7d6ddcb Run background seeding periodically instead of unpredictably (Pieter Wuille) 4679181 Add information gathered through getauxval() (Pieter Wuille) 88d97d0 Feed CPUID data into RNG (Pieter Wuille) 8f5b9c9 Use sysctl for seeding on MacOS/BSD (Pieter Wuille) 67de246 Gather additional entropy from the environment (Pieter Wuille) 6142e1f Seed randomness with process id / thread id / various clocks (Pieter Wuille) 7bde8b7 [MOVEONLY] Move cpuid code from random to compat/cpuid (Fuzzbawls) 52b5336 [MOVEONLY] Move perfmon data gathering to new randomenv module (Pieter Wuille) 27cf995 doc: minor corrections in random.cpp (fanquake) fccd2b8 doc: correct function name in ReportHardwareRand() (fanquake) 909473e Fix FreeBSD build by including utilstrencodings.h (Fuzzbawls) 630931f break circular dependency: random/sync -> util -> random/sync (Fuzzbawls) 5eed08c random: remove call to RAND_screen() (Windows only) (fanquake) ada9868 gui: remove OpenSSL PRNG seeding (Windows, Qt only) (fanquake) 22a7121 Fix non-deterministic coverage of test DoS_mapOrphans (Fuzzbawls) 79e7fd3 Add ChaCha20 bench (Jonas Schnelli) 6966aa9 Add ChaCha20 encryption option (XOR) (Jonas Schnelli) 28c9cdb tests: Add script checking for deterministic line coverage (practicalswift) c82e359 test: Make bloom tests deterministic (MarcoFalke) 7b33223 Document strenghtening (Pieter Wuille) 0190dec Add hash strengthening to the RNG (Pieter Wuille) 67e336d Use RdSeed when available, and reduce RdRand load (Pieter Wuille) 4ffda1f Document RNG design in random.h (Pieter Wuille) 2b6381e Use secure allocator for RNG state (Pieter Wuille) 080deb3 Encapsulate RNGState better (Pieter Wuille) 787d72f DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille) 5bc2583 Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille) 774899f Remove hwrand_initialized. (Pieter Wuille) 698d133 Switch all RNG code to the built-in PRNG. (Pieter Wuille) 038a45a Integrate util/system's CInit into RNGState (Fuzzbawls) 5f20e62 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille) 298f97c Add thread safety annotations to RNG state (Pieter Wuille) 2326535 Rename some hardware RNG related functions (Pieter Wuille) d76ee83 Automatically initialize RNG on first use. (Pieter Wuille) 1a5dbc5 Don't log RandAddSeedPerfmon details (Pieter Wuille) 32e6c42 Simplify testing RNG code (Fuzzbawls) 972effa Make unit tests use the insecure_rand_ctx exclusively (Fuzzbawls) af52bf5 Use a FastRandomContext in LimitOrphanTxSize (Fuzzbawls) 746d466 Introduce a Shuffle for FastRandomContext and use it in wallet (Fuzzbawls) 1cdf124 Use a local FastRandomContext in a few more places in net (Fuzzbawls) e862564 Make addrman use its local RNG exclusively (Fuzzbawls) 94b2ead Make FastRandomContext support standard C++11 RNG interface (Pieter Wuille) Pull request description: This is a collection of upstream PRs that have been backported to bring our RNG (`src/random`) code more up-to-date. The following upstream PRs have been included here: - bitcoin#12742 - bitcoin#14624 - some of this had already been merged previously - bitcoin#14955 - bitcoin#15250 - bitcoin#15224 - bitcoin#15324 - bitcoin#15296 - bitcoin#15512 - bitcoin#16878 - bitcoin#17151 - bitcoin#17191 - bitcoin#13236 - bitcoin#13314 - bitcoin#17169 - bitcoin#17270 - omitted last commit as our testing framework doesn't support it currently - omitted bitcoin@64e1e02, to be pulled in after our time utility is updated in a separate PR - bitcoin#17573 - bitcoin#17507 - bitcoin#17670 - bitcoin#17527 - bitcoin#14127 - bitcoin#21486 ACKs for top commit: furszy: ACK cecbf6c with a minor nit that can be easily tackled later. random-zebra: rebase utACK cecbf6c and merging... Tree-SHA512: 3463b693cc9bddc1ec15228d264a794f5c2f159073fafa2ccf6e2563abfeb4369e49505f97ca84f2478ca792bd07b66d2cd83c58044d6a0cae6af42d22f5784b

fanquake added 2 commits November 18, 2019 10:22

doc: correct random.h docs after bitcoin#17270

461e547

random: mark RandAddPeriodic and SeedPeriodic as noexcept

55b2cb1

The usage of MilliSleep() in SeedPeriodic (previously SeedSleep) was removed in bitcoin#17270, meaning it, and its users can now be marked noexcept.

fanquake added the Utils/log/libs label Nov 18, 2019

fanquake requested a review from sipa November 18, 2019 15:27

laanwj reviewed Nov 18, 2019

View reviewed changes

sipa reviewed Nov 18, 2019

View reviewed changes

laanwj mentioned this pull request Nov 19, 2019

util: Make logging noexcept #17514

Closed

laanwj merged commit 55b2cb1 into bitcoin:master Dec 5, 2019

laanwj mentioned this pull request Dec 5, 2019

Move events_hasher into RNGState() #17670

Merged

fanquake deleted the random_followups branch December 5, 2019 14:16

Naviabheeman mentioned this pull request Apr 3, 2020

Remove Openssl dependency from Tapyrus chaintope/tapyrus-core#61

Closed

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

Fuzzbawls mentioned this pull request Mar 31, 2021

[Refactor] Update RNG code from upstream PIVX-Project/PIVX#2278

Merged

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

random: mark RandAddPeriodic and SeedPeriodic as noexcept #17507

random: mark RandAddPeriodic and SeedPeriodic as noexcept #17507

Uh oh!

fanquake commented Nov 18, 2019

Uh oh!

laanwj Nov 18, 2019

Uh oh!

practicalswift commented Nov 18, 2019

Uh oh!

laanwj commented Nov 18, 2019 •

edited

Loading

Uh oh!

practicalswift commented Nov 18, 2019

Uh oh!

maflcko commented Nov 18, 2019

Uh oh!

practicalswift commented Nov 18, 2019

Uh oh!

maflcko commented Nov 18, 2019

Uh oh!

sipa left a comment

Uh oh!

laanwj commented Nov 19, 2019 •

edited

Loading

Uh oh!

practicalswift commented Nov 19, 2019 •

edited

Loading

Uh oh!

laanwj commented Nov 19, 2019

Uh oh!

DrahtBot commented Dec 5, 2019

Uh oh!

sipa commented Dec 5, 2019 •

edited

Loading

Uh oh!

Uh oh!

random: mark RandAddPeriodic and SeedPeriodic as noexcept #17507

random: mark RandAddPeriodic and SeedPeriodic as noexcept #17507

Uh oh!

Conversation

fanquake commented Nov 18, 2019

Uh oh!

laanwj Nov 18, 2019

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Nov 18, 2019

Uh oh!

laanwj commented Nov 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Nov 18, 2019

Uh oh!

maflcko commented Nov 18, 2019

Uh oh!

practicalswift commented Nov 18, 2019

Uh oh!

maflcko commented Nov 18, 2019

Uh oh!

sipa left a comment

Choose a reason for hiding this comment

Uh oh!

laanwj commented Nov 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Nov 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Nov 19, 2019

Uh oh!

DrahtBot commented Dec 5, 2019

Conflicts

Uh oh!

sipa commented Dec 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

laanwj commented Nov 18, 2019 •

edited

Loading

laanwj commented Nov 19, 2019 •

edited

Loading

practicalswift commented Nov 19, 2019 •

edited

Loading

sipa commented Dec 5, 2019 •

edited

Loading