Resolved merge conflict by incorporating both suggestions.

bip39bug.md

@@ -0,0 +1,13 @@
+## Zecwallet-cli BIP39 derivation bug
+
+In v1.0 of zecwallet-cli, there was a bug that incorrectly derived HD wallet keys after the first key. That is, the first key, address was correct, but subsequent ones were not. 
+
+The issue was that the 32-byte seed was directly being used to derive then subsequent addresses instead of the 64-byte pkdf2(seed). The issue affected both t and z addresses.
+
+Note that no funds are at risk. The issue is that, if in the future, you import the seed into a different wallet, you might not see all your addresses in the new wallet, so it's better to fix it now. 
+
+## Fix
+If you start a wallet that has this bug, you'll be notified. 
+The bug can be automatically fixed by the wallet by running the `fixbip39bug` command. Just start `zecwallet-cli` and type `fixbip39bug`.
+
+If you have any funds in the incorrect addresses, they'll be sent to yourself, and the correct addresses re-derived. 

lib/src/commands.rs

@@ -196,47 +196,9 @@ impl Command for ExportCommand {
         }
 
         let address = if args.is_empty() { None } else { Some(args[0].to_string()) };
-
-        format!("{}", lightclient.do_export(address).pretty(2))
-    }
-}
-
-struct EncryptCommand {}
-impl Command for EncryptCommand {
-    fn help(&self) -> String {
-        let mut h = vec![];
-        h.push("Encrypt the wallet with a password");
-        h.push("Note 1: This will encrypt the seed and the sapling and transparent private keys.");
-        h.push("        Use 'unlock' to temporarily unlock the wallet for spending or 'decrypt' ");
-        h.push("        to permanatly remove the encryption");
-        h.push("Note 2: If you forget the password, the only way to recover the wallet is to restore");
-        h.push("        from the seed phrase.");
-        h.push("Usage:");
-        h.push("encrypt password");
-        h.push("");
-        h.push("Example:");
-        h.push("encrypt my_strong_password");
-
-        h.join("\n")
-    }
-
-    fn short_help(&self) -> String {
-        "Encrypt the wallet with a password".to_string()
-    }
-
-    fn exec(&self, args: &[&str], lightclient: &LightClient) -> String {
-        if args.len() != 1 {
-            return self.help();
-        }
-
-        let passwd = args[0].to_string();
-
-        match lightclient.wallet.write().unwrap().encrypt(passwd) {
-            Ok(_)  => object!{ "result" => "success" },
-            Err(e) => object!{
-                "result" => "error",
-                "error"  => e.to_string()
-            }
+        match lightclient.do_export(address) {
+            Ok(j)  => j,
+            Err(e) => object!{ "error" => e }
         }.pretty(2)
     }
 }
@@ -347,11 +309,11 @@ impl Command for SendCommand {
         // 2 - A single argument in the form of a JSON string that is "[{address: address, value: value, memo: memo},...]"
 
         // 1 - Destination address. T or Z address
-         if args.len() < 1 || args.len() > 3 {
+        if args.len() < 1 || args.len() > 3 {
             return self.help();
         }
 
-     // Check for a single argument that can be parsed as JSON
+        // Check for a single argument that can be parsed as JSON
         if args.len() == 1 {
             // Sometimes on the command line, people use "'" for the quotes, which json::parse doesn't
             // understand. So replace it with double-quotes
@@ -365,11 +327,11 @@ impl Command for SendCommand {
                 }
             };
 
-         if !json_args.is_array() {
+            if !json_args.is_array() {
                 return format!("Couldn't parse argument as array\n{}", self.help());
             }
 
-                    let maybe_send_args = json_args.members().map( |j| {
+            let maybe_send_args = json_args.members().map( |j| {
                 if !j.has_key("address") || !j.has_key("amount") {
                     Err(format!("Need 'address' and 'amount'\n"))
                 } else {
@@ -397,7 +359,7 @@ impl Command for SendCommand {
             };
 
             let memo = if args.len() == 3 { Some(args[2].to_string()) } else {None};
-
+            
             lightclient.do_sync(true);
             match lightclient.do_send(vec!((args[0], value, memo))) {
                 Ok(txid) => { object!{ "txid" => txid } },
@@ -428,7 +390,7 @@ impl Command for SaveCommand {
     }
 
     fn exec(&self, _args: &[&str], lightclient: &LightClient) -> String {
-                match lightclient.do_save() {
+        match lightclient.do_save() {
             Ok(_) => {
                 let r = object!{ "result" => "success" };
                 r.pretty(2)
@@ -462,7 +424,10 @@ impl Command for SeedCommand {
     }
 
     fn exec(&self, _args: &[&str], lightclient: &LightClient) -> String {
-        format!("{}", lightclient.do_seed_phrase().pretty(2))
+        match lightclient.do_seed_phrase() {
+            Ok(j)  => j,
+            Err(e) => object!{ "error" => e }
+        }.pretty(2)
     }
 }
 
@@ -537,7 +502,10 @@ impl Command for NewAddressCommand {
             return format!("No address type specified\n{}", self.help());
         }
 
-        format!("{}", lightclient.do_new_address(args[0]).pretty(2))
+        match lightclient.do_new_address(args[0]) {
+            Ok(j)  => j,
+            Err(e) => object!{ "error" => e }
+        }.pretty(2)
     }
 }
 
@@ -620,7 +588,7 @@ impl Command for QuitCommand {
     }
 
     fn exec(&self, _args: &[&str], lightclient: &LightClient) -> String {
-                match lightclient.do_save() {
+        match lightclient.do_save() {
             Ok(_) => {"".to_string()},
             Err(e) => e
         }

lib/src/grpcconnector.rs

@@ -284,7 +284,7 @@ pub fn broadcast_raw_tx(uri: &http::Uri, no_cert: bool, tx_bytes: Box<[u8]>) ->
                             txid = txid[1..txid.len()-1].to_string();
                         }
 
-                        Ok(txid)  
+                        Ok(txid)
                     } else {
                         Err(format!("Error: {:?}", sendresponse))
                     }

lib/src/lightclient.rs

@@ -168,7 +168,8 @@ impl LightClientConfig {
     pub fn base58_pubkey_address(&self) -> [u8; 1] {
         match &self.chain_name[..] {
             "main"    => mainnet::B58_PUBKEY_ADDRESS_PREFIX,
-            
+        
+
             c         => panic!("Unknown chain {}", c)
         }
     }
@@ -177,7 +178,8 @@ impl LightClientConfig {
     pub fn base58_script_address(&self) -> [u8; 1] {
         match &self.chain_name[..] {
             "main"    => mainnet::B58_SCRIPT_ADDRESS_PREFIX,
-           
+            
+
             c         => panic!("Unknown chain {}", c)
         }
     }
@@ -222,6 +224,26 @@ impl LightClient {
 
     }
 
+    /// Method to create a test-only version of the LightClient
+    #[allow(dead_code)]
+    fn unconnected(seed_phrase: String) -> io::Result<Self> {
+        let config = LightClientConfig::create_unconnected("test".to_string());
+        let mut l = LightClient {
+                wallet          : Arc::new(RwLock::new(LightWallet::new(Some(seed_phrase), &config, 0)?)),
+                config          : config.clone(),
+                sapling_output  : vec![], 
+                sapling_spend   : vec![]
+            };
+
+        l.set_wallet_initial_state();
+        l.read_sapling_params();
+
+        info!("Created new wallet!");
+        info!("Created LightClient to {}", &config.server);
+
+        Ok(l)
+    }
+
     pub fn new_from_phrase(seed_phrase: String, config: &LightClientConfig, latest_block: u64) -> io::Result<Self> {
         if config.get_wallet_path().exists() {
             return Err(Error::new(ErrorKind::AlreadyExists,
@@ -279,12 +301,10 @@ impl LightClient {
     }
 
     // Export private keys
-    pub fn do_export(&self, addr: Option<String>) -> JsonValue {
+    pub fn do_export(&self, addr: Option<String>) -> Result<JsonValue, &str> {
         if !self.wallet.read().unwrap().is_unlocked_for_spending() {
             error!("Wallet is locked");
-            return object!{
-                "error" => "Wallet is locked"
-            };
+            return Err("Wallet is locked");
         }
 
         // Clone address so it can be moved into the closure
@@ -317,11 +337,12 @@ impl LightClient {
         all_keys.extend_from_slice(&z_keys);
         all_keys.extend_from_slice(&t_keys);
 
-        all_keys.into()
+        Ok(all_keys.into())
     }
 
     pub fn do_address(&self) -> JsonValue {
         let wallet = self.wallet.read().unwrap();
+
         // Collect z addresses
         let z_addresses = wallet.zaddress.read().unwrap().iter().map( |ad| {
             encode_payment_address(self.config.hrp_sapling_address(), &ad)
@@ -370,8 +391,7 @@ impl LightClient {
         }
     }
 
-    pub fn do_save(&self) -> Result<(), String> {
-        
+    pub fn do_save(&self) -> Result<(), String> {        
         // If the wallet is encrypted but unlocked, lock it again.
         {
             let mut wallet = self.wallet.write().unwrap();
@@ -423,19 +443,17 @@ impl LightClient {
         }
     }
 
-    pub fn do_seed_phrase(&self) -> JsonValue {
+    pub fn do_seed_phrase(&self) -> Result<JsonValue, &str> {
         if !self.wallet.read().unwrap().is_unlocked_for_spending() {
             error!("Wallet is locked");
-            return object!{
-                "error" => "Wallet is locked"
-            };
+            return Err("Wallet is locked");
         }
 
         let wallet = self.wallet.read().unwrap();
-        object!{
+        Ok(object!{
             "seed"     => wallet.get_seed_phrase(),
             "birthday" => wallet.get_birthday()
-        }
+        })
     }
 
     // Return a list of all notes, spent and unspent
@@ -610,12 +628,10 @@ impl LightClient {
     }
 
     /// Create a new address, deriving it from the seed.
-    pub fn do_new_address(&self, addr_type: &str) -> JsonValue {
+    pub fn do_new_address(&self, addr_type: &str) -> Result<JsonValue, String> {
         if !self.wallet.read().unwrap().is_unlocked_for_spending() {
             error!("Wallet is locked");
-            return object!{
-                "error" => "Wallet is locked"
-            };
+            return Err("Wallet is locked".to_string());
         }
 
         let wallet = self.wallet.write().unwrap();
@@ -626,13 +642,11 @@ impl LightClient {
             _   => {
                 let e = format!("Unrecognized address type: {}", addr_type);
                 error!("{}", e);
-                return object!{
-                    "error" => e
-                };
+                return Err(e);
             }
         };
 
-        array![new_address]
+        Ok(array![new_address])
     }
 
     pub fn do_rescan(&self) -> String {
@@ -872,3 +886,61 @@ impl LightClient {
         }
     }
 }
+
+
+pub mod tests {
+    use lazy_static::lazy_static;
+    //use super::LightClient;
+
+    lazy_static!{
+        static ref TEST_SEED: String = "youth strong sweet gorilla hammer unhappy congress stamp left stereo riot salute road tag clean toilet artefact fork certain leopard entire civil degree wonder".to_string();
+    }
+
+    #[test]
+    pub fn test_encrypt_decrypt() {
+        let lc = super::LightClient::unconnected(TEST_SEED.to_string()).unwrap();
+
+        assert!(!lc.do_export(None).is_err());
+        assert!(!lc.do_new_address("z").is_err());
+        assert!(!lc.do_new_address("t").is_err());
+        assert_eq!(lc.do_seed_phrase().unwrap()["seed"], TEST_SEED.to_string());
+
+        // Encrypt and Lock the wallet
+        lc.wallet.write().unwrap().encrypt("password".to_string()).unwrap();
+        assert!(lc.do_export(None).is_err());
+        assert!(lc.do_seed_phrase().is_err());
+        assert!(lc.do_new_address("t").is_err());
+        assert!(lc.do_new_address("z").is_err());
+        assert!(lc.do_send(vec![("z", 0, None)]).is_err());
+
+        // Do a unlock, and make sure it all works now
+        lc.wallet.write().unwrap().unlock("password".to_string()).unwrap();
+        assert!(!lc.do_export(None).is_err());
+        assert!(!lc.do_seed_phrase().is_err());
+        assert!(!lc.do_new_address("t").is_err());
+        assert!(!lc.do_new_address("z").is_err());
+    }
+
+    #[test]
+    pub fn test_addresses() {
+        let lc = super::LightClient::unconnected(TEST_SEED.to_string()).unwrap();
+
+        // Add new z and t addresses
+            
+        let taddr1 = lc.do_new_address("t").unwrap()[0].as_str().unwrap().to_string();
+        let taddr2 = lc.do_new_address("t").unwrap()[0].as_str().unwrap().to_string();        
+        let zaddr1 = lc.do_new_address("z").unwrap()[0].as_str().unwrap().to_string();
+        let zaddr2 = lc.do_new_address("z").unwrap()[0].as_str().unwrap().to_string();
+        
+        let addresses = lc.do_address();
+        assert_eq!(addresses["z_addresses"].len(), 3);
+        assert_eq!(addresses["z_addresses"][1], zaddr1);
+        assert_eq!(addresses["z_addresses"][2], zaddr2);
+
+        assert_eq!(addresses["t_addresses"].len(), 3);
+        assert_eq!(addresses["t_addresses"][1], taddr1);
+        assert_eq!(addresses["t_addresses"][2], taddr2);
+    }
+
+
+}

lib/src/lightwallet/bugs.rs

@@ -70,6 +70,7 @@ impl BugBip39Derivation {
         
         // Tranfer money
         // 1. The desination is z address #0
+        println!("Sending funds to ourself.");
         let zaddr = client.do_address()["z_addresses"][0].as_str().unwrap().to_string();
         let balance_json = client.do_balance();
         let fee: u64 = DEFAULT_FEE.try_into().unwrap();

lib/src/startup_helpers.rs

@@ -17,4 +17,4 @@ pub fn report_permission_error() {
                   user,
                   home);
     }
-}
+}

src/main.rs

@@ -160,7 +160,7 @@ fn startup(server: http::Uri, dangerous: bool, seed: Option<String>, first_sync:
         std::io::Error::new(ErrorKind::Other, e)
     })?;
 
-      let lightclient = match seed {
+    let lightclient = match seed {
         Some(phrase) => Arc::new(LightClient::new_from_phrase(phrase, &config, latest_block_height)?),
         None => Arc::new(LightClient::read_from_disk(&config)?)
     };