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

Use a BIP 44 change address for change when sending from legacy t-addrs.

This commit is contained in:
Kris Nuttycombe 2022-01-26 19:32:43 -07:00
parent b46d85976d
commit 1733c6bc0c
5 changed files with 127 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/wallet/asyncrpcoperation_sendmany.cpp
View File

@ -266,15 +266,15 @@ uint256 AsyncRPCOperation_sendmany::main_impl() {
std::visit(match {
[&](const CKeyID& keyId) {
auto accountId = selectorAccountId.value_or(ZCASH_LEGACY_ACCOUNT);
builder_.SendChangeTo(
pwalletMain->GenerateChangeAddressForAccount(accountId, { libzcash::ChangeType::Sapling }).value(),
ovks.first);
auto changeAddr = pwalletMain->GenerateChangeAddressForAccount(
accountId, { libzcash::ChangeType::Transparent }).value();
builder_.SendChangeTo(changeAddr, ovks.first);
},
[&](const CScriptID& scriptId) {
auto accountId = selectorAccountId.value_or(ZCASH_LEGACY_ACCOUNT);
builder_.SendChangeTo(
pwalletMain->GenerateChangeAddressForAccount(accountId, { libzcash::ChangeType::Sapling }).value(),
ovks.first);
auto changeAddr = pwalletMain->GenerateChangeAddressForAccount(
accountId, { libzcash::ChangeType::Transparent }).value();
builder_.SendChangeTo(changeAddr, ovks.first);
},
[&](const libzcash::SproutPaymentAddress& addr) {
// for Sprout, we return change to the originating address.

33
src/wallet/rpcwallet.cpp
View File

@ -4010,18 +4010,15 @@ UniValue z_viewtransaction(const UniValue& params, bool fHelp)
// Collect OutgoingViewingKeys for recovering output information
std::set<uint256> ovks;
{
// Generate the common ovk for recovering t->z outputs.
// Generate the old, pre-UA accounts OVK for recovering t->z outputs.
HDSeed seed = pwalletMain->GetHDSeedForRPC();
ovks.insert(ovkForShieldingFromTaddr(seed));
auto legacyAcctUFVK = pwalletMain->GetUnifiedFullViewingKeyByAccount(ZCASH_LEGACY_ACCOUNT);
if (legacyAcctUFVK.has_value()) {
auto legacyAcctOVKs = legacyAcctUFVK.value().GetTransparentOVKsForShielding();
if (legacyAcctOVKs.has_value()) {
ovks.insert(legacyAcctOVKs.value().first);
ovks.insert(legacyAcctOVKs.value().second);
}
}
// Generate the OVKs for shielding from the legacy UA account
auto legacyKey = pwalletMain->GetLegacyAccountKey().ToAccountPubKey();
auto legacyAcctOVKs = legacyKey.GetOVKsForShielding();
ovks.insert(legacyAcctOVKs.first);
ovks.insert(legacyAcctOVKs.second);
}
// Sapling spends
@ -4386,6 +4383,24 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
"Invalid from address, no payment source found for address.");
}
auto selectorAccount = pwalletMain->FindAccountForSelector(ztxoSelectorOpt.value());
std::visit(match {
[&](const libzcash::UnifiedAddress& ua) {
if (!selectorAccount.has_value() || selectorAccount.value() == ZCASH_LEGACY_ACCOUNT) {
throw JSONRPCError(
RPC_INVALID_ADDRESS_OR_KEY,
"Invalid from address, UA does not correspond to a known account.");
}
},
[&](const auto& other) {
if (selectorAccount.has_value() && selectorAccount.value() != ZCASH_LEGACY_ACCOUNT) {
throw JSONRPCError(
RPC_INVALID_ADDRESS_OR_KEY,
"Invalid from address, bare address does not correspond the legacy account.");
}
}
}, decoded.value());
return ztxoSelectorOpt.value();
}
}();

37
src/wallet/test/rpc_wallet_tests.cpp
View File

@ -134,7 +134,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
LOCK2(cs_main, pwalletMain->cs_wallet);
CPubKey demoPubkey = pwalletMain->GenerateNewKey();
CPubKey demoPubkey = pwalletMain->GenerateNewKey(false);
CTxDestination demoAddress(CTxDestination(demoPubkey.GetID()));
UniValue retValue;
string strPurpose = "receive";
@ -143,7 +143,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
pwalletMain->SetAddressBook(demoPubkey.GetID(), "", strPurpose);
});
CPubKey setaccountDemoPubkey = pwalletMain->GenerateNewKey();
CPubKey setaccountDemoPubkey = pwalletMain->GenerateNewKey(false);
CTxDestination setaccountDemoAddress(CTxDestination(setaccountDemoPubkey.GetID()));
/*********************************
@ -1245,19 +1245,6 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals)
BOOST_CHECK( msg.find("Insufficient funds") != string::npos);
}
// minconf cannot be zero when sending from zaddr
{
TransactionBuilder builder(consensusParams, nHeight + 1, pwalletMain);
try {
auto selector = pwalletMain->ToZTXOSelector(zaddr1, true).value();
std::vector<SendManyRecipient> recipients = {SendManyRecipient(taddr1, 100*COIN, "DEADBEEF")};
std::shared_ptr<AsyncRPCOperation> operation(new AsyncRPCOperation_sendmany(builder, selector, recipients, 0));
BOOST_CHECK(false); // Fail test if an exception is not thrown
} catch (const UniValue& objError) {
BOOST_CHECK(find_error(objError, "Minconf cannot be zero when sending from a shielded address"));
}
}
// there are no unspent notes to spend
{
auto selector = pwalletMain->ToZTXOSelector(zaddr1, true).value();
@ -1338,7 +1325,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_taddr_to_sapling)
KeyIO keyIO(Params());
// add keys manually
auto taddr = pwalletMain->GenerateNewKey().GetID();
auto taddr = pwalletMain->GenerateNewKey(false).GetID();
std::string taddr1 = keyIO.EncodeDestination(taddr);
auto pa = pwalletMain->GenerateNewLegacySaplingZKey();
@ -1402,13 +1389,19 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_taddr_to_sapling)
tx.vShieldedOutput[0].cmu,
tx.vShieldedOutput[0].ephemeralKey));
// We should be able to decrypt the outCiphertext with the ovk
// generated for transparent addresses
std::optional<MnemonicSeed> seed = pwalletMain->GetMnemonicSeed();
BOOST_ASSERT(seed.has_value());
auto accountKey = pwalletMain->GetLegacyAccountKey().ToAccountPubKey();
auto ovks = accountKey.GetOVKsForShielding();
// We should not be able to decrypt with the internal change OVK for shielding
BOOST_CHECK(!AttemptSaplingOutDecryption(
tx.vShieldedOutput[0].outCiphertext,
ovks.first,
tx.vShieldedOutput[0].cv,
tx.vShieldedOutput[0].cmu,
tx.vShieldedOutput[0].ephemeralKey));
// We should be able to decrypt with the external OVK for shielding
BOOST_CHECK(AttemptSaplingOutDecryption(
tx.vShieldedOutput[0].outCiphertext,
ovkForShieldingFromTaddr(seed.value()),
ovks.second,
tx.vShieldedOutput[0].cv,
tx.vShieldedOutput[0].cmu,
tx.vShieldedOutput[0].ephemeralKey));
@ -1994,7 +1987,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_internals)
void TestWTxStatus(const Consensus::Params consensusParams, const int delta) {
auto AddTrx = [&consensusParams]() {
auto taddr = pwalletMain->GenerateNewKey().GetID();
auto taddr = pwalletMain->GenerateNewKey(false).GetID();
CMutableTransaction mtx = CreateNewContextualCMutableTransaction(consensusParams, 1);
CScript scriptPubKey = CScript() << OP_DUP << OP_HASH160 << ToByteVector(taddr) << OP_EQUALVERIFY << OP_CHECKSIG;
mtx.vout.push_back(CTxOut(5 * COIN, scriptPubKey));

117
src/wallet/wallet.cpp
View File

@ -256,9 +256,10 @@ bool CWallet::AddSproutZKey(const libzcash::SproutSpendingKey &key)
return true;
}
CPubKey CWallet::GenerateNewKey()
CPubKey CWallet::GenerateNewKey(bool isChangeKey)
{
AssertLockHeld(cs_wallet); // mapKeyMetadata
bool external = !isChangeKey;
if (!mnemonicHDChain.has_value()) {
throw std::runtime_error(
@ -270,13 +271,16 @@ CPubKey CWallet::GenerateNewKey()
std::optional<CPubKey> pubkey = std::nullopt;
do {
auto index = hdChain.GetLegacyTKeyCounter();
auto key = accountKey.DeriveExternalSpendingKey(index);
auto key = external ?
accountKey.DeriveExternalSpendingKey(index) :
accountKey.DeriveInternalSpendingKey(index);
hdChain.IncrementLegacyTKeyCounter();
if (key.has_value()) {
auto keyPath = transparent::AccountKey::KeyPath(BIP44CoinType(), ZCASH_LEGACY_ACCOUNT, true, index);
pubkey = AddTransparentSecretKey(
hdChain.GetSeedFingerprint(),
std::make_pair(key.value(), keyPath)
key.value(),
transparent::AccountKey::KeyPath(BIP44CoinType(), ZCASH_LEGACY_ACCOUNT, external, index)
);
}
// if we did not successfully generate a key, try again.
@ -292,15 +296,15 @@ CPubKey CWallet::GenerateNewKey()
CPubKey CWallet::AddTransparentSecretKey(
const uint256& seedFingerprint,
const std::pair<CKey, HDKeyPath>& extSecret)
const CKey& secret,
const HDKeyPath& keyPath)
{
CKey secret = extSecret.first;
CPubKey pubkey = secret.GetPubKey();
assert(secret.VerifyPubKey(pubkey));
// Create new metadata
CKeyMetadata keyMeta(GetTime());
keyMeta.hdKeypath = extSecret.second;
keyMeta.hdKeypath = keyPath,
keyMeta.seedFp = seedFingerprint;
mapKeyMetadata[pubkey.GetID()] = keyMeta;
if (nTimeFirstKey == 0 || keyMeta.nCreateTime < nTimeFirstKey)
@ -481,7 +485,12 @@ std::optional<libzcash::ZcashdUnifiedSpendingKey>
// Set up the bidirectional maps between the account ID and the UFVK ID.
auto metaKey = std::make_pair(skmeta.GetSeedFingerprint(), skmeta.GetAccountId());
mapUnifiedAccountKeys.insert({metaKey, ufvkid});
const auto [it, is_new_key] = mapUnifiedAccountKeys.insert({metaKey, skmeta.GetKeyID()});
if (!is_new_key) {
// key was already present, so just return the USK.
return usk.value();
}
// We set up the UFVKAddressMetadata with the correct account ID (so we identify
// the UFVK as corresponding to this account) and empty receivers data (as we
// haven't generated any addresses yet). We don't need to persist this directly,
@ -655,8 +664,36 @@ UAGenerationResult CWallet::GenerateUnifiedAddress(
}
assert(mapUfvkAddressMetadata[ufvkid].SetReceivers(diversifierIndex, receiverTypes));
// Writing this data is handled by `CWalletDB::WriteUnifiedAddressMetadata` below.
assert(CCryptoKeyStore::AddTransparentReceiverForUnifiedAddress(ufvkid, diversifierIndex, address.value()));
if (hasTransparent) {
// We must construct the and add transparent spending key associated
// with the external and internal transparent child addresses to the
// transparent keystore.
auto usk = GenerateUnifiedSpendingKeyForAccount(accountId).value();
auto accountKey = usk.GetTransparentKey();
// this .value is known to be safe from the earlier check
auto childIndex = diversifierIndex.ToTransparentChildIndex().value();
auto externalKey = accountKey.DeriveExternalSpendingKey(childIndex);
if (!externalKey.has_value()) {
return AddressGenerationError::NoAddressForDiversifier;
}
AddTransparentSecretKey(
mnemonicHDChain.value().GetSeedFingerprint(),
externalKey.value(),
transparent::AccountKey::KeyPath(BIP44CoinType(), accountId, true, childIndex)
);
// We do not add the change address for the transparent key, because
// we do not send transparent change when using unified accounts.
// Writing this data is handled by `CWalletDB::WriteUnifiedAddressMetadata` below.
assert(
CCryptoKeyStore::AddTransparentReceiverForUnifiedAddress(
ufvkid, diversifierIndex, address.value()
)
);
}
// Save the metadata for the generated address so that we can re-derive
// it in the future.
@ -666,27 +703,6 @@ UAGenerationResult CWallet::GenerateUnifiedAddress(
"CWallet::AddUnifiedAddress(): Writing unified address metadata failed");
}
if (hasTransparent) {
// Regenerate the secret key for the transparent address and add it to
// the wallet.
auto seed = GetMnemonicSeed().value();
auto accountKey = transparent::AccountKey::ForAccount(seed, BIP44CoinType(), accountId).value();
// this .value is known to be safe from the earlier check
auto childIndex = diversifierIndex.ToTransparentChildIndex().value();
auto key = accountKey.DeriveExternalSpendingKey(childIndex).value();
AddTransparentSecretKey(
seed.Fingerprint(),
std::make_pair(
key,
transparent::AccountKey::KeyPath(BIP44CoinType(), accountId, true, childIndex)
)
);
// We do not add the change address for the transparent key, because
// we do not send transparent change when using unified accounts.
}
return std::make_pair(address.value(), diversifierIndex);
} else {
return AddressGenerationError::NoSuchAccount;
@ -722,8 +738,12 @@ bool CWallet::LoadUnifiedFullViewingKey(const libzcash::UnifiedFullViewingKey &k
// restore unified addresses that have been previously generated to the
// keystore
for (const auto &[j, receiverTypes] : metadata->second.GetKnownReceiverSetsByDiversifierIndex()) {
auto addr = zufvk.Address(j, receiverTypes).value();
if (!CCryptoKeyStore::AddTransparentReceiverForUnifiedAddress(zufvk.GetKeyID(), j, addr)) {
auto addr = zufvk.Address(j, receiverTypes);
// failure to restore the generated address is an error
if (!addr.has_value()) return false;
if (!CCryptoKeyStore::AddTransparentReceiverForUnifiedAddress(zufvk.GetKeyID(), j, addr.value())) {
return false;
}
}
@ -754,6 +774,7 @@ bool CWallet::LoadUnifiedAddressMetadata(const ZcashdUnifiedAddressMetadata &add
// Regenerate the unified address and add it to the keystore.
auto j = addrmeta.GetDiversifierIndex();
auto addr = ufvk.value().Address(j, addrmeta.GetReceiverTypes()).value();
return CCryptoKeyStore::AddTransparentReceiverForUnifiedAddress(addrmeta.GetKeyID(), j, addr);
}
@ -1573,16 +1594,26 @@ bool CWallet::SelectorMatchesAddress(
std::optional<RecipientAddress> CWallet::GenerateChangeAddressForAccount(
libzcash::AccountId accountId,
std::set<libzcash::ChangeType> changeOptions) {
auto ufvk = this->GetUnifiedFullViewingKeyByAccount(accountId);
if (ufvk.has_value()) {
for (libzcash::ChangeType t : changeOptions) {
if (t == libzcash::ChangeType::Transparent && accountId == ZCASH_LEGACY_ACCOUNT) {
return GenerateNewKey().GetID();
} else {
return ufvk.value().GetChangeAddress(SaplingChangeRequest());
AssertLockHeld(cs_wallet);
// changeOptions is sorted in preference order, so return
// the first (and therefore most preferred) change address that
// we're able to generate.
for (libzcash::ChangeType t : changeOptions) {
if (t == libzcash::ChangeType::Transparent && accountId == ZCASH_LEGACY_ACCOUNT) {
return GenerateNewKey(true).GetID();
} else {
auto ufvk = this->GetUnifiedFullViewingKeyByAccount(accountId);
if (ufvk.has_value()) {
// Default to Sapling shielded change (TODO ORCHARD: update this)
auto changeAddr = ufvk.value().GetChangeAddress(SaplingChangeRequest());
if (changeAddr.has_value()) {
return changeAddr.value();
}
}
}
}
return std::nullopt;
}
@ -4901,7 +4932,7 @@ bool CWallet::NewKeyPool()
for (int i = 0; i < nKeys; i++)
{
int64_t nIndex = i+1;
walletdb.WritePool(nIndex, CKeyPool(GenerateNewKey()));
walletdb.WritePool(nIndex, CKeyPool(GenerateNewKey(false)));
setKeyPool.insert(nIndex);
}
LogPrintf("CWallet::NewKeyPool wrote %d new keys\n", nKeys);
@ -4931,7 +4962,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
int64_t nEnd = 1;
if (!setKeyPool.empty())
nEnd = *(--setKeyPool.end()) + 1;
if (!walletdb.WritePool(nEnd, CKeyPool(GenerateNewKey())))
if (!walletdb.WritePool(nEnd, CKeyPool(GenerateNewKey(false))))
throw runtime_error("TopUpKeyPool(): writing generated key failed");
setKeyPool.insert(nEnd);
LogPrintf("keypool added key %d, size=%u\n", nEnd, setKeyPool.size());
@ -4998,7 +5029,7 @@ std::optional<CPubKey> CWallet::GetKeyFromPool()
if (nIndex == -1)
{
if (IsLocked()) return std::nullopt;
return GenerateNewKey();
return GenerateNewKey(false);
}
KeepKey(nIndex);
return keypool.vchPubKey;

10
src/wallet/wallet.h
View File

@ -1052,7 +1052,8 @@ private:
/* Add a transparent secret key to the wallet. Internal use only. */
CPubKey AddTransparentSecretKey(
const uint256& seedFingerprint,
const std::pair<CKey, HDKeyPath>& extSecret);
const CKey& secret,
const HDKeyPath& keyPath);
protected:
bool UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx);
@ -1255,6 +1256,11 @@ public:
*/
std::optional<libzcash::AccountId> FindAccountForSelector(const ZTXOSelector& paymentSource) const;
/**
* Generate a change address for the specified account. If a transparent change
* address is requested, this will generate a fresh diversified unified address,
* and return the associated transparent change address.
*/
std::optional<libzcash::RecipientAddress> GenerateChangeAddressForAccount(
libzcash::AccountId accountId,
std::set<libzcash::ChangeType> changeOptions);
@ -1294,7 +1300,7 @@ public:
* keystore implementation
* Generate a new key
*/
CPubKey GenerateNewKey();
CPubKey GenerateNewKey(bool isChangeKey);
//! Adds a key to the store, and saves it to disk.
bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey);
//! Adds a key to the store, without saving it to disk (used by LoadWallet)