handle dex cert update adequately

- Add and use new core.addDexConnection
- Refactor core.UpdateCert

don't return an error if DEX host was found in db but not found in core conns map

reject cert update for already connected dex

ensure user provides a new cert

client/core/account.go

@@ -1,10 +1,12 @@
 package core
 
 import (
+	"bytes"
 	"encoding/hex"
 	"errors"
 	"fmt"
 
+	"decred.org/dcrdex/client/comms"
 	"decred.org/dcrdex/client/db"
 	"decred.org/dcrdex/server/account"
 	"github.com/decred/dcrd/dcrec/secp256k1/v4"
@@ -208,24 +210,57 @@ func (c *Core) AccountImport(pw []byte, acct *Account, bonds []*db.Bond) error {
 
 // UpdateCert attempts to connect to a server using a new TLS certificate. If
 // the connection is successful, then the cert in the database is updated.
+// Updating cert for already connected dex will return an error.
 func (c *Core) UpdateCert(host string, cert []byte) error {
-	accountInfo, err := c.db.Account(host)
+	c.connMtx.RLock()
+	dc, found := c.conns[host]
+	c.connMtx.RUnlock()
+	if found && dc.status() == comms.Connected {
+		return errors.New("dex is already connected")
+	}
+
+	acct, err := c.db.Account(host)
 	if err != nil {
 		return err
 	}
 
-	accountInfo.Cert = cert
+	// Ensure user provides a new cert.
+	if bytes.Equal(acct.Cert, cert) {
+		return errors.New("provided cert is the same with the old cert")
+	}
+
+	// Stop reconnect retry for previous dex connection first but leave it in
+	// the map so it remains listed incase we need it in the interim.
+	if found {
+		dc.connMaster.Disconnect()
+		dc.acct.lock()
+		dc.booksMtx.Lock()
+		for m, b := range dc.books {
+			b.closeFeeds()
+			if b.closeTimer != nil {
+				b.closeTimer.Stop()
+			}
+			delete(dc.books, m)
+		}
+		dc.booksMtx.Unlock()
+	}
 
-	_, connected := c.connectAccount(accountInfo)
-	if !connected {
-		return errors.New("failed to connect using new cert")
+	acct.Cert = cert
+	dc, err = c.connectDEX(acct)
+	if err != nil {
+		if dc != nil {
+			dc.connMaster.Disconnect() // stop any retry loop for this new connection.
+		}
+		return fmt.Errorf("failed to connect using new cert (will attempt to restore old connection): %v", err)
 	}
 
-	err = c.db.UpdateAccountInfo(accountInfo)
+	err = c.db.UpdateAccountInfo(acct)
 	if err != nil {
 		return fmt.Errorf("failed to update account info: %w", err)
 	}
 
+	c.addDexConnection(dc)
+
 	return nil
 }
 

client/core/account_test.go

@@ -307,9 +307,7 @@ func TestUpdateDEXHost(t *testing.T) {
 		rig.db.acct.LegacyFeeCoin = encode.RandomBytes(32)
 		rig.db.acct.Host = tDexHost
 
-		tCore.connMtx.Lock()
-		tCore.conns[rig.acct.host] = rig.dc
-		tCore.connMtx.Unlock()
+		tCore.addDexConnection(rig.dc)
 
 		rig.dc.pendingFee = nil
 		if test.feePending {

client/core/bond.go

@@ -860,9 +860,7 @@ func (c *Core) PostBond(form *PostBondForm) (*PostBondResult, error) {
 		if paid {
 			success = true
 			// The listen goroutine is already running, now track the conn.
-			c.connMtx.Lock()
-			c.conns[dc.acct.host] = dc
-			c.connMtx.Unlock()
+			c.addDexConnection(dc)
 			return &PostBondResult{ /* no new bond */ }, nil
 		}
 	}
@@ -997,9 +995,7 @@ func (c *Core) makeAndPostBond(dc *dexConnection, acctExists bool, wallet *xcWal
 	dc.acct.authMtx.Unlock()
 
 	if !acctExists { // *after* setting pendingBonds for rotateBonds accounting if targetTier>0
-		c.connMtx.Lock()
-		c.conns[dc.acct.host] = dc
-		c.connMtx.Unlock()
+		c.addDexConnection(dc)
 		// NOTE: it's still not authed if this was the first bond
 	}
 

client/core/core.go

@@ -1193,6 +1193,16 @@ func (c *Core) dex(addr string) (*dexConnection, bool, error) {
 	return dc, dc.status() == comms.Connected, nil
 }
 
+// addDexConnection is a helper used to add a dex connection.
+func (c *Core) addDexConnection(dc *dexConnection) {
+	if dc == nil {
+		return
+	}
+	c.connMtx.Lock()
+	c.conns[dc.acct.host] = dc
+	c.connMtx.Unlock()
+}
+
 // Get the *dexConnection for the host. Return an error if the DEX is not
 // connected.
 func (c *Core) connectedDEX(addr string) (*dexConnection, error) {
@@ -3723,9 +3733,7 @@ func (c *Core) upgradeConnection(dc *dexConnection) {
 		go c.listen(dc)
 		go dc.subPriceFeed()
 	}
-	c.connMtx.Lock()
-	c.conns[dc.acct.host] = dc
-	c.connMtx.Unlock()
+	c.addDexConnection(dc)
 }
 
 // DiscoverAccount fetches the DEX server's config, and if the server supports
@@ -3944,9 +3952,7 @@ func (c *Core) Register(form *RegisterForm) (*RegisterResult, error) {
 	if paid {
 		registrationComplete = true
 		// The listen goroutine is already running, now track the conn.
-		c.connMtx.Lock()
-		c.conns[dc.acct.host] = dc
-		c.connMtx.Unlock()
+		c.addDexConnection(dc)
 
 		feeCoinStr := coinIDString(dc.acct.feeAssetID, dc.acct.feeCoin)
 		return &RegisterResult{FeeID: feeCoinStr, ReqConfirms: 0}, nil
@@ -3975,10 +3981,7 @@ func (c *Core) Register(form *RegisterForm) (*RegisterResult, error) {
 		return nil, err
 	}
 	if paid { // would have gotten this from discoverAccount
-		c.connMtx.Lock()
-		c.conns[dc.acct.host] = dc
-		c.connMtx.Unlock()
-
+		c.addDexConnection(dc)
 		registrationComplete = true
 		// register already promoted the connection
 		feeCoinStr := coinIDString(dc.acct.feeAssetID, dc.acct.feeCoin)
@@ -4020,9 +4023,7 @@ func (c *Core) Register(form *RegisterForm) (*RegisterResult, error) {
 
 	// Registration complete.
 	registrationComplete = true
-	c.connMtx.Lock()
-	c.conns[host] = dc
-	c.connMtx.Unlock()
+	c.addDexConnection(dc)
 
 	err = c.db.CreateAccount(&db.AccountInfo{
 		Host:             dc.acct.host,
@@ -6341,8 +6342,7 @@ func (c *Core) initialize() {
 // connectAccount makes a connection to the DEX for the given account. If a
 // non-nil dexConnection is returned, it was inserted into the conns map even if
 // the initial connection attempt failed (connected == false), and the connect
-// retry / keepalive loop is active. If there was already a dexConnection, it is
-// first stopped.
+// retry / keepalive loop is active.
 func (c *Core) connectAccount(acct *db.AccountInfo) (dc *dexConnection, connected bool) {
 	// if !acct.Paid && len(acct.FeeCoin) == 0 {
 	// 	// Register should have set this when creating the account that was
@@ -6358,22 +6358,6 @@ func (c *Core) connectAccount(acct *db.AccountInfo) (dc *dexConnection, connecte
 		return
 	}
 
-	c.connMtx.RLock()
-	if dc := c.conns[host]; dc != nil {
-		dc.connMaster.Disconnect()
-		dc.acct.lock()
-		dc.booksMtx.Lock()
-		for m, b := range dc.books {
-			b.closeFeeds()
-			if b.closeTimer != nil {
-				b.closeTimer.Stop()
-			}
-			delete(dc.books, m)
-		}
-		dc.booksMtx.Unlock()
-	} // leave it in the map so it remains listed if connectDEX fails
-	c.connMtx.RUnlock()
-
 	dc, err = c.connectDEX(acct)
 	if dc == nil {
 		c.log.Errorf("Unable to prepare DEX %s: %v", host, err)
@@ -6386,9 +6370,7 @@ func (c *Core) connectAccount(acct *db.AccountInfo) (dc *dexConnection, connecte
 		connected = true
 	}
 
-	c.connMtx.Lock()
-	c.conns[host] = dc
-	c.connMtx.Unlock()
+	c.addDexConnection(dc)
 
 	return
 }

client/core/core_test.go

@@ -2066,9 +2066,7 @@ func TestRegister(t *testing.T) {
 	form.Addr = tDexHost
 
 	// account already exists
-	tCore.connMtx.Lock()
-	tCore.conns[tDexHost] = dc
-	tCore.connMtx.Unlock()
+	tCore.addDexConnection(dc)
 	_, err = tCore.Register(form)
 	if !errorHasCode(err, dupeDEXErr) {
 		t.Fatalf("wrong account exists error: %v", err)