Apply suggestions from code review

Co-authored-by: str4d <jack@electriccoin.co>
This commit is contained in:
Kris Nuttycombe 2021-12-22 09:14:41 -07:00 committed by Kris Nuttycombe
parent 9b72fff365
commit 735ecd0906
5 changed files with 25 additions and 22 deletions

View File

@ -528,6 +528,7 @@ TEST(KeystoreTests, StoreAndRetrieveUFVK) {
auto zufvk = usk.value().ToFullViewingKey();
auto ufvk = UnifiedFullViewingKey::FromZcashdUFVK(zufvk);
auto ufvkid = ufvk.GetKeyID(Params());
EXPECT_FALSE(keyStore.GetUnifiedFullViewingKey(ufvkid).has_value());
EXPECT_TRUE(keyStore.AddUnifiedFullViewingKey(ufvkid, zufvk));
EXPECT_EQ(keyStore.GetUnifiedFullViewingKey(ufvkid).value(), zufvk);

View File

@ -364,7 +364,7 @@ extern "C" {
* Arguments:
* - dk: [c_uchar; 32] the byte representation of a Sapling diversifier key
* - addr: [c_uchar; 11] the bytes of the diversifier
* - j_ret: [c_uchar; 11] array that will store the retulgin diversifier index
* - j_ret: [c_uchar; 11] array that will store the resulting diversifier index
*/
bool librustzcash_sapling_diversifier_index(
const unsigned char *dk,

View File

@ -2180,6 +2180,8 @@ TEST(WalletTests, GenerateUnifiedAddress) {
// If the wallet does not have a mnemonic seed available, it is
// treated as if the wallet is encrypted.
EXPECT_FALSE(wallet.IsCrypted());
EXPECT_FALSE(wallet.GetMnemonicSeed().has_value());
UAGenerationResult expected = AddressGenerationError::WalletEncrypted;
EXPECT_EQ(uaResult, expected);

View File

@ -590,8 +590,9 @@ UAGenerationResult CWallet::GenerateUnifiedAddress(
// being requested is the same as the set of receiver types that was
// previously generated; if so, return the previously generated address,
// otherwise return an error.
if (mapUnifiedFullViewingKeyMeta.count(ufvkid) > 0) {
auto receivers = mapUnifiedFullViewingKeyMeta.at(ufvkid).GetReceivers(j);
auto metadata = mapUnifiedFullViewingKeyMeta.find(ufvkid);
if (metadata != mapUnifiedFullViewingKeyMeta.end()) {
auto receivers = metadata->second.GetReceivers(j);
if (receivers.has_value()) {
if (receivers.value() == receiverTypes) {
ZcashdUnifiedAddressMetadata addrmeta(ufvkid, j, receiverTypes);
@ -617,13 +618,13 @@ UAGenerationResult CWallet::GenerateUnifiedAddress(
// the wallet.
auto seed = GetMnemonicSeed().value();
auto b44 = libzcash::Bip44AccountChains::ForAccount(seed, BIP44CoinType(), accountId).value();
auto key = b44.DeriveExternal(j.ToTransparentChildIndex().value()).value();
auto key = b44.DeriveExternal(diversifierIndex.ToTransparentChildIndex().value()).value();
AddTransparentSecretKey(seed.Fingerprint(), key, ufvkid);
}
// Save the metadata for the generated address so that we can re-derive
// it in the future.
ZcashdUnifiedAddressMetadata addrmeta(ufvkid, foundAddress.second, receiverTypes);
ZcashdUnifiedAddressMetadata addrmeta(ufvkid, diversifierIndex, receiverTypes);
// we can safely ignore the return value here; we know that we're adding a new
// set of receivers given the receivers.has_value() check above.
@ -641,10 +642,11 @@ bool CWallet::LoadUnifiedFullViewingKey(const libzcash::UnifiedFullViewingKey &k
{
auto ufvkid = key.GetKeyID(Params());
auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(key);
if (mapUnifiedFullViewingKeyMeta.count(ufvkid) > 0) {
auto metadata = mapUnifiedFullViewingKeyMeta.find(ufvkid);
if (metadata != mapUnifiedFullViewingKeyMeta.end()) {
// restore unified addresses that have been previously generated to the
// keystore
for (const auto &[j, receiverTypes] : mapUnifiedFullViewingKeyMeta.at(ufvkid).GetAllReceivers()) {
for (const auto &[j, receiverTypes] : metadata->second.GetAllReceivers()) {
auto addr = zufvk.Address(j, receiverTypes).value();
AddUnifiedAddress(ufvkid, std::make_pair(addr, j));
}
@ -5820,12 +5822,13 @@ std::optional<libzcash::UnifiedAddress> LookupUnifiedAddress::operator()(const l
}
diversifier_index_t j;
if (wallet.mapUnifiedFullViewingKeyMeta.count(ufvkid) > 0 &&
auto metadata = wallet.mapUnifiedFullViewingKeyMeta.find(ufvkid);
if (metadata != wallet.mapUnifiedFullViewingKeyMeta.end()) {
librustzcash_sapling_diversifier_index(
ufvk.value().GetSaplingKey().value().dk.begin(),
saplingAddr.d.begin(),
j.begin())) {
auto receivers = wallet.mapUnifiedFullViewingKeyMeta.at(ufvkid).GetReceivers(j);
j.begin());
auto receivers = metadata->second.GetReceivers(j);
if (receivers.has_value()) {
return ufvk.value().Address(j, receivers.value());
} else {
@ -5859,8 +5862,9 @@ std::optional<libzcash::UnifiedAddress> LookupUnifiedAddress::operator()(const C
// Find the set of receivers at the diversifier index. If no metadata is available
// for the ufvk, or we do not know the receiver types for the address produced
// at this diversifier, we cannot reconstruct the address.
if (wallet.mapUnifiedFullViewingKeyMeta.count(ufvkid) > 0) {
auto receivers = wallet.mapUnifiedFullViewingKeyMeta.at(ufvkid).GetReceivers(j);
auto metadata = wallet.mapUnifiedFullViewingKeyMeta.find(ufvkid);
if (metadata != wallet.mapUnifiedFullViewingKeyMeta.end()) {
auto receivers = metadata->second.GetReceivers(j);
if (receivers.has_value()) {
return ufvk.value().Address(j, receivers.value());
} else {

View File

@ -696,8 +696,9 @@ public:
std::optional<std::set<libzcash::ReceiverType>> GetReceivers(
const libzcash::diversifier_index_t& j) const {
if (addressReceivers.count(j) > 0) {
return addressReceivers.at(j);
auto receivers = addressReceivers.find(j);
if (receivers != addressReceivers.end()) {
return receivers->second;
} else {
return std::nullopt;
}
@ -706,12 +707,7 @@ public:
bool SetReceivers(
const libzcash::diversifier_index_t& j,
const std::set<libzcash::ReceiverType>& receivers) {
if (addressReceivers.count(j) > 0) {
return false;
} else {
addressReceivers[j] = receivers;
return true;
}
return addressReceivers.insert(std::make_pair(j, receivers)).second;
}
bool SetAccountId(libzcash::AccountId accountIdIn) {
@ -753,7 +749,7 @@ private:
bool fBroadcastTransactions;
/**
* A map from protocol-specifiec transaction output identifier to
* A map from a protocol-specific transaction output identifier to
* a txid.
*/
template <class T>
@ -859,7 +855,7 @@ private:
void SyncMetaData(std::pair<typename TxSpendMap<T>::iterator, typename TxSpendMap<T>::iterator>);
void ChainTipAdded(const CBlockIndex *pindex, const CBlock *pblock, SproutMerkleTree sproutTree, SaplingMerkleTree saplingTree);
/* Add an extended secret key to the wallet. Internal use only. */
/* Add a transparent secret key to the wallet. Internal use only. */
CPubKey AddTransparentSecretKey(
const uint256& seedFingerprint,
const std::pair<CKey, HDKeyPath>& extSecret,