client/asset/btc: work with unprotected Electrum wallets

This also fixes the ability to work work with unprotected Electrum
wallets, which have no password. This is handled by tagging the
"password" JSON fields "omitempty", otherwise a cryptic "incorrect
padding" error is returned from Electrum.

client/asset/btc/electrum/wallet_methods.go

@@ -28,14 +28,14 @@ const (
 	methodGetUnusedAddress = "getunusedaddress"
 	methodGetTransaction   = "gettransaction"
 	methodListUnspent      = "listunspent"
-	methodGetPrivateKeys   = "getprivatekeys"
-	methodPayTo            = "payto"
+	methodGetPrivateKeys   = "getprivatekeys" // requires password for protected wallets
+	methodPayTo            = "payto"          // requires password for protected wallets
 	methodAddLocalTx       = "addtransaction"
 	methodRemoveLocalTx    = "removelocaltx"
 	methodGetTxStatus      = "get_tx_status" // only wallet txns
 	methodGetBalance       = "getbalance"
 	methodIsMine           = "ismine"
-	methodSignTransaction  = "signtransaction"
+	methodSignTransaction  = "signtransaction" // requires password for protected wallets
 	methodFreezeUTXO       = "freeze_utxo"
 	methodUnfreezeUTXO     = "unfreeze_utxo"
 )
@@ -312,7 +312,7 @@ type paytoReq struct {
 	NoCheck        bool   `json:"nocheck"`
 	Unsigned       bool   `json:"unsigned"` // unsigned returns a base64 psbt thing
 	RBF            bool   `json:"rbf"`      // default to false
-	Password       string `json:"password"`
+	Password       string `json:"password,omitempty"`
 	LockTime       *int64 `json:"locktime,omitempty"`
 	AddTransaction bool   `json:"addtransaction"`
 	Wallet         string `json:"wallet,omitempty"`
@@ -409,7 +409,7 @@ func (wc *WalletClient) Sweep(ctx context.Context, walletPass string, addr strin
 
 type signTransactionArgs struct {
 	Tx   string `json:"tx"`
-	Pass string `json:"password"`
+	Pass string `json:"password,omitempty"`
 	// 4.0.9 has privkey in this request, but 4.2 does not since it has a
 	// signtransaction_with_privkey request. (this RPC should not use positional
 	// arguments)
@@ -476,7 +476,7 @@ func (wc *WalletClient) RemoveLocalTx(ctx context.Context, txid string) error {
 
 type getPrivKeyArgs struct {
 	Addr   string `json:"address"`
-	Pass   string `json:"password"`
+	Pass   string `json:"password,omitempty"`
 	Wallet string `json:"wallet,omitempty"`
 }
 

client/asset/btc/electrum_client.go

@@ -274,6 +274,10 @@ func (ew *electrumWallet) connect(ctx context.Context, wg *sync.WaitGroup) error
 	ew.resetChain(chain)
 	ew.ctx = ctx // for requests via methods that lack a context arg
 
+	// This wallet may not be "protected", in which case we omit the password
+	// from the requests. Detect this now and flag the wallet as unlocked.
+	_ = ew.walletUnlock([]byte{})
+
 	// Start a goroutine to keep the chain client alive and on the same
 	// ElectrumX server as the external Electrum wallet if possible.
 	wg.Add(1)
@@ -895,10 +899,39 @@ func (ew *electrumWallet) pass() (pw string, unlocked bool) {
 	return ew.pw, ew.unlocked
 }
 
+func (ew *electrumWallet) testPass(pw []byte) error {
+	addr, err := ew.wallet.GetUnusedAddress(ew.ctx)
+	if err != nil {
+		return err
+	}
+	wifStr, err := ew.wallet.GetPrivateKeys(ew.ctx, string(pw), addr)
+	if err != nil {
+		// When providing a password to an unprotected wallet, and other cases,
+		// a cryptic error containing "incorrect padding" is returned.
+		if strings.Contains(strings.ToLower(err.Error()), "incorrect padding") {
+			return errors.New("incorrect password (no password required?)")
+		}
+		return fmt.Errorf("GetPrivateKeys: %v", err)
+	}
+	// That should be enough, but validate the returned keys in case they are
+	// empty or invalid.
+	if _, err = btcutil.DecodeWIF(wifStr); err != nil {
+		return fmt.Errorf("DecodeWIF: %v", err)
+	}
+	return nil
+}
+
 // walletLock locks the wallet. Part of the btc.Wallet interface.
 func (ew *electrumWallet) walletLock() error {
 	ew.pwMtx.Lock()
 	defer ew.pwMtx.Unlock()
+	if ew.pw == "" && ew.unlocked {
+		// This is an unprotected wallet (can't actually lock it). But confirm
+		// the password is still empty in case it changed externally.
+		if err := ew.testPass([]byte{}); err == nil {
+			return nil
+		} // must have changed! "lock" it!
+	}
 	ew.pw, ew.unlocked = "", false
 	return nil
 }
@@ -922,20 +955,11 @@ func (ew *electrumWallet) walletPass() string {
 // success, the password is stored and may be accessed via pass or walletPass.
 // Part of the btc.Wallet interface.
 func (ew *electrumWallet) walletUnlock(pw []byte) error {
-	addr, err := ew.wallet.GetUnusedAddress(ew.ctx)
-	if err != nil {
-		return err
-	}
-	pass := string(pw)
-	wifStr, err := ew.wallet.GetPrivateKeys(ew.ctx, pass, addr)
-	if err != nil {
-		return err
-	} // that should be enough, but validate the returned keys in case they are empty or something
-	if _, err = btcutil.DecodeWIF(wifStr); err != nil {
+	if err := ew.testPass(pw); err != nil {
 		return err
 	}
 	ew.pwMtx.Lock()
-	ew.pw, ew.unlocked = pass, true
+	ew.pw, ew.unlocked = string(pw), true
 	ew.pwMtx.Unlock()
 	return nil
 }

client/asset/btc/rpcclient.go

@@ -322,21 +322,6 @@ func (wc *rpcClient) getTxOutput(txHash *chainhash.Hash, index uint32) (*btcjson
 		&res)
 }
 
-// locked returns the wallet's lock state.
-func (wc *rpcClient) locked() bool {
-	walletInfo, err := wc.GetWalletInfo()
-	if err != nil {
-		wc.log.Errorf("GetWalletInfo error: %w", err)
-		return false
-	}
-	if walletInfo.UnlockedUntil == nil {
-		// This wallet is not encrypted.
-		return false
-	}
-
-	return time.Unix(*walletInfo.UnlockedUntil, 0).Before(time.Now())
-}
-
 func (wc *rpcClient) callHashGetter(method string, args anylist) (*chainhash.Hash, error) {
 	var txid string
 	err := wc.call(method, args, &txid)
@@ -770,6 +755,21 @@ func (wc *rpcClient) walletLock() error {
 	return wc.call(methodLock, nil, nil)
 }
 
+// locked returns the wallet's lock state.
+func (wc *rpcClient) locked() bool {
+	walletInfo, err := wc.GetWalletInfo()
+	if err != nil {
+		wc.log.Errorf("GetWalletInfo error: %w", err)
+		return false
+	}
+	if walletInfo.UnlockedUntil == nil {
+		// This wallet is not encrypted.
+		return false
+	}
+
+	return time.Unix(*walletInfo.UnlockedUntil, 0).Before(time.Now())
+}
+
 // sendToAddress sends the amount to the address. feeRate is in units of
 // sats/byte. If there is not a fee rate positional param, it is used
 // legacySendToAddress instead.

client/core/core.go

@@ -1848,7 +1848,7 @@ func (c *Core) ToggleWalletStatus(assetID uint32, disable bool) error {
 		}
 
 		if wallet.connected() {
-			wallet.Disconnect()
+			wallet.Disconnect() // before disable or it refuses
 		}
 
 		wallet.setDisabled(true)
@@ -1985,7 +1985,7 @@ func (c *Core) CreateWallet(appPW, walletPW []byte, form *WalletForm) error {
 		return err
 	}
 
-	// Prevent two different tokens from trying to create the parent simulataneously.
+	// Prevent two different tokens from trying to create the parent simultaneously.
 	if err = c.setWalletCreationPending(token.ParentID); err != nil {
 		return err
 	}
@@ -2091,7 +2091,7 @@ func (c *Core) createWalletOrToken(crypter encrypt.Crypter, walletPW []byte, for
 		return nil, fmt.Errorf(s, a...)
 	}
 
-	err = wallet.Unlock(crypter)
+	err = wallet.Unlock(crypter) // no-op if !wallet.Wallet.Locked() && len(encPW) == 0
 	if err != nil {
 		wallet.Disconnect()
 		return nil, fmt.Errorf("%s wallet authentication error: %w", symbol, err)
@@ -2841,7 +2841,7 @@ func (c *Core) ReconfigureWallet(appPW, newWalletPW []byte, form *WalletForm) er
 			Settings: form.Config,
 			DataDir:  c.assetDataDirectory(assetID),
 		}, oldWallet.currentDepositAddress()); err != nil {
-			return err
+			return fmt.Errorf("Reconfigure: %v", err)
 		} else if !restart {
 			// Config was updated without a need to restart.
 			if owns, err := oldWallet.OwnsDepositAddress(oldWallet.currentDepositAddress()); err != nil {
@@ -2936,7 +2936,7 @@ func (c *Core) ReconfigureWallet(appPW, newWalletPW []byte, form *WalletForm) er
 	// are redundant with the rest of this function.
 	dbWallet.Address, err = c.connectWallet(wallet)
 	if err != nil {
-		return err
+		return fmt.Errorf("connectWallet: %w", err)
 	}
 
 	// If there are active trades, make sure they can be settled by the
@@ -2963,7 +2963,7 @@ func (c *Core) ReconfigureWallet(appPW, newWalletPW []byte, form *WalletForm) er
 		err = c.setWalletPassword(wallet, newWalletPW, crypter)
 		if err != nil {
 			wallet.Disconnect()
-			return err
+			return fmt.Errorf("setWalletPassword: %v", err)
 		}
 		// Update dbWallet so db.UpdateWallet below reflects the new password.
 		dbWallet.EncryptedPW = make([]byte, len(wallet.encPass))
@@ -3064,8 +3064,8 @@ func (c *Core) setWalletPassword(wallet *xcWallet, newPW []byte, crypter encrypt
 	if err != nil {
 		return err
 	}
-	if walletDef.Seeded {
-		return newError(passwordErr, "cannot set a password on a seeded wallet")
+	if walletDef.Seeded || asset.TokenInfo(wallet.AssetID) != nil {
+		return newError(passwordErr, "cannot set a password on a seeded or token wallet")
 	}
 
 	// Connect if necessary.
@@ -3079,11 +3079,7 @@ func (c *Core) setWalletPassword(wallet *xcWallet, newPW []byte, crypter encrypt
 	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.
-	// 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?
+	// Check that the new password works.
 	if newPasswordSet {
 		// Encrypt password if it's not an empty string.
 		encNewPW, err := crypter.Encrypt(newPW)
@@ -3097,6 +3093,20 @@ func (c *Core) setWalletPassword(wallet *xcWallet, newPW []byte, crypter encrypt
 		}
 		wallet.setEncPW(encNewPW)
 	} else {
+		// Test that the wallet is actually good with no password. At present,
+		// this means the backend either cannot be locked or unlocks with an
+		// empty password. The following Lock->Unlock cycle but may be required
+		// to detect a newly-unprotected wallet without reconnecting. We will
+		// ignore errors in this process as we are discovering the true state.
+		backend := wallet.Wallet // check the backend directly, not using the xcWallet
+		_ = backend.Lock()
+		_ = backend.Unlock([]byte{})
+		if backend.Locked() {
+			if wasUnlocked { // try to re-unlock the wallet with previous encPW
+				_ = wallet.Unlock(crypter)
+			}
+			return newError(authErr, "wallet appears to require a password")
+		}
 		wallet.setEncPW(nil)
 	}
 
@@ -3106,7 +3116,7 @@ func (c *Core) setWalletPassword(wallet *xcWallet, newPW []byte, crypter encrypt
 	}
 
 	// Re-lock the wallet if it was previously locked.
-	if !wasUnlocked {
+	if !wasUnlocked && newPasswordSet {
 		if err = wallet.Lock(2 * time.Second); err != nil {
 			c.log.Warnf("Unable to relock %s wallet: %v", unbip(wallet.AssetID), err)
 		}