refactor: share and use GenerateRandomKey helper

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(...)`.

src/bench/ellswift.cpp

@@ -11,9 +11,7 @@ static void EllSwiftCreate(benchmark::Bench& bench)
 {
     ECC_Start();
 
-    CKey key;
+    CKey key = GenerateRandomKey();
-    key.MakeNewKey(true);
-
     uint256 entropy = GetRandHash();
 
     bench.batch(1).unit("pubkey").run([&] {

src/key.cpp

@@ -369,6 +369,13 @@ ECDHSecret CKey::ComputeBIP324ECDHSecret(const EllSwiftPubKey& their_ellswift, c
     return output;
 }
 
+CKey GenerateRandomKey(bool compressed) noexcept
+{
+    CKey key;
+    key.MakeNewKey(/*fCompressed=*/compressed);
+    return key;
+}
+
 bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const {
     if (nDepth == std::numeric_limits<unsigned char>::max()) return false;
     out.nDepth = nDepth + 1;
@@ -420,8 +427,7 @@ void CExtKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) {
 }
 
 bool ECC_InitSanityCheck() {
-    CKey key;
+    CKey key = GenerateRandomKey();
-    key.MakeNewKey(true);
     CPubKey pubkey = key.GetPubKey();
     return key.VerifyPubKey(pubkey);
 }

src/key.h

@@ -205,6 +205,8 @@ public:
                                        bool initiating) const;
 };
 
+CKey GenerateRandomKey(bool compressed = true) noexcept;
+
 struct CExtKey {
     unsigned char nDepth;
     unsigned char vchFingerprint[4];

src/net.cpp

@@ -18,6 +18,7 @@
 #include <consensus/consensus.h>
 #include <crypto/sha256.h>
 #include <i2p.h>
+#include <key.h>
 #include <logging.h>
 #include <memusage.h>
 #include <net_permissions.h>
@@ -938,13 +939,6 @@ public:
 
 const V2MessageMap V2_MESSAGE_MAP;
 
-CKey GenerateRandomKey() noexcept
-{
-    CKey key;
-    key.MakeNewKey(/*fCompressed=*/true);
-    return key;
-}
-
 std::vector<uint8_t> GenerateRandomGarbage() noexcept
 {
     std::vector<uint8_t> ret;

src/qt/test/addressbooktests.cpp

@@ -85,8 +85,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
     }
 
     auto build_address = [&wallet]() {
-        CKey key;
+        CKey key = GenerateRandomKey();
-        key.MakeNewKey(true);
         CTxDestination dest(GetDestinationForKey(
             key.GetPubKey(), wallet->m_default_address_type));
 

src/test/blockfilter_index_tests.cpp

@@ -162,9 +162,8 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup)
         LOCK(cs_main);
         tip = m_node.chainman->ActiveChain().Tip();
     }
-    CKey coinbase_key_A, coinbase_key_B;
+    CKey coinbase_key_A = GenerateRandomKey();
-    coinbase_key_A.MakeNewKey(true);
+    CKey coinbase_key_B = GenerateRandomKey();
-    coinbase_key_B.MakeNewKey(true);
     CScript coinbase_script_pub_key_A = GetScriptForDestination(PKHash(coinbase_key_A.GetPubKey()));
     CScript coinbase_script_pub_key_B = GetScriptForDestination(PKHash(coinbase_key_B.GetPubKey()));
     std::vector<std::shared_ptr<CBlock>> chainA, chainB;

src/test/compress_tests.cpp

@@ -65,8 +65,7 @@ BOOST_AUTO_TEST_CASE(compress_amounts)
 BOOST_AUTO_TEST_CASE(compress_script_to_ckey_id)
 {
     // case CKeyID
-    CKey key;
+    CKey key = GenerateRandomKey();
-    key.MakeNewKey(true);
     CPubKey pubkey = key.GetPubKey();
 
     CScript script = CScript() << OP_DUP << OP_HASH160 << ToByteVector(pubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
@@ -101,8 +100,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_cscript_id)
 
 BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id)
 {
-    CKey key;
+    CKey key = GenerateRandomKey(); // case compressed PubKeyID
-    key.MakeNewKey(true); // case compressed PubKeyID
 
     CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // COMPRESSED_PUBLIC_KEY_SIZE (33)
     BOOST_CHECK_EQUAL(script.size(), 35U);
@@ -119,8 +117,7 @@ BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id)
 
 BOOST_AUTO_TEST_CASE(compress_script_to_uncompressed_pubkey_id)
 {
-    CKey key;
+    CKey key = GenerateRandomKey(/*compressed=*/false); // case uncompressed PubKeyID
-    key.MakeNewKey(false); // case uncompressed PubKeyID
     CScript script =  CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // PUBLIC_KEY_SIZE (65)
     BOOST_CHECK_EQUAL(script.size(), 67U);                   // 1 char code + 65 char pubkey + OP_CHECKSIG
 

src/test/script_standard_tests.cpp

@@ -136,10 +136,8 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_success)
 
 BOOST_AUTO_TEST_CASE(script_standard_Solver_failure)
 {
-    CKey key;
+    CKey key = GenerateRandomKey();
-    CPubKey pubkey;
+    CPubKey pubkey = key.GetPubKey();
-    key.MakeNewKey(true);
-    pubkey = key.GetPubKey();
 
     CScript s;
     std::vector<std::vector<unsigned char> > solutions;
@@ -192,10 +190,8 @@ BOOST_AUTO_TEST_CASE(script_standard_Solver_failure)
 
 BOOST_AUTO_TEST_CASE(script_standard_ExtractDestination)
 {
-    CKey key;
+    CKey key = GenerateRandomKey();
-    CPubKey pubkey;
+    CPubKey pubkey = key.GetPubKey();
-    key.MakeNewKey(true);
-    pubkey = key.GetPubKey();
 
     CScript s;
     CTxDestination address;

src/test/script_tests.cpp

@@ -1051,10 +1051,9 @@ sign_multisig(const CScript& scriptPubKey, const CKey& key, const CTransaction&
 BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG12)
 {
     ScriptError err;
-    CKey key1, key2, key3;
+    CKey key1 = GenerateRandomKey();
-    key1.MakeNewKey(true);
+    CKey key2 = GenerateRandomKey(/*compressed=*/false);
-    key2.MakeNewKey(false);
+    CKey key3 = GenerateRandomKey();
-    key3.MakeNewKey(true);
 
     CScript scriptPubKey12;
     scriptPubKey12 << OP_1 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << OP_2 << OP_CHECKMULTISIG;
@@ -1081,11 +1080,10 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG12)
 BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG23)
 {
     ScriptError err;
-    CKey key1, key2, key3, key4;
+    CKey key1 = GenerateRandomKey();
-    key1.MakeNewKey(true);
+    CKey key2 = GenerateRandomKey(/*compressed=*/false);
-    key2.MakeNewKey(false);
+    CKey key3 = GenerateRandomKey();
-    key3.MakeNewKey(true);
+    CKey key4 = GenerateRandomKey(/*compressed=*/false);
-    key4.MakeNewKey(false);
 
     CScript scriptPubKey23;
     scriptPubKey23 << OP_2 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << ToByteVector(key3.GetPubKey()) << OP_3 << OP_CHECKMULTISIG;
@@ -1165,8 +1163,7 @@ BOOST_AUTO_TEST_CASE(script_combineSigs)
     std::vector<CPubKey> pubkeys;
     for (int i = 0; i < 3; i++)
     {
-        CKey key;
+        CKey key = GenerateRandomKey(/*compressed=*/i%2 == 1);
-        key.MakeNewKey(i%2 == 1);
         keys.push_back(key);
         pubkeys.push_back(key.GetPubKey());
         BOOST_CHECK(keystore.AddKey(key));

src/test/sigopcount_tests.cpp

@@ -50,8 +50,7 @@ BOOST_AUTO_TEST_CASE(GetSigOpCount)
     std::vector<CPubKey> keys;
     for (int i = 0; i < 3; i++)
     {
-        CKey k;
+        CKey k = GenerateRandomKey();
-        k.MakeNewKey(true);
         keys.push_back(k.GetPubKey());
     }
     CScript s2 = GetScriptForMultisig(1, keys);
@@ -120,8 +119,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
     CCoinsView coinsDummy;
     CCoinsViewCache coins(&coinsDummy);
     // Create key
-    CKey key;
+    CKey key = GenerateRandomKey();
-    key.MakeNewKey(true);
     CPubKey pubkey = key.GetPubKey();
     // Default flags
     const uint32_t flags{SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH};

src/test/transaction_tests.cpp

@@ -487,8 +487,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction)
     CMutableTransaction mtx;
     mtx.nVersion = 1;
 
-    CKey key;
+    CKey key = GenerateRandomKey(); // Need to use compressed keys in segwit or the signing will fail
-    key.MakeNewKey(true); // Need to use compressed keys in segwit or the signing will fail
     FillableSigningProvider keystore;
     BOOST_CHECK(keystore.AddKeyPubKey(key, key.GetPubKey()));
     CKeyID hash = key.GetPubKey().GetID();
@@ -564,18 +563,16 @@ SignatureData CombineSignatures(const CMutableTransaction& input1, const CMutabl
 BOOST_AUTO_TEST_CASE(test_witness)
 {
     FillableSigningProvider keystore, keystore2;
-    CKey key1, key2, key3, key1L, key2L;
+    CKey key1 = GenerateRandomKey();
-    CPubKey pubkey1, pubkey2, pubkey3, pubkey1L, pubkey2L;
+    CKey key2 = GenerateRandomKey();
-    key1.MakeNewKey(true);
+    CKey key3 = GenerateRandomKey();
-    key2.MakeNewKey(true);
+    CKey key1L = GenerateRandomKey(/*compressed=*/false);
-    key3.MakeNewKey(true);
+    CKey key2L = GenerateRandomKey(/*compressed=*/false);
-    key1L.MakeNewKey(false);
+    CPubKey pubkey1 = key1.GetPubKey();
-    key2L.MakeNewKey(false);
+    CPubKey pubkey2 = key2.GetPubKey();
-    pubkey1 = key1.GetPubKey();
+    CPubKey pubkey3 = key3.GetPubKey();
-    pubkey2 = key2.GetPubKey();
+    CPubKey pubkey1L = key1L.GetPubKey();
-    pubkey3 = key3.GetPubKey();
+    CPubKey pubkey2L = key2L.GetPubKey();
-    pubkey1L = key1L.GetPubKey();
-    pubkey2L = key2L.GetPubKey();
     BOOST_CHECK(keystore.AddKeyPubKey(key1, pubkey1));
     BOOST_CHECK(keystore.AddKeyPubKey(key2, pubkey2));
     BOOST_CHECK(keystore.AddKeyPubKey(key1L, pubkey1L));
@@ -756,8 +753,7 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
     t.vin[0].scriptSig << std::vector<unsigned char>(65, 0);
     t.vout.resize(1);
     t.vout[0].nValue = 90*CENT;
-    CKey key;
+    CKey key = GenerateRandomKey();
-    key.MakeNewKey(true);
     t.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
 
     constexpr auto CheckIsStandard = [](const auto& t) {

src/test/txpackage_tests.cpp

@@ -116,8 +116,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup)
     unsigned int initialPoolSize = m_node.mempool->size();
 
     // Parent and Child Package
-    CKey parent_key;
+    CKey parent_key = GenerateRandomKey();
-    parent_key.MakeNewKey(true);
     CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey()));
     auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0,
                                                     /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
@@ -125,8 +124,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup)
                                                     /*output_amount=*/CAmount(49 * COIN), /*submit=*/false);
     CTransactionRef tx_parent = MakeTransactionRef(mtx_parent);
 
-    CKey child_key;
+    CKey child_key = GenerateRandomKey();
-    child_key.MakeNewKey(true);
     CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey()));
     auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent, /*input_vout=*/0,
                                                    /*input_height=*/101, /*input_signing_key=*/parent_key,
@@ -170,11 +168,9 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup)
 BOOST_FIXTURE_TEST_CASE(noncontextual_package_tests, TestChain100Setup)
 {
     // The signatures won't be verified so we can just use a placeholder
-    CKey placeholder_key;
+    CKey placeholder_key = GenerateRandomKey();
-    placeholder_key.MakeNewKey(true);
     CScript spk = GetScriptForDestination(PKHash(placeholder_key.GetPubKey()));
-    CKey placeholder_key_2;
+    CKey placeholder_key_2 = GenerateRandomKey();
-    placeholder_key_2.MakeNewKey(true);
     CScript spk2 = GetScriptForDestination(PKHash(placeholder_key_2.GetPubKey()));
 
     // Parent and Child Package
@@ -266,8 +262,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
 {
     LOCK(cs_main);
     unsigned int expected_pool_size = m_node.mempool->size();
-    CKey parent_key;
+    CKey parent_key = GenerateRandomKey();
-    parent_key.MakeNewKey(true);
     CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey()));
 
     // Unrelated transactions are not allowed in package submission.
@@ -298,8 +293,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
     package_parent_child.push_back(tx_parent);
     package_3gen.push_back(tx_parent);
 
-    CKey child_key;
+    CKey child_key = GenerateRandomKey();
-    child_key.MakeNewKey(true);
     CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey()));
     auto mtx_child = CreateValidMempoolTransaction(/*input_transaction=*/tx_parent, /*input_vout=*/0,
                                                    /*input_height=*/101, /*input_signing_key=*/parent_key,
@@ -309,8 +303,7 @@ BOOST_FIXTURE_TEST_CASE(package_submission_tests, TestChain100Setup)
     package_parent_child.push_back(tx_child);
     package_3gen.push_back(tx_child);
 
-    CKey grandchild_key;
+    CKey grandchild_key = GenerateRandomKey();
-    grandchild_key.MakeNewKey(true);
     CScript grandchild_locking_script = GetScriptForDestination(PKHash(grandchild_key.GetPubKey()));
     auto mtx_grandchild = CreateValidMempoolTransaction(/*input_transaction=*/tx_child, /*input_vout=*/0,
                                                        /*input_height=*/101, /*input_signing_key=*/child_key,
@@ -434,8 +427,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
     witness2.stack.emplace_back(2);
     witness2.stack.emplace_back(witnessScript.begin(), witnessScript.end());
 
-    CKey child_key;
+    CKey child_key = GenerateRandomKey();
-    child_key.MakeNewKey(true);
     CScript child_locking_script = GetScriptForDestination(WitnessV0KeyHash(child_key.GetPubKey()));
     CMutableTransaction mtx_child1;
     mtx_child1.nVersion = 1;
@@ -504,8 +496,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
     // This tests a potential censorship vector in which an attacker broadcasts a competing package
     // where a parent's witness is mutated. The honest package should be accepted despite the fact
     // that we don't allow witness replacement.
-    CKey grandchild_key;
+    CKey grandchild_key = GenerateRandomKey();
-    grandchild_key.MakeNewKey(true);
     CScript grandchild_locking_script = GetScriptForDestination(WitnessV0KeyHash(grandchild_key.GetPubKey()));
     auto mtx_grandchild = CreateValidMempoolTransaction(/*input_transaction=*/ptx_child2, /*input_vout=*/0,
                                                         /*input_height=*/0, /*input_signing_key=*/child_key,
@@ -595,8 +586,7 @@ BOOST_FIXTURE_TEST_CASE(package_witness_swap_tests, TestChain100Setup)
     BOOST_CHECK(m_node.mempool->m_min_relay_feerate.GetFee(GetVirtualTransactionSize(*ptx_parent3)) <= low_fee_amt);
 
     // child spends parent1, parent2, and parent3
-    CKey mixed_grandchild_key;
+    CKey mixed_grandchild_key = GenerateRandomKey();
-    mixed_grandchild_key.MakeNewKey(true);
     CScript mixed_child_spk = GetScriptForDestination(WitnessV0KeyHash(mixed_grandchild_key.GetPubKey()));
 
     CMutableTransaction mtx_mixed_child;
@@ -648,11 +638,9 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
     MockMempoolMinFee(CFeeRate(5000));
     LOCK(::cs_main);
     size_t expected_pool_size = m_node.mempool->size();
-    CKey child_key;
+    CKey child_key = GenerateRandomKey();
-    child_key.MakeNewKey(true);
     CScript parent_spk = GetScriptForDestination(WitnessV0KeyHash(child_key.GetPubKey()));
-    CKey grandchild_key;
+    CKey grandchild_key = GenerateRandomKey();
-    grandchild_key.MakeNewKey(true);
     CScript child_spk = GetScriptForDestination(WitnessV0KeyHash(grandchild_key.GetPubKey()));
 
     // low-fee parent and high-fee child package

src/wallet/scriptpubkeyman.cpp

@@ -1183,8 +1183,7 @@ bool LegacyScriptPubKeyMan::CanGenerateKeys() const
 CPubKey LegacyScriptPubKeyMan::GenerateNewSeed()
 {
     assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));
-    CKey key;
+    CKey key = GenerateRandomKey();
-    key.MakeNewKey(true);
     return DeriveNewSeed(key);
 }
 

src/wallet/test/ismine_tests.cpp

@@ -47,8 +47,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
         pubkeys[i] = keys[i].GetPubKey();
     }
 
-    CKey uncompressedKey;
+    CKey uncompressedKey = GenerateRandomKey(/*compressed=*/false);
-    uncompressedKey.MakeNewKey(false);
     CPubKey uncompressedPubkey = uncompressedKey.GetPubKey();
     std::unique_ptr<interfaces::Chain>& chain = m_node.chain;
 

src/wallet/test/wallet_tests.cpp

@@ -231,8 +231,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
         keys.push_back(key);
         key.clear();
         key.setObject();
-        CKey futureKey;
+        CKey futureKey = GenerateRandomKey();
-        futureKey.MakeNewKey(true);
         key.pushKV("scriptPubKey", HexStr(GetScriptForRawPubKey(futureKey.GetPubKey())));
         key.pushKV("timestamp", newTip->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1);
         key.pushKV("internal", UniValue(true));
@@ -704,8 +703,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
 static size_t CalculateNestedKeyhashInputSize(bool use_max_sig)
 {
     // Generate ephemeral valid pubkey
-    CKey key;
+    CKey key = GenerateRandomKey();
-    key.MakeNewKey(true);
     CPubKey pubkey = key.GetPubKey();
 
     // Generate pubkey hash
@@ -789,8 +787,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
     context.args = &m_args;
     context.chain = m_node.chain.get();
     auto wallet = TestLoadWallet(context);
-    CKey key;
+    CKey key = GenerateRandomKey();
-    key.MakeNewKey(true);
     AddKey(*wallet, key);
     TestUnloadWallet(std::move(wallet));
 
@@ -898,8 +895,7 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup)
     context.args = &m_args;
     context.chain = m_node.chain.get();
     auto wallet = TestLoadWallet(context);
-    CKey key;
+    CKey key = GenerateRandomKey();
-    key.MakeNewKey(true);
     AddKey(*wallet, key);
 
     std::string error;

src/wallet/wallet.cpp

@@ -3578,8 +3578,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans()
 
     if (!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) {
         // Make a seed
-        CKey seed_key;
+        CKey seed_key = GenerateRandomKey();
-        seed_key.MakeNewKey(true);
         CPubKey seed = seed_key.GetPubKey();
         assert(seed_key.VerifyPubKey(seed));