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

Make `FindAddress` correctly check for maximum transparent child index. 

ZcashdUnifiedFullViewingKey::FindAddress was previously not checking
that the transparent child index did not overflow in unified address
generation. GenerateUnifiedAddress has been modified to search from
the last known diversifier index if no diversifier is specified, and
boundary conditions for transparent addresses are now correctly handled.
This commit is contained in:
Kris Nuttycombe 2021-12-30 09:53:55 -07:00
parent 5d07a8ae79
commit 16ba83ab1e
9 changed files with 160 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/gtest/test_keystore.cpp
View File

@ -533,7 +533,7 @@ TEST(KeystoreTests, StoreAndRetrieveUFVK) {
EXPECT_TRUE(keyStore.AddUnifiedFullViewingKey(zufvk));
EXPECT_EQ(keyStore.GetUnifiedFullViewingKey(ufvkid).value(), zufvk);
auto addrPair = zufvk.FindAddress(diversifier_index_t(0), {ReceiverType::Sapling});
auto addrPair = zufvk.FindAddress(diversifier_index_t(0), {ReceiverType::Sapling}).value();
EXPECT_TRUE(addrPair.first.GetSaplingReceiver().has_value());
auto saplingReceiver = addrPair.first.GetSaplingReceiver().value();
@ -557,7 +557,7 @@ TEST(KeystoreTests, AddUnifiedAddress) {
auto ufvk = usk.value().ToFullViewingKey();
auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(Params(), ufvk);
auto ufvkid = zufvk.GetKeyID();
auto addrPair = zufvk.FindAddress(diversifier_index_t(0), {ReceiverType::P2PKH, ReceiverType::Sapling});
auto addrPair = zufvk.FindAddress(diversifier_index_t(0), {ReceiverType::P2PKH, ReceiverType::Sapling}).value();
EXPECT_TRUE(addrPair.first.GetP2PKHReceiver().has_value());
keyStore.AddUnifiedAddress(ufvkid, addrPair.second, addrPair.first);

36
src/wallet/gtest/test_wallet.cpp
View File

@ -2170,7 +2170,7 @@ TEST(WalletTests, GenerateUnifiedAddress) {
(void) RegtestActivateSapling();
TestWallet wallet(Params());
UAGenerationResult uaResult = wallet.GenerateUnifiedAddress(0, diversifier_index_t(0), {ReceiverType::P2PKH, ReceiverType::Sapling});
UAGenerationResult uaResult = wallet.GenerateUnifiedAddress(0, {ReceiverType::P2PKH, ReceiverType::Sapling});
// If the wallet does not have a mnemonic seed available, it is
// treated as if the wallet is encrypted.
@ -2186,16 +2186,13 @@ TEST(WalletTests, GenerateUnifiedAddress) {
// If the user has not generated a unified spending key,
// we cannot create an address for the account corresponding
// to that spending key.
uaResult = wallet.GenerateUnifiedAddress(0, diversifier_index_t(0), {ReceiverType::P2PKH, ReceiverType::Sapling});
uaResult = wallet.GenerateUnifiedAddress(0, {ReceiverType::P2PKH, ReceiverType::Sapling});
expected = AddressGenerationError::NoSuchAccount;
EXPECT_EQ(uaResult, expected);
// Create an account, then generate an address for that account.
auto skpair = wallet.GenerateNewUnifiedSpendingKey();
uaResult = wallet.GenerateUnifiedAddress(
skpair.second,
diversifier_index_t(0),
{ReceiverType::P2PKH, ReceiverType::Sapling});
uaResult = wallet.GenerateUnifiedAddress(skpair.second, {ReceiverType::P2PKH, ReceiverType::Sapling});
auto ua = std::get_if<std::pair<libzcash::UnifiedAddress, libzcash::diversifier_index_t>>(&uaResult);
EXPECT_NE(ua, nullptr);
@ -2208,4 +2205,31 @@ TEST(WalletTests, GenerateUnifiedAddress) {
auto u4r = wallet.GetUnifiedForReceiver(ua->first.GetSaplingReceiver().value());
EXPECT_EQ(u4r, ua->first);
// Explicitly trigger the invalid transparent child index failure
uaResult = wallet.GenerateUnifiedAddress(
0,
{ReceiverType::P2PKH, ReceiverType::Sapling},
MAX_TRANSPARENT_CHILD_IDX.succ().value());
expected = AddressGenerationError::InvalidTransparentChildIndex;
EXPECT_EQ(uaResult, expected);
// Attempt to generate a UA at the maximum transparent child index. This might fail
// due to this index being invalid for Sapling; if so, it will return an error that
// the diversifier index is out of range. If it succeeds, we'll attempt to generate
// the next available diversifier, and this should always fail
uaResult = wallet.GenerateUnifiedAddress(
0,
{ReceiverType::P2PKH, ReceiverType::Sapling},
MAX_TRANSPARENT_CHILD_IDX);
ua = std::get_if<std::pair<libzcash::UnifiedAddress, libzcash::diversifier_index_t>>(&uaResult);
if (ua == nullptr) {
expected = AddressGenerationError::NoAddressForDiversifier;
EXPECT_EQ(uaResult, expected);
} else {
// the previous generation attempt succeeded, so this one should definitely fail.
uaResult = wallet.GenerateUnifiedAddress(0, {ReceiverType::P2PKH, ReceiverType::Sapling});
expected = AddressGenerationError::DiversifierSpaceExhausted;
EXPECT_EQ(uaResult, expected);
}
}

77
src/wallet/wallet.cpp
View File

@ -547,9 +547,10 @@ std::optional<ZcashdUnifiedFullViewingKey> CWallet::GetUnifiedFullViewingKeyByAc
UAGenerationResult CWallet::GenerateUnifiedAddress(
const libzcash::AccountId& accountId,
const libzcash::diversifier_index_t& j,
const std::set<libzcash::ReceiverType>& receiverTypes)
const std::set<libzcash::ReceiverType>& receiverTypes,
std::optional<libzcash::diversifier_index_t> j)
{
bool searchDiversifiers = !j.has_value();
if (!libzcash::HasShielded(receiverTypes)) {
return AddressGenerationError::InvalidReceiverTypes;
}
@ -561,7 +562,11 @@ UAGenerationResult CWallet::GenerateUnifiedAddress(
// ours rather than considering them as watch-only.
bool hasTransparent = receiverTypes.find(ReceiverType::P2PKH) != receiverTypes.end();
if (hasTransparent) {
if (!j.ToTransparentChildIndex().has_value()) {
// A preemptive check to ensure that the user has not specified an
// invalid transparent child index. If we search from a valid transparent
// child index into invalid child index space, later checks will return
// this error as `AddressGenerationError::DiversifierSpaceExhausted`
if (j.has_value() && !j.value().ToTransparentChildIndex().has_value()) {
return AddressGenerationError::InvalidTransparentChildIndex;
}
@ -574,30 +579,66 @@ UAGenerationResult CWallet::GenerateUnifiedAddress(
if (ufvk.has_value()) {
auto ufvkid = ufvk.value().GetKeyID();
// Check whether an address has already been generated for this
// diversifier index. If so, ensure that the set of receiver types
// being requested is the same as the set of receiver types that was
// previously generated & return the previously generated address,
// otherwise return an error.
// Check whether an address has already been generated for any provided
// diversifier index. Return that address, or set the diversifier index
// at which we'll begin searching for the next available diversified
// address.
auto metadata = mapUfvkAddressMetadata.find(ufvkid);
if (metadata != mapUfvkAddressMetadata.end()) {
auto receivers = metadata->second.GetReceivers(j);
if (receivers.has_value()) {
if (receivers.value() == receiverTypes) {
return std::make_pair(ufvk.value().Address(j, receiverTypes).value(), j);
} else {
return AddressGenerationError::ExistingAddressMismatch;
if (j.has_value()) {
auto receivers = metadata->second.GetReceivers(j.value());
if (receivers.has_value()) {
// Ensure that the set of receiver types being requested is
// the same as the set of receiver types that was previously
// generated. If they match, simply return that address.
if (receivers.value() == receiverTypes) {
return std::make_pair(ufvk.value().Address(j.value(), receiverTypes).value(), j.value());
} else {
return AddressGenerationError::ExistingAddressMismatch;
}
}
} else {
// Set the diversifier index to one greater than the last used
// diversifier
j = metadata->second.GetNextDiversifierIndex();
if (!j.has_value()) {
return AddressGenerationError::DiversifierSpaceExhausted;
}
}
} else {
// Begin searching from the zero diversifier index if we haven't
// yet generated an address from the specified UFVK and no
// diversifier index has been specified.
if (!j.has_value()) {
j = libzcash::diversifier_index_t(0);
}
}
// Find a working diversifier and construct the associated address.
auto foundAddress = ufvk.value().FindAddress(j, receiverTypes);
auto diversifierIndex = foundAddress.second;
// At this point, we know that `j` will contain a value. Optimistically
// attempt to find an address at that diversifier; we'll search if we
// don't find one.
auto diversifierIndex = j.value();
auto address = ufvk.value().Address(diversifierIndex, receiverTypes);
if (!address.has_value() && searchDiversifiers) {
auto found = ufvk.value().FindAddress(j.value(), receiverTypes);
if (found.has_value()) {
address = found.value().first;
diversifierIndex = found.value().second;
} else {
return AddressGenerationError::DiversifierSpaceExhausted;
}
}
// Now that we're done searching, check that we actually got an address
if (!address.has_value()) {
return AddressGenerationError::NoAddressForDiversifier;
}
// Persist the newly created address to the keystore
mapUfvkAddressMetadata[ufvkid].SetReceivers(diversifierIndex, receiverTypes);
CCryptoKeyStore::AddUnifiedAddress(ufvkid, diversifierIndex, foundAddress.first);
CCryptoKeyStore::AddUnifiedAddress(ufvkid, diversifierIndex, address.value());
// Save the metadata for the generated address so that we can re-derive
// it in the future.
@ -616,7 +657,7 @@ UAGenerationResult CWallet::GenerateUnifiedAddress(
AddTransparentSecretKey(seed.Fingerprint(), key);
}
return foundAddress;
return std::make_pair(address.value(), diversifierIndex);
} else {
return AddressGenerationError::NoSuchAccount;
}

30
src/wallet/wallet.h
View File

@ -406,7 +406,8 @@ enum class AddressGenerationError {
NoSuchAccount,
InvalidReceiverTypes,
ExistingAddressMismatch,
NoSaplingAddressForDiversifier,
NoAddressForDiversifier,
DiversifierSpaceExhausted,
WalletEncrypted,
InvalidTransparentChildIndex
};
@ -729,6 +730,26 @@ public:
return true;
}
}
/**
* Search the for the maximum diversifier that has already been used to
* generate a new address, and return the next diversifier. Returns the
* zero diversifier index if no addresses have yet been generated,
* and returns std::nullopt if the increment operation would cause an
* overflow.
*/
std::optional<libzcash::diversifier_index_t> GetNextDiversifierIndex() {
if (addressReceivers.empty()) {
return libzcash::diversifier_index_t(0);
} else {
auto lastIndex = addressReceivers.rbegin()->first;
if (lastIndex.increment()) {
return lastIndex;
} else {
return std::nullopt;
}
}
}
};
/**
@ -1189,10 +1210,13 @@ public:
//! Generate a new unified address for the specified account, diversifier, and
//! set of receiver types.
//!
//! If no diversifier index is provided, the next unused diversifier index
//! will be selected.
UAGenerationResult GenerateUnifiedAddress(
const libzcash::AccountId& accountId,
const libzcash::diversifier_index_t& j,
const std::set<libzcash::ReceiverType>& receivers);
const std::set<libzcash::ReceiverType>& receivers,
std::optional<libzcash::diversifier_index_t> j = std::nullopt);
bool AddUnifiedFullViewingKey(const libzcash::UnifiedFullViewingKey &ufvk);

1
src/zcash/Address.cpp
View File

@ -95,6 +95,7 @@ std::pair<std::string, PaymentAddress> AddressInfoFromViewingKey::operator()(con
"unified",
ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(keyConstants, ufvk)
.FindAddress(diversifier_index_t(0))
.value() //safe because we're searching from 0
.first
);
}

19
src/zcash/address/unified.cpp
View File

@ -14,12 +14,20 @@ using namespace libzcash;
bool libzcash::HasShielded(const std::set<ReceiverType>& receiverTypes) {
auto has_shielded = [](ReceiverType r) {
// TODO: update this as support for new protocols is added.
// TODO: update this as support for new shielded protocols is added.
return r == ReceiverType::Sapling;
};
return std::find_if(receiverTypes.begin(), receiverTypes.end(), has_shielded) != receiverTypes.end();
}
bool libzcash::HasTransparent(const std::set<ReceiverType>& receiverTypes) {
auto has_shielded = [](ReceiverType r) {
// TODO: update this as support for new transparent protocols is added.
return r == ReceiverType::P2PKH || r == ReceiverType::P2SH;
};
return std::find_if(receiverTypes.begin(), receiverTypes.end(), has_shielded) != receiverTypes.end();
}
std::optional<ZcashdUnifiedSpendingKey> ZcashdUnifiedSpendingKey::ForAccount(
const HDSeed& seed,
uint32_t bip44CoinType,
@ -108,20 +116,21 @@ std::optional<UnifiedAddress> ZcashdUnifiedFullViewingKey::Address(
return ua;
}
std::pair<UnifiedAddress, diversifier_index_t> ZcashdUnifiedFullViewingKey::FindAddress(
std::optional<std::pair<UnifiedAddress, diversifier_index_t>> ZcashdUnifiedFullViewingKey::FindAddress(
const diversifier_index_t& j,
const std::set<ReceiverType>& receiverTypes) const {
diversifier_index_t j0(j);
bool hasTransparent = HasTransparent(receiverTypes);
auto addr = Address(j0, receiverTypes);
while (!addr.has_value()) {
if (!j0.increment())
throw std::runtime_error(std::string(__func__) + ": diversifier index overflow.");;
if (!j0.increment() || (hasTransparent && !j0.ToTransparentChildIndex().has_value()))
return std::nullopt;
addr = Address(j0, receiverTypes);
}
return std::make_pair(addr.value(), j0);
}
std::pair<UnifiedAddress, diversifier_index_t> ZcashdUnifiedFullViewingKey::FindAddress(
std::optional<std::pair<UnifiedAddress, diversifier_index_t>> ZcashdUnifiedFullViewingKey::FindAddress(
const diversifier_index_t& j) const {
return FindAddress(j, {ReceiverType::P2PKH, ReceiverType::Sapling, ReceiverType::Orchard});
}

18
src/zcash/address/unified.h
View File

@ -24,6 +24,12 @@ enum class ReceiverType: uint32_t {
*/
bool HasShielded(const std::set<ReceiverType>& receiverTypes);
/**
* Test whether the specified list of receiver types contains a
* shielded receiver type
*/
bool HasTransparent(const std::set<ReceiverType>& receiverTypes);
class ZcashdUnifiedSpendingKey;
// prototypes for the classes handling ZIP-316 encoding (in Address.hpp)
@ -94,15 +100,21 @@ public:
* Find the smallest diversifier index >= `j` such that it generates a valid
* unified address according to the conditions specified in the documentation
* for the `Address` method above, and returns the newly created address along
* with the diversifier index used to produce it.
* with the diversifier index used to produce it. Returns `std::nullopt` if the
* diversifier space is exhausted, or if the set of receiver types contains a
* transparent receiver and the diversifier exceeds the maximum transparent
* child index.
*
* This method will throw if `receiverTypes` does not include a shielded receiver type.
*/
std::pair<UnifiedAddress, diversifier_index_t> FindAddress(
std::optional<std::pair<UnifiedAddress, diversifier_index_t>> FindAddress(
const diversifier_index_t& j,
const std::set<ReceiverType>& receiverTypes) const;
std::pair<UnifiedAddress, diversifier_index_t> FindAddress(const diversifier_index_t& j) const;
/**
* Find the next available address that contains all supported receiver types.
*/
std::optional<std::pair<UnifiedAddress, diversifier_index_t>> FindAddress(const diversifier_index_t& j) const;
friend bool operator==(const ZcashdUnifiedFullViewingKey& a, const ZcashdUnifiedFullViewingKey& b)
{

2
src/zcash/address/zip32.cpp
View File

@ -19,8 +19,6 @@ const unsigned char ZCASH_HD_SEED_FP_PERSONAL[BLAKE2bPersonalBytes] =
const unsigned char ZCASH_TADDR_OVK_PERSONAL[BLAKE2bPersonalBytes] =
{'Z', 'c', 'T', 'a', 'd', 'd', 'r', 'T', 'o', 'S', 'a', 'p', 'l', 'i', 'n', 'g'};
const libzcash::diversifier_index_t MAX_TRANSPARENT_CHILD_IDX(0x7FFFFFFF);
uint256 HDSeed::Fingerprint() const
{
CBLAKE2bWriter h(SER_GETHASH, 0, ZCASH_HD_SEED_FP_PERSONAL);

12
src/zcash/address/zip32.h
View File

@ -103,6 +103,15 @@ public:
return false; //overflow
}
std::optional<diversifier_index_t> succ() const {
diversifier_index_t next(*this);
if (next.increment()) {
return next;
} else {
return std::nullopt;
}
}
std::optional<uint32_t> ToTransparentChildIndex() const;
friend bool operator<(const diversifier_index_t& a, const diversifier_index_t& b) {
@ -118,6 +127,9 @@ public:
}
};
// The maximum allowed transparent child index according to BIP-44
const libzcash::diversifier_index_t MAX_TRANSPARENT_CHILD_IDX(0x7FFFFFFF);
class SaplingDiversifiableFullViewingKey {
public:
libzcash::SaplingFullViewingKey fvk;