From b37dc4e22fa2702af53e71082620cfe0b3d63ce4 Mon Sep 17 00:00:00 2001
From: Eirik Ogilvie-Wigley <eirik@z.cash>
Date: Fri, 14 Sep 2018 18:36:24 -0600
Subject: [PATCH] Don't export empty zip32 metadata

---
 qa/rpc-tests/wallet_import_export.py | 23 ++++++++++++++++-------
 src/wallet/rpcdump.cpp               | 10 ++++++++--
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/qa/rpc-tests/wallet_import_export.py b/qa/rpc-tests/wallet_import_export.py
index 8169e36ba..ee9cc42ce 100755
--- a/qa/rpc-tests/wallet_import_export.py
+++ b/qa/rpc-tests/wallet_import_export.py
@@ -8,10 +8,15 @@ from test_framework.util import assert_equal, assert_true, start_nodes
 
 class WalletImportExportTest (BitcoinTestFramework):
     def setup_network(self, split=False):
-        extra_args = [["-exportdir={}/export{}".format(self.options.tmpdir, i)] for i in range(2)]
-        self.nodes = start_nodes(2, self.options.tmpdir, extra_args)
+        num_nodes = 3
+        extra_args = [["-exportdir={}/export{}".format(self.options.tmpdir, i)] for i in range(num_nodes)]
+        self.nodes = start_nodes(num_nodes, self.options.tmpdir, extra_args)
 
     def run_test(self):
+        sapling_address2 = self.nodes[2].z_getnewaddress('sapling')
+        privkey2 = self.nodes[2].z_exportkey(sapling_address2)
+        self.nodes[0].z_importkey(privkey2)
+
         sprout_address0 = self.nodes[0].z_getnewaddress('sprout')
         sapling_address0 = self.nodes[0].z_getnewaddress('sapling')
 
@@ -19,11 +24,14 @@ class WalletImportExportTest (BitcoinTestFramework):
         dump_path0 = self.nodes[0].z_exportwallet('walletdump')
         (t_keys0, sprout_keys0, sapling_keys0) = parse_wallet_file(dump_path0)
 
-        for sapling_key0 in sapling_keys0.splitlines():
-            assert_equal(4, len(sapling_key0.split(' #')[0].split()), "Should have 4 parameters before ' #'")
-        
+        sapling_line_lengths = [len(sapling_key0.split(' #')[0].split()) for sapling_key0 in sapling_keys0.splitlines()]
+        assert_equal(2, len(sapling_line_lengths), "Should have 2 sapling keys")
+        assert_true(2 in sapling_line_lengths, "Should have a key with 2 parameters")
+        assert_true(4 in sapling_line_lengths, "Should have a key with 4 parameters")
+
         assert_true(sprout_address0 in sprout_keys0)
         assert_true(sapling_address0 in sapling_keys0)
+        assert_true(sapling_address2 in sapling_keys0)
 
         # node 1 should not have the keys
         dump_path1 = self.nodes[1].z_exportwallet('walletdumpbefore')
@@ -41,6 +49,7 @@ class WalletImportExportTest (BitcoinTestFramework):
         
         assert_true(sprout_address0 in sprout_keys1)
         assert_true(sapling_address0 in sapling_keys1)
+        assert_true(sapling_address2 in sapling_keys1)
 
         # make sure we have perserved the metadata
         for sapling_key0 in sapling_keys0.splitlines():
@@ -49,7 +58,7 @@ class WalletImportExportTest (BitcoinTestFramework):
 # Helper functions
 def parse_wallet_file(dump_path):
     file_lines = open(dump_path, "r").readlines()
-    # WE expect information about the HDSeed and fingerpring in the header
+    # We expect information about the HDSeed and fingerpring in the header
     assert_true("HDSeed" in file_lines[4], "Expected HDSeed")
     assert_true("fingerprint" in file_lines[4], "Expected fingerprint")
     (t_keys, i) = parse_wallet_file_lines(file_lines, 0)
@@ -67,7 +76,7 @@ def parse_wallet_file_lines(file_lines, i):
     while  i < len(file_lines) and not (file_lines[i] == '\n' or file_lines[i].startswith("#")):
         keys.append(file_lines[i])
         i += 1
-    return ("\n".join(keys), i)
+    return ("".join(keys), i)
 
 if __name__ == '__main__':
     WalletImportExportTest().main()
\ No newline at end of file
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index e6bd0fe3e..81e2969d5 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -298,7 +298,8 @@ UniValue importwallet_impl(const UniValue& params, bool fHelp, bool fImportZKeys
         if (fImportZKeys) {
             auto spendingkey = DecodeSpendingKey(vstr[0]);
             int64_t nTime = DecodeDumpTime(vstr[1]);
-            boost::optional<std::string> hdKeypath = (vstr.size() > 2) ? boost::optional<std::string>(vstr[2]) : boost::none;
+            // Only include hdKeypath and seedFpStr if we have both
+            boost::optional<std::string> hdKeypath = (vstr.size() > 3) ? boost::optional<std::string>(vstr[2]) : boost::none;
             boost::optional<std::string> seedFpStr = (vstr.size() > 3) ? boost::optional<std::string>(vstr[3]) : boost::none;
             if (IsValidSpendingKey(spendingkey)) {
                 auto addResult = boost::apply_visitor(
@@ -552,7 +553,12 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys)
                 auto ivk = extsk.expsk.full_viewing_key().in_viewing_key();
                 CKeyMetadata keyMeta = pwalletMain->mapSaplingZKeyMetadata[ivk];
                 std::string strTime = EncodeDumpTime(keyMeta.nCreateTime);
-                file << strprintf("%s %s %s %s # zaddr=%s\n", EncodeSpendingKey(extsk), strTime, keyMeta.hdKeypath, keyMeta.seedFp.GetHex(), EncodePaymentAddress(addr));
+                // Keys imported with z_importkey do not have zip32 metadata
+                if (keyMeta.hdKeypath.empty() || keyMeta.seedFp.IsNull()) {
+                    file << strprintf("%s %s # zaddr=%s\n", EncodeSpendingKey(extsk), strTime, EncodePaymentAddress(addr));
+                } else {
+                    file << strprintf("%s %s %s %s # zaddr=%s\n", EncodeSpendingKey(extsk), strTime, keyMeta.hdKeypath, keyMeta.seedFp.GetHex(), EncodePaymentAddress(addr));
+                }
             }
         }
         file << "\n";