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

Do not add Sapling addresses to the wallet by default when adding extfvks. 

Adding the default address to the wallet automatically made sense
prior to support of diversified addresses, but this is no longer
desirable behavior as we have separated key generation from address
derivation.
This commit is contained in:
Kris Nuttycombe 2022-01-30 11:21:56 -07:00
parent 66530d97e1
commit 90e59c3be0
6 changed files with 68 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/gtest/test_keystore.cpp
View File

@ -270,6 +270,7 @@ TEST(KeystoreTests, StoreAndRetrieveSaplingSpendingKey) {
EXPECT_TRUE(keyStore.GetSaplingSpendingKey(extfvk, skOut));
EXPECT_TRUE(keyStore.HaveSaplingFullViewingKey(ivk));
EXPECT_TRUE(keyStore.GetSaplingFullViewingKey(ivk, extfvkOut));
keyStore.AddSaplingPaymentAddress(ivk, addr);
EXPECT_TRUE(keyStore.HaveSaplingIncomingViewingKey(addr));
EXPECT_TRUE(keyStore.GetSaplingIncomingViewingKey(addr, ivkOut));
EXPECT_EQ(sk, skOut);
@ -313,7 +314,8 @@ TEST(KeystoreTests, StoreAndRetrieveSaplingFullViewingKey) {
EXPECT_FALSE(keyStore.HaveSaplingSpendingKey(extfvk));
EXPECT_FALSE(keyStore.GetSaplingSpendingKey(extfvk, skOut));
// ... but we should have an incoming viewing key
// The IVK must be manually associated with the address...
keyStore.AddSaplingPaymentAddress(ivk, addr);
EXPECT_TRUE(keyStore.HaveSaplingIncomingViewingKey(addr));
EXPECT_TRUE(keyStore.GetSaplingIncomingViewingKey(addr, ivkOut));
EXPECT_EQ(ivk, ivkOut);

2
src/keystore.cpp
View File

@ -196,7 +196,7 @@ bool CBasicKeyStore::AddSaplingFullViewingKey(
auto ivk = extfvk.fvk.in_viewing_key();
mapSaplingFullViewingKeys[ivk] = extfvk;
return CBasicKeyStore::AddSaplingPaymentAddress(ivk, extfvk.DefaultAddress());
return true;
}
// This function updates the wallet's internal address->ivk map.

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

@ -59,11 +59,11 @@ TEST(WalletZkeysTest, StoreAndLoadSaplingZkeys) {
wallet.GetSaplingSpendingKey(extfvk, keyOut);
ASSERT_EQ(sk, keyOut);
// verify there are two keys
// verify there is still only one key; adding the spending
// key no longer adds the default address
wallet.GetSaplingPaymentAddresses(addrs);
EXPECT_EQ(2, addrs.size());
EXPECT_EQ(1, addrs.size());
EXPECT_EQ(1, addrs.count(address));
EXPECT_EQ(1, addrs.count(sk.ToXFVK().DefaultAddress()));
// 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
@ -71,6 +71,9 @@ TEST(WalletZkeysTest, StoreAndLoadSaplingZkeys) {
libzcash::diversifier_index_t j(10);
auto dpa = sk.ToXFVK().FindAddress(j).first;
// add the default address
EXPECT_TRUE(wallet.AddSaplingPaymentAddress(sk.ToXFVK().fvk.in_viewing_key(), sk.ToXFVK().DefaultAddress()));
// verify wallet only has the default address
EXPECT_TRUE(wallet.HaveSaplingIncomingViewingKey(sk.ToXFVK().DefaultAddress()));
EXPECT_FALSE(wallet.HaveSaplingIncomingViewingKey(dpa));
@ -98,7 +101,7 @@ TEST(WalletZkeysTest, StoreAndLoadSaplingZkeys) {
// Load a diversified address for the third key into the wallet
auto dpa2 = sk2.ToXFVK().FindAddress(j).first;
EXPECT_TRUE(wallet.HaveSaplingIncomingViewingKey(sk2.ToXFVK().DefaultAddress()));
EXPECT_FALSE(wallet.HaveSaplingIncomingViewingKey(sk2.ToXFVK().DefaultAddress()));
EXPECT_FALSE(wallet.HaveSaplingIncomingViewingKey(dpa2));
EXPECT_TRUE(wallet.LoadSaplingPaymentAddress(dpa2, ivk2));
EXPECT_TRUE(wallet.HaveSaplingIncomingViewingKey(dpa2));
@ -469,6 +472,9 @@ TEST(WalletZkeysTest, WriteCryptedSaplingZkeyDirectToDb) {
wallet.Unlock(strWalletPass);
auto address2 = wallet.GenerateNewLegacySaplingZKey();
wallet.GetSaplingPaymentAddresses(addrs);
ASSERT_EQ(3, addrs.size());
// flush the wallet to prevent race conditions
wallet.Flush();

4
src/wallet/rpcdump.cpp
View File

@ -404,7 +404,7 @@ UniValue importwallet_impl(const UniValue& params, bool fImportZKeys)
std::optional<std::string> seedFpStr = (vstr.size() > 3) ? std::optional<std::string>(vstr[3]) : std::nullopt;
if (spendingkey.has_value()) {
auto addResult = std::visit(
AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus(), nTime, hdKeypath, seedFpStr, true), spendingkey.value());
AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus(), nTime, hdKeypath, seedFpStr, true, true), spendingkey.value());
if (addResult == KeyAlreadyExists){
LogPrint("zrpc", "Skipping import of zaddr (key already present)\n");
} else if (addResult == KeyNotAdded) {
@ -878,7 +878,7 @@ UniValue z_importviewingkey(const UniValue& params, bool fHelp)
result.pushKV("type", addrInfo.first);
result.pushKV("address", strAddress);
auto addResult = std::visit(AddViewingKeyToWallet(pwalletMain), viewingkey.value());
auto addResult = std::visit(AddViewingKeyToWallet(pwalletMain, true), viewingkey.value());
if (addResult == SpendingKeyExists) {
throw JSONRPCError(
RPC_WALLET_ERROR,

60
src/wallet/wallet.cpp
View File

@ -170,6 +170,10 @@ std::pair<SaplingPaymentAddress, bool> CWallet::GenerateLegacySaplingZKey(uint32
throw std::runtime_error("CWallet::GenerateLegacySaplingZKey(): AddSaplingZKey failed.");
}
if (!AddSaplingPaymentAddress(ivk, extfvk.DefaultAddress())) {
throw std::runtime_error("CWallet::GenerateLegacySaplingZKey(): AddSaplingPaymentAddress failed.");
};
return std::make_pair(extfvk.DefaultAddress(), true) ;
} else {
return std::make_pair(extfvk.DefaultAddress(), false);
@ -227,9 +231,7 @@ bool CWallet::AddSaplingPaymentAddress(
return true;
}
if (!IsCrypted()) {
return CWalletDB(strWalletFile).WriteSaplingPaymentAddress(addr, ivk);
}
return CWalletDB(strWalletFile).WriteSaplingPaymentAddress(addr, ivk);
return true;
}
@ -503,24 +505,35 @@ std::optional<libzcash::ZcashdUnifiedSpendingKey>
// the secret keys that we need to store are the child spending keys
// that are produced whenever we create a transparent address.
// Add Sapling component to the wallet
auto saplingEsk = usk.value().GetSaplingExtendedSpendingKey();
auto saplingKeyPath = libzcash::Zip32AccountKeyPath(BIP44CoinType(), accountId);
auto addSpendingKey = AddSpendingKeyToWallet(
// Create the function that we'll use to add Sapling keys
// to the wallet.
auto addSaplingKey = AddSpendingKeyToWallet(
this, Params().GetConsensus(), GetTime(),
saplingKeyPath, skmeta.GetSeedFingerprint().GetHex(), true);
if (addSpendingKey(saplingEsk) == KeyNotAdded) {
libzcash::Zip32AccountKeyPath(BIP44CoinType(), accountId),
skmeta.GetSeedFingerprint().GetHex(), true, false
);
// Add the Sapling spending key to the wallet
auto saplingEsk = usk.value().GetSaplingExtendedSpendingKey();
if (addSaplingKey(saplingEsk) == KeyNotAdded) {
// If adding the Sapling key to the wallet failed, abort the process.
throw std::runtime_error("CWalletDB::GenerateUnifiedSpendingKeyForAccount(): Unable to add Sapling key component to the wallet.");
}
// Add the Sapling change key to the wallet
// Add the Sapling change spending key to the wallet
auto saplingChangeEsk = saplingEsk.DeriveInternalKey();
if (addSpendingKey(saplingChangeEsk) == KeyNotAdded) {
if (addSaplingKey(saplingChangeEsk) == KeyNotAdded) {
// If adding the Sapling change key to the wallet failed, abort the process.
throw std::runtime_error("CWalletDB::GenerateUnifiedSpendingKeyForAccount(): Unable to add Sapling key component to the wallet.");
}
// Associate the Sapling default change address with the IVK
auto saplingChangeDFVK = saplingEsk.ToXFVK().GetInternalDFVK();
auto saplingChangeIVK = saplingChangeDFVK.fvk.in_viewing_key();
if (!AddSaplingPaymentAddress(saplingChangeIVK, saplingChangeDFVK.GetChangeAddress())) {
throw std::runtime_error("CWallet::GenerateUnifiedSpendingKeyForAccount(): Failed to add Sapling change address to the wallet.");
};
auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(Params(), ufvk);
if (!CCryptoKeyStore::AddUnifiedFullViewingKey(zufvk)) {
throw std::runtime_error("CWalletDB::GenerateUnifiedSpendingKeyForAccount(): Failed to add UFVK to the keystore.");
@ -696,6 +709,17 @@ UAGenerationResult CWallet::GenerateUnifiedAddress(
);
}
// If the address has a Sapling component, add an association between
// that address and the Sapling IVK corresponding to the ufvk
auto hasSapling = receiverTypes.find(ReceiverType::Sapling) != receiverTypes.end();
if (hasSapling) {
auto dfvk = ufvk.value().GetSaplingKey();
auto saplingAddress = address.value().GetSaplingReceiver();
assert (dfvk.has_value() && saplingAddress.has_value());
AddSaplingPaymentAddress(dfvk.value().fvk.in_viewing_key(), saplingAddress.value());
}
// Save the metadata for the generated address so that we can re-derive
// it in the future.
ZcashdUnifiedAddressMetadata addrmeta(ufvkid, diversifierIndex, receiverTypes);
@ -6208,7 +6232,9 @@ KeyAddResult AddViewingKeyToWallet::operator()(const libzcash::SaplingExtendedFu
return SpendingKeyExists;
} else if (m_wallet->HaveSaplingFullViewingKey(extfvk.fvk.in_viewing_key())) {
return KeyAlreadyExists;
} else if (m_wallet->AddSaplingFullViewingKey(extfvk)) {
} else if (
m_wallet->AddSaplingFullViewingKey(extfvk) &&
(!addDefaultAddress || m_wallet->AddSaplingPaymentAddress(extfvk.fvk.in_viewing_key(), extfvk.DefaultAddress()))) {
return KeyAdded;
} else {
return KeyNotAdded;
@ -6238,16 +6264,20 @@ KeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SproutSpendingKe
KeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SaplingExtendedSpendingKey &sk) const {
auto extfvk = sk.ToXFVK();
auto ivk = extfvk.fvk.in_viewing_key();
auto addr = extfvk.DefaultAddress();
KeyIO keyIO(Params());
{
{ // TODO: why is this extra scope here?
if (log){
LogPrint("zrpc", "Importing zaddr %s...\n", keyIO.EncodePaymentAddress(sk.ToXFVK().DefaultAddress()));
LogPrint("zrpc", "Importing zaddr %s...\n", keyIO.EncodePaymentAddress(addr));
}
// Don't throw error in case a key is already there
if (m_wallet->HaveSaplingSpendingKey(extfvk)) {
return KeyAlreadyExists;
} else {
if (!m_wallet->AddSaplingZKey(sk)) {
if (!(
m_wallet->AddSaplingZKey(sk) &&
(!addDefaultAddress || m_wallet->AddSaplingPaymentAddress(ivk, addr))
)) {
return KeyNotAdded;
}

11
src/wallet/wallet.h
View File

@ -1810,8 +1810,9 @@ class AddViewingKeyToWallet
{
private:
CWallet *m_wallet;
bool addDefaultAddress;
public:
AddViewingKeyToWallet(CWallet *wallet) : m_wallet(wallet) {}
AddViewingKeyToWallet(CWallet *wallet, bool addDefaultAddressIn) : m_wallet(wallet), addDefaultAddress(addDefaultAddressIn) {}
KeyAddResult operator()(const libzcash::SproutViewingKey &sk) const;
KeyAddResult operator()(const libzcash::SaplingExtendedFullViewingKey &sk) const;
@ -1827,17 +1828,19 @@ private:
std::optional<std::string> hdKeypath; // currently sapling only
std::optional<std::string> seedFpStr; // currently sapling only
bool log;
bool addDefaultAddress;
public:
AddSpendingKeyToWallet(CWallet *wallet, const Consensus::Params &params) :
m_wallet(wallet), params(params), nTime(1), hdKeypath(std::nullopt), seedFpStr(std::nullopt), log(false) {}
m_wallet(wallet), params(params), nTime(1), hdKeypath(std::nullopt), seedFpStr(std::nullopt), log(false), addDefaultAddress(true) {}
AddSpendingKeyToWallet(
CWallet *wallet,
const Consensus::Params &params,
int64_t _nTime,
std::optional<std::string> _hdKeypath,
std::optional<std::string> _seedFp,
bool _log
) : m_wallet(wallet), params(params), nTime(_nTime), hdKeypath(_hdKeypath), seedFpStr(_seedFp), log(_log) {}
bool _log,
bool _addDefaultAddress
) : m_wallet(wallet), params(params), nTime(_nTime), hdKeypath(_hdKeypath), seedFpStr(_seedFp), log(_log), addDefaultAddress(_addDefaultAddress) {}
KeyAddResult operator()(const libzcash::SproutSpendingKey &sk) const;