Merge pull request #5807 from superbaud/fix-btest-failures

fix sporadic failures in WriteCryptedSaplingZkeyDirectToDb, StoreAndLoadSaplingZkeys, and in StoreAndRetrieveMnemonicSeedInEncryptedStore

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));

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

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;