diff --git a/src/key.h b/src/key.h index 4f48f3bbd..654e8bcd7 100644 --- a/src/key.h +++ b/src/key.h @@ -15,13 +15,6 @@ #include #include -/** - * The account identifier used for HD derivation of the transparent - * p2pkh public key from which all child transparent addresses are - * derived in accordance with ZIP-316. - */ -const uint32_t ZCASH_LEGACY_TRANSPARENT_ACCOUNT = 0x7FFFFFFE; - /** * secure_allocator is defined in allocators.h * CPrivKey is a serialized private key, with all parameters included diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index 49540be6c..b0c0ddf01 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -35,10 +35,10 @@ TEST(WalletZkeysTest, StoreAndLoadSaplingZkeys) { // Load the all-zeroes seed std::string mnemonic("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art"); MnemonicSeed seed(English, mnemonic); - // The legacy seed used to be automatically derived from randomness; since - // non-mnemonic random generation has been removed we just use the + // The legacy seed used to be automatically derived from randomness; since + // non-mnemonic random generation has been removed we just use the // all-zeros mnemonic for these tests. - wallet.LoadLegacyHDSeed(seed); + wallet.LoadMnemonicSeed(seed); // Now this call succeeds legacyKey = wallet.GenerateNewLegacySaplingZKey(); @@ -438,7 +438,7 @@ TEST(WalletZkeysTest, WriteCryptedSaplingZkeyDirectToDb) { // Load the all-zeroes seed as the legacy seed std::string mnemonic("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art"); MnemonicSeed seed(English, mnemonic); - wallet.LoadLegacyHDSeed(seed); + wallet.LoadMnemonicSeed(seed); // wallet should be empty std::set addrs; diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index b54b33fa5..177ae373f 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -614,7 +614,7 @@ UniValue dumpwallet_impl(const UniValue& params, bool fDumpZKeys) if (hdSeed.has_value()) { auto mSeed = hdSeed.value(); file << strprintf( - "# Recovery Phrase=\"%s\" \n# language=%s \n# fingerprint=%s\n", + "# Emergency Recovery Phrase=\"%s\" \n# language=%s \n# fingerprint=%s\n", mSeed.GetMnemonic(), MnemonicSeed::LanguageName(mSeed.GetLanguage()), mSeed.Fingerprint().GetHex() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c9a7a2169..93600bf24 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2034,10 +2034,13 @@ UniValue getwalletinfo(const UniValue& params, bool fHelp) obj.pushKV("paytxfee", ValueFromAmount(payTxFee.GetFeePerK())); auto mnemonicChain = pwalletMain->GetMnemonicHDChain(); if (mnemonicChain.has_value()) - obj.pushKV("seedfp", mnemonicChain.value().GetSeedFingerprint().GetHex()); - auto legacyChain = pwalletMain->GetLegacyHDChain(); - if (legacyChain.has_value()) - obj.pushKV("legacy_seedfp", legacyChain.value().GetSeedFingerprint().GetHex()); + obj.pushKV("mnemonic_seedfp", mnemonicChain.value().GetSeedFingerprint().GetHex()); + // TODO: do we really need to return the legacy seed fingerprint if we're + // no longer using it to generate any new keys? What do people actually use + // the fingerprint for? + auto legacySeed = pwalletMain->GetLegacyHDSeed(); + if (legacySeed.has_value()) + obj.pushKV("legacy_seedfp", legacySeed.value().Fingerprint().GetHex()); return obj; } diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 806bcec93..dd591af56 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -11,8 +11,10 @@ WalletTestingSetup::WalletTestingSetup(): TestingSetup() bool fFirstRun; pwalletMain = new CWallet(Params(), "wallet_test.dat"); pwalletMain->LoadWallet(fFirstRun); - RegisterValidationInterface(pwalletMain); + if (!pwalletMain->HaveMnemonicSeed()) + pwalletMain->GenerateNewSeed(); + RegisterValidationInterface(pwalletMain); RegisterWalletRPCCommands(tableRPC); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a02a6aa25..6635f81e4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -121,7 +121,7 @@ libzcash::SproutPaymentAddress CWallet::GenerateNewSproutZKey() std::optional CWallet::GenerateNewLegacySaplingZKey() { AssertLockHeld(cs_wallet); - auto seedOpt = GetLegacyHDSeed(); + auto seedOpt = GetMnemonicSeed(); if (seedOpt.has_value()) { auto seed = seedOpt.value(); if (!legacyHDChain.has_value()) { @@ -129,21 +129,21 @@ std::optional CWallet::GenerateNewLegacySaplingZKey() { } CHDChain& hdChain = legacyHDChain.value(); - // loop until we find an unused account id + // loop until we find an unused address index while (true) { - auto xsk = libzcash::SaplingExtendedSpendingKey::ForAccount( + auto xsk = libzcash::SaplingExtendedSpendingKey::Legacy( seed, BIP44CoinType(), - hdChain.GetAccountCounter()); - // advance the account counter so that the next time we need to generate + hdChain.GetLegacySaplingKeyCounter()); + // advance the address index counter so that the next time we need to generate // a key we're pointing at a free index. - hdChain.IncrementAccountCounter(); + hdChain.IncrementLegacySaplingKeyCounter(); if (HaveSaplingSpendingKey(xsk.first.ToXFVK())) { - // try the next account ID + // try the next index continue; } else { // Update the persisted chain information - if (fFileBacked && !CWalletDB(strWalletFile).WriteLegacyHDChain(hdChain)) { + if (fFileBacked && !CWalletDB(strWalletFile).WriteMnemonicHDChain(hdChain)) { throw std::runtime_error("CWallet::GenerateNewLegacySaplingZKey(): Writing HD chain model failed"); } @@ -252,12 +252,12 @@ std::optional CWallet::GenerateNewKey() CHDChain& hdChain = mnemonicHDChain.value(); if (seedOpt.has_value()) { // All mnemonic seeds are checked at construction to ensure that we can obtain - // a valid spending key for the account ZCASH_LEGACY_TRANSPARENT_ACCOUNT; + // a valid spending key for the account ZCASH_LEGACY_ACCOUNT; // therefore, the `value()` call here is safe. BIP32AccountChains accountChains = BIP32AccountChains::ForAccount( seedOpt.value(), BIP44CoinType(), - ZCASH_LEGACY_TRANSPARENT_ACCOUNT).value(); + ZCASH_LEGACY_ACCOUNT).value(); while (true) { auto extKey = accountChains.DeriveExternal(hdChain.GetLegacyTKeyCounter()); @@ -272,7 +272,7 @@ std::optional CWallet::GenerateNewKey() // Create new metadata const CKeyMetadata& keyMeta = extKey.value().second; mapKeyMetadata[pubkey.GetID()] = keyMeta; - if (!nTimeFirstKey || keyMeta.nCreateTime < nTimeFirstKey) + if (nTimeFirstKey == 0 || keyMeta.nCreateTime < nTimeFirstKey) nTimeFirstKey = keyMeta.nCreateTime; if (!AddKeyPubKey(secret, pubkey)) @@ -2356,16 +2356,6 @@ void CWallet::SetMnemonicHDChain(const CHDChain& chain, bool memonly) mnemonicHDChain = chain; } -// TODO: make private -void CWallet::SetLegacyHDChain(const CHDChain& chain, bool memonly) -{ - LOCK(cs_wallet); - if (!memonly && fFileBacked && !CWalletDB(strWalletFile).WriteLegacyHDChain(chain)) - throw std::runtime_error(std::string(__func__) + ": writing chain failed"); - - legacyHDChain = chain; -} - bool CWallet::CheckNetworkInfo(std::pair readNetworkInfo) { LOCK(cs_wallet); @@ -4864,6 +4854,7 @@ 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. diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 35e13bcaf..6a7da3965 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1320,10 +1320,6 @@ public: void SetMnemonicHDChain(const CHDChain& chain, bool memonly); const std::optional& GetMnemonicHDChain() const { return mnemonicHDChain; } - /* Set the metadata for the legacy HD seed (chain child index counters) */ - void SetLegacyHDChain(const CHDChain& chain, bool memonly); - const std::optional GetLegacyHDChain() const { return legacyHDChain; } - bool CheckNetworkInfo(std::pair networkInfo); uint32_t BIP44CoinType(); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 0dad1fdda..417a5f8d8 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -749,11 +749,6 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, } wss.fIsEncrypted = true; } - else if (strType == "hdchain") - { - auto chain = CHDChain::Read(ssValue); - pwallet->SetLegacyHDChain(chain, true); - } else if (strType == "mnemonichdchain") { auto chain = CHDChain::Read(ssValue); @@ -1208,14 +1203,6 @@ bool CWalletDB::WriteCryptedMnemonicSeed(const uint256& seedFp, const std::vecto return Write(std::make_pair(std::string("chdmnemonicseed"), seedFp), vchCryptedSecret); } -// This can be removed once generation of Sapling addresses using the legacy -// seed has been disabled. -bool CWalletDB::WriteLegacyHDChain(const CHDChain& chain) -{ - nWalletDBUpdateCounter++; - return Write(std::string("hdchain"), chain); -} - bool CWalletDB::WriteMnemonicHDChain(const CHDChain& chain) { nWalletDBUpdateCounter++; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index aef6789cd..2f808822d 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -50,6 +50,7 @@ private: int64_t nCreateTime; // 0 means unknown uint32_t accountCounter; uint32_t legacyTKeyCounter; + uint32_t legacySaplingKeyCounter; CHDChain() { SetNull(); } @@ -60,12 +61,13 @@ private: nCreateTime = 0; accountCounter = 0; legacyTKeyCounter = 0; + legacySaplingKeyCounter = 0; } public: static const int VERSION_HD_BASE = 1; static const int CURRENT_VERSION = VERSION_HD_BASE; - CHDChain(uint256 seedFpIn, int64_t nCreateTimeIn): nVersion(CHDChain::CURRENT_VERSION), seedFp(seedFpIn), nCreateTime(nCreateTimeIn), accountCounter(0), legacyTKeyCounter(0) {} + CHDChain(uint256 seedFpIn, int64_t nCreateTimeIn): nVersion(CHDChain::CURRENT_VERSION), seedFp(seedFpIn), nCreateTime(nCreateTimeIn), accountCounter(0), legacyTKeyCounter(0), legacySaplingKeyCounter(0) {} ADD_SERIALIZE_METHODS; @@ -77,6 +79,7 @@ public: READWRITE(nCreateTime); READWRITE(accountCounter); READWRITE(legacyTKeyCounter); + READWRITE(legacySaplingKeyCounter); } template @@ -105,6 +108,14 @@ public: void IncrementLegacyTKeyCounter() { legacyTKeyCounter += 1; } + + uint32_t GetLegacySaplingKeyCounter() const { + return legacySaplingKeyCounter; + } + + void IncrementLegacySaplingKeyCounter() { + legacySaplingKeyCounter += 1; + } }; /** Access to the wallet database */ @@ -164,11 +175,6 @@ public: bool WriteCryptedMnemonicSeed(const uint256& seedFp, const std::vector& vchCryptedSecret); bool WriteMnemonicHDChain(const CHDChain& chain); - //! Write the legacy hdchain metadata to the database - //! - //! TODO: remove when generation of new legacy-seed-based keys has been disabled. - bool WriteLegacyHDChain(const CHDChain& chain); - /// Write spending key to wallet database, where key is payment address and value is spending key. bool WriteZKey(const libzcash::SproutPaymentAddress& addr, const libzcash::SproutSpendingKey& key, const CKeyMetadata &keyMeta); bool WriteSaplingZKey(const libzcash::SaplingIncomingViewingKey &ivk, diff --git a/src/zcash/address/zip32.cpp b/src/zcash/address/zip32.cpp index fe09bc183..19e28db4b 100644 --- a/src/zcash/address/zip32.cpp +++ b/src/zcash/address/zip32.cpp @@ -33,9 +33,13 @@ MnemonicSeed MnemonicSeed::Random(uint32_t bip44CoinType, Language language, siz MnemonicSeed seed(language, mnemonic); // Verify that the seed data is valid entropy for unified spending keys at - // account 0 and at both the public & private chain levels for account 0x7FFFFFFE + // account 0 and at both the public & private chain levels for account 0x7FFFFFFE. + // It is not necessary to check for a valid diversified Sapling address at + // account 0x7FFFFFFE because derivation via the legacy path can simply search + // for a valid diversifier; unlike in the unified spending key case, diversifier + // indices don't need to line up with anything. if (libzcash::UnifiedSpendingKey::ForAccount(seed, bip44CoinType, 0).has_value() && - libzcash::BIP32AccountChains::ForAccount(seed, bip44CoinType, ZCASH_LEGACY_TRANSPARENT_ACCOUNT).has_value()) { + libzcash::BIP32AccountChains::ForAccount(seed, bip44CoinType, ZCASH_LEGACY_ACCOUNT).has_value()) { return seed; } } @@ -268,6 +272,34 @@ std::pair SaplingExtendedSpendingKey:: return std::make_pair(xsk, keyMeta); } +std::pair 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' + + // Derive m/32' + auto m_32h = m.Derive(32 | ZIP32_HARDENED_KEY_LIMIT); + // Derive m/32'/coin_type' + auto m_32h_cth = m_32h.Derive(bip44CoinType | ZIP32_HARDENED_KEY_LIMIT); + + // Derive account key at the legacy account index + auto m_32h_cth_l = m_32h_cth.Derive(ZCASH_LEGACY_ACCOUNT | ZIP32_HARDENED_KEY_LIMIT); + + // Derive key at the specified address index (non-hardened) + auto xsk = m_32h_cth_l.Derive(addressIndex); + + // Create new metadata + int64_t nCreationTime = GetTime(); + CKeyMetadata metadata(nCreationTime); + metadata.hdKeypath = "m/32'/" + + std::to_string(bip44CoinType) + "'/" + + std::to_string(ZCASH_LEGACY_ACCOUNT) + "'/" + + std::to_string(addressIndex); + metadata.seedFp = seed.Fingerprint(); + + return std::make_pair(xsk, metadata); +} + SaplingExtendedFullViewingKey SaplingExtendedSpendingKey::ToXFVK() const { SaplingExtendedFullViewingKey ret; diff --git a/src/zcash/address/zip32.h b/src/zcash/address/zip32.h index cc7eb9f6e..1b4da4777 100644 --- a/src/zcash/address/zip32.h +++ b/src/zcash/address/zip32.h @@ -21,6 +21,14 @@ const uint32_t ZIP32_HARDENED_KEY_LIMIT = 0x80000000; const size_t ZIP32_XFVK_SIZE = 169; const size_t ZIP32_XSK_SIZE = 169; +/** + * The account identifier used for HD derivation of + * transparent and Sapling addresses via the legacy + * `getnewaddress` and `z_getnewaddress` code paths, + */ +const uint32_t ZCASH_LEGACY_ACCOUNT = 0x7FFFFFFE; + + class CWalletDB; typedef std::vector> RawHDSeed; @@ -275,6 +283,8 @@ struct SaplingExtendedSpendingKey { static SaplingExtendedSpendingKey Master(const HDSeed& seed); static std::pair ForAccount(const HDSeed& seed, uint32_t bip44CoinType, uint32_t accountId); + static std::pair Legacy(const HDSeed& seed, uint32_t bip44CoinType, uint32_t addressIndex); + SaplingExtendedSpendingKey Derive(uint32_t i) const;