wallet: Fix bugs in `listaddresses`

- Legacy transparent addresses derived from the mnemonic seed are no
  longer duplicated in the `legacy_random` source.
- Legacy transparent change addresses derived from the mnemonic seed are
  now shown under that source.
- Sapling addresses that aren't part of a UA are now identified
  correctly when derived from the mnemonic seed, rather than being
  assumed to be derived from the legacy HD seed.
This commit is contained in:
Jack Grigg 2022-03-22 03:40:17 +00:00
parent e576f6616f
commit 24089109b2
2 changed files with 89 additions and 46 deletions

View File

@ -399,9 +399,14 @@ UniValue listaddresses(const UniValue& params, bool fHelp)
UniValue ret(UniValue::VARR);
// keypool-derived and imported/watchonly transparent addresses
// Split transparent addresses into several categories:
// - Generated randomly.
// - Imported watchonly.
// - Derived from mnemonic seed.
std::set<CTxDestination> t_generated_dests;
std::set<CTxDestination> t_change_dests;
std::set<CTxDestination> t_generated_change_dests;
std::set<CTxDestination> t_mnemonic_dests;
std::set<CTxDestination> t_mnemonic_change_dests;
std::set<CTxDestination> t_watchonly_dests;
// Get the CTxDestination values for all the entries in the transparent address book.
// This will include any address that has been generated by this wallet.
@ -424,6 +429,9 @@ UniValue listaddresses(const UniValue& params, bool fHelp)
case PaymentAddressSource::ImportedWatchOnly:
t_watchonly_dests.insert(item.first);
break;
case PaymentAddressSource::MnemonicHDSeed:
t_mnemonic_dests.insert(item.first);
break;
default:
// Not going to be in the address book.
assert(false);
@ -437,24 +445,35 @@ UniValue listaddresses(const UniValue& params, bool fHelp)
// a future unified incoming viewing key) will have been added to the address book.
for (const std::pair<CTxDestination, CAmount>& item : pwalletMain->GetAddressBalances()) {
if (t_generated_dests.count(item.first) == 0 &&
t_watchonly_dests.count(item.first) == 0) {
std::optional<PaymentAddressSource> source;
std::visit(match {
[&](const CKeyID& addr) {
source = GetSourceForPaymentAddress(pwalletMain)(addr);
},
[&](const CScriptID& addr) {
source = GetSourceForPaymentAddress(pwalletMain)(addr);
},
[&](const CNoDestination& addr) {}
}, item.first);
if (source.has_value() && source.value() == PaymentAddressSource::Random) {
// assume that if we didn't add the address to the addrbook
// that it's a change address. Ideally we'd have a better way
// of checking this by exploring the transaction graph;
t_change_dests.insert(item.first);
t_mnemonic_dests.count(item.first) == 0 &&
t_watchonly_dests.count(item.first) == 0)
{
std::optional<PaymentAddressSource> source;
std::visit(match {
[&](const CKeyID& addr) {
source = GetSourceForPaymentAddress(pwalletMain)(addr);
},
[&](const CScriptID& addr) {
source = GetSourceForPaymentAddress(pwalletMain)(addr);
},
[&](const CNoDestination& addr) {}
}, item.first);
if (source.has_value()) {
switch (source.value()) {
case PaymentAddressSource::Random:
t_generated_change_dests.insert(item.first);
break;
case PaymentAddressSource::MnemonicHDSeed:
t_mnemonic_change_dests.insert(item.first);
break;
default:
// assume that if we didn't add the address to the addrbook
// that it's a change address. Ideally we'd have a better way
// of checking this by exploring the transaction graph;
break;
}
}
}
}
/// sprout addresses
@ -486,16 +505,16 @@ UniValue listaddresses(const UniValue& params, bool fHelp)
hasData = true;
}
if (!t_change_dests.empty()) {
if (!t_generated_change_dests.empty()) {
UniValue random_t_change_addrs(UniValue::VARR);
for (const CTxDestination& dest : t_change_dests) {
for (const CTxDestination& dest : t_generated_change_dests) {
random_t_change_addrs.push_back(keyIO.EncodeDestination(dest));
}
random_t.pushKV("changeAddresses", random_t_change_addrs);
hasData = true;
}
if (!t_generated_dests.empty() || !t_change_dests.empty()) {
if (!t_generated_dests.empty() || !t_generated_change_dests.empty()) {
entry.pushKV("transparent", random_t);
}
@ -538,11 +557,11 @@ UniValue listaddresses(const UniValue& params, bool fHelp)
// Do not include any address that is associated with a unified key.
auto ua = pwalletMain->FindUnifiedAddressByReceiver(addr);
if (!ua.has_value()) {
ivkAddrs[ivkRet].push_back(addr);
ivkAddrs[ivkRet].push_back(addr);
}
}
}
}
}
{
UniValue ivk_groups(UniValue::VARR);
@ -662,25 +681,29 @@ UniValue listaddresses(const UniValue& params, bool fHelp)
{
UniValue entry(UniValue::VOBJ);
entry.pushKV("source", "mnemonic_seed");
bool hasData = false;
// transparent
// For each address in pwalletMain->mapKeyMetadata, include any address
// for which its HD keypath metadata indicates it is associated with the legacy
// account ID. All such addresses are derived from the mnemonic seed.
UniValue mnemonic_taddrs(UniValue::VARR);
for (const auto& [keyId, keyMeta] : pwalletMain->mapKeyMetadata) {
auto account = libzcash::ParseHDKeypathAccount(44, Params().BIP44CoinType(), keyMeta.hdKeypath);
if (account.has_value() && account.value() == ZCASH_LEGACY_ACCOUNT) {
mnemonic_taddrs.push_back(keyIO.EncodeDestination(keyId));
hasData = true;
UniValue mnemonic_transparent(UniValue::VOBJ);
if (!t_mnemonic_dests.empty()) {
UniValue mnemonic_taddrs(UniValue::VARR);
for (const CTxDestination& dest : t_mnemonic_dests) {
mnemonic_taddrs.push_back(keyIO.EncodeDestination(dest));
}
}
if (hasData) {
// This extra "level" of JSON allows attributes to be added in
// the future in a backward-compatible way.
UniValue mnemonic_transparent(UniValue::VOBJ);
mnemonic_transparent.pushKV("addresses", mnemonic_taddrs);
hasData = true;
}
if (!t_mnemonic_change_dests.empty()) {
UniValue mnemonic_change_taddrs(UniValue::VARR);
for (const CTxDestination& dest : t_mnemonic_change_dests) {
mnemonic_change_taddrs.push_back(keyIO.EncodeDestination(dest));
}
mnemonic_transparent.pushKV("changeAddresses", mnemonic_change_taddrs);
hasData = true;
}
if (!t_mnemonic_dests.empty() || !t_mnemonic_change_dests.empty()) {
entry.pushKV("transparent", mnemonic_transparent);
}

View File

@ -6852,12 +6852,14 @@ PaymentAddressSource GetSourceForPaymentAddress::operator()(const CKeyID &addr)
{
auto ufvkSource = this->GetUnifiedSource(addr);
if (ufvkSource == PaymentAddressSource::AddressNotFound) {
if (m_wallet->HaveKey(addr)) {
auto keyMeta = pwalletMain->mapKeyMetadata[addr];
auto account = libzcash::ParseHDKeypathAccount(44, Params().BIP44CoinType(), keyMeta.hdKeypath);
if (account.has_value() && account.value() == ZCASH_LEGACY_ACCOUNT) {
return PaymentAddressSource::MnemonicHDSeed;
} else if (m_wallet->HaveKey(addr)) {
return PaymentAddressSource::Random;
} else {
if (m_wallet->HaveWatchOnly(GetScriptForDestination(addr))) {
return PaymentAddressSource::ImportedWatchOnly;
}
} else if (m_wallet->HaveWatchOnly(GetScriptForDestination(addr))) {
return PaymentAddressSource::ImportedWatchOnly;
}
}
@ -6896,10 +6898,28 @@ PaymentAddressSource GetSourceForPaymentAddress::operator()(const libzcash::Sapl
// also have the corresponding SaplingExtendedFullViewingKey.
if (m_wallet->GetSaplingIncomingViewingKey(zaddr, ivk)) {
if (m_wallet->HaveSaplingFullViewingKey(ivk)) {
// If we have the HD keypath, it's related to the legacy seed
// If we have the HD keypath, it's related to a seed.
if (m_wallet->mapSaplingZKeyMetadata.count(ivk) > 0 &&
m_wallet->mapSaplingZKeyMetadata[ivk].hdKeypath != "") {
return PaymentAddressSource::LegacyHDSeed;
// The following keypaths have been used in zcashd:
// - Legacy seed: m/32'/coin_type'/account_counter'
// - Mnemonic phrase: m/32'/coin_type'/ZCASH_LEGACY_ACCOUNT'/account_counter'
//
// Thus if the keypath uses account ZCASH_LEGACY_ACCOUNT and
// is longer than "m/32'/coin_type'/ZCASH_LEGACY_ACCOUNT'",
// it is derived from the mnemonic.
auto account = libzcash::ParseHDKeypathAccount(
32, Params().BIP44CoinType(), m_wallet->mapSaplingZKeyMetadata[ivk].hdKeypath);
if (account.has_value() &&
account.value() == ZCASH_LEGACY_ACCOUNT && (
libzcash::Zip32AccountKeyPath(Params().BIP44CoinType(), ZCASH_LEGACY_ACCOUNT).size()
< m_wallet->mapSaplingZKeyMetadata[ivk].hdKeypath.size()
))
{
return PaymentAddressSource::MnemonicHDSeed;
} else {
return PaymentAddressSource::LegacyHDSeed;
}
} else if (m_wallet->HaveSaplingSpendingKeyForAddress(zaddr)) {
return PaymentAddressSource::Imported;
} else {