client: Update wallet's settings & password simultaneously

This adds the possibility to update wallet's settings/name & password simultaneously.
With this users which used an unencrypted wallet could switch to a different encrypted wallet.

client/core/core.go

@@ -1561,8 +1561,9 @@ func (c *Core) WalletSettings(assetID uint32) (map[string]string, error) {
 	return dbWallet.Settings, nil
 }
 
-// ReconfigureWallet updates the wallet configuration settings.
-func (c *Core) ReconfigureWallet(appPW []byte, assetID uint32, cfg map[string]string) error {
+// ReconfigureWallet updates the wallet configuration settings, it also updates
+// the password if newWalletPW is non-nil.
+func (c *Core) ReconfigureWallet(appPW, newWalletPW []byte, assetID uint32, cfg map[string]string) error {
 	crypter, err := c.encryptionKey(appPW)
 	if err != nil {
 		return newError(authErr, "ReconfigureWallet password error: %v", err)
@@ -1571,7 +1572,8 @@ func (c *Core) ReconfigureWallet(appPW []byte, assetID uint32, cfg map[string]st
 	defer c.walletMtx.Unlock()
 	oldWallet, found := c.wallets[assetID]
 	if !found {
-		return newError(missingWalletErr, "%d -> %s wallet not found", assetID, unbip(assetID))
+		return newError(missingWalletErr, "%d -> %s wallet not found",
+			assetID, unbip(assetID))
 	}
 	dbWallet := &db.Wallet{
 		AssetID:     oldWallet.AssetID,
@@ -1583,7 +1585,17 @@ func (c *Core) ReconfigureWallet(appPW []byte, assetID uint32, cfg map[string]st
 	// Reload the wallet with the new settings.
 	wallet, err := c.loadWallet(dbWallet)
 	if err != nil {
-		return newError(walletErr, "error loading wallet for %d -> %s: %v", assetID, unbip(assetID), err)
+		return newError(walletErr, "error loading wallet for %d -> %s: %v",
+			assetID, unbip(assetID), err)
+	}
+
+	isSettingNewPW := newWalletPW != nil // Includes empty but non-nil
+	// If newWalletPW is non-nil, update the wallet's password.
+	if isSettingNewPW {
+		err = c.setWalletPassword(wallet, newWalletPW, crypter)
+		if err != nil {
+			return err
+		}
 	}
 
 	// Must connect to ensure settings are good.
@@ -1592,10 +1604,11 @@ func (c *Core) ReconfigureWallet(appPW []byte, assetID uint32, cfg map[string]st
 		return err
 	}
 
-	// Carry over any cached password regardless of backend lock state.
-	// loadWallet already copied encPW, so this will decrypt pw rather than
-	// actually copying it, and it will ensure the backend is also unlocked.
-	if oldWallet.locallyUnlocked() {
+	// If the password was not changed, carry over any cached password
+	// regardless of backend lock state. loadWallet already copied encPW, so
+	// this will decrypt pw rather than actually copying it, and it will
+	// ensure the backend is also unlocked.
+	if !isSettingNewPW && oldWallet.locallyUnlocked() {
 		err := wallet.Unlock(crypter)
 		if err != nil {
 			wallet.Disconnect()
@@ -1642,8 +1655,10 @@ func (c *Core) ReconfigureWallet(appPW []byte, assetID uint32, cfg map[string]st
 	c.wallets[assetID] = wallet
 
 	c.notify(newBalanceNote(assetID, balances)) // redundant with wallet config note?
-	details := fmt.Sprintf("Configuration for %s wallet has been updated. Deposit address = %s", unbip(assetID), wallet.address)
-	c.notify(newWalletConfigNote(SubjectWalletConfigurationUpdated, details, db.Success, wallet.state()))
+	details := fmt.Sprintf("Configuration for %s wallet has been updated. Deposit address = %s",
+		unbip(assetID), wallet.address)
+	c.notify(newWalletConfigNote(SubjectWalletConfigurationUpdated,
+		details, db.Success, wallet.state()))
 
 	// Clear any existing tickGovernors for suspect matches.
 	c.connMtx.RLock()
@@ -1673,15 +1688,19 @@ func (c *Core) ReconfigureWallet(appPW []byte, assetID uint32, cfg map[string]st
 }
 
 // SetWalletPassword updates the (encrypted) password for the wallet.
+// Returns passwordErr if provided newPW is nil.
 func (c *Core) SetWalletPassword(appPW []byte, assetID uint32, newPW []byte) error {
+	// Ensure newPW isn't nil.
+	if newPW == nil {
+		return newError(passwordErr, "SetWalletPassword password can't be nil")
+	}
+
 	// Check the app password and get the crypter.
 	crypter, err := c.encryptionKey(appPW)
 	if err != nil {
 		return newError(authErr, "SetWalletPassword password error: %v", err)
 	}
 
-	newPasswordSet := len(newPW) > 0
-
 	// Check that the specified wallet exists.
 	c.walletMtx.Lock()
 	defer c.walletMtx.Unlock()
@@ -1690,50 +1709,66 @@ func (c *Core) SetWalletPassword(appPW []byte, assetID uint32, newPW []byte) err
 		return newError(missingWalletErr, "wallet for %s (%d) is not known", unbip(assetID), assetID)
 	}
 
+	// Set new password.
+	err = c.setWalletPassword(wallet, newPW, crypter)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// setWalletPassword updates the (encrypted) password for the wallet.
+func (c *Core) setWalletPassword(wallet *xcWallet, newPW []byte, crypter encrypt.Crypter) error {
 	// Connect if necessary.
 	wasConnected := wallet.connected()
 	if !wasConnected {
-		if err = c.connectAndUpdateWallet(wallet); err != nil {
+		if err := c.connectAndUpdateWallet(wallet); err != nil {
 			return newError(connectionErr, "SetWalletPassword connection error: %v", err)
 		}
 	}
 
+	wasUnlocked := wallet.unlocked()
+	newPasswordSet := len(newPW) > 0 // excludes empty but non-nil
+
 	// Check that the new password works. If the new password is empty, skip
 	// this step, since an empty password signifies an unencrypted wallet.
-	wasUnlocked := wallet.unlocked()
+	// TODO: find a way to verify that the wallet actually is unencrypted or
+	// otherwise does not require a password. Perhaps an
+	// asset.Wallet.RequiresPassword wallet method?
 	if newPasswordSet {
+		// Encrypt password if it's not an empty string
+		encNewPW, err := crypter.Encrypt(newPW)
+		if err != nil {
+			return newError(encryptionErr, "encryption error: %v", err)
+		}
 		err = wallet.Wallet.Unlock(string(newPW))
 		if err != nil {
-			return newError(authErr, "Error unlocking wallet. Is the new password correct?: %v", err)
+			return newError(authErr,
+				"setWalletPassword unlocking wallet error, is the new password correct?: %v", err)
 		}
+		wallet.encPW = encNewPW
+	} else {
+		wallet.encPW = nil
+	}
+
+	err := c.db.SetWalletPassword(wallet.dbID, wallet.encPW)
+	if err != nil {
+		return codedError(dbErr, err)
 	}
 
 	if !wasConnected {
 		wallet.Disconnect()
 	} else if !wasUnlocked {
 		if err = wallet.Lock(); err != nil {
-			c.log.Warnf("Unable to relock %s wallet: %v", unbip(assetID), err)
+			c.log.Warnf("Unable to relock %s wallet: %v", unbip(wallet.AssetID), err)
 		}
 	}
 
-	// Encrypt the password.
-	var encPW []byte
-	if newPasswordSet {
-		encPW, err = crypter.Encrypt(newPW)
-		if err != nil {
-			return newError(encryptionErr, "encryption error: %v", err)
-		}
-	}
-
-	err = c.db.SetWalletPassword(wallet.dbID, encPW)
-	if err != nil {
-		return codedError(dbErr, err)
-	}
-
-	wallet.encPW = encPW
-
-	details := fmt.Sprintf("Password for %s wallet has been updated.", unbip(assetID))
-	c.notify(newWalletConfigNote(SubjectWalletPasswordUpdated, details, db.Success, wallet.state()))
+	details := fmt.Sprintf("Password for %s wallet has been updated.",
+		unbip(wallet.AssetID))
+	c.notify(newWalletConfigNote(SubjectWalletPasswordUpdated, details,
+		db.Success, wallet.state()))
 
 	return nil
 }

client/core/core_test.go

@@ -4735,16 +4735,16 @@ func TestReconfigureWallet(t *testing.T) {
 	}
 	var assetID uint32 = 54321
 
-	// Password error
+	// App Password error
 	rig.crypter.recryptErr = tErr
-	err := tCore.ReconfigureWallet(tPW, assetID, newSettings)
+	err := tCore.ReconfigureWallet(tPW, nil, assetID, newSettings)
 	if !errorHasCode(err, authErr) {
 		t.Fatalf("wrong error for password error: %v", err)
 	}
 	rig.crypter.recryptErr = nil
 
 	// Missing wallet error
-	err = tCore.ReconfigureWallet(tPW, assetID, newSettings)
+	err = tCore.ReconfigureWallet(tPW, nil, assetID, newSettings)
 	if !errorHasCode(err, missingWalletErr) {
 		t.Fatalf("wrong error for missing wallet: %v", err)
 	}
@@ -4760,7 +4760,7 @@ func TestReconfigureWallet(t *testing.T) {
 
 	// Connect error
 	tXyzWallet.connectErr = tErr
-	err = tCore.ReconfigureWallet(tPW, assetID, newSettings)
+	err = tCore.ReconfigureWallet(tPW, nil, assetID, newSettings)
 	if !errorHasCode(err, connectWalletErr) {
 		t.Fatalf("wrong error when expecting connection error: %v", err)
 	}
@@ -4769,9 +4769,9 @@ func TestReconfigureWallet(t *testing.T) {
 	// Unlock error
 	tXyzWallet.Unlock(wPW)
 	tXyzWallet.unlockErr = tErr
-	err = tCore.ReconfigureWallet(tPW, assetID, newSettings)
+	err = tCore.ReconfigureWallet(tPW, nil, assetID, newSettings)
 	if !errorHasCode(err, walletAuthErr) {
-		t.Fatalf("wrong error when expecting connection error: %v", err)
+		t.Fatalf("wrong error when expecting auth error: %v", err)
 	}
 	tXyzWallet.unlockErr = nil
 
@@ -4798,8 +4798,8 @@ func TestReconfigureWallet(t *testing.T) {
 		},
 	}
 
-	// Success
-	err = tCore.ReconfigureWallet(tPW, assetID, newSettings)
+	// Success updating settings.
+	err = tCore.ReconfigureWallet(tPW, nil, assetID, newSettings)
 	if err != nil {
 		t.Fatalf("ReconfigureWallet error: %v", err)
 	}
@@ -4812,6 +4812,19 @@ func TestReconfigureWallet(t *testing.T) {
 	if match.tickGovernor != nil {
 		t.Fatalf("tickGovernor not removed")
 	}
+
+	// Success updating wallet PW.
+	newWalletPW := []byte("password")
+	err = tCore.ReconfigureWallet(tPW, newWalletPW, assetID, newSettings)
+	if err != nil {
+		t.Fatalf("ReconfigureWallet error: %v", err)
+	}
+
+	// Check that the xcWallet was updated.
+	xyzWallet = tCore.wallets[assetID]
+	if !bytes.Equal(xyzWallet.encPW, newWalletPW) {
+		t.Fatalf("xcWallet encPW field not updated want: %x got: %x", newWalletPW, xyzWallet.encPW)
+	}
 }
 
 func TestSetWalletPassword(t *testing.T) {
@@ -4823,11 +4836,17 @@ func TestSetWalletPassword(t *testing.T) {
 	newPW := []byte("def")
 	var assetID uint32 = 54321
 
-	// Password error
+	// Nil password error
+	err := tCore.SetWalletPassword(tPW, assetID, nil)
+	if !errorHasCode(err, passwordErr) {
+		t.Fatalf("wrong error for nil password error: %v", err)
+	}
+
+	// Auth error
 	rig.crypter.recryptErr = tErr
-	err := tCore.SetWalletPassword(tPW, assetID, newPW)
+	err = tCore.SetWalletPassword(tPW, assetID, newPW)
 	if !errorHasCode(err, authErr) {
-		t.Fatalf("wrong error for password error: %v", err)
+		t.Fatalf("wrong error for auth error: %v", err)
 	}
 	rig.crypter.recryptErr = nil
 

client/webserver/api.go

@@ -339,26 +339,6 @@ func (s *WebServer) apiWalletSettings(w http.ResponseWriter, r *http.Request) {
 	}, s.indent)
 }
 
-// apiSetWalletPass updates the current password for the specified wallet.
-func (s *WebServer) apiSetWalletPass(w http.ResponseWriter, r *http.Request) {
-	form := &struct {
-		AssetID uint32           `json:"assetID"`
-		NewPW   encode.PassBytes `json:"newPW"`
-		AppPW   encode.PassBytes `json:"appPW"`
-	}{}
-	defer form.NewPW.Clear()
-	defer form.AppPW.Clear()
-	if !readPost(w, r, form) {
-		return
-	}
-	err := s.core.SetWalletPassword(form.AppPW, form.AssetID, form.NewPW)
-	if err != nil {
-		s.writeAPIError(w, "password change error: %v", err)
-		return
-	}
-	writeJSON(w, simpleAck(), s.indent)
-}
-
 // apiDefaultWalletCfg attempts to load configuration settings from the
 // asset's default path on the server.
 func (s *WebServer) apiDefaultWalletCfg(w http.ResponseWriter, r *http.Request) {
@@ -429,17 +409,26 @@ func (s *WebServer) apiReconfig(w http.ResponseWriter, r *http.Request) {
 	form := &struct {
 		AssetID uint32            `json:"assetID"`
 		Config  map[string]string `json:"config"`
-		AppPW   encode.PassBytes  `json:"pw"`
+		// newWalletPW json field should be omitted in case caller isn't interested
+		// in setting new password, passing null JSON value will cause an unmarshal
+		// error.
+		NewWalletPW encode.PassBytes `json:"newWalletPW"`
+		AppPW       encode.PassBytes `json:"appPW"`
 	}{}
+	defer form.NewWalletPW.Clear()
 	defer form.AppPW.Clear()
 	if !readPost(w, r, form) {
 		return
 	}
-	err := s.core.ReconfigureWallet(form.AppPW, form.AssetID, form.Config)
+
+	// Update wallet settings.
+	err := s.core.ReconfigureWallet(form.AppPW, form.NewWalletPW, form.AssetID,
+		form.Config)
 	if err != nil {
 		s.writeAPIError(w, "reconfig error: %v", err)
 		return
 	}
+
 	writeJSON(w, simpleAck(), s.indent)
 }
 

client/webserver/live_test.go

@@ -907,7 +907,7 @@ func (c *TCore) Wallets() []*core.WalletState {
 func (c *TCore) WalletSettings(assetID uint32) (map[string]string, error) {
 	return c.wallets[assetID].settings, nil
 }
-func (c *TCore) ReconfigureWallet(pw []byte, assetID uint32, cfg map[string]string) error {
+func (c *TCore) ReconfigureWallet(aPW, nPW []byte, assetID uint32, cfg map[string]string) error {
 	c.wallets[assetID].settings = cfg
 	return nil
 }