From 62937f2844b25e6aa63349761e3cfa16807a475b Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Tue, 3 Sep 2024 20:39:39 +0100 Subject: [PATCH 01/10] multi: update AccountDisable method on core and db - Rename AccountDisable to ToggleAccountStatus and allow re-enabling a disabled account. Signed-off-by: Philemon Ukane --- client/core/account.go | 47 ++++---- client/core/account_test.go | 57 +++++----- client/core/core.go | 6 ++ client/core/core_test.go | 8 +- client/core/errors.go | 2 +- client/db/bolt/db.go | 113 +++++++------------- client/db/bolt/db_test.go | 44 +++++--- client/db/interface.go | 5 +- client/db/types.go | 1 + client/webserver/api.go | 14 +-- client/webserver/live_test.go | 18 ++-- client/webserver/site/src/js/dexsettings.ts | 4 +- client/webserver/types.go | 7 +- client/webserver/webserver.go | 4 +- client/webserver/webserver_test.go | 2 +- 15 files changed, 169 insertions(+), 163 deletions(-) diff --git a/client/core/account.go b/client/core/account.go index 04b031c8d5..9e60f969a4 100644 --- a/client/core/account.go +++ b/client/core/account.go @@ -41,35 +41,44 @@ func (c *Core) disconnectDEX(dc *dexConnection) { 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) if err != nil { return codedError(passwordErr, err) } - // Get dex connection by host. - dc, _, err := c.dex(addr) - if err != nil { - return newError(unknownDEXErr, "error retrieving dex conn: %w", err) - } + var dc *dexConnection + if disable { + // Get dex connection by host. + 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.hasUnspentBond() { - return fmt.Errorf("cannot disable account with unspent bonds") + // Check active orders or bonds. + if dc.hasActiveOrders() { + return fmt.Errorf("cannot disable account with active orders") + } } - 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 { + c.disconnectDEX(dc) + } else { + acct, err := c.db.Account(addr) + if err != nil { + return err + } + + c.connectAccount(acct) + } return nil } @@ -368,9 +377,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 index ddad6213ce..09a6aa1139 100644 --- a/client/core/account_test.go +++ b/client/core/account_test.go @@ -59,32 +59,36 @@ 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}}, } + // TODO: Add test case for enable dex account 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", + host: tDexHost, + wantDisable: true, }, { - name: "password error", - host: tDexHost, - recryptErr: tErr, - wantErr: true, - errCode: passwordErr, + 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, @@ -92,18 +96,21 @@ func TestAccountDisable(t *testing.T) { 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 +129,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 +142,17 @@ 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 _, found := tCore.conns[test.host]; found { + t.Fatal("found disabled account dex connection") + } + 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) + } } } } diff --git a/client/core/core.go b/client/core/core.go index 8527266925..164b676284 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -7115,6 +7115,12 @@ func (c *Core) initialize() error { var liveConns uint32 var wg sync.WaitGroup for _, acct := range accts { + if !acct.Active { + // TODO: We should list this account separatly for display on the + // dex settings page to allow re-enabling this server. But we should + // listen for unspent bond refund if any. + continue + } wg.Add(1) go func(acct *db.AccountInfo) { defer wg.Done() diff --git a/client/core/core_test.go b/client/core/core_test.go index 8f6fa9c5b0..85a3aa07f9 100644 --- a/client/core/core_test.go +++ b/client/core/core_test.go @@ -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 } diff --git a/client/core/errors.go b/client/core/errors.go index f5de3a1f8a..1c30e850dc 100644 --- a/client/core/errors.go +++ b/client/core/errors.go @@ -42,7 +42,7 @@ const ( fileReadErr unknownDEXErr accountRetrieveErr - accountDisableErr + accountStatusUpdateErr suspendedAcctErr existenceCheckErr createWalletErr diff --git a/client/db/bolt/db.go b/client/db/bolt/db.go index 09130b986c..a1dd34986b 100644 --- a/client/db/bolt/db.go +++ b/client/db/bolt/db.go @@ -55,20 +55,19 @@ var ( // value encodings. var ( // bucket keys - appBucket = []byte("appBucket") - accountsBucket = []byte("accounts") - bondIndexesBucket = []byte("bondIndexes") - bondsSubBucket = []byte("bonds") // sub bucket of accounts - disabledAccountsBucket = []byte("disabledAccounts") - activeOrdersBucket = []byte("activeOrders") - archivedOrdersBucket = []byte("orders") - activeMatchesBucket = []byte("activeMatches") - archivedMatchesBucket = []byte("matches") - botProgramsBucket = []byte("botPrograms") - walletsBucket = []byte("wallets") - notesBucket = []byte("notes") - pokesBucket = []byte("pokes") - credentialsBucket = []byte("credentials") + appBucket = []byte("appBucket") + accountsBucket = []byte("accounts") + bondIndexesBucket = []byte("bondIndexes") + bondsSubBucket = []byte("bonds") // sub bucket of accounts + activeOrdersBucket = []byte("activeOrders") + archivedOrdersBucket = []byte("orders") + activeMatchesBucket = []byte("activeMatches") + archivedMatchesBucket = []byte("matches") + botProgramsBucket = []byte("botPrograms") + walletsBucket = []byte("wallets") + notesBucket = []byte("notes") + pokesBucket = []byte("pokes") + credentialsBucket = []byte("credentials") // value keys versionKey = []byte("version") @@ -179,7 +178,7 @@ func NewDB(dbPath string, logger dex.Logger, opts ...Opts) (dexdb.DB, error) { } if err = bdb.makeTopLevelBuckets([][]byte{ - appBucket, accountsBucket, bondIndexesBucket, disabledAccountsBucket, + appBucket, accountsBucket, bondIndexesBucket, activeOrdersBucket, archivedOrdersBucket, activeMatchesBucket, archivedMatchesBucket, walletsBucket, notesBucket, credentialsBucket, @@ -548,6 +547,8 @@ func loadAccountInfo(acct *bbolt.Bucket, log dex.Logger) (*db.AccountInfo, error return nil, err } + acctInfo.Active = bytes.Equal(acct.Get(activeKey), byteTrue) + bondsBkt := acct.Bucket(bondsSubBucket) if bondsBkt == nil { return acctInfo, nil // no bonds, OK for legacy account @@ -627,7 +628,7 @@ func (db *BoltDB) CreateAccount(ai *dexdb.AccountInfo) error { if err != nil { return fmt.Errorf("accountKey put error: %w", err) } - err = acct.Put(activeKey, byteTrue) // huh? + err = acct.Put(activeKey, byteTrue) if err != nil { return fmt.Errorf("activeKey put error: %w", err) } @@ -710,63 +711,31 @@ func (db *BoltDB) UpdateAccountInfo(ai *dexdb.AccountInfo) error { }) } -// deleteAccount removes the account by host. -func (db *BoltDB) deleteAccount(host string) error { - acctKey := []byte(host) +// ToggleAccountStatus enables or disables the account associated with the given +// host. +func (db *BoltDB) ToggleAccountStatus(host string, disable bool) error { return db.acctsUpdate(func(accts *bbolt.Bucket) error { - return accts.DeleteBucket(acctKey) - }) -} - -// DisableAccount disables the account associated with the given host -// and archives it. The Accounts and Account methods will no longer find -// the disabled account. -// -// TODO: Add disabledAccounts method for retrieval of a disabled account and -// possible recovery of the account data. -func (db *BoltDB) DisableAccount(url string) error { - // Get account's info. - ai, err := db.Account(url) - if err != nil { - return err - } - // Copy AccountInfo to disabledAccounts. Not necessary for view-only - // accounts. - acctKey := ai.EncKey() - if len(acctKey) > 0 { - err = db.disabledAcctsUpdate(func(disabledAccounts *bbolt.Bucket) error { - return disabledAccounts.Put(acctKey, ai.Encode()) - }) - if err != nil { - return err + acct := accts.Bucket([]byte(host)) + if acct == nil { + return fmt.Errorf("account not found for %s", host) } - } - // WARNING/TODO: account proof (fee paid info) not saved! - err = db.deleteAccount(ai.Host) - if err != nil { - if errors.Is(err, bbolt.ErrBucketNotFound) { - db.log.Warnf("Cannot delete account from active accounts"+ - " table. Host: not found. %s err: %v", ai.Host, err) - } else { - return err + + newStatus := byteTrue + if disable { + newStatus = byteFalse } - } - return nil -} -// disabledAccount gets the AccountInfo from disabledAccount associated with -// the specified EncKey. -func (db *BoltDB) disabledAccount(encKey []byte) (*dexdb.AccountInfo, error) { - var acctInfo *dexdb.AccountInfo - return acctInfo, db.disabledAcctsView(func(accts *bbolt.Bucket) error { - acct := accts.Get(encKey) - if acct == nil { - return fmt.Errorf("account not found for key") + if bytes.Equal(acct.Get(activeKey), newStatus) { + msg := "account is already enabled" + if disable { + msg = "account is already disabled" + } + return errors.New(msg) } - var err error - acctInfo, err = dexdb.DecodeAccountInfo(acct) + + err := acct.Put(activeKey, newStatus) if err != nil { - return err + return fmt.Errorf("accountKey put error: %w", err) } return nil }) @@ -782,16 +751,6 @@ func (db *BoltDB) acctsUpdate(f bucketFunc) error { return db.withBucket(accountsBucket, db.Update, f) } -// disabledAcctsView is a convenience function for reading from the disabledAccounts bucket. -func (db *BoltDB) disabledAcctsView(f bucketFunc) error { - return db.withBucket(disabledAccountsBucket, db.View, f) -} - -// disabledAcctsUpdate is a convenience function for inserting into the disabledAccounts bucket. -func (db *BoltDB) disabledAcctsUpdate(f bucketFunc) error { - return db.withBucket(disabledAccountsBucket, db.Update, f) -} - func (db *BoltDB) storeBond(bondBkt *bbolt.Bucket, bond *db.Bond) error { err := bondBkt.Put(bondKey, bond.Encode()) if err != nil { diff --git a/client/db/bolt/db_test.go b/client/db/bolt/db_test.go index f34d096fd2..35c48039b1 100644 --- a/client/db/bolt/db_test.go +++ b/client/db/bolt/db_test.go @@ -266,7 +266,7 @@ func TestAccounts(t *testing.T) { acct.DEXPubKey = dexKey } -func TestDisableAccount(t *testing.T) { +func TestToggleAccountStatus(t *testing.T) { boltdb, shutdown := newTestDB(t) defer shutdown() @@ -276,29 +276,43 @@ func TestDisableAccount(t *testing.T) { if err != nil { t.Fatalf("Unexpected CreateAccount error: %v", err) } - actualDisabledAccount, err := boltdb.disabledAccount(acct.EncKey()) - if err == nil { - t.Fatalf("Expected disabledAccount error but there was none.") + + accounts, err := boltdb.Accounts() + if err != nil { + t.Fatalf("Unexpected boltdb.Accounts error: %v", err) } - if actualDisabledAccount != nil { - t.Fatalf("Expected not to retrieve a disabledAccount.") + if len(accounts) != 1 { + t.Fatalf("Expected 1 account but got %d", len(accounts)) } - err = boltdb.DisableAccount(host) + // Test disable account + err = boltdb.ToggleAccountStatus(host, true) + if err != nil { + t.Fatalf("Unexpected ToggleAccountStatus error: %v", err) + } + actualAcct, err := boltdb.Account(host) if err != nil { - t.Fatalf("Unexpected DisableAccount error: %v", err) + t.Fatalf("Unexpected boltdb.Account error: %v", err) } - actualAcct, _ := boltdb.Account(host) - if actualAcct != nil { - t.Fatalf("Expected retrieval of deleted account to be nil") + + if actualAcct.Active { + t.Fatalf("Expected a disabled account.") } - actualDisabledAccount, err = boltdb.disabledAccount(acct.EncKey()) + + // Test enable account + err = boltdb.ToggleAccountStatus(host, false) if err != nil { - t.Fatalf("Unexpected disabledAccount error: %v", err) + t.Fatalf("Unexpected ToggleAccountStatus error: %v", err) } - if actualDisabledAccount == nil { - t.Fatalf("Expected to retrieve a disabledAccount.") + + actualAcct, err = boltdb.Account(host) + if err != nil { + t.Fatalf("Unexpected boltdb.Account error: %v", err) + } + + if !actualAcct.Active { + t.Fatalf("Expected an active account.") } } diff --git a/client/db/interface.go b/client/db/interface.go index beba5b8ec9..215de2dcec 100644 --- a/client/db/interface.go +++ b/client/db/interface.go @@ -47,8 +47,9 @@ type DB interface { ConfirmBond(host string, assetID uint32, bondCoinID []byte) error // BondRefunded records that a bond has been refunded. BondRefunded(host string, assetID uint32, bondCoinID []byte) error - // DisableAccount sets the AccountInfo disabled status to true. - DisableAccount(host string) error + // ToggleAccountStatus enables or disables the account associated with the + // given host. + ToggleAccountStatus(host string, disable bool) error // UpdateOrder saves the order information in the database. Any existing // order info will be overwritten without indication. UpdateOrder(m *MetaOrder) error diff --git a/client/db/types.go b/client/db/types.go index 05494c1121..e23c1b6cc0 100644 --- a/client/db/types.go +++ b/client/db/types.go @@ -232,6 +232,7 @@ type AccountInfo struct { MaxBondedAmt uint64 PenaltyComps uint16 BondAsset uint32 // the asset to use when auto-posting bonds + Active bool // whether the account is enabled // DEPRECATED reg fee data. Bond txns are in a sub-bucket. // Left until we need to upgrade just for serialization simplicity. diff --git a/client/webserver/api.go b/client/webserver/api.go index a1aa701643..23bb0067fe 100644 --- a/client/webserver/api.go +++ b/client/webserver/api.go @@ -846,9 +846,9 @@ func (s *WebServer) apiRestoreWalletInfo(w http.ResponseWriter, r *http.Request) writeJSON(w, resp) } -// apiAccountDisable is the handler for the '/disableaccount' API request. -func (s *WebServer) apiAccountDisable(w http.ResponseWriter, r *http.Request) { - form := new(accountDisableForm) +// apiToggleAccountStatus is the handler for the '/toggleaccountstatus' API request. +func (s *WebServer) apiToggleAccountStatus(w http.ResponseWriter, r *http.Request) { + form := new(updateAccountStatusForm) defer form.Pass.Clear() if !readPost(w, r, form) { return @@ -860,12 +860,14 @@ func (s *WebServer) apiAccountDisable(w http.ResponseWriter, r *http.Request) { return } // Disable account. - err = s.core.AccountDisable(appPW, form.Host) + err = s.core.ToggleAccountStatus(appPW, form.Host, form.Disable) if err != nil { - s.writeAPIError(w, fmt.Errorf("error disabling account: %w", err)) + s.writeAPIError(w, fmt.Errorf("error updating account status: %w", err)) return } - w.Header().Set("Connection", "close") + if form.Disable { + w.Header().Set("Connection", "close") + } writeJSON(w, simpleAck()) } diff --git a/client/webserver/live_test.go b/client/webserver/live_test.go index 66f012998f..cae2faf0c1 100644 --- a/client/webserver/live_test.go +++ b/client/webserver/live_test.go @@ -772,7 +772,7 @@ func (c *TCore) AccountExport(pw []byte, host string) (*core.Account, []*db.Bond func (c *TCore) AccountImport(pw []byte, account *core.Account, bond []*db.Bond) error { return nil } -func (c *TCore) AccountDisable(pw []byte, host string) error { return nil } +func (c *TCore) ToggleAccountStatus(pw []byte, host string, disable bool) error { return nil } func (c *TCore) TxHistory(assetID uint32, n int, refID *string, past bool) ([]*asset.WalletTransaction, error) { return nil, nil @@ -2176,13 +2176,13 @@ func (m *TMarketMaker) StartBot(startCfg *mm.StartConfig, alternateConfigPath *s mkt.BaseID: randomBalance(), }, DEXBalances: map[uint32]*mm.BotBalance{ - mkt.BaseID: &mm.BotBalance{ + mkt.BaseID: { Available: randomBalance(), Locked: randomBalance(), Pending: randomBalance(), Reserved: randomBalance(), }, - mkt.BaseID: &mm.BotBalance{ + mkt.BaseID: { Available: randomBalance(), Locked: randomBalance(), Pending: randomBalance(), @@ -2190,13 +2190,13 @@ func (m *TMarketMaker) StartBot(startCfg *mm.StartConfig, alternateConfigPath *s }, }, CEXBalances: map[uint32]*mm.BotBalance{ - mkt.BaseID: &mm.BotBalance{ + mkt.BaseID: { Available: randomBalance(), Locked: randomBalance(), Pending: randomBalance(), Reserved: randomBalance(), }, - mkt.BaseID: &mm.BotBalance{ + mkt.BaseID: { Available: randomBalance(), Locked: randomBalance(), Pending: randomBalance(), @@ -2313,12 +2313,12 @@ func (m *TMarketMaker) Status() *mm.Status { stats = &mm.RunStats{ InitialBalances: make(map[uint32]uint64), DEXBalances: map[uint32]*mm.BotBalance{ - botCfg.BaseID: &mm.BotBalance{Available: randomBalance()}, - botCfg.QuoteID: &mm.BotBalance{Available: randomBalance()}, + botCfg.BaseID: {Available: randomBalance()}, + botCfg.QuoteID: {Available: randomBalance()}, }, CEXBalances: map[uint32]*mm.BotBalance{ - botCfg.BaseID: &mm.BotBalance{Available: randomBalance()}, - botCfg.QuoteID: &mm.BotBalance{Available: randomBalance()}, + botCfg.BaseID: {Available: randomBalance()}, + botCfg.QuoteID: {Available: randomBalance()}, }, ProfitLoss: randomProfitLoss(botCfg.BaseID, botCfg.QuoteID), StartTime: time.Now().Add(-time.Duration(float64(time.Hour*10) * rand.Float64())).Unix(), diff --git a/client/webserver/site/src/js/dexsettings.ts b/client/webserver/site/src/js/dexsettings.ts index 0fa3d0efa4..a23839a7cc 100644 --- a/client/webserver/site/src/js/dexsettings.ts +++ b/client/webserver/site/src/js/dexsettings.ts @@ -325,9 +325,9 @@ export default class DexSettingsPage extends BasePage { async disableAccount () { const page = this.page const host = page.disableAccountHost.textContent - const req = { host } + const req = { host, disable: true } const loaded = app().loading(this.body) - const res = await postJSON('/api/disableaccount', req) + const res = await postJSON('/api/toggleaccountstatus', req) loaded() if (!app().checkResponse(res)) { page.disableAccountErr.textContent = res.msg diff --git a/client/webserver/types.go b/client/webserver/types.go index 6ec08206ab..f5b8e48be4 100644 --- a/client/webserver/types.go +++ b/client/webserver/types.go @@ -133,9 +133,10 @@ type accountImportForm struct { Bonds []*db.Bond `json:"bonds"` } -type accountDisableForm struct { - Pass encode.PassBytes `json:"pw"` - Host string `json:"host"` +type updateAccountStatusForm struct { + Pass encode.PassBytes `json:"pw"` + Host string `json:"host"` + Disable bool `json:"disable"` } type deleteRecordsForm struct { diff --git a/client/webserver/webserver.go b/client/webserver/webserver.go index 7a44b1da68..1ea09fa6fe 100644 --- a/client/webserver/webserver.go +++ b/client/webserver/webserver.go @@ -133,7 +133,7 @@ type clientCore interface { MaxSell(host string, base, quote uint32) (*core.MaxOrderEstimate, error) AccountExport(pw []byte, host string) (*core.Account, []*db.Bond, error) AccountImport(pw []byte, account *core.Account, bonds []*db.Bond) error - AccountDisable(pw []byte, host string) error + ToggleAccountStatus(pw []byte, host string, disable bool) error IsInitialized() bool ExportSeed(pw []byte) (string, error) PreOrder(*core.TradeForm) (*core.OrderEstimate, error) @@ -536,7 +536,7 @@ func New(cfg *Config) (*WebServer, error) { apiAuth.Post("/exportaccount", s.apiAccountExport) apiAuth.Post("/exportseed", s.apiExportSeed) apiAuth.Post("/importaccount", s.apiAccountImport) - apiAuth.Post("/disableaccount", s.apiAccountDisable) + apiAuth.Post("/toggleaccountstatus", s.apiToggleAccountStatus) apiAuth.Post("/accelerateorder", s.apiAccelerateOrder) apiAuth.Post("/preaccelerate", s.apiPreAccelerate) apiAuth.Post("/accelerationestimate", s.apiAccelerationEstimate) diff --git a/client/webserver/webserver_test.go b/client/webserver/webserver_test.go index 4bfb500780..40cbf0d52c 100644 --- a/client/webserver/webserver_test.go +++ b/client/webserver/webserver_test.go @@ -246,7 +246,7 @@ func (c *TCore) AccountExport(pw []byte, host string) (*core.Account, []*db.Bond func (c *TCore) AccountImport(pw []byte, account *core.Account, bonds []*db.Bond) error { return nil } -func (c *TCore) AccountDisable(pw []byte, host string) error { return nil } +func (c *TCore) ToggleAccountStatus(pw []byte, host string, disable bool) error { return nil } func (c *TCore) ExportSeed(pw []byte) (string, error) { return "seed words here", nil From b226aed518a10780eedb62febf0f39c6f80414c4 Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Wed, 4 Sep 2024 09:24:53 +0100 Subject: [PATCH 02/10] site: don't convert null/undefined values to objects Signed-off-by: Philemon Ukane --- client/webserver/site/src/js/forms.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/client/webserver/site/src/js/forms.ts b/client/webserver/site/src/js/forms.ts index 30f53e2213..edcf920601 100644 --- a/client/webserver/site/src/js/forms.ts +++ b/client/webserver/site/src/js/forms.ts @@ -1037,13 +1037,17 @@ export class FeeAssetSelectionForm { this.marketRows.push({ mkt, tmpl, setTier }) } - for (const { symbol, id: assetID } of Object.values(xc.assets)) { - if (!app().assets[assetID]) continue - const bondAsset = xc.bondAssets[symbol] - if (bondAsset) addBondRow(assetID, bondAsset) + if (xc.assets) { + for (const { symbol, id: assetID } of Object.values(xc.assets)) { + if (!app().assets[assetID]) continue + const bondAsset = xc.bondAssets[symbol] + if (bondAsset) addBondRow(assetID, bondAsset) + } } - for (const mkt of Object.values(xc.markets)) addMarketRow(mkt) + if (xc.markets) { + for (const mkt of Object.values(xc.markets)) addMarketRow(mkt) + } // page.host.textContent = xc.host page.tradingTierInput.value = xc.auth.targetTier ? String(xc.auth.targetTier) : '1' From e717b096ff703c14725ed885c34b73e02f3bd8a0 Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Wed, 4 Sep 2024 09:44:41 +0100 Subject: [PATCH 03/10] multi: allow enabling and disabling of dex account Signed-off-by: Philemon Ukane --- client/core/account.go | 38 +++++++++------ client/core/bond.go | 8 +++- client/core/core.go | 23 ++++++---- client/core/types.go | 15 ++++++ client/db/bolt/db.go | 2 +- client/db/bolt/db_test.go | 4 +- client/db/types.go | 2 +- client/webserver/jsintl.go | 8 ++++ client/webserver/locales/en-us.go | 3 +- .../webserver/site/src/html/dexsettings.tmpl | 32 ++++++++----- client/webserver/site/src/js/dexsettings.ts | 46 ++++++++++++++----- client/webserver/site/src/js/locales.ts | 4 ++ client/webserver/site/src/js/markets.ts | 4 +- client/webserver/site/src/js/registry.ts | 1 + 14 files changed, 136 insertions(+), 54 deletions(-) diff --git a/client/core/account.go b/client/core/account.go index 9e60f969a4..c604c95c9b 100644 --- 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,8 +34,13 @@ 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() @@ -45,19 +50,19 @@ func (c *Core) disconnectDEX(dc *dexConnection) { // 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) } - var dc *dexConnection - if disable { - // Get dex connection by host. - dc, _, err = c.dex(addr) - if err != nil { - return newError(unknownDEXErr, "error retrieving dex conn: %w", err) - } + // 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) + } + if disable { // Check active orders or bonds. if dc.hasActiveOrders() { return fmt.Errorf("cannot disable account with active orders") @@ -70,14 +75,19 @@ func (c *Core) ToggleAccountStatus(pw []byte, addr string, disable bool) error { } if disable { - c.disconnectDEX(dc) + dc.acct.toggleAccountStatus(true) + c.stopDEXConnection(dc) } else { acct, err := c.db.Account(addr) if err != nil { return err } - c.connectAccount(acct) + if !c.connectAccount(acct) { + c.log.Errorf("Failed to establish connection to %s (will retry)", addr) + } + + c.initializeDEXConnections(crypter) } return nil diff --git a/client/core/bond.go b/client/core/bond.go index 291ee5b838..b544d2c5ec 100644 --- a/client/core/bond.go +++ b/client/core/bond.go @@ -713,8 +713,6 @@ func (c *Core) rotateBonds(ctx context.Context) { } 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 +722,12 @@ func (c *Core) rotateBonds(ctx context.Context) { c.updateAssetBalance(assetID) } + if dc.acct.isDisabled() { + continue // we can only attempt bond refund(if any) for disabled accounts + } + + c.repostPendingBonds(dc, bondCfg, acctBondState, unlocked) + bondAsset := bondCfg.bondAssets[acctBondState.BondAssetID] if bondAsset == nil { if acctBondState.TargetTier > 0 { diff --git a/client/core/core.go b/client/core/core.go index 164b676284..a91a3cf907 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -455,6 +455,7 @@ func (c *Core) exchangeInfo(dc *dexConnection) *Exchange { Host: dc.acct.host, AcctID: acctID, ConnectionStatus: dc.status(), + Disabled: dc.acct.isDisabled(), } } @@ -493,6 +494,7 @@ func (c *Core) exchangeInfo(dc *dexConnection) *Exchange { Auth: acctBondState.ExchangeAuth, MaxScore: cfg.MaxScore, PenaltyThreshold: cfg.PenaltyThreshold, + Disabled: dc.acct.isDisabled(), } } @@ -5140,6 +5142,10 @@ func (c *Core) initializeDEXConnections(crypter encrypt.Crypter) { continue } + if dc.acct.isDisabled() { + continue // we can only unlock the dex account to init 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", @@ -7115,12 +7121,6 @@ func (c *Core) initialize() error { var liveConns uint32 var wg sync.WaitGroup for _, acct := range accts { - if !acct.Active { - // TODO: We should list this account separatly for display on the - // dex settings page to allow re-enabling this server. But we should - // listen for unspent bond refund if any. - continue - } wg.Add(1) go func(acct *db.AccountInfo) { defer wg.Done() @@ -7178,7 +7178,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 { @@ -8173,7 +8174,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) @@ -8220,6 +8221,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. diff --git a/client/core/types.go b/client/core/types.go index 39e830836f..f05a9f806e 100644 --- a/client/core/types.go +++ b/client/core/types.go @@ -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 @@ -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 @@ -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 @@ -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 { diff --git a/client/db/bolt/db.go b/client/db/bolt/db.go index a1dd34986b..5e0b13c6b9 100644 --- a/client/db/bolt/db.go +++ b/client/db/bolt/db.go @@ -547,7 +547,7 @@ func loadAccountInfo(acct *bbolt.Bucket, log dex.Logger) (*db.AccountInfo, error return nil, err } - acctInfo.Active = bytes.Equal(acct.Get(activeKey), byteTrue) + acctInfo.Disabled = bytes.Equal(acct.Get(activeKey), byteFalse) bondsBkt := acct.Bucket(bondsSubBucket) if bondsBkt == nil { diff --git a/client/db/bolt/db_test.go b/client/db/bolt/db_test.go index 35c48039b1..341ee69d17 100644 --- a/client/db/bolt/db_test.go +++ b/client/db/bolt/db_test.go @@ -296,7 +296,7 @@ func TestToggleAccountStatus(t *testing.T) { t.Fatalf("Unexpected boltdb.Account error: %v", err) } - if actualAcct.Active { + if !actualAcct.Disabled { t.Fatalf("Expected a disabled account.") } @@ -311,7 +311,7 @@ func TestToggleAccountStatus(t *testing.T) { t.Fatalf("Unexpected boltdb.Account error: %v", err) } - if !actualAcct.Active { + if actualAcct.Disabled { t.Fatalf("Expected an active account.") } } diff --git a/client/db/types.go b/client/db/types.go index e23c1b6cc0..d20bd4fae5 100644 --- a/client/db/types.go +++ b/client/db/types.go @@ -232,7 +232,7 @@ type AccountInfo struct { MaxBondedAmt uint64 PenaltyComps uint16 BondAsset uint32 // the asset to use when auto-posting bonds - Active bool // whether the account is enabled + Disabled bool // whether the account is disabled // DEPRECATED reg fee data. Bond txns are in a sub-bucket. // Left until we need to upgrade just for serialization simplicity. diff --git a/client/webserver/jsintl.go b/client/webserver/jsintl.go index cde67ece1f..733be59e2e 100644 --- a/client/webserver/jsintl.go +++ b/client/webserver/jsintl.go @@ -194,6 +194,10 @@ const ( archivedSettingsID = "ARCHIVED_SETTINGS" idTransparent = "TRANSPARENT" idNoCodeProvided = "NO_CODE_PROVIDED" + enableAccount = "ENABLE_ACCOUNT" + disableAccount = "DISABLE_ACCOUNT" + accountDisabledMsg = "ACCOUNT_DISABLED_MSG" + dexDisabledMsg = "DEX_DISABLED_MSG" ) var enUS = map[string]*intl.Translation{ @@ -387,6 +391,10 @@ var enUS = map[string]*intl.Translation{ archivedSettingsID: {T: "Archived Settings"}, idTransparent: {T: "Transparent"}, idNoCodeProvided: {T: "no code provided"}, + enableAccount: {T: "Enable Account"}, + disableAccount: {T: "Disable Account"}, + accountDisabledMsg: {T: "account disabled - re-enable to update settings"}, + dexDisabledMsg: {T: "DEX server is disabled. Visit the settings page to enable and connect to this server."}, } var ptBR = map[string]*intl.Translation{ diff --git a/client/webserver/locales/en-us.go b/client/webserver/locales/en-us.go index cff6a9089e..841795abb2 100644 --- a/client/webserver/locales/en-us.go +++ b/client/webserver/locales/en-us.go @@ -51,7 +51,7 @@ var EnUS = map[string]*intl.Translation{ "Authorize Export": {T: "Authorize Export"}, "export_app_pw_msg": {T: "Enter your app password to confirm account export for"}, "Disable Account": {T: "Disable Account"}, - "disable_dex_server": {T: "This DEX server may be re-enabled at any time in the future by adding it again."}, + "disable_dex_server": {T: "This DEX server may be re-enabled at any time in the future on the settings page.", Version: 1}, "Authorize Import": {T: "Authorize Import"}, "app_pw_import_msg": {T: "Enter your app password to confirm account import"}, "Account File": {T: "Account File"}, @@ -653,4 +653,5 @@ var EnUS = map[string]*intl.Translation{ "Transaction": {T: "Transaction"}, "Value": {T: "Value"}, "Prepaid bond redeemed": {T: "Prepaid bond redeemed!"}, + "Enable Account": {T: "Enable Account"}, } diff --git a/client/webserver/site/src/html/dexsettings.tmpl b/client/webserver/site/src/html/dexsettings.tmpl index 55fd19c50b..36eb61640f 100644 --- a/client/webserver/site/src/html/dexsettings.tmpl +++ b/client/webserver/site/src/html/dexsettings.tmpl @@ -1,6 +1,6 @@ {{define "dexsettings"}} {{template "top" .}} -
+
@@ -21,11 +21,13 @@
- [[[target_tier]]] + [[[target_tier]]]
- [[[Actual Tier]]] + [[[Actual Tier]]]
@@ -39,7 +41,7 @@
- +
@@ -47,7 +49,7 @@
Auto Renew
-
+
@@ -61,7 +63,7 @@
- +
@@ -73,18 +75,24 @@
+
+ +
-
- -
-
+
[[[successful_cert_update]]]
-
+
@@ -144,4 +152,4 @@
{{template "bottom"}} -{{end}} +{{end}} \ No newline at end of file diff --git a/client/webserver/site/src/js/dexsettings.ts b/client/webserver/site/src/js/dexsettings.ts index a23839a7cc..5936441d8f 100644 --- a/client/webserver/site/src/js/dexsettings.ts +++ b/client/webserver/site/src/js/dexsettings.ts @@ -32,6 +32,7 @@ export default class DexSettingsPage extends BasePage { currentForm: PageElement page: Record host: string + accountDisabled:boolean keyup: (e: KeyboardEvent) => void dexAddrForm: forms.DEXAddressForm bondFeeBufferCache: Record @@ -101,7 +102,12 @@ export default class DexSettingsPage extends BasePage { this.reputationMeter.setHost(host) Doc.bind(page.exportDexBtn, 'click', () => this.exportAccount()) - Doc.bind(page.disableAcctBtn, 'click', () => this.prepareAccountDisable(page.disableAccountForm)) + + this.accountDisabled = body.dataset.disabled === 'true' + Doc.bind(page.toggleAccountStatusBtn, 'click', () => { + if (!this.accountDisabled) this.prepareAccountDisable(page.disableAccountForm) + else this.toggleAccountStatus(false) + }) Doc.bind(page.updateCertBtn, 'click', () => page.certFileInput.click()) Doc.bind(page.updateHostBtn, 'click', () => this.prepareUpdateHost()) Doc.bind(page.certFileInput, 'change', () => this.onCertFileChange()) @@ -114,12 +120,13 @@ export default class DexSettingsPage extends BasePage { Doc.bind(page.changeTier, 'click', () => { showTierForm() }) const willAutoRenew = xc.auth.targetTier > 0 this.renewToggle = new AniToggle(page.toggleAutoRenew, page.renewErr, willAutoRenew, async (newState: boolean) => { + if (this.accountDisabled) return if (newState) showTierForm() else return this.disableAutoRenew() }) Doc.bind(page.autoRenewBox, 'click', (e: MouseEvent) => { e.stopPropagation() - page.toggleAutoRenew.click() + if (!this.accountDisabled) page.toggleAutoRenew.click() }) page.penaltyComps.textContent = String(xc.auth.penaltyComps) @@ -173,7 +180,7 @@ export default class DexSettingsPage extends BasePage { }, this.host) // forms.bind(page.bondDetailsForm, page.updateBondOptionsConfirm, () => this.updateBondOptions()) - forms.bind(page.disableAccountForm, page.disableAccountConfirm, () => this.disableAccount()) + forms.bind(page.disableAccountForm, page.disableAccountConfirm, () => this.toggleAccountStatus(true)) Doc.bind(page.forms, 'mousedown', (e: MouseEvent) => { if (!Doc.mouseInElement(e, this.currentForm)) { this.closePopups() } @@ -321,21 +328,35 @@ export default class DexSettingsPage extends BasePage { Doc.hide(page.forms) } - // disableAccount disables the account associated with the provided host. - async disableAccount () { + // toggleAccountStatus enables or disables the account associated with the + // provided host. + async toggleAccountStatus (disable:boolean) { const page = this.page - const host = page.disableAccountHost.textContent - const req = { host, disable: true } + Doc.hide(page.errMsg) + let host: string|null = this.host + if (disable) host = page.disableAccountHost.textContent + const req = { host, disable: disable } const loaded = app().loading(this.body) const res = await postJSON('/api/toggleaccountstatus', req) loaded() if (!app().checkResponse(res)) { - page.disableAccountErr.textContent = res.msg - Doc.show(page.disableAccountErr) + if (disable) { + page.disableAccountErr.textContent = res.msg + Doc.show(page.disableAccountErr) + } else { + page.errMsg.textContent = res.msg + Doc.show(page.errMsg) + } return } - Doc.hide(page.forms) - window.location.assign('/settings') + if (disable) { + this.page.toggleAccountStatusBtn.textContent = intl.prep(intl.ID_ENABLE_ACCOUNT) + Doc.hide(page.forms) + } else { + this.page.toggleAccountStatusBtn.textContent = intl.prep(intl.ID_DISABLE_ACCOUNT) + } + this.accountDisabled = disable + window.location.assign(`/dexsettings/${host}`) } async prepareAccountDisable (disableAccountForm: HTMLElement) { @@ -402,7 +423,8 @@ export default class DexSettingsPage extends BasePage { break case ConnectionStatus.Disconnected: displayIcons(false) - page.connectionStatus.textContent = intl.prep(intl.ID_DISCONNECTED) + if (this.accountDisabled) page.connectionStatus.textContent = intl.prep(intl.ID_ACCOUNT_DISABLED_MSG) + else page.connectionStatus.textContent = intl.prep(intl.ID_DISCONNECTED) break case ConnectionStatus.InvalidCert: displayIcons(false) diff --git a/client/webserver/site/src/js/locales.ts b/client/webserver/site/src/js/locales.ts index a84e0cf228..1dd2ee7ddf 100644 --- a/client/webserver/site/src/js/locales.ts +++ b/client/webserver/site/src/js/locales.ts @@ -194,6 +194,10 @@ export const ID_PENDING = 'PENDING' export const ID_COMPLETE = 'COMPLETE' export const ID_ARCHIVED_SETTINGS = 'ARCHIVED_SETTINGS' export const ID_NO_CODE_PROVIDED = 'NO_CODE_PROVIDED' +export const ID_ENABLE_ACCOUNT = 'ENABLE_ACCOUNT' +export const ID_DISABLE_ACCOUNT = 'DISABLE_ACCOUNT' +export const ID_ACCOUNT_DISABLED_MSG = 'ACCOUNT_DISABLED_MSG' +export const ID_DEX_DISABLED_MSG = 'DEX_DISABLED_MSG' let locale: Locale diff --git a/client/webserver/site/src/js/markets.ts b/client/webserver/site/src/js/markets.ts index 8447a1799a..3a63ae88bb 100644 --- a/client/webserver/site/src/js/markets.ts +++ b/client/webserver/site/src/js/markets.ts @@ -1104,7 +1104,9 @@ export default class MarketsPage extends BasePage { // exchange data, so just put up a message and wait for the connection to be // established, at which time handleConnNote will refresh and reload. if (!dex || !dex.markets || dex.connectionStatus !== ConnectionStatus.Connected) { - page.chartErrMsg.textContent = intl.prep(intl.ID_CONNECTION_FAILED) + let errMsg = intl.prep(intl.ID_CONNECTION_FAILED) + if (dex.disabled) errMsg = intl.prep(intl.ID_DEX_DISABLED_MSG) + page.chartErrMsg.textContent = errMsg Doc.show(page.chartErrMsg) return } diff --git a/client/webserver/site/src/js/registry.ts b/client/webserver/site/src/js/registry.ts index 5e8c088bf2..98ad0c3a4c 100644 --- a/client/webserver/site/src/js/registry.ts +++ b/client/webserver/site/src/js/registry.ts @@ -63,6 +63,7 @@ export interface Exchange { candleDurs: string[] maxScore: number penaltyThreshold: number + disabled:boolean } export interface Candle { From 836853f759dc52a237742e0153f6b8d7ab12a521 Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Wed, 4 Sep 2024 10:06:36 +0100 Subject: [PATCH 04/10] client: add test case for enabling a dex account Signed-off-by: Philemon Ukane --- client/core/account.go | 4 ++++ client/core/account_test.go | 15 +++++++++++---- client/webserver/site/src/html/dexsettings.tmpl | 2 +- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/client/core/account.go b/client/core/account.go index c604c95c9b..167908b13f 100644 --- a/client/core/account.go +++ b/client/core/account.go @@ -62,6 +62,10 @@ func (c *Core) ToggleAccountStatus(pw []byte, addr string, disable bool) error { return newError(unknownDEXErr, "error retrieving dex conn: %w", err) } + if dc.acct.isDisabled() == disable { + return nil // no-op + } + if disable { // Check active orders or bonds. if dc.hasActiveOrders() { diff --git a/client/core/account_test.go b/client/core/account_test.go index 09a6aa1139..62f20bb4a6 100644 --- a/client/core/account_test.go +++ b/client/core/account_test.go @@ -64,7 +64,6 @@ func TestToggleAccountStatus(t *testing.T) { {}: {metaData: &db.OrderMetaData{Status: order.OrderStatusBooked}}, } - // TODO: Add test case for enable dex account tests := []struct { name, host string recryptErr, acctErr, disableAcctErr error @@ -72,9 +71,13 @@ func TestToggleAccountStatus(t *testing.T) { activeTrades map[order.OrderID]*trackedTrade errCode int }{{ - name: "ok", + name: "ok: disable account", host: tDexHost, wantDisable: true, + }, { + name: "ok: enable account", + host: tDexHost, + wantDisable: false, }, { name: "password error", host: tDexHost, @@ -143,8 +146,8 @@ func TestToggleAccountStatus(t *testing.T) { t.Fatalf("unexpected error for test %v: %v", test.name, err) } if test.wantDisable { - if _, found := tCore.conns[test.host]; found { - t.Fatal("found disabled account dex connection") + 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") @@ -153,6 +156,10 @@ func TestToggleAccountStatus(t *testing.T) { 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/webserver/site/src/html/dexsettings.tmpl b/client/webserver/site/src/html/dexsettings.tmpl index 36eb61640f..acb38bc92d 100644 --- a/client/webserver/site/src/html/dexsettings.tmpl +++ b/client/webserver/site/src/html/dexsettings.tmpl @@ -152,4 +152,4 @@
{{template "bottom"}} -{{end}} \ No newline at end of file +{{end}} From 23e9354eda74aa11330ad78426362b5afeccfc30 Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Tue, 10 Sep 2024 21:10:45 +0100 Subject: [PATCH 05/10] buck review changes Signed-off-by: Philemon Ukane --- client/core/account.go | 4 ++++ client/core/bond.go | 4 ++-- client/core/core.go | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/client/core/account.go b/client/core/account.go index 167908b13f..0ec2051a95 100644 --- a/client/core/account.go +++ b/client/core/account.go @@ -71,6 +71,10 @@ func (c *Core) ToggleAccountStatus(pw []byte, addr string, disable bool) error { 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.") + } } err = c.db.ToggleAccountStatus(addr, disable) diff --git a/client/core/bond.go b/client/core/bond.go index b544d2c5ec..c42f5fe537 100644 --- a/client/core/bond.go +++ b/client/core/bond.go @@ -705,7 +705,7 @@ 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") } @@ -723,7 +723,7 @@ func (c *Core) rotateBonds(ctx context.Context) { } if dc.acct.isDisabled() { - continue // we can only attempt bond refund(if any) for disabled accounts + 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) diff --git a/client/core/core.go b/client/core/core.go index a91a3cf907..c1d1167a63 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -5143,7 +5143,7 @@ func (c *Core) initializeDEXConnections(crypter encrypt.Crypter) { } if dc.acct.isDisabled() { - continue // we can only unlock the dex account to init the account ID. + 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. From 02e33d26772fe505a335b38e815a4a8636350527 Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Sun, 22 Sep 2024 21:44:34 +0100 Subject: [PATCH 06/10] buck review changes Signed-off-by: Philemon Ukane --- client/core/account.go | 18 +-- client/core/core.go | 122 ++++++++++---------- client/webserver/site/src/js/dexsettings.ts | 9 +- client/webserver/site/src/js/forms.ts | 14 +-- 4 files changed, 83 insertions(+), 80 deletions(-) diff --git a/client/core/account.go b/client/core/account.go index 0ec2051a95..7df7cce7b7 100644 --- a/client/core/account.go +++ b/client/core/account.go @@ -73,7 +73,7 @@ func (c *Core) ToggleAccountStatus(pw []byte, addr string, disable bool) error { } if dc.hasUnspentBond() { - c.log.Warnf("Disabling dex server with unspent bonds. Bonds will be refunded when expired.") + c.log.Info("Disabling dex server with unspent bonds. Bonds will be refunded when expired.") } } @@ -86,16 +86,18 @@ func (c *Core) ToggleAccountStatus(pw []byte, addr string, disable bool) error { dc.acct.toggleAccountStatus(true) c.stopDEXConnection(dc) } else { - acct, err := c.db.Account(addr) + acctInfo, 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) + dc, err := c.connectDEX(acctInfo) + if err != nil { + c.log.Errorf("Trouble establishing connection to %s (will retry). Error: %v", acctInfo.Host, err) } - - c.initializeDEXConnections(crypter) + // Connected or not, add dex connection to the connections map. + c.addDexConnection(dc) + c.initializeDEXConnection(dc, crypter) } return nil @@ -215,7 +217,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 } @@ -282,7 +284,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 } diff --git a/client/core/core.go b/client/core/core.go index c1d1167a63..f858389d0a 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -5128,74 +5128,80 @@ func (c *Core) initializeDEXConnections(crypter encrypt.Crypter) { var wg sync.WaitGroup conns := c.dexConnections() for _, dc := range conns { - if dc.acct.isViewOnly() { - continue // don't attempt authDEX for view-only conn - } + wg.Add(1) + go func(dc *dexConnection) { + defer wg.Done() + c.initializeDEXConnection(dc, crypter) + }(dc) + } - // Unlock before checking auth and continuing, because if the user - // logged out and didn't shut down, the account is still authed, but - // locked, and needs unlocked. - err := dc.acct.unlock(crypter) - if err != nil { - subject, details := c.formatDetails(TopicAccountUnlockError, dc.acct.host, err) - c.notify(newFeePaymentNote(TopicAccountUnlockError, subject, details, db.ErrorLevel, dc.acct.host)) // newDEXAuthNote? - continue - } + wg.Wait() +} - if dc.acct.isDisabled() { - continue // For disabled account, we only want dc.acct.unlock above to initialize the account ID. - } +// initializeDEXConnection connects to the DEX server in the conns map and +// authenticates the connection. +func (c *Core) initializeDEXConnection(dc *dexConnection, crypter encrypt.Crypter) { + if dc.acct.isViewOnly() { + return // don't attempt authDEX for view-only conn + } - // 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", - unbip(bondAssetID), targetTier, dc.acct.host, maxBondedAmt) - wallet, exists := c.wallet(bondAssetID) - if !exists || !wallet.connected() { // connectWallets already run, just fail - subject, details := c.formatDetails(TopicBondWalletNotConnected, unbip(bondAssetID)) - var w *WalletState - if exists { - w = wallet.state() - } - c.notify(newWalletConfigNote(TopicBondWalletNotConnected, subject, details, db.ErrorLevel, w)) - } else if !wallet.unlocked() { - err = wallet.Unlock(crypter) - if err != nil { - subject, details := c.formatDetails(TopicWalletUnlockError, dc.acct.host, err) - c.notify(newFeePaymentNote(TopicWalletUnlockError, subject, details, db.ErrorLevel, dc.acct.host)) - } - } - } + // Unlock before checking auth and continuing, because if the user + // logged out and didn't shut down, the account is still authed, but + // locked, and needs unlocked. + err := dc.acct.unlock(crypter) + if err != nil { + subject, details := c.formatDetails(TopicAccountUnlockError, dc.acct.host, err) + c.notify(newFeePaymentNote(TopicAccountUnlockError, subject, details, db.ErrorLevel, dc.acct.host)) // newDEXAuthNote? + return + } - if dc.acct.authed() { // should not be possible with newly idempotent login, but there's AccountImport... - continue // authDEX already done + if dc.acct.isDisabled() { + return // 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", + unbip(bondAssetID), targetTier, dc.acct.host, maxBondedAmt) + wallet, exists := c.wallet(bondAssetID) + if !exists || !wallet.connected() { // connectWallets already run, just fail + subject, details := c.formatDetails(TopicBondWalletNotConnected, unbip(bondAssetID)) + var w *WalletState + if exists { + w = wallet.state() + } + c.notify(newWalletConfigNote(TopicBondWalletNotConnected, subject, details, db.ErrorLevel, w)) + } else if !wallet.unlocked() { + err = wallet.Unlock(crypter) + if err != nil { + subject, details := c.formatDetails(TopicWalletUnlockError, dc.acct.host, err) + c.notify(newFeePaymentNote(TopicWalletUnlockError, subject, details, db.ErrorLevel, dc.acct.host)) + } } + } - // Pending bonds will be handled by authDEX. Expired bonds will be - // refunded by rotateBonds. + if dc.acct.authed() { // should not be possible with newly idempotent login, but there's AccountImport... + return // authDEX already done + } - // If the connection is down, authDEX will fail on Send. - if dc.IsDown() { - c.log.Warnf("Connection to %v not available for authorization. "+ - "It will automatically authorize when it connects.", dc.acct.host) - subject, details := c.formatDetails(TopicDEXDisconnected, dc.acct.host) - c.notify(newConnEventNote(TopicDEXDisconnected, subject, dc.acct.host, comms.Disconnected, details, db.ErrorLevel)) - continue - } + // Pending bonds will be handled by authDEX. Expired bonds will be + // refunded by rotateBonds. - wg.Add(1) - go func(dc *dexConnection) { - defer wg.Done() - err := c.authDEX(dc) - if err != nil { - subject, details := c.formatDetails(TopicDexAuthError, dc.acct.host, err) - c.notify(newDEXAuthNote(TopicDexAuthError, subject, dc.acct.host, false, details, db.ErrorLevel)) - return - } - }(dc) + // If the connection is down, authDEX will fail on Send. + if dc.IsDown() { + c.log.Warnf("Connection to %v not available for authorization. "+ + "It will automatically authorize when it connects.", dc.acct.host) + subject, details := c.formatDetails(TopicDEXDisconnected, dc.acct.host) + c.notify(newConnEventNote(TopicDEXDisconnected, subject, dc.acct.host, comms.Disconnected, details, db.ErrorLevel)) + return } - wg.Wait() + // Authenticate dex connection + err = c.authDEX(dc) + if err != nil { + subject, details := c.formatDetails(TopicDexAuthError, dc.acct.host, err) + c.notify(newDEXAuthNote(TopicDexAuthError, subject, dc.acct.host, false, details, db.ErrorLevel)) + } } // resolveActiveTrades loads order and match data from the database. Only active diff --git a/client/webserver/site/src/js/dexsettings.ts b/client/webserver/site/src/js/dexsettings.ts index 5936441d8f..43f4f226cc 100644 --- a/client/webserver/site/src/js/dexsettings.ts +++ b/client/webserver/site/src/js/dexsettings.ts @@ -176,7 +176,7 @@ export default class DexSettingsPage extends BasePage { }) this.dexAddrForm = new forms.DEXAddressForm(page.dexAddrForm, async (xc: Exchange) => { - window.location.assign(`/dexsettings/${xc.host}`) + app().loadPage(`/dexsettings/${xc.host}`) }, this.host) // forms.bind(page.bondDetailsForm, page.updateBondOptionsConfirm, () => this.updateBondOptions()) @@ -352,11 +352,10 @@ export default class DexSettingsPage extends BasePage { if (disable) { this.page.toggleAccountStatusBtn.textContent = intl.prep(intl.ID_ENABLE_ACCOUNT) Doc.hide(page.forms) - } else { - this.page.toggleAccountStatusBtn.textContent = intl.prep(intl.ID_DISABLE_ACCOUNT) - } + } else this.page.toggleAccountStatusBtn.textContent = intl.prep(intl.ID_DISABLE_ACCOUNT) + this.accountDisabled = disable - window.location.assign(`/dexsettings/${host}`) + app().loadPage(`dexsettings/${host}`) } async prepareAccountDisable (disableAccountForm: HTMLElement) { diff --git a/client/webserver/site/src/js/forms.ts b/client/webserver/site/src/js/forms.ts index edcf920601..e253d9cec5 100644 --- a/client/webserver/site/src/js/forms.ts +++ b/client/webserver/site/src/js/forms.ts @@ -1037,17 +1037,13 @@ export class FeeAssetSelectionForm { this.marketRows.push({ mkt, tmpl, setTier }) } - if (xc.assets) { - for (const { symbol, id: assetID } of Object.values(xc.assets)) { - if (!app().assets[assetID]) continue - const bondAsset = xc.bondAssets[symbol] - if (bondAsset) addBondRow(assetID, bondAsset) - } + for (const { symbol, id: assetID } of Object.values(xc.assets || {})) { + if (!app().assets[assetID]) continue + const bondAsset = xc.bondAssets[symbol] + if (bondAsset) addBondRow(assetID, bondAsset) } - if (xc.markets) { - for (const mkt of Object.values(xc.markets)) addMarketRow(mkt) - } + for (const mkt of Object.values(xc.markets || {})) addMarketRow(mkt) // page.host.textContent = xc.host page.tradingTierInput.value = xc.auth.targetTier ? String(xc.auth.targetTier) : '1' From bda10a3ba4eb18140bbc0bdb1d1ff348fa14e07e Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Thu, 10 Oct 2024 02:32:31 +0100 Subject: [PATCH 07/10] attempt to unlock wallet during expired bond refund Signed-off-by: Philemon Ukane --- client/core/bond.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/client/core/bond.go b/client/core/bond.go index c42f5fe537..f6adc4b465 100644 --- 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. From fc4b355dbbc57703abca7ff373ce2ab53515e467 Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Fri, 11 Oct 2024 02:14:52 +0100 Subject: [PATCH 08/10] use core.connectAccount to connect newly enabled dex server Signed-off-by: Philemon Ukane --- client/core/account.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/client/core/account.go b/client/core/account.go index 7df7cce7b7..95cdd21580 100644 --- a/client/core/account.go +++ b/client/core/account.go @@ -91,12 +91,7 @@ func (c *Core) ToggleAccountStatus(pw []byte, addr string, disable bool) error { return err } - dc, err := c.connectDEX(acctInfo) - if err != nil { - c.log.Errorf("Trouble establishing connection to %s (will retry). Error: %v", acctInfo.Host, err) - } - // Connected or not, add dex connection to the connections map. - c.addDexConnection(dc) + c.connectAccount(acctInfo) c.initializeDEXConnection(dc, crypter) } From c9bf340a81f4aae30e59ce9043c1b498706d0539 Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Fri, 11 Oct 2024 02:15:33 +0100 Subject: [PATCH 09/10] check if dexAccount.encKey is set before attempting to unlock dexAccount Signed-off-by: Philemon Ukane --- client/core/core.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client/core/core.go b/client/core/core.go index f858389d0a..c73e1c3f05 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -5145,6 +5145,10 @@ func (c *Core) initializeDEXConnection(dc *dexConnection, crypter encrypt.Crypte return // don't attempt authDEX for view-only conn } + if initialized, _ := dc.acct.status(); !initialized { + return // dex account is not yet initialized, so we can't unlock it. + } + // Unlock before checking auth and continuing, because if the user // logged out and didn't shut down, the account is still authed, but // locked, and needs unlocked. From 0eca856d663516159b304d34cac4d642d1caae32 Mon Sep 17 00:00:00 2001 From: Brian Stafford Date: Fri, 11 Oct 2024 08:22:31 -0500 Subject: [PATCH 10/10] review changes --- client/core/account.go | 14 ++++++++------ client/core/core.go | 14 +++++--------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/client/core/account.go b/client/core/account.go index 95cdd21580..ce7760385f 100644 --- a/client/core/account.go +++ b/client/core/account.go @@ -48,7 +48,7 @@ func (c *Core) disconnectDEX(dc *dexConnection) { // 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 { +func (c *Core) ToggleAccountStatus(pw []byte, host string, disable bool) error { // Validate password. crypter, err := c.encryptionKey(pw) if err != nil { @@ -57,7 +57,7 @@ func (c *Core) ToggleAccountStatus(pw []byte, addr string, disable bool) error { // 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) + dc, _, err := c.dex(host) if err != nil { return newError(unknownDEXErr, "error retrieving dex conn: %w", err) } @@ -77,7 +77,7 @@ func (c *Core) ToggleAccountStatus(pw []byte, addr string, disable bool) error { } } - err = c.db.ToggleAccountStatus(addr, disable) + err = c.db.ToggleAccountStatus(host, disable) if err != nil { return newError(accountStatusUpdateErr, "error updating account status: %w", err) } @@ -86,12 +86,14 @@ func (c *Core) ToggleAccountStatus(pw []byte, addr string, disable bool) error { dc.acct.toggleAccountStatus(true) c.stopDEXConnection(dc) } else { - acctInfo, err := c.db.Account(addr) + acctInfo, err := c.db.Account(host) if err != nil { return err } - - c.connectAccount(acctInfo) + dc, connected := c.connectAccount(acctInfo) + if !connected { + return fmt.Errorf("failed to connected re-enabled account: %w", err) + } c.initializeDEXConnection(dc, crypter) } diff --git a/client/core/core.go b/client/core/core.go index c73e1c3f05..e6a965b0d0 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -5145,10 +5145,6 @@ func (c *Core) initializeDEXConnection(dc *dexConnection, crypter encrypt.Crypte return // don't attempt authDEX for view-only conn } - if initialized, _ := dc.acct.status(); !initialized { - return // dex account is not yet initialized, so we can't unlock it. - } - // Unlock before checking auth and continuing, because if the user // logged out and didn't shut down, the account is still authed, but // locked, and needs unlocked. @@ -7134,7 +7130,7 @@ func (c *Core) initialize() error { wg.Add(1) go func(acct *db.AccountInfo) { defer wg.Done() - if c.connectAccount(acct) { + if _, connected := c.connectAccount(acct); connected { atomic.AddUint32(&liveConns, 1) } }(acct) @@ -7190,7 +7186,7 @@ func (c *Core) initialize() error { // the conns map even if the connection attempt failed (connected == false), and // 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) { +func (c *Core) connectAccount(acct *db.AccountInfo) (dc *dexConnection, connected bool) { host, err := addrHost(acct.Host) if err != nil { c.log.Errorf("skipping loading of %s due to address parse error: %v", host, err) @@ -7199,7 +7195,7 @@ func (c *Core) connectAccount(acct *db.AccountInfo) (connected bool) { if c.cfg.TheOneHost != "" && c.cfg.TheOneHost != host { c.log.Infof("Running with --onehost = %q.", c.cfg.TheOneHost) - return false + return } var connectFlag connectDEXFlag @@ -7207,7 +7203,7 @@ func (c *Core) connectAccount(acct *db.AccountInfo) (connected bool) { connectFlag |= connectDEXFlagViewOnly } - dc, err := c.newDEXConnection(acct, connectFlag) + dc, err = c.newDEXConnection(acct, connectFlag) if err != nil { c.log.Errorf("Unable to prepare DEX %s: %v", host, err) return @@ -7220,7 +7216,7 @@ func (c *Core) connectAccount(acct *db.AccountInfo) (connected bool) { // Connected or not, the dexConnection goes in the conns map now. c.addDexConnection(dc) - return err == nil + return dc, err == nil } func (c *Core) dbOrders(host string) ([]*db.MetaOrder, error) {