From 0c38ad920040312e5bcd25e2b4e389e5b005255f Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@nutty.land>
Date: Fri, 11 Mar 2022 18:47:01 -0700
Subject: [PATCH] Return std::optional<CExtKey> from CExtKey::Master

Master key generation is fallible, but the type of master
key generation did not previously reflect that the generated
key might be invalid.
---
 src/key.cpp                       | 8 ++++++--
 src/key.h                         | 2 +-
 src/test/bip32_tests.cpp          | 2 +-
 src/zcash/address/transparent.cpp | 3 ++-
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/key.cpp b/src/key.cpp
index dfbd3019c..cb4aa8343 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -290,7 +290,7 @@ std::optional<CExtKey> CExtKey::Derive(unsigned int _nChild) const {
     }
 }
 
-CExtKey CExtKey::Master(const unsigned char *seed, unsigned int nSeedLen) {
+std::optional<CExtKey> CExtKey::Master(const unsigned char *seed, unsigned int nSeedLen) {
     CExtKey xk;
     static const unsigned char hashkey[] = {'B','i','t','c','o','i','n',' ','s','e','e','d'};
     std::vector<unsigned char, secure_allocator<unsigned char>> vout(64);
@@ -301,7 +301,11 @@ CExtKey CExtKey::Master(const unsigned char *seed, unsigned int nSeedLen) {
     xk.nChild = 0;
     memset(xk.vchFingerprint, 0, sizeof(xk.vchFingerprint));
 
-    return xk;
+    if (xk.key.IsValid()) {
+        return xk;
+    } else {
+        return std::nullopt;
+    }
 }
 
 CExtPubKey CExtKey::Neuter() const {
diff --git a/src/key.h b/src/key.h
index d5354d61f..0122ec9fa 100644
--- a/src/key.h
+++ b/src/key.h
@@ -166,7 +166,7 @@ struct CExtKey {
             a.key == b.key;
     }
 
-    static CExtKey Master(const unsigned char* seed, unsigned int nSeedLen);
+    static std::optional<CExtKey> Master(const unsigned char* seed, unsigned int nSeedLen);
 
     void Encode(unsigned char code[BIP32_EXTKEY_SIZE]) const;
     void Decode(const unsigned char code[BIP32_EXTKEY_SIZE]);
diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp
index 64e9b87ab..2a78dc9d6 100644
--- a/src/test/bip32_tests.cpp
+++ b/src/test/bip32_tests.cpp
@@ -80,7 +80,7 @@ TestVector test2 =
 
 void RunTest(const TestVector &test) {
     std::vector<unsigned char> seed = ParseHex(test.strHexMaster);
-    CExtKey key = CExtKey::Master(&seed[0], seed.size());
+    CExtKey key = CExtKey::Master(&seed[0], seed.size()).value();
     CExtPubKey pubkey = key.Neuter();
     KeyIO keyIO(Params());
     for (const TestDerivation &derive : test.vDerive) {
diff --git a/src/zcash/address/transparent.cpp b/src/zcash/address/transparent.cpp
index c1a4d9de2..824debfe8 100644
--- a/src/zcash/address/transparent.cpp
+++ b/src/zcash/address/transparent.cpp
@@ -74,10 +74,11 @@ std::optional<AccountKey> AccountKey::ForAccount(
         AccountId accountId) {
     auto rawSeed = seed.RawSeed();
     auto m = CExtKey::Master(rawSeed.data(), rawSeed.size());
+    if (!m.has_value()) return std::nullopt;
 
     // We use a fixed keypath scheme of m/44'/coin_type'/account'
     // Derive m/44'
-    auto m_44h = m.Derive(44 | HARDENED_KEY_LIMIT);
+    auto m_44h = m.value().Derive(44 | HARDENED_KEY_LIMIT);
     if (!m_44h.has_value()) return std::nullopt;
 
     // Derive m/44'/coin_type'