From 0951fe22d8301c342440e52eb030d239bdb74986 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris.nuttycombe@gmail.com>
Date: Fri, 29 Oct 2021 16:16:15 -0600
Subject: [PATCH] Apply suggestions from code review

Co-authored-by: str4d <jack@electriccoin.co>
---
 src/gtest/test_zip32.cpp  |  3 ---
 src/keystore.h            |  3 +++
 src/zcash/address/zip32.h | 20 ++++++++++----------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/gtest/test_zip32.cpp b/src/gtest/test_zip32.cpp
index a6fce09a9..3f06b341a 100644
--- a/src/gtest/test_zip32.cpp
+++ b/src/gtest/test_zip32.cpp
@@ -11,9 +11,6 @@ TEST(ZIP32, TestVectors) {
         0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16,
         17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31};
     HDSeed seed(rawSeed);
-    // TODO: Regenerate test vectors using a BIP-44-derived seed.
-    // std::string mnemonic("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art");
-    // HDSeed seed(English, mnemonic);
 
     auto m = libzcash::SaplingExtendedSpendingKey::Master(seed);
     EXPECT_EQ(m.depth, 0);
diff --git a/src/keystore.h b/src/keystore.h
index 7bad46943..78bf5c400 100644
--- a/src/keystore.h
+++ b/src/keystore.h
@@ -124,6 +124,9 @@ typedef std::map<libzcash::SaplingPaymentAddress, libzcash::SaplingIncomingViewi
 class CBasicKeyStore : public CKeyStore
 {
 protected:
+    // All wallets will have a mnemonic seed, but this field must be declared
+    // as optional to avoid the need to construct or hold an invalid seed before the
+    // wallet's contents have been loaded from the database.
     std::optional<MnemonicSeed> mnemonicSeed;
     std::optional<HDSeed> legacySeed;
     KeyMap mapKeys;
diff --git a/src/zcash/address/zip32.h b/src/zcash/address/zip32.h
index ce15a0b31..bcbb1a03c 100644
--- a/src/zcash/address/zip32.h
+++ b/src/zcash/address/zip32.h
@@ -83,7 +83,7 @@ public:
     /**
      * Randomly generate a new mnemonic seed. A SLIP-44 coin type is required to make it possible
      * to check that the generated seed can produce valid transparent and unified addresses at account
-     * numbers 0x7FFFFFFF and 0x0 respectively.
+     * numbers 0x7FFFFFFF and 0x00 respectively.
      */
     static MnemonicSeed Random(uint32_t bip44CoinType, Language language = English, size_t entropyLen = 32);
 
@@ -173,14 +173,14 @@ public:
     diversifier_index_t() {}
     diversifier_index_t(const base_blob<88>& b) : base_blob<88>(b) {}
     diversifier_index_t(uint64_t i): base_blob<88>() {
-        data[0] = (uint8_t) i;
-        data[1] = (uint8_t) (i >> 8);
-        data[2] = (uint8_t) (i >> 16);
-        data[3] = (uint8_t) (i >> 24);
-        data[4] = (uint8_t) (i >> 32);
-        data[5] = (uint8_t) (i >> 40);
-        data[6] = (uint8_t) (i >> 48);
-        data[7] = (uint8_t) (i >> 56);
+        data[0] = i & 0xFF;
+        data[1] = (i >> 8) & 0xFF;
+        data[2] = (i >> 16) & 0xFF;
+        data[3] = (i >> 24) & 0xFF;
+        data[4] = (i >> 32) & 0xFF;
+        data[5] = (i >> 40) & 0xFF;
+        data[6] = (i >> 48) & 0xFF;
+        data[7] = (i >> 56) & 0xFF;
     }
     explicit diversifier_index_t(const std::vector<unsigned char>& vch) : base_blob<88>(vch) {}
 
@@ -229,7 +229,7 @@ struct SaplingExtendedFullViewingKey {
 
     std::optional<SaplingExtendedFullViewingKey> Derive(uint32_t i) const;
 
-    // Attempt to construct a valid payment address with diversifier index
+    // Attempts to construct a valid payment address with diversifier index
     // `j`; returns std::nullopt if `j` does not result in a valid diversifier
     // given this xfvk.
     std::optional<libzcash::SaplingPaymentAddress> Address(diversifier_index_t j) const;