From f146029b0a50864e2c111034786a36e53f6f02db Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 19 Sep 2017 16:49:52 -0700 Subject: [PATCH] Replace CBitcoinSecret with {Encode,Decode}Secret --- src/base58.cpp | 48 ++++++++++++++++++--------------------- src/base58.h | 17 ++------------ src/bitcoin-tx.cpp | 8 +++---- src/rpcrawtransaction.cpp | 8 ++----- src/test/base58_tests.cpp | 21 ++++++++--------- src/test/bloom_tests.cpp | 7 ++---- src/test/key_tests.cpp | 25 ++++++++------------ src/wallet/rpcdump.cpp | 22 +++++++----------- 8 files changed, 58 insertions(+), 98 deletions(-) diff --git a/src/base58.cpp b/src/base58.cpp index f3eb73e45..55b065cbe 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -257,39 +257,35 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par } } // namespace -void CBitcoinSecret::SetKey(const CKey& vchSecret) +CKey DecodeSecret(const std::string& str) { - assert(vchSecret.IsValid()); - SetData(Params().Base58Prefix(CChainParams::SECRET_KEY), vchSecret.begin(), vchSecret.size()); - if (vchSecret.IsCompressed()) - vchData.push_back(1); + CKey key; + std::vector data; + if (DecodeBase58Check(str, data)) { + const std::vector& privkey_prefix = Params().Base58Prefix(CChainParams::SECRET_KEY); + if ((data.size() == 32 + privkey_prefix.size() || (data.size() == 33 + privkey_prefix.size() && data.back() == 1)) && + std::equal(privkey_prefix.begin(), privkey_prefix.end(), data.begin())) { + bool compressed = data.size() == 33 + privkey_prefix.size(); + key.Set(data.begin() + privkey_prefix.size(), data.begin() + privkey_prefix.size() + 32, compressed); + } + } + memory_cleanse(data.data(), data.size()); + return key; } -CKey CBitcoinSecret::GetKey() +std::string EncodeSecret(const CKey& key) { - CKey ret; - assert(vchData.size() >= 32); - ret.Set(vchData.begin(), vchData.begin() + 32, vchData.size() > 32 && vchData[32] == 1); + assert(key.IsValid()); + std::vector data = Params().Base58Prefix(CChainParams::SECRET_KEY); + data.insert(data.end(), key.begin(), key.end()); + if (key.IsCompressed()) { + data.push_back(1); + } + std::string ret = EncodeBase58Check(data); + memory_cleanse(data.data(), data.size()); return ret; } -bool CBitcoinSecret::IsValid() const -{ - bool fExpectedFormat = vchData.size() == 32 || (vchData.size() == 33 && vchData[32] == 1); - bool fCorrectVersion = vchVersion == Params().Base58Prefix(CChainParams::SECRET_KEY); - return fExpectedFormat && fCorrectVersion; -} - -bool CBitcoinSecret::SetString(const char* pszSecret) -{ - return CBase58Data::SetString(pszSecret, 1) && IsValid(); -} - -bool CBitcoinSecret::SetString(const std::string& strSecret) -{ - return SetString(strSecret.c_str()); -} - std::string EncodeDestination(const CTxDestination& dest) { return boost::apply_visitor(DestinationEncoder(Params()), dest); diff --git a/src/base58.h b/src/base58.h index 9e90db78f..f30479214 100644 --- a/src/base58.h +++ b/src/base58.h @@ -139,21 +139,8 @@ public: CZCSpendingKey(const libzcash::SpendingKey& addr) { Set(addr); } }; -/** - * A base58-encoded secret key - */ -class CBitcoinSecret : public CBase58Data -{ -public: - void SetKey(const CKey& vchSecret); - CKey GetKey(); - bool IsValid() const; - bool SetString(const char* pszSecret); - bool SetString(const std::string& strSecret); - - CBitcoinSecret(const CKey& vchSecret) { SetKey(vchSecret); } - CBitcoinSecret() {} -}; +CKey DecodeSecret(const std::string& str); +std::string EncodeSecret(const CKey& key); template class CBitcoinExtKeyBase : public CBase58Data { diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 1158563f2..aa2523039 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -400,12 +400,10 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& strInput) for (size_t kidx = 0; kidx < keysObj.size(); kidx++) { if (!keysObj[kidx].isStr()) throw std::runtime_error("privatekey not a std::string"); - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(keysObj[kidx].getValStr()); - if (!fGood) + CKey key = DecodeSecret(keysObj[kidx].getValStr()); + if (!key.IsValid()) { throw std::runtime_error("privatekey not valid"); - - CKey key = vchSecret.GetKey(); + } tempKeystore.AddKey(key); } diff --git a/src/rpcrawtransaction.cpp b/src/rpcrawtransaction.cpp index e59a9b369..75af5264d 100644 --- a/src/rpcrawtransaction.cpp +++ b/src/rpcrawtransaction.cpp @@ -777,13 +777,9 @@ UniValue signrawtransaction(const UniValue& params, bool fHelp) UniValue keys = params[2].get_array(); for (size_t idx = 0; idx < keys.size(); idx++) { UniValue k = keys[idx]; - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(k.get_str()); - if (!fGood) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); - CKey key = vchSecret.GetKey(); + CKey key = DecodeSecret(k.get_str()); if (!key.IsValid()) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key"); tempKeystore.AddKey(key); } } diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 8e7f77496..351c722c0 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -78,7 +78,7 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58) BOOST_AUTO_TEST_CASE(base58_keys_valid_parse) { UniValue tests = read_json(std::string(json_tests::base58_keys_valid, json_tests::base58_keys_valid + sizeof(json_tests::base58_keys_valid))); - CBitcoinSecret secret; + CKey privkey; CTxDestination destination; SelectParams(CBaseChainParams::MAIN); @@ -102,9 +102,8 @@ BOOST_AUTO_TEST_CASE(base58_keys_valid_parse) if (isPrivkey) { bool isCompressed = find_value(metadata, "isCompressed").get_bool(); // Must be valid private key - BOOST_CHECK_MESSAGE(secret.SetString(exp_base58string), "!SetString:"+ strTest); - BOOST_CHECK_MESSAGE(secret.IsValid(), "!IsValid:" + strTest); - CKey privkey = secret.GetKey(); + privkey = DecodeSecret(exp_base58string); + BOOST_CHECK_MESSAGE(privkey.IsValid(), "!IsValid:" + strTest); BOOST_CHECK_MESSAGE(privkey.IsCompressed() == isCompressed, "compressed mismatch:" + strTest); BOOST_CHECK_MESSAGE(privkey.size() == exp_payload.size() && std::equal(privkey.begin(), privkey.end(), exp_payload.begin()), "key mismatch:" + strTest); @@ -119,8 +118,8 @@ BOOST_AUTO_TEST_CASE(base58_keys_valid_parse) BOOST_CHECK_EQUAL(HexStr(script), HexStr(exp_payload)); // Public key must be invalid private key - secret.SetString(exp_base58string); - BOOST_CHECK_MESSAGE(!secret.IsValid(), "IsValid pubkey as privkey:" + strTest); + privkey = DecodeSecret(exp_base58string); + BOOST_CHECK_MESSAGE(!privkey.IsValid(), "IsValid pubkey as privkey:" + strTest); } } } @@ -153,9 +152,7 @@ BOOST_AUTO_TEST_CASE(base58_keys_valid_gen) CKey key; key.Set(exp_payload.begin(), exp_payload.end(), isCompressed); assert(key.IsValid()); - CBitcoinSecret secret; - secret.SetKey(key); - BOOST_CHECK_MESSAGE(secret.ToString() == exp_base58string, "result mismatch: " + strTest); + BOOST_CHECK_MESSAGE(EncodeSecret(key) == exp_base58string, "result mismatch: " + strTest); } else { CTxDestination dest; CScript exp_script(exp_payload.begin(), exp_payload.end()); @@ -172,7 +169,7 @@ BOOST_AUTO_TEST_CASE(base58_keys_valid_gen) BOOST_AUTO_TEST_CASE(base58_keys_invalid) { UniValue tests = read_json(std::string(json_tests::base58_keys_invalid, json_tests::base58_keys_invalid + sizeof(json_tests::base58_keys_invalid))); // Negative testcases - CBitcoinSecret secret; + CKey privkey; CTxDestination destination; for (size_t idx = 0; idx < tests.size(); idx++) { @@ -188,8 +185,8 @@ BOOST_AUTO_TEST_CASE(base58_keys_invalid) // must be invalid as public and as private key destination = DecodeDestination(exp_base58string); BOOST_CHECK_MESSAGE(!IsValidDestination(destination), "IsValid pubkey:" + strTest); - secret.SetString(exp_base58string); - BOOST_CHECK_MESSAGE(!secret.IsValid(), "IsValid privkey:" + strTest); + privkey = DecodeSecret(exp_base58string); + BOOST_CHECK_MESSAGE(!privkey.IsValid(), "IsValid privkey:" + strTest); } } diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index a8f3c0454..554882899 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -86,11 +86,8 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize_with_tweak) BOOST_AUTO_TEST_CASE(bloom_create_insert_key) { - string strSecret = string("5Kg1gnAjaLfKiwhhPpGS3QfRg2m6awQvaj98JCZBZQ5SuS2F15C"); - CBitcoinSecret vchSecret; - BOOST_CHECK(vchSecret.SetString(strSecret)); - - CKey key = vchSecret.GetKey(); + std::string strSecret = std::string("5Kg1gnAjaLfKiwhhPpGS3QfRg2m6awQvaj98JCZBZQ5SuS2F15C"); + CKey key = DecodeSecret(strSecret); CPubKey pubkey = key.GetPubKey(); vector vchPubKey(pubkey.begin(), pubkey.end()); diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index ed997a95a..6caaed391 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -65,21 +65,16 @@ BOOST_FIXTURE_TEST_SUITE(key_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(key_test1) { - CBitcoinSecret bsecret1, bsecret2, bsecret1C, bsecret2C, baddress1; - BOOST_CHECK( bsecret1.SetString (strSecret1)); - BOOST_CHECK( bsecret2.SetString (strSecret2)); - BOOST_CHECK( bsecret1C.SetString(strSecret1C)); - BOOST_CHECK( bsecret2C.SetString(strSecret2C)); - BOOST_CHECK(!baddress1.SetString(strAddressBad)); - - CKey key1 = bsecret1.GetKey(); - BOOST_CHECK(key1.IsCompressed() == false); - CKey key2 = bsecret2.GetKey(); - BOOST_CHECK(key2.IsCompressed() == false); - CKey key1C = bsecret1C.GetKey(); - BOOST_CHECK(key1C.IsCompressed() == true); - CKey key2C = bsecret2C.GetKey(); - BOOST_CHECK(key2C.IsCompressed() == true); + CKey key1 = DecodeSecret(strSecret1); + BOOST_CHECK(key1.IsValid() && !key1.IsCompressed()); + CKey key2 = DecodeSecret(strSecret2); + BOOST_CHECK(key2.IsValid() && !key2.IsCompressed()); + CKey key1C = DecodeSecret(strSecret1C); + BOOST_CHECK(key1C.IsValid() && key1C.IsCompressed()); + CKey key2C = DecodeSecret(strSecret2C); + BOOST_CHECK(key2C.IsValid() && key2C.IsCompressed()); + CKey bad_key = DecodeSecret(strAddressBad); + BOOST_CHECK(!bad_key.IsValid()); CPubKey pubkey1 = key1. GetPubKey(); CPubKey pubkey2 = key2. GetPubKey(); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 6768db611..c29972fc1 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -112,13 +112,8 @@ UniValue importprivkey(const UniValue& params, bool fHelp) if (params.size() > 2) fRescan = params[2].get_bool(); - CBitcoinSecret vchSecret; - bool fGood = vchSecret.SetString(strSecret); - - if (!fGood) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); - - CKey key = vchSecret.GetKey(); - if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Private key outside allowed range"); + CKey key = DecodeSecret(strSecret); + if (!key.IsValid()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid private key encoding"); CPubKey pubkey = key.GetPubKey(); assert(key.VerifyPubKey(pubkey)); @@ -325,10 +320,9 @@ UniValue importwallet_impl(const UniValue& params, bool fHelp, bool fImportZKeys } } - CBitcoinSecret vchSecret; - if (!vchSecret.SetString(vstr[0])) + CKey key = DecodeSecret(vstr[0]); + if (!key.IsValid()) continue; - CKey key = vchSecret.GetKey(); CPubKey pubkey = key.GetPubKey(); assert(key.VerifyPubKey(pubkey)); CKeyID keyid = pubkey.GetID(); @@ -418,7 +412,7 @@ UniValue dumpprivkey(const UniValue& params, bool fHelp) if (!pwalletMain->GetKey(*keyID, vchSecret)) { throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known"); } - return CBitcoinSecret(vchSecret).ToString(); + return EncodeSecret(vchSecret); } @@ -522,11 +516,11 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) CKey key; if (pwalletMain->GetKey(keyid, key)) { if (pwalletMain->mapAddressBook.count(keyid)) { - file << strprintf("%s %s label=%s # addr=%s\n", CBitcoinSecret(key).ToString(), strTime, EncodeDumpString(pwalletMain->mapAddressBook[keyid].name), strAddr); + file << strprintf("%s %s label=%s # addr=%s\n", EncodeSecret(key), strTime, EncodeDumpString(pwalletMain->mapAddressBook[keyid].name), strAddr); } else if (setKeyPool.count(keyid)) { - file << strprintf("%s %s reserve=1 # addr=%s\n", CBitcoinSecret(key).ToString(), strTime, strAddr); + file << strprintf("%s %s reserve=1 # addr=%s\n", EncodeSecret(key), strTime, strAddr); } else { - file << strprintf("%s %s change=1 # addr=%s\n", CBitcoinSecret(key).ToString(), strTime, strAddr); + file << strprintf("%s %s change=1 # addr=%s\n", EncodeSecret(key), strTime, strAddr); } } }