HomePhabricator

Kill insecure_random and associated global state
5eaaa83ac1f5Unpublished

Unpublished Commit ยท Learn More

Repository Importing: This repository is still importing.

Description

Kill insecure_random and associated global state

There are only a few uses of insecure_random outside the tests.
This PR replaces uses of insecure_random (and its accompanying global
state) in the core code with an FastRandomContext that is automatically
seeded on creation.

This is meant to be used for inner loops. The FastRandomContext
can be in the outer scope, or the class itself, then rand32() is used
inside the loop. Useful e.g. for pushing addresses in CNode or the fee
rounding, or randomization for coin selection.

As a context is created per purpose, thus it gets rid of
cross-thread unprotected shared usage of a single set of globals, this
should also get rid of the potential race conditions.

  • I'd say TxMempool::check is not called enough to warrant using a special fast random context, this is switched to GetRand() (open for discussion...)
  • The use of insecure_rand in ConnectThroughProxy has been replaced by an atomic integer counter. The only goal here is to have a different credentials pair for each connection to go on a different Tor circuit, it does not need to be random nor unpredictable.
  • To avoid having a FastRandomContext on every CNode, the context is passed into PushAddress as appropriate.

There remains an insecure_random for test usage in test_random.h.

Details

Provenance
Wladimir J. van der Laan <laanwj@gmail.com>Authored on Oct 13 2016, 14:19
schancelPushed on Jan 5 2018, 21:58
schancelPushed on Jan 5 2018, 21:39
schancelPushed on Jan 5 2018, 21:17
Parents
rSTAGING8d46429c83ec: Merge #8911: qt: Translate all files, even if wallet disabled
Branches
Unknown
Tags
Unknown

Event Timeline

Wladimir J. van der Laan <laanwj@gmail.com> committed rSTAGING5eaaa83ac1f5: Kill insecure_random and associated global state (authored by Wladimir J. van der Laan <laanwj@gmail.com>).Oct 17 2016, 11:08