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

ukane-philemon · 2024-09-03T19:48:51Z

This PR introduces a new behaviour for disabled dex accounts. The account is no longer deleted and will be loaded into memory when core is initialized as an inactive dexConnection to facilitate active bond refunds(if any) every bond rotation. Users can now disable an account with active unspent bonds if they wish to.

Screen.Recording.2024-09-04.at.10.07.53.AM.mov

disabledAccountsBucket was removed. This means previously disabled accounts are lost. If we intend to allow users to re-enable a previously disabled account without having to add it again, I would restore disabledAccountsBucket and allow for backward compatibility.

As a follow up, I'll implement an endpoint to support deleting a dex server from the db. @buck54321 what do you think?

buck54321

utAck just some comments for now. I'm OK with just ignoring the old disabledAccountsBucket from now on. As far as permanent deletion, it has to occur after all bonds are refunded of course, but yeah, do it in a follow up. We'll need this PR or a minimal version of this PR to go on top of release-v1.0 branch too.

client/core/account.go

client/core/bond.go

client/core/core.go

client/core/bond.go

JoeGruffins

Looks good testing fine.

client/core/account.go

client/webserver/site/src/js/dexsettings.ts

client/webserver/site/src/js/forms.ts

client/webserver/site/src/js/dexsettings.ts

client/webserver/site/src/js/forms.ts

client/core/core.go

buck54321

Trying to re-enable the account is resulting in a panic.

buck54321 · 2024-10-10T00:37:49Z

@ukane-philemon please look into the panic. I'd like to get this into a patch release asap.

ukane-philemon · 2024-10-10T00:41:14Z

Sure, I’ll look into it in a moment:

ukane-philemon · 2024-10-10T01:05:20Z

@buck54321 do you have a trace log?

EDIT: Is this what you saw? #3013

- Rename AccountDisable to ToggleAccountStatus and allow re-enabling a disabled account. Signed-off-by: Philemon Ukane <[email protected]>

Signed-off-by: Philemon Ukane <[email protected]>

ukane-philemon · 2024-10-10T01:34:08Z

Screen.Recording.2024-10-10.at.2.30.53.AM.mov

Tested again, and noticed no panic concerning this.

The only unrelated panic I got is #3013 and I'm unable to reproduce it.

buck54321 · 2024-10-10T23:46:07Z

This first one simulates what we might encounter if a server goes offline permanently or just happens to be offline when we try to re-enable it. This results in a panic.

Fresh simnet node
Create account, view-only or bonded, doesn't matter
Shut down client
Shut down server
Restart client
Disable account
Enable account

panic serving 127.0.0.1:54230: runtime error: slice bounds out of range [-1:]
decred.org/dcrdex/client/core.(*Core).initializeDEXConnection(0xc00043ee00, 0x0, {0x2a1eba0, 0xc0007ec320})
	/home/buck/github/decred/dcrdex/client/core/core.go:5144 +0x3a
decred.org/dcrdex/client/core.(*Core).ToggleAccountStatus(0xc00043ee00, {0xc000e00550?, 0xc000388c80?, 0x1a26aa0?}, {0xc000e004e0, 0xf}, 0x0)
	/home/buck/github/decred/dcrdex/client/core/account.go:100 +0x495
decred.org/dcrdex/client/webserver.(*WebServer).apiToggleAccountStatus(0xc0003823c0, {0x7fd1fc7d5ff8, 0xc000153800}, 0xc000388c80)
	/home/buck/github/decred/dcrdex/client/webserver/api.go:863 +0x21d

This other one is just something I stumbled across.

Fresh simnet node
Create view-only account during initial setup
Disable account
Re-enable account

[ERR] CORE: notify: |ERROR| (feepayment) Account unlock error - error unlocking account for 127.0.0.1:17273: DecodeBlob: zero length blob not allowed`

Signed-off-by: Philemon Ukane <[email protected]>

ukane-philemon · 2024-10-11T01:17:02Z

Both issues have been resolved in separate commits.

buck54321

Just for convenience, here's the changes I think are needed buck54321/dcrdex@c9bf340...buck54321:dcrdex:toggle-account-fixes

buck54321 · 2024-10-11T13:41:03Z

client/core/core.go

+	if initialized, _ := dc.acct.status(); !initialized {
+		return // dex account is not yet initialized, so we can't unlock it.
+	}


This isn't necessary. The problem you had before was that you were using connectDEX in ToggleAccountStatus, and connectDEX doesn't set the connectDEXFlag so dc.acct.viewOnly was not being set correctly. Now that you're using connectAccount, this isn't a problem.

I traced your last error report to this: dc.acct.encKey was not set hence the error, which means it hasn't been initialized and this was an attempt to fix that.

Please read again and look at the code. Nothing I said was incorrect. Your use of connectDEX, which doesn't set the connectDEXFlag correctly, was the problem.

client/core/account.go

ukane-philemon · 2024-10-11T19:50:53Z

Just for convenience, here's the changes I think are needed buck54321/[email protected]:dcrdex:toggle-account-fixes

Thanks, cherry-picked as is.

* 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]>

ukane-philemon marked this pull request as ready for review September 4, 2024 09:11

ukane-philemon changed the title ~~multi: update AccountDisable method on core and db~~ multi: add support for enabling/disabling a dex account Sep 4, 2024

ukane-philemon force-pushed the disable-server branch from d2b0482 to 5e1dc4d Compare September 4, 2024 09:16

buck54321 reviewed Sep 7, 2024

View reviewed changes

client/core/account.go Show resolved Hide resolved

client/core/bond.go Outdated Show resolved Hide resolved

client/core/core.go Outdated Show resolved Hide resolved

client/core/bond.go Show resolved Hide resolved

JoeGruffins approved these changes Sep 12, 2024

View reviewed changes

buck54321 requested changes Sep 20, 2024

View reviewed changes

buck54321 reviewed Oct 3, 2024

View reviewed changes

client/core/core.go Show resolved Hide resolved

buck54321 reviewed Oct 6, 2024

View reviewed changes

ukane-philemon added 7 commits October 10, 2024 02:05

multi: update AccountDisable method on core and db

62937f2

- Rename AccountDisable to ToggleAccountStatus and allow re-enabling a disabled account. Signed-off-by: Philemon Ukane <[email protected]>

site: don't convert null/undefined values to objects

b226aed

Signed-off-by: Philemon Ukane <[email protected]>

multi: allow enabling and disabling of dex account

e717b09

Signed-off-by: Philemon Ukane <[email protected]>

client: add test case for enabling a dex account

836853f

Signed-off-by: Philemon Ukane <[email protected]>

buck review changes

23e9354

Signed-off-by: Philemon Ukane <[email protected]>

buck review changes

02e33d2

Signed-off-by: Philemon Ukane <[email protected]>

attempt to unlock wallet during expired bond refund

bda10a3

Signed-off-by: Philemon Ukane <[email protected]>

ukane-philemon force-pushed the disable-server branch from 8f8ccdc to bda10a3 Compare October 10, 2024 01:32

ukane-philemon added 2 commits October 11, 2024 02:14

use core.connectAccount to connect newly enabled dex server

fc4b355

Signed-off-by: Philemon Ukane <[email protected]>

check if dexAccount.encKey is set before attempting to unlock dexAccount

c9bf340

Signed-off-by: Philemon Ukane <[email protected]>

buck54321 requested changes Oct 11, 2024

View reviewed changes

review changes

0eca856

buck54321 approved these changes Oct 14, 2024

View reviewed changes

buck54321 merged commit 7b24153 into decred:master Oct 14, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ukane-philemon commented Sep 3, 2024 •

edited

Loading

buck54321 left a comment

JoeGruffins left a comment

buck54321 left a comment

buck54321 commented Oct 10, 2024

ukane-philemon commented Oct 10, 2024

ukane-philemon commented Oct 10, 2024 •

edited

Loading

ukane-philemon commented Oct 10, 2024

buck54321 commented Oct 10, 2024

ukane-philemon commented Oct 11, 2024

buck54321 left a comment

buck54321 Oct 11, 2024

ukane-philemon Oct 11, 2024

buck54321 Oct 14, 2024

ukane-philemon commented Oct 11, 2024

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

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

Conversation

ukane-philemon commented Sep 3, 2024 • edited Loading

buck54321 left a comment

Choose a reason for hiding this comment

JoeGruffins left a comment

Choose a reason for hiding this comment

buck54321 left a comment

Choose a reason for hiding this comment

buck54321 commented Oct 10, 2024

ukane-philemon commented Oct 10, 2024

ukane-philemon commented Oct 10, 2024 • edited Loading

ukane-philemon commented Oct 10, 2024

buck54321 commented Oct 10, 2024

ukane-philemon commented Oct 11, 2024

buck54321 left a comment

Choose a reason for hiding this comment

buck54321 Oct 11, 2024

Choose a reason for hiding this comment

ukane-philemon Oct 11, 2024

Choose a reason for hiding this comment

buck54321 Oct 14, 2024

Choose a reason for hiding this comment

ukane-philemon commented Oct 11, 2024

ukane-philemon commented Sep 3, 2024 •

edited

Loading

ukane-philemon commented Oct 10, 2024 •

edited

Loading