multi: add support for enabling/disabling a dex account (decred#2946)

* multi: update AccountDisable method on core and db - Rename AccountDisable to ToggleAccountStatus and allow re-enabling a disabled account. --------- Signed-off-by: Philemon Ukane <[email protected]> Co-authored-by: Brian Stafford <[email protected]>
buck54321 · Oct 17, 2024 · 04fbd6f · 04fbd6f
1 parent 008266a
commit 04fbd6f
Show file tree

Hide file tree

Showing 24 changed files with 372 additions and 258 deletions.
diff --git a/client/core/account.go b/client/core/account.go
@@ -14,9 +14,9 @@ import (
 	"github.com/decred/dcrd/dcrec/secp256k1/v4"
 )
 
-// disconnectDEX unsubscribes from the dex's orderbooks, ends the connection
-// with the dex, and removes it from the connection map.
-func (c *Core) disconnectDEX(dc *dexConnection) {
+// stopDEXConnection unsubscribes from the dex's orderbooks and ends the
+// connection with the dex. The dexConnection will still remain in c.conns map.
+func (c *Core) stopDEXConnection(dc *dexConnection) {
 	// Stop dexConnection books.
 	dc.cfgMtx.RLock()
 	if dc.cfg != nil {
@@ -34,42 +34,68 @@ func (c *Core) disconnectDEX(dc *dexConnection) {
 		}
 	}
 	dc.cfgMtx.RUnlock()
+	dc.connMaster.Disconnect() // disconnect
+}
+
+// disconnectDEX disconnects a dex and removes it from the connection map.
+func (c *Core) disconnectDEX(dc *dexConnection) {
 	// Disconnect and delete connection from map.
-	dc.connMaster.Disconnect()
+	c.stopDEXConnection(dc)
 	c.connMtx.Lock()
 	delete(c.conns, dc.acct.host)
 	c.connMtx.Unlock()
 }
 
-// AccountDisable is used to disable an account by given host and application
-// password.
-func (c *Core) AccountDisable(pw []byte, addr string) error {
+// ToggleAccountStatus is used to disable or enable an account by given host and
+// application password.
+func (c *Core) ToggleAccountStatus(pw []byte, host string, disable bool) error {
 	// Validate password.
-	_, err := c.encryptionKey(pw)
+	crypter, err := c.encryptionKey(pw)
 	if err != nil {
 		return codedError(passwordErr, err)
 	}
 
-	// Get dex connection by host.
-	dc, _, err := c.dex(addr)
+	// Get dex connection by host. All exchange servers (enabled or not) are loaded as
+	// dexConnections but disabled servers are not connected.
+	dc, _, err := c.dex(host)
 	if err != nil {
 		return newError(unknownDEXErr, "error retrieving dex conn: %w", err)
 	}
 
-	// Check active orders or bonds.
-	if dc.hasActiveOrders() {
-		return fmt.Errorf("cannot disable account with active orders")
+	if dc.acct.isDisabled() == disable {
+		return nil // no-op
 	}
-	if dc.hasUnspentBond() {
-		return fmt.Errorf("cannot disable account with unspent bonds")
+
+	if disable {
+		// Check active orders or bonds.
+		if dc.hasActiveOrders() {
+			return fmt.Errorf("cannot disable account with active orders")
+		}
+
+		if dc.hasUnspentBond() {
+			c.log.Info("Disabling dex server with unspent bonds. Bonds will be refunded when expired.")
+		}
 	}
 
-	err = c.db.DisableAccount(dc.acct.host)
+	err = c.db.ToggleAccountStatus(host, disable)
 	if err != nil {
-		return newError(accountDisableErr, "error disabling account: %w", err)
+		return newError(accountStatusUpdateErr, "error updating account status: %w", err)
 	}
 
-	c.disconnectDEX(dc)
+	if disable {
+		dc.acct.toggleAccountStatus(true)
+		c.stopDEXConnection(dc)
+	} else {
+		acctInfo, err := c.db.Account(host)
+		if err != nil {
+			return err
+		}
+		dc, connected := c.connectAccount(acctInfo)
+		if !connected {
+			return fmt.Errorf("failed to connected re-enabled account: %w", err)
+		}
+		c.initializeDEXConnection(dc, crypter)
+	}
 
 	return nil
 }
@@ -188,7 +214,7 @@ func (c *Core) AccountImport(pw []byte, acct *Account, bonds []*db.Bond) error {
 			return err
 		}
 		c.addDexConnection(dc)
-		c.initializeDEXConnections(crypter)
+		c.initializeDEXConnection(dc, crypter)
 		return nil
 	}
 
@@ -255,7 +281,7 @@ func (c *Core) AccountImport(pw []byte, acct *Account, bonds []*db.Bond) error {
 		return err
 	}
 	c.addDexConnection(dc)
-	c.initializeDEXConnections(crypter)
+	c.initializeDEXConnection(dc, crypter)
 	return nil
 }
 
@@ -368,9 +394,9 @@ func (c *Core) UpdateDEXHost(oldHost, newHost string, appPW []byte, certI any) (
 		}
 	}
 
-	err = c.db.DisableAccount(oldDc.acct.host)
+	err = c.db.ToggleAccountStatus(oldDc.acct.host, true)
 	if err != nil {
-		return nil, newError(accountDisableErr, "error disabling account: %w", err)
+		return nil, newError(accountStatusUpdateErr, "error updating account status: %w", err)
 	}
 
 	updatedHost = true

diff --git a/client/core/account_test.go b/client/core/account_test.go
@@ -59,51 +59,61 @@ func TestAccountExport(t *testing.T) {
 }
 */
 
-func TestAccountDisable(t *testing.T) {
+func TestToggleAccountStatus(t *testing.T) {
 	activeTrades := map[order.OrderID]*trackedTrade{
 		{}: {metaData: &db.OrderMetaData{Status: order.OrderStatusBooked}},
 	}
 
 	tests := []struct {
-		name, host                          string
-		recryptErr, acctErr, disableAcctErr error
-		wantErr, wantErrCode, loseConns     bool
-		activeTrades                        map[order.OrderID]*trackedTrade
-		errCode                             int
+		name, host                                   string
+		recryptErr, acctErr, disableAcctErr          error
+		wantErr, wantErrCode, loseConns, wantDisable bool
+		activeTrades                                 map[order.OrderID]*trackedTrade
+		errCode                                      int
 	}{{
-		name: "ok",
-		host: tDexHost,
+		name:        "ok: disable account",
+		host:        tDexHost,
+		wantDisable: true,
 	}, {
-		name:       "password error",
-		host:       tDexHost,
-		recryptErr: tErr,
-		wantErr:    true,
-		errCode:    passwordErr,
+		name:        "ok: enable account",
+		host:        tDexHost,
+		wantDisable: false,
+	}, {
+		name:        "password error",
+		host:        tDexHost,
+		recryptErr:  tErr,
+		wantErr:     true,
+		errCode:     passwordErr,
+		wantDisable: true,
 	}, {
 		name:        "host error",
 		host:        ":bad:",
 		wantErr:     true,
 		wantErrCode: true,
 		errCode:     unknownDEXErr,
+		wantDisable: true,
 	}, {
 		name:        "dex not in conns",
 		host:        tDexHost,
 		loseConns:   true,
 		wantErr:     true,
 		wantErrCode: true,
 		errCode:     unknownDEXErr,
+		wantDisable: true,
 	}, {
 		name:         "has active orders",
 		host:         tDexHost,
 		activeTrades: activeTrades,
 		wantErr:      true,
+		wantDisable:  true,
 	}, {
 		name:           "disable account error",
 		host:           tDexHost,
 		disableAcctErr: errors.New(""),
 		wantErr:        true,
 		wantErrCode:    true,
-		errCode:        accountDisableErr,
+		errCode:        accountStatusUpdateErr,
+		wantDisable:    true,
 	}}
 
 	for _, test := range tests {
@@ -122,7 +132,7 @@ func TestAccountDisable(t *testing.T) {
 		}
 		tCore.connMtx.Unlock()
 
-		err := tCore.AccountDisable(tPW, test.host)
+		err := tCore.ToggleAccountStatus(tPW, test.host, test.wantDisable)
 		if test.wantErr {
 			if err == nil {
 				t.Fatalf("expected error for test %v", test.name)
@@ -135,15 +145,21 @@ func TestAccountDisable(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unexpected error for test %v: %v", test.name, err)
 		}
-		if _, found := tCore.conns[test.host]; found {
-			t.Fatal("found disabled account dex connection")
-		}
-		if rig.db.disabledHost == nil {
-			t.Fatal("expected execution of db.DisableAccount")
-		}
-		if *rig.db.disabledHost != test.host {
-			t.Fatalf("expected db disabled account to match test host, want: %v"+
-				" got: %v", test.host, *rig.db.disabledHost)
+		if test.wantDisable {
+			if dc, found := tCore.conns[test.host]; found && !dc.acct.isDisabled() {
+				t.Fatal("expected disabled dex account")
+			}
+			if rig.db.disabledHost == nil {
+				t.Fatal("expected a disable dex server host")
+			}
+			if *rig.db.disabledHost != test.host {
+				t.Fatalf("expected db account to match test host, want: %v"+
+					" got: %v", test.host, *rig.db.disabledHost)
+			}
+		} else {
+			if dc, found := tCore.conns[test.host]; found && dc.acct.isDisabled() {
+				t.Fatal("expected enabled dex account")
+			}
 		}
 	}
 }

diff --git a/client/core/bond.go b/client/core/bond.go
@@ -510,6 +510,13 @@ func (c *Core) refundExpiredBonds(ctx context.Context, acct *dexAccount, cfg *de
 		//
 		// TODO: if mustPost > 0 { wallet.RenewBond(...) }
 
+		// Ensure wallet is unlocked for use below.
+		_, err = wallet.refreshUnlock()
+		if err != nil {
+			c.log.Errorf("failed to unlock bond asset wallet %v: %v", unbip(state.BondAssetID), err)
+			continue
+		}
+
 		// Generate a refund tx paying to an address from the currently
 		// connected wallet, using bond.KeyIndex to create the signed
 		// transaction. The RefundTx is really a backup.
@@ -705,16 +712,14 @@ func (c *Core) rotateBonds(ctx context.Context) {
 		// locked. However, we must refund bonds regardless.
 
 		bondCfg := c.dexBondConfig(dc, now)
-		if len(bondCfg.bondAssets) == 0 {
+		if len(bondCfg.bondAssets) == 0 && !dc.acct.isDisabled() {
 			if !dc.IsDown() && dc.config() != nil {
 				dc.log.Meter("no-bond-assets", time.Minute*10).Warnf("Zero bond assets reported for apparently connected DCRDEX server")
 			}
 			continue
 		}
 		acctBondState := c.bondStateOfDEX(dc, bondCfg)
 
-		c.repostPendingBonds(dc, bondCfg, acctBondState, unlocked)
-
 		refundedAssets, expiredStrength, err := c.refundExpiredBonds(ctx, dc.acct, bondCfg, acctBondState, now)
 		if err != nil {
 			c.log.Errorf("Failed to refund expired bonds for %v: %v", dc.acct.host, err)
@@ -724,6 +729,12 @@ func (c *Core) rotateBonds(ctx context.Context) {
 			c.updateAssetBalance(assetID)
 		}
 
+		if dc.acct.isDisabled() {
+			continue // For disabled account, we should only bother about unspent bonds that might have been refunded by refundExpiredBonds above.
+		}
+
+		c.repostPendingBonds(dc, bondCfg, acctBondState, unlocked)
+
 		bondAsset := bondCfg.bondAssets[acctBondState.BondAssetID]
 		if bondAsset == nil {
 			if acctBondState.TargetTier > 0 {