From d711a7c5297f793217427acf9966d7ef9d4e84d8 Mon Sep 17 00:00:00 2001 From: sasha Date: Thu, 31 Mar 2022 11:12:57 -0700 Subject: [PATCH 1/3] Fix test_wallet_zkeys failures by increasing diversifier search offset 1 chance in 2^-128 is unobservable, whereas 1 chance in 1024 happens often. --- src/wallet/gtest/test_wallet_zkeys.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index a51ec83f9..5a2ce53da 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -67,9 +67,9 @@ TEST(WalletZkeysTest, StoreAndLoadSaplingZkeys) { EXPECT_EQ(1, addrs.count(address)); // 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 + // By starting our search at `128` we ensure there's no more than a 2^-128 chance that we // collide with the default diversifier. - libzcash::diversifier_index_t j(10); + libzcash::diversifier_index_t j(128); auto dpa = sk.ToXFVK().FindAddress(j).first; // add the default address @@ -461,11 +461,12 @@ TEST(WalletZkeysTest, WriteCryptedSaplingZkeyDirectToDb) { wallet.GetSaplingPaymentAddresses(addrs); ASSERT_EQ(1, addrs.size()); - // Generate a diversified address different to the default - // If we can't get an early diversified address, we are very unlucky + // Find a diversified address that does not use the same diversifier as the default address. + // By starting our search at `128` we ensure there's no more than a 2^-128 chance that we + // collide with the default diversifier. libzcash::SaplingExtendedSpendingKey extsk; EXPECT_TRUE(wallet.GetSaplingExtendedSpendingKey(address, extsk)); - libzcash::diversifier_index_t j(10); + libzcash::diversifier_index_t j(128); auto dpa = extsk.ToXFVK().FindAddress(j).first; // Add diversified address to the wallet From 3cf2ba8853bd198a142d03e846af46c905942c4f Mon Sep 17 00:00:00 2001 From: sasha Date: Fri, 1 Apr 2022 14:35:47 -0700 Subject: [PATCH 2/3] Fix sporadic failures in StoreAndRetrieveMnemonicSeedInEncryptedStore --- src/gtest/test_keystore.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index 342e274b5..64e3ea97d 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -359,15 +359,14 @@ TEST(KeystoreTests, StoreAndRetrieveMnemonicSeedInEncryptedStore) { seedOut = keyStore.GetMnemonicSeed(); EXPECT_FALSE(seedOut.has_value()); - // Unlocking with a random key should fail - CKeyingMaterial vRandomKey(32, 0); - GetRandBytes(vRandomKey.data(), 32); - EXPECT_FALSE(keyStore.Unlock(vRandomKey)); + // Unlocking with a random key causes sporadic failures, since we currently + // don't use an authenticated encryption scheme for CCryptoKeyStore. - // Unlocking with a slightly-modified vMasterKey should fail - CKeyingMaterial vModifiedKey(vMasterKey); - vModifiedKey[0] += 1; - EXPECT_FALSE(keyStore.Unlock(vModifiedKey)); + // Currently, DecryptMnemonicSeed tests if a key is invalid by looking at + // the return value of CBCDecrypt. If keyStore.Unlock is called with an + // invalid key, there's roughly a 257/65536 chance that the padding check + // in CBCDecrypt will pass, in which case DecryptMnemonicSeed then calls + // the deserialization code in mnemonic.h with random data. // Unlocking with vMasterKey should succeed ASSERT_TRUE(keyStore.Unlock(vMasterKey)); From d7b13560c50fcd55bb39ea9257b491f63fa2d9f9 Mon Sep 17 00:00:00 2001 From: sasha Date: Fri, 1 Apr 2022 15:46:10 -0700 Subject: [PATCH 3/3] Fix the exception message for SetSeedFromMnemonic failure It's possible for SetSeedFromMnemonic (a wrapper around zip339_phrase_to_seed) to fail for multiple reasons, including an invalid language code or seed string -- a message about non-UTF8ness is not correct. --- src/zcash/address/mnemonic.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zcash/address/mnemonic.h b/src/zcash/address/mnemonic.h index e93fd6fe4..3b5ed7ed3 100644 --- a/src/zcash/address/mnemonic.h +++ b/src/zcash/address/mnemonic.h @@ -79,7 +79,7 @@ public: READWRITE(mnemonic); language = (Language) language0; if (!SetSeedFromMnemonic()) { - throw std::ios_base::failure("Could not interpret the mnemonic phrase as a valid UTF-8 string."); + throw std::ios_base::failure("Invalid mnemonic phrase or language code."); } } else { uint32_t language0 = (uint32_t) language;