Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 46 additions & 19 deletions client/core/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -34,42 +34,69 @@ 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, addr 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.
// 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(addr)
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.
buck54321 marked this conversation as resolved.
Show resolved Hide resolved
if dc.hasActiveOrders() {
return fmt.Errorf("cannot disable account with active orders")
}

if dc.hasUnspentBond() {
c.log.Warnf("Disabling dex server with unspent bonds. Bonds will be refunded when expired.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably just Info is good.

}
}

err = c.db.DisableAccount(dc.acct.host)
err = c.db.ToggleAccountStatus(addr, 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 {
acct, err := c.db.Account(addr)
if err != nil {
return err
}

if !c.connectAccount(acct) {
c.log.Errorf("Failed to establish connection to %s (will retry)", addr)
}

c.initializeDEXConnections(crypter)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this do all the servers?

}

return nil
}
Expand Down Expand Up @@ -368,9 +395,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
Expand Down
64 changes: 40 additions & 24 deletions client/core/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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")
}
}
}
}
Expand Down
10 changes: 7 additions & 3 deletions client/core/bond.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,16 +705,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
}
buck54321 marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand All @@ -724,6 +722,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 {
Expand Down
17 changes: 15 additions & 2 deletions client/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ func (c *Core) exchangeInfo(dc *dexConnection) *Exchange {
Host: dc.acct.host,
AcctID: acctID,
ConnectionStatus: dc.status(),
Disabled: dc.acct.isDisabled(),
}
}

Expand Down Expand Up @@ -493,6 +494,7 @@ func (c *Core) exchangeInfo(dc *dexConnection) *Exchange {
Auth: acctBondState.ExchangeAuth,
MaxScore: cfg.MaxScore,
PenaltyThreshold: cfg.PenaltyThreshold,
Disabled: dc.acct.isDisabled(),
}
}

Expand Down Expand Up @@ -5138,6 +5140,10 @@ func (c *Core) initializeDEXConnections(crypter encrypt.Crypter) {
continue
}

if dc.acct.isDisabled() {
continue // For disabled account, we only want dc.acct.unlock above to initialize the account ID.
}

// Unlock the bond wallet if a target tier is set.
if bondAssetID, targetTier, maxBondedAmt := dc.bondOpts(); targetTier > 0 {
c.log.Debugf("Preparing %s wallet to maintain target tier of %d for %v, bonding limit %v",
Expand Down Expand Up @@ -7166,7 +7172,8 @@ func (c *Core) initialize() error {
// connectAccount makes a connection to the DEX for the given account. If a
// non-nil dexConnection is returned from newDEXConnection, it was inserted into
// the conns map even if the connection attempt failed (connected == false), and
// the connect retry / keepalive loop is active.
// the connect retry / keepalive loop is active. The intial connection attempt
// or keepalive loop will not run if acct is disabled.
func (c *Core) connectAccount(acct *db.AccountInfo) (connected bool) {
host, err := addrHost(acct.Host)
if err != nil {
Expand Down Expand Up @@ -8150,7 +8157,7 @@ func (c *Core) startDexConnection(acctInfo *db.AccountInfo, dc *dexConnection) e
// the dexConnection's ConnectionMaster is shut down. This goroutine should
// be started as long as the reconnect loop is running. It only returns when
// the wsConn is stopped.
listen := dc.broadcastingConnect()
listen := dc.broadcastingConnect() && !dc.acct.isDisabled()
if listen {
c.wg.Add(1)
go c.listen(dc)
Expand Down Expand Up @@ -8197,6 +8204,12 @@ func (c *Core) startDexConnection(acctInfo *db.AccountInfo, dc *dexConnection) e
// according to ConnectResult.Bonds slice.
}

if dc.acct.isDisabled() {
// Sort out the bonds with current time to indicate refundable bonds.
categorizeBonds(time.Now().Unix())
return nil // nothing else to do
}

err := dc.connMaster.Connect(c.ctx)
if err != nil {
// Sort out the bonds with current time to indicate refundable bonds.
Expand Down
8 changes: 6 additions & 2 deletions client/core/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,12 @@ func (tdb *TDB) BondRefunded(host string, assetID uint32, bondCoinID []byte) err
return nil
}

func (tdb *TDB) DisableAccount(url string) error {
tdb.disabledHost = &url
func (tdb *TDB) ToggleAccountStatus(host string, disable bool) error {
if disable {
tdb.disabledHost = &host
} else {
tdb.disabledHost = nil
}
return tdb.disableAccountErr
}

Expand Down
2 changes: 1 addition & 1 deletion client/core/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const (
fileReadErr
unknownDEXErr
accountRetrieveErr
accountDisableErr
accountStatusUpdateErr
suspendedAcctErr
existenceCheckErr
createWalletErr
Expand Down
15 changes: 15 additions & 0 deletions client/core/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,7 @@ type Exchange struct {
Auth ExchangeAuth `json:"auth"`
PenaltyThreshold uint32 `json:"penaltyThreshold"`
MaxScore uint32 `json:"maxScore"`
Disabled bool `json:"disabled"`
}

// newDisplayIDFromSymbols creates a display-friendly market ID for a base/quote
Expand Down Expand Up @@ -817,6 +818,7 @@ type dexAccount struct {

authMtx sync.RWMutex
isAuthed bool
disabled bool
pendingBondsConfs map[string]uint32
pendingBonds []*db.Bond // not yet confirmed
bonds []*db.Bond // confirmed, and not yet expired
Expand All @@ -835,6 +837,7 @@ func newDEXAccount(acctInfo *db.AccountInfo, viewOnly bool) *dexAccount {
cert: acctInfo.Cert,
dexPubKey: acctInfo.DEXPubKey,
viewOnly: viewOnly,
disabled: acctInfo.Disabled,
encKey: acctInfo.EncKey(), // privKey and id on decrypt
pendingBondsConfs: make(map[string]uint32),
// bonds are set separately when categorized in connectDEX
Expand Down Expand Up @@ -958,6 +961,18 @@ func (a *dexAccount) status() (initialized, unlocked bool) {
return len(a.encKey) > 0, a.privKey != nil
}

func (a *dexAccount) isDisabled() bool {
a.authMtx.RLock()
defer a.authMtx.RUnlock()
return a.disabled
}

func (a *dexAccount) toggleAccountStatus(disable bool) {
a.authMtx.Lock()
defer a.authMtx.Unlock()
a.disabled = disable
}

// locked will be true if the account private key is currently decrypted, or
// there are no account keys generated yet.
func (a *dexAccount) locked() bool {
Expand Down
Loading
Loading