zecfoundation
/
zcashd
mirror of https://github.com/ZcashFoundation/zcashd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Apply suggestions from code review 

Co-authored-by: str4d <jack@electriccoin.co>
This commit is contained in:
Kris Nuttycombe 2021-11-03 16:36:42 -06:00 committed by Kris Nuttycombe
parent 399ed5b4d7
commit a67fb00d0a
12 changed files with 40 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/gtest/test_keystore.cpp
View File

@ -53,6 +53,7 @@ TEST(KeystoreTests, StoreAndRetrieveLegacyHDSeed) {
EXPECT_FALSE(seedOut.has_value());
// Generate a random seed
// (We use MnemonicSeed purely to generate a seed, and then drop the mnemonic part.)
HDSeed seed = MnemonicSeed::Random(SLIP44_TESTNET_TYPE);
// We should be able to set and retrieve the seed

2
src/test/alert_tests.cpp
View File

@ -117,7 +117,7 @@ bool SignAlert(CAlert &alert)
// sign alert
std::vector<unsigned char> vchTmp(ParseHex(pszPrivKey));
CPrivKey vchPrivKey(vchTmp.begin(), vchTmp.end());
std::optional<CKey> key = CKey::FromPrivKey(SetPrivKey(vchPrivKey, false);
std::optional<CKey> key = CKey::FromPrivKey(vchPrivKey, false);
if (!key.has_value()) {
printf("key.SetPrivKey failed\n");
return false;

5
src/wallet/asyncrpcoperation_saplingmigration.cpp
View File

@ -191,9 +191,8 @@ CAmount AsyncRPCOperation_saplingmigration::chooseAmount(const CAmount& availabl
return amount;
}
// Unless otherwise specified, the migration destination address is the address
// the legacy Sapling account (0x7FFFFFFF) at the smallest diversifier index
// that produces a valid diversified address.
// Unless otherwise specified, the migration destination address is the
// default address for the key at m/32'/coin_type'/0x7FFFFFFF'/0'
libzcash::SaplingPaymentAddress AsyncRPCOperation_saplingmigration::getMigrationDestAddress(const HDSeed& seed) {
KeyIO keyIO(Params());
if (mapArgs.count("-migrationdestaddress")) {

6
src/wallet/crypter.cpp
View File

@ -372,7 +372,8 @@ bool CCryptoKeyStore::SetCryptedMnemonicSeed(
{
LOCK(cs_KeyStore);
if (!fUseCrypto || !cryptedMnemonicSeed.first.IsNull()) {
// Require encryption & don't allow an existing seed to be changed.
// Either wallet encryption is disabled, or the wallet already has an
// existing mnemonic. In either case, don't allow a new mnemonic.
return false;
} else {
cryptedMnemonicSeed = std::make_pair(seedFp, vchCryptedSecret);
@ -439,7 +440,8 @@ bool CCryptoKeyStore::SetCryptedLegacyHDSeed(
{
LOCK(cs_KeyStore);
if (!fUseCrypto || cryptedLegacySeed.has_value()) {
// Require encryption & don't allow an existing seed to be changed.
// Either wallet encryption is disabled, or the wallet already has an
// existing legacy seed. In either case, don't allow a new seed.
return false;
} else {
cryptedLegacySeed = std::make_pair(seedFp, vchCryptedSecret);

9
src/wallet/gtest/test_wallet_zkeys.cpp
View File

@ -65,9 +65,10 @@ TEST(WalletZkeysTest, StoreAndLoadSaplingZkeys) {
EXPECT_EQ(1, addrs.count(address));
EXPECT_EQ(1, addrs.count(sk.ToXFVK().DefaultAddress()));
// Generate a diversified address different to the default
// If we can't get an early diversified address, we are very unlucky
libzcash::diversifier_index_t j(2);
// Find a diversified address that does not use the same diversifier as the default address.
// By starting our search at `10` we ensure there's no more than a 2^-10 chance that we
// collide with the default diversifier.
libzcash::diversifier_index_t j(10);
auto dpa = sk.ToXFVK().FindAddress(j).first;
// verify wallet only has the default address
@ -448,7 +449,7 @@ TEST(WalletZkeysTest, WriteCryptedSaplingZkeyDirectToDb) {
// If we can't get an early diversified address, we are very unlucky
libzcash::SaplingExtendedSpendingKey extsk;
EXPECT_TRUE(wallet.GetSaplingExtendedSpendingKey(address, extsk));
libzcash::diversifier_index_t j(2);
libzcash::diversifier_index_t j(10);
auto dpa = extsk.ToXFVK().FindAddress(j).first;
// Add diversified address to the wallet

4
src/wallet/rpcdump.cpp
View File

@ -614,7 +614,7 @@ UniValue dumpwallet_impl(const UniValue& params, bool fDumpZKeys)
if (hdSeed.has_value()) {
auto mSeed = hdSeed.value();
file << strprintf(
"# Emergency Recovery Information\n"
"# Emergency Recovery Information:\n"
"# - recovery_phrase=\"%s\"\n"
"# - language=%s\n"
"# - fingerprint=%s\n",
@ -628,7 +628,7 @@ UniValue dumpwallet_impl(const UniValue& params, bool fDumpZKeys)
if (legacySeed.has_value()) {
auto rawSeed = legacySeed.value().RawSeed();
file << strprintf(
"# Legacy HD Seed\n"
"# Legacy HD Seed:\n"
"# - seed=%s\n"
"# - fingerprint=%s\n",
HexStr(rawSeed.begin(), rawSeed.end()),

3
src/wallet/rpcwallet.cpp
View File

@ -54,7 +54,6 @@ using namespace libzcash;
const std::string ADDR_TYPE_SPROUT = "sprout";
const std::string ADDR_TYPE_SAPLING = "sapling";
const bool DEFAULT_WALLET_REQUIRE_BACKUP = true;
extern UniValue TxJoinSplitToJSON(const CTransaction& tx);
@ -89,7 +88,7 @@ void EnsureWalletIsBackedUp(const CChainParams& params)
throw JSONRPCError(
RPC_WALLET_BACKUP_REQUIRED,
"Error: Please acknowledge that you have backed up the wallet's emergency recovery phrase "
"with walletconfirmbackup first."
"by using the walletconfirmbackup RPC method first."
);
}

13
src/wallet/wallet.cpp
View File

@ -110,8 +110,8 @@ libzcash::SproutPaymentAddress CWallet::GenerateNewSproutZKey()
return addr;
}
// Generate a new Sapling spending key as a child of the legacy Sapling account
// return its public payment address.
// Generates a new Sapling spending key as a child of the legacy Sapling account,
// and returns its public payment address.
//
// The z_getnewaddress API must use the mnemonic HD seed, and fail if that seed
// is not present. The account index is determined by trial of values of
@ -435,7 +435,7 @@ ZcashdUnifiedSpendingKey CWallet::GenerateNewUnifiedSpendingKey() {
std::optional<libzcash::ZcashdUnifiedSpendingKey> CWallet::GenerateUnifiedSpendingKeyForAccount(uint32_t accountId) {
auto seed = GetMnemonicSeed();
if (!seed.has_value()) {
throw std::runtime_error(std::string(__func__) + ": Wallet has no mnemonic HD seed. Please upgrade this wallet.");
throw std::runtime_error(std::string(__func__) + ": Wallet has no mnemonic HD seed.");
}
auto usk = ZcashdUnifiedSpendingKey::ForAccount(seed.value(), BIP44CoinType(), accountId);
@ -4240,8 +4240,7 @@ bool CWallet::NewKeyPool()
for (int i = 0; i < nKeys; i++)
{
int64_t nIndex = i+1;
auto key = GenerateNewKey();
walletdb.WritePool(nIndex, CKeyPool(key));
walletdb.WritePool(nIndex, CKeyPool(GenerateNewKey()));
setKeyPool.insert(nIndex);
}
LogPrintf("CWallet::NewKeyPool wrote %d new keys\n", nKeys);
@ -4271,8 +4270,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
int64_t nEnd = 1;
if (!setKeyPool.empty())
nEnd = *(--setKeyPool.end()) + 1;
auto newKey = GenerateNewKey();
if (!walletdb.WritePool(nEnd, CKeyPool(newKey)))
if (!walletdb.WritePool(nEnd, CKeyPool(GenerateNewKey())))
throw runtime_error("TopUpKeyPool(): writing generated key failed");
setKeyPool.insert(nEnd);
LogPrintf("keypool added key %d, size=%u\n", nEnd, setKeyPool.size());
@ -4898,7 +4896,6 @@ bool CWallet::InitLoadWallet(const CChainParams& params, bool clearWitnessCaches
walletInstance->SetMaxVersion(nMaxVersion);
}
// TODO: should this go in `LoadWallet` instead?
if (!walletInstance->HaveMnemonicSeed())
{
// We can't set the new HD seed until the wallet is decrypted.

6
src/wallet/wallet.h
View File

@ -1060,7 +1060,7 @@ public:
libzcash::SaplingPaymentAddress GenerateNewLegacySaplingZKey();
//! Generates Sapling key at the specified address index, and stores the newly generated
//! spending key to the wallet if it has not alreay been persisted.
//! Returns the newly created key, its metadata, and a flag distinguishing
//! Returns the newly created key, and a flag distinguishing
//! whether or not the key was already known by the wallet.
std::pair<libzcash::SaplingExtendedSpendingKey, bool> GenerateLegacySaplingZKey(uint32_t addrIndex);
//! Adds Sapling spending key to the store, and saves it to disk
@ -1308,12 +1308,12 @@ public:
bool VerifyMnemonicSeed(const SecureString& mnemonic);
bool MnemonicVerified();
/* Set the current HD seed, without saving it to disk (used by LoadWallet) */
/* Set the current mnemonic phrase, without saving it to disk (used by LoadWallet) */
bool LoadMnemonicSeed(const MnemonicSeed& seed);
/* Set the legacy HD seed, without saving it to disk (used by LoadWallet) */
bool LoadLegacyHDSeed(const HDSeed& seed);
/* Set the current encrypted HD seed, without saving it to disk (used by LoadWallet) */
/* Set the current encrypted mnemonic phrase, without saving it to disk (used by LoadWallet) */
bool LoadCryptedMnemonicSeed(const uint256& seedFp, const std::vector<unsigned char>& seed);
/* Set the legacy encrypted HD seed, without saving it to disk (used by LoadWallet) */
bool LoadCryptedLegacyHDSeed(const uint256& seedFp, const std::vector<unsigned char>& seed);

14
src/wallet/walletdb.cpp
View File

@ -685,7 +685,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
{
ssValue >> pwallet->nWitnessCacheSize;
}
else if (strType == "mnemonicseed")
else if (strType == "mnemonicphrase")
{
uint256 seedFp;
ssKey >> seedFp;
@ -693,17 +693,17 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
if (seed.Fingerprint() != seedFp)
{
strErr = "Error reading wallet database: HDSeed corrupt";
strErr = "Error reading wallet database: mnemonic phrase corrupt";
return false;
}
if (!pwallet->LoadMnemonicSeed(seed))
{
strErr = "Error reading wallet database: LoadHDSeed failed";
strErr = "Error reading wallet database: LoadMnemonicSeed failed";
return false;
}
}
else if (strType == "chdmnemonicseed")
else if (strType == "cmnemonicphrase")
{
uint256 seedFp;
vector<unsigned char> vchCryptedSecret;
@ -774,7 +774,7 @@ static bool IsKeyType(string strType)
{
return (strType== "key" || strType == "wkey" ||
strType == "hdseed" || strType == "chdseed" ||
strType == "mnemonicseed" || strType == "chdmnemonicseed" ||
strType == "mnemonicphrase" || strType == "cmnemonicphrase" ||
strType == "zkey" || strType == "czkey" ||
strType == "sapzkey" || strType == "csapzkey" ||
strType == "vkey" || strType == "sapextfvk" ||
@ -1194,13 +1194,13 @@ bool CWalletDB::WriteNetworkInfo(const std::string& networkId)
bool CWalletDB::WriteMnemonicSeed(const MnemonicSeed& seed)
{
nWalletDBUpdateCounter++;
return Write(std::make_pair(std::string("mnemonicseed"), seed.Fingerprint()), seed);
return Write(std::make_pair(std::string("mnemonicphrase"), seed.Fingerprint()), seed);
}
bool CWalletDB::WriteCryptedMnemonicSeed(const uint256& seedFp, const std::vector<unsigned char>& vchCryptedSecret)
{
nWalletDBUpdateCounter++;
return Write(std::make_pair(std::string("chdmnemonicseed"), seedFp), vchCryptedSecret);
return Write(std::make_pair(std::string("cmnemonicphrase"), seedFp), vchCryptedSecret);
}
bool CWalletDB::WriteMnemonicHDChain(const CHDChain& chain)

11
src/zcash/address/zip32.cpp
View File

@ -25,7 +25,7 @@ MnemonicSeed MnemonicSeed::Random(uint32_t bip44CoinType, Language language, siz
{
assert(entropyLen >= 32);
while (true) { // loop until we find usable entropy
std::vector<unsigned char> entropy(entropyLen, 0);
RawHDSeed entropy(entropyLen, 0);
GetRandBytes(entropy.data(), entropyLen);
const char* phrase = zip339_entropy_to_phrase(language, entropy.data(), entropyLen);
SecureString mnemonic(phrase);
@ -99,7 +99,7 @@ std::optional<std::pair<CExtKey, HDKeyPath>> DeriveBip44TransparentAccountKey(co
}
std::optional<Bip44AccountChains> Bip44AccountChains::ForAccount(
const HDSeed& seed,
const MnemonicSeed& seed,
uint32_t bip44CoinType,
uint32_t accountId) {
auto accountKeyOpt = DeriveBip44TransparentAccountKey(seed, bip44CoinType, accountId);
@ -263,7 +263,10 @@ std::pair<SaplingExtendedSpendingKey, HDKeyPath> SaplingExtendedSpendingKey::For
std::pair<SaplingExtendedSpendingKey, HDKeyPath> SaplingExtendedSpendingKey::Legacy(const HDSeed& seed, uint32_t bip44CoinType, uint32_t addressIndex) {
auto m = Master(seed);
// We use a fixed keypath scheme of m/32'/coin_type'/account'/addressIndex'
// We use a fixed keypath scheme of m/32'/coin_type'/0x7FFFFFFF'/addressIndex'
// This is not a "standard" path, but instead is a predictable location for
// legacy zcashd-derived keys that is minimally different from the UA account
// path, while unlikely to collide with normal UA account usage.
// Derive m/32'
auto m_32h = m.Derive(32 | ZIP32_HARDENED_KEY_LIMIT);
@ -301,7 +304,7 @@ SaplingExtendedFullViewingKey SaplingExtendedSpendingKey::ToXFVK() const
// Unified
//
std::optional<std::pair<ZcashdUnifiedSpendingKey, HDKeyPath>> ZcashdUnifiedSpendingKey::ForAccount(const HDSeed& seed, uint32_t bip44CoinType, uint32_t accountId) {
std::optional<std::pair<ZcashdUnifiedSpendingKey, HDKeyPath>> ZcashdUnifiedSpendingKey::ForAccount(const MnemonicSeed& seed, uint32_t bip44CoinType, uint32_t accountId) {
ZcashdUnifiedSpendingKey usk;
usk.accountId = accountId;

4
src/zcash/address/zip32.h
View File

@ -367,7 +367,7 @@ private:
ZcashdUnifiedSpendingKey() {}
public:
static std::optional<std::pair<ZcashdUnifiedSpendingKey, HDKeyPath>> ForAccount(
const HDSeed& seed,
const MnemonicSeed& mnemonic,
uint32_t bip44CoinType,
uint32_t accountId);
@ -396,7 +396,7 @@ private:
seedFp(seedFpIn), accountId(accountIdIn), bip44CoinType(bip44CoinTypeIn), external(externalIn), internal(internalIn) {}
public:
static std::optional<Bip44AccountChains> ForAccount(
const HDSeed& seed,
const MnemonicSeed& mnemonic,
uint32_t bip44CoinType,
uint32_t accountId);