RandAddSeedPerfmon #5495

CKey::MakeNewKey is responsible for generating a new private key using a cryptographic PRNG. The rest of key metadata is then generated by CWallet::GenerateNewKey.

However, the GenerateNewKey function does not call MakeNewKey before ensuring RandAddSeedPerfmon is done. If increasing the uncertainty about the state and making the PRNG output less predictable is the message to send, the step then might as well be implemented in the MakeNewKey function itself.

The initial Sanity Check, and tests are the two other consumers of the function, neither RandAddSeedPerfmon-ing. The latter out of, presumably, performance considerations. Nevertheless, the CKey access modifiers, and the ongoing libification tipped me over to log this, as it is something to consider as we move forward (in light of recent events...).

Not sure I follow you. CWallet::GenerateNewKey() does a RandAddSeedPerfmon() as first thing. All private key generation goes through that wallet method. The only call to MakeNewKey is there, apart from the sanity check.

I wouldn't be opposed to move the call closer to the actual generation for software engineering reasons, but it will not improve the state of the PRNG.

Yes, effectively, the software engineering reasons i.e. ensuring that no publicly accessible MakeNewKey in some future wallet lib circumvents RandAddSeedPerfmon, as the tests do now (presumably inadvertently).

OK well I'd say go ahead move the call.

RandAddSeedPerfmon is of minimal value. I think it's not unlikely that we'll replace the RNG infrastructure during 0.11... I have no objection to moving things around so long as people are confident that it's low risk, but I don't think a lot of time would be well spent micro-twiddling the details on this. (Also, not sure about which recent events you refer to, but ... we have never had a randomness related incident in Bitcoin core and take auditing, review, and testing of these components very seriously. Some places where there have been issues have had issues in part due to unnecessary small changes, something to keep in mind.)

Agreed. And, yes, not a randomness related incident in Bitcoin Core, but a third party.

Right. The only places where bitcoin core require cryptographically secure randomness after the RFC6979 merge are:

• Generation of private keys (for the wallet)
• Generation of master key (for wallet encryption)
• Generation of example password for RPC server

The other uses of GetRand* such as those in the network code and address manager could feasibly (but carefully) be replaced by a more efficient and less secure random number generator.

Merged & closed.