search

decred / dcrdex

The Decred Decentralized Exchange (DEX), powered by atomic-swaps.
Other
186 stars 95 forks source link

client: disable account on dex server don't seem to have effect #1631

Closed ukane-philemon closed 2 years ago

ukane-philemon commented 2 years ago

Disabled my account on the test dex server and after a few minutes I connected to the same server and nothing seem to go wrong. Don't know if it's expected behaviour but the warning message on disable account form says otherwise.

Note: This action is irreversible - once an account is disabled it can't be re-enabled.

To reproduce: disable a registered dex account then reconnect after a few minutes.

LasTshaMAN commented 2 years ago

Adding some links for context,

(original) related issue: https://github.com/decred/dcrdex/issues/249 bulk of the work was done in https://github.com/decred/dcrdex/pull/519 disable account button (manual action) implemented in https://github.com/decred/dcrdex/pull/985

I might cycle back to this issue in the following week or two, and try to find the root cause, if nobody picks it up in the meantime.

chappjc commented 2 years ago

This is expected behavior. The warning is just old. Everything changed when account discovery was done:

When you say "disable a registered dex account then reconnect after a few minutes." that is not what you are doing. You are re-registering. You actually clicking the button in the "Add a DEX" dialog:

image

This is different from simply restarting and logging in to dexc.

The disable function now just prevents dexc from trying to authenticate with that server on login. It puts the account on ice. The actually account cannot be forgotten server-side. And because of how the HD seed is used to derive accounts, and the account discovery methods linked, the original account is restored. This is intentional.

I don't think there's a strong use case for making it unrecoverable, and instead requiring a new reg fee.

ukane-philemon commented 2 years ago

I assumed clicking on add dex sever should trigger new reg fee payment.

I don't think there's a strong use case for making it unrecoverable, and instead requiring a new reg fee.

We could make client pay a new reg fee if account was previously disabled or display a better warning message?

chappjc commented 2 years ago

Hmm, the whole point of discovery was to avoid having to repay. This was a feature we worked toward for a while. Let's just change that warning.

ukane-philemon commented 2 years ago

Let's just change that warning

You have a better warning message in mind?

LasTshaMAN commented 2 years ago

Hmm, the whole point of discovery was to avoid having to repay. This was a feature we worked toward for a while. Let's just change that warning.

that sounds like most reasonable thing to do to me

The disable function now just prevents dexc from trying to authenticate with that server on login. It puts the account on ice. The actually account cannot be forgotten server-side.

while we are on it, can you clarify why "a user will want to prevent dexc from trying to authenticate with a particular server" ? So that we can incorporate this into warning message for him maybe ? And maybe it no longer will be a warning message, rather, just a message ?

I can try to prepare these changes.

ukane-philemon commented 2 years ago

"a user will want to prevent dexc from trying to authenticate with a particular server" ?

Currently, it looks like disabling a registered account on a dex server is more like an account freeze. Dexc won't auto connect/authenticate with that particular dex server (even after restarting), unless the user "re-register" by clicking the button on the "Add a DEX" dialog.

LasTshaMAN commented 2 years ago

I'm wondering why the user might even need this feature in the first place,

later I'll take a look at the PRs supplied above (maybe there is an answer).

chappjc commented 2 years ago

I'm wondering why the user might even need this feature in the first place,

Because you might be done with a DEX and you don't want it connecting to the DEX every time you login. Maybe the DEX server has vanished and it will just try to connect and fail forever. Many reasons. It would be a striking missing feature if you could not remove or disable a DEX server that you had once configured.

Currently, it looks like disabling a registered account on a dex server is more like an account freeze

I don't think this is even named incorrectly. "Disable" is perfectly appropriate IMO. It's not deleting it, it's just disabling it. You can re-enable by re-adding it.

We should stop talking about this issue and just change the incorrect warning to a plain message that says something like "This dex server may be re-enabled at any time by adding the DEX again. You will not have to pay the fee again."