GenerateRandomKey helper
#28455
Making the GeneratingRandomKey helper (recently introduced in PR #28433, commit b6934fd03f080d437acb1fd2b665503c3d6de785) available to other modules via key.{h.cpp} allows us to create random private keys directly at CKey instantiation, in contrast to the currently needed two-step process of creating an (invalid) CKey instance first and then having to call MakeNewKey(...).
This is mostly used in unit tests and a few instances in the wallet.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | Sjors, sipa, kristapsk, stratospher, achow101 |
| Stale ACK | kevkevinpal |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
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.
I see MakeNewKey being used in
./src/test/multisig_tests.cpp
./src/test/descriptor_tests.cpp
./src/test/script_p2sh_tests.cpp
./src/test/script_standard_tests.cpp
./src/wallet/test/scriptpubkeyman_tests.cpp
./src/wallet/test/ismine_tests.cpp
./src/wallet/test/wallet_tests.cpp
./src/wallet/scriptpubkeyman.cpp
I found these by doing
grep -nri "MakeNewKey" ./src --binary-files=without-match
do we plan on making this modification to these files aswell?
978@@ -979,13 +979,6 @@ class V2MessageMap
979
980 const V2MessageMap V2_MESSAGE_MAP;
981
982-CKey GenerateRandomKey() noexcept
#include <key.h> since we’re still using GenerateRandomKey() in src/net.cpp
key.h makes sense.
Concept ACK. I think this is a useful refactor to have since it’s more modular to have GenerateRandomKey in key.cpp rather than net code.
I found these by doing grep -nri “MakeNewKey” ./src –binary-files=without-match do we plan on making this modification to these files aswell? @kevkevinpal, it makes sense to do this refactor when we’re instantiating a CKey object and also filling the value together in the same line. we can’t do this in arrays for example, where the instantiation is done prior to filling values. we can do this refactor in:
ACK 81187d04c2925bbd1281fe3cbdc2a30c2a02cca5
The Stratum v2 Template Provider in #28983 also uses the CKey static_key; static_key.MakeNewKey(true); pattern, so would (slightly) benefit from this change.
do we plan on making this modification to these files aswell?
I can re-review if more tests are modified to use this new helper, but I’m also fine with leaving as is. New code can use it.
Making the `GenerateRandomKey` helper available to other modules via
key.{h.cpp} allows us to create random private keys directly at
instantiation of CKey, in contrast to the two-step process of creating
the instance and then having to call `MakeNewKey(...)`.$ git range-diff 81187d0...fa1d495.
