client: allow skipping registration when adding a dex (view-only) #1986

itswisdomagain · 2022-12-05T21:18:32Z

Resolves #1882. Also resolves #726.

This adds a new flow for connecting to a DEX.

The registration page shows a checkbox to connect a DEX in view-only mode (no registration required).

The markets page shows a "Create Account" button for view-only DEX connections.

The "Create Account" button from the markets page redirects to the register page with a confirmation form that performs account discovery on submission, and if necessary, starts the fee payment process.

buck54321

Damn good work! Looks and tests great. Just a couple little things.

client/webserver/site/src/html/markets.tmpl

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

client/webserver/locales/en-us.go

chappjc

I think we're conflating two concepts: (a) being registered at the DEX host, and (b) having a generated account identity/keys for authentication/authorization purposes regardless of registration and fee/bond status.

In Register of course we are concerned with both, particularly the first one so we don't pay a fee twice.

In many other places, we really don't need to know about fee coins or bonds or anything like that; we just want to know if we've run setupCryptoV2 and created account keys. I think that is what distinguishes a view-only account from one intended for authorization (regardless of payment details).

See my comments on the new (*dexAccount).isRegistered method.

client/core/core.go

client/core/types.go

client/core/core.go

client/webserver/webserver.go

itswisdomagain · 2022-12-06T21:25:26Z

we really don't need to know about fee coins or bonds or anything like that; we just want to know if we've run setupCryptoV2 and created account keys. I think that is what distinguishes a view-only account from one intended for authorization (regardless of payment details).

Hmm. A dexAccount that started as a view-only account (no keys setup) but later has keys setup is still a view-only account unless the fee is paid? If not, how would the markets page UI look for such a dexAccount?

chappjc · 2022-12-06T23:18:19Z

we really don't need to know about fee coins or bonds or anything like that; we just want to know if we've run setupCryptoV2 and created account keys. I think that is what distinguishes a view-only account from one intended for authorization (regardless of payment details).

Hmm. A dexAccount that started as a view-only account (no keys setup) but later has keys setup is still a view-only account unless the fee is paid? If not, how would the markets page UI look for such a dexAccount?

I think I just answered this in #1986 (comment), but no, it is no longer a view-only account if it has it's keys set. Fee is a legacy concept, and feeCoin is deprecated. It is expected for accounts created via postbond to have no feeCoin ever set. Further, it's expected to have no live bonds if they've all expired. Neither of these things means the account is not intended for trading, or non-existent on the server. Trading should only be blocked if tier<1, and that's for authDEX to determine.

As far as how the UI looks, note that with the legacy registration fee, there's a "pending" state between when you broadcast the fee transaction and when it gets confirmed and then submitted to the DEX. Similarly, with bonds, if you create an account with a bond ('postbond') or post a bond on an existing account that is presently tier<1, there's a period where the client waits for confirmations before submitting the bond for acceptance (and subsequent tier adjustment). In both cases, the UI has to have special handling. For the legacy fee, it's the confirmation countdown that takes the place of the order form. With bonds, it needs to be something indicating "tier zero, you must post new bonds", and that state can come and go as bond are posted and expire.

I don't know exactly what "not completed" Register outcomes there might be, but we indeed would either need to zero the keys to revert the conversion from view-only to active trading, or have UI handling to indicate this state when they need to attempt to Register again (after funding the wallet or fixing some connectivity issue perhaps?).

itswisdomagain · 2022-12-07T23:25:26Z

Your explanations above clarify things for me @chappjc, thanks.

I think we're conflating two concepts: (a) being registered at the DEX host, and (b) having a generated account identity/keys for authentication/authorization purposes regardless of registration and fee/bond status.

So, this PR adds a third concept (c) view-only DEX (not registered, no keys).

Among other changes in my last commit,

authDEX continues to be done for (a) and (b), excluding only (c)
markets page shows "Create account to trade" prompt even if keys have been generated as long as the account is not yet registered with the server.

JoeGruffins · 2022-12-08T02:32:47Z

There is a limit on the number of websocket connections to a server.

dcrdex/server/comms/server.go

Lines 36 to 38 in 884100b

    
           // rpcMaxClients is the maximum number of active websocket connections 
        
           // allowed. 
        
           rpcMaxClients = 10000

If dex becomes popular, unpaid users could, accidentally, shut the dex down when paid/bond posted users cannot connect. Is it possible to limit the number of unpaid connections, and give them a time limit where they are kicked and must re-connect, maybe with a forced wait buffer if we are close to max unpaid connections?

Can be a todo, imo.

JoeGruffins

Starting fresh on simnet not working

itswisdomagain · 2022-12-08T09:05:01Z

Starting fresh on simnet not working

Silly if statement bug. Fixed.

itswisdomagain · 2022-12-08T10:42:14Z

With bonds, it needs to be something indicating "tier zero, you must post new bonds", and that state can come and go as bond are posted and expire.

Also, a "tier zero, bond posted, waiting confirmations" state.

Is the UI bits something you're working on in your next bond PR @chappjc?

chappjc · 2022-12-08T18:19:35Z

Also, a "tier zero, bond posted, waiting confirmations" state.

Is the UI bits something you're working on in your next bond PR @chappjc?

I'm starting the next "in progress" item on the project page here: DCRDEX Fidelity Bonds (view)

chappjc · 2022-12-08T18:26:55Z

Next bond steps include UI, updated messaging (suspended -> sub-tier or whatever), client bond maintenance, settings, switches, etc.

I'm starting client db and core work for maintaining a certain tier with limits on total bonding amount.

For the UI, what we do depends on if we intend to support both the legacy fee and bonds in the next release. I don't think we should give the choice, only bonds.

So, while we transition in development, I think we should start by showing two options at /register - one that uses the legacy fee and one that uses a bond. But since it's only for developers to use either as needed, I don't think we should make it a single polished form with a selector. In fact, two separate forms shown simultaneously where you just use one of the forms might make the most sense (not intended for release that way). Otherwise if we make a nice selector on a single form, we're going to have to change that when the legacy fee is dropped. If there are just two forms shown, we just remove one when it's time to drop the fee.

client/core/types.go

JoeGruffins

Working great.

One nit is that the viewing dex will appear in the settings under Registered Dexes:

None of the functions there seem to work, so maybe just don't show it there if possible.

client/db/types.go

chappjc · 2022-12-16T20:28:15Z

client/db/types.go

+	if ai.LegacyFeePaid || len(ai.LegacyFeeCoin) > 0 {
+		return true
+	}
+	return len(ai.Bonds) > 0


One tangential but fundamental improvement with the bonds registration protocol is that the account is not created server-side until the bond is confirmed. Thus, we need the Confirmed loop in 91d6124#diff-592072666205a423f50bc3061a74c2cda51bfe4689d4044315ff285d29645bc3R196-R199 to discern if the account should exist on the server ("may auth"). If there is a need to discern the pending initial bond state, we'll have to account for and flag that separately.

I see. Yeah, there's a need to discern if a bond tx or the legacy fee tx has been broadcast to prevent re-attempting a new legacy tx. But that's about it, and since legacy registration will soon be a thing of the past, I'm beginning to wonder if there's any need to account for this.

The only thing we'd need to know is if there's NO active/pending bond and then display a prompt on the UI to post a bond to trade. If there's a pending bond, we display a confirmations counter.

We may also choose to prevent posting a new bond if there's a pending bond.

Alright, sounds fine for now. Will test it all out today.

client/webserver/http.go

client/webserver/site/src/html/bodybuilder.tmpl

client/webserver/site/src/html/forms.tmpl

chappjc · 2022-12-19T22:28:16Z

Sorry, didn't mean to request review from you @buck54321. Feel free to review of course, but no urgent need.

client/webserver/site/src/html/markets.tmpl

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

client/core/account.go

client/webserver/site/src/html/forms.tmpl

client/core/core.go

itswisdomagain · 2023-01-10T14:07:17Z

client/webserver/http.go

+	// Host is optional. If provided, the register page will not display the add
+	// dex form, instead this host will be pre-selected for registration.
+	Host string


If #2025 goes in first (I think it should), I'd use the dexsettings page for this instead of the register page. It would already contain the mechanics for posting bond for an existing dex connection and would naturally fit.

buck54321

Testing well.

buck54321 · 2023-01-27T16:50:36Z

client/core/core.go

@@ -1195,6 +1196,10 @@ func (c *Core) connectedDEX(addr string) (*dexConnection, error) {
 		return nil, err
 	}

+	if dc.acct.isViewOnly() {


This method maybe doesn't make sense named connectedDEX anymore. A view-only connection is still a connection.

buck54321 · 2023-01-27T17:44:32Z

client/db/bolt/db.go

+	// TODO: Accept a viewOnly bool param to prevent unintentionally saving
+	// zero-length Enckey.


Still applicable?

buck54321 · 2023-01-27T18:13:27Z

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

+    const page = this.page
+    page.appPW.value = ''
+    Doc.hide(page.err)
+    const hidePWBox = State.passwordIsCached() || (this.pwCache && this.pwCache.pw)


Could we skip this form altogether if the password is cached?

buck54321 · 2023-01-27T18:15:33Z

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

+    // TODO: Use dexsettings page?
+    const registerBttn = Doc.tmplElement(page.notRegistered, 'registerBttn')
+    bind(registerBttn, 'click', () => {
+      window.location.assign(`/register/${this.market.dex.host}`)


This isn't our preferred method of navigation, since it forces the js and css to be reloaded. How about using loadPage with a data argument?

buck54321 requested changes Dec 6, 2022

View reviewed changes

client/webserver/site/src/html/markets.tmpl Outdated Show resolved Hide resolved

client/webserver/site/src/js/forms.ts Outdated Show resolved Hide resolved

client/webserver/locales/en-us.go Outdated Show resolved Hide resolved

chappjc requested changes Dec 6, 2022

View reviewed changes

itswisdomagain force-pushed the no-reg-dex branch from 8dcfd55 to e0812cd Compare December 7, 2022 23:28

JoeGruffins reviewed Dec 8, 2022

View reviewed changes

itswisdomagain force-pushed the no-reg-dex branch from e0812cd to 19d9ca4 Compare December 8, 2022 09:14

chappjc reviewed Dec 9, 2022

View reviewed changes

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

itswisdomagain force-pushed the no-reg-dex branch from 8967ea7 to 2f6694a Compare December 13, 2022 10:56

JoeGruffins approved these changes Dec 16, 2022

View reviewed changes

chappjc requested changes Dec 16, 2022

View reviewed changes

chappjc requested review from buck54321 and chappjc and removed request for buck54321 December 19, 2022 22:27

itswisdomagain force-pushed the no-reg-dex branch 2 times, most recently from c5d7c26 to b872504 Compare December 20, 2022 21:07

chappjc added this to the 0.6/1.0 milestone Dec 27, 2022

chappjc changed the title ~~client: allow skipping registration when adding a dex~~ client: allow skipping registration when adding a dex (view-only) Dec 29, 2022

buck54321 requested changes Jan 3, 2023

View reviewed changes

chappjc linked an issue Jan 4, 2023 that may be closed by this pull request

Lite book subscriptions #142

Closed

itswisdomagain commented Jan 10, 2023

View reviewed changes

itswisdomagain force-pushed the no-reg-dex branch 2 times, most recently from b079447 to 6237a08 Compare January 10, 2023 14:32

chappjc mentioned this pull request Jan 10, 2023

enable eth and usdc.eth on mainnet #2013

Merged

buck54321 reviewed Jan 23, 2023

View reviewed changes

buck54321 reviewed Jan 27, 2023

View reviewed changes

itswisdomagain force-pushed the no-reg-dex branch from 6237a08 to f506663 Compare February 13, 2023 21:01

itswisdomagain and others added 9 commits February 18, 2023 12:16

client: allow skipping registration when adding a dex

612a0a8

perform acct discovery before registering view-only dex

d1079c6

don't auth unregistered dex on reconnect

52f1935

differentiate view-only dc from dc with keys from registered dc

307cc4b

allow postbond on view-only dex conn

92ecfd1

isRegistered based on DB knowledge

31f5b0b

allow updating cert, host for view-only dcs, other improvs

3cc6f2f

special viewonly flag

b7bd82e

constructor data path. reload user on legacy reg.

8abd8b2

buck54321 force-pushed the no-reg-dex branch from 4ff9ba8 to 8abd8b2 Compare February 18, 2023 18:18

buck54321 approved these changes Feb 18, 2023

View reviewed changes

chappjc merged commit 02374f7 into decred:master Feb 18, 2023

itswisdomagain deleted the no-reg-dex branch February 18, 2023 20:33

buck54321 mentioned this pull request Jun 14, 2023

ui: rework app initialization #2385

Merged

		// TODO: Accept a viewOnly bool param to prevent unintentionally saving
		// zero-length Enckey.

client: allow skipping registration when adding a dex (view-only) #1986

client: allow skipping registration when adding a dex (view-only) #1986

Uh oh!

Conversation

itswisdomagain commented Dec 5, 2022

Uh oh!

buck54321 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chappjc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

itswisdomagain commented Dec 6, 2022

Uh oh!

chappjc commented Dec 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

itswisdomagain commented Dec 7, 2022

Uh oh!

JoeGruffins commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoeGruffins left a comment

Choose a reason for hiding this comment

Uh oh!

itswisdomagain commented Dec 8, 2022

Uh oh!

itswisdomagain commented Dec 8, 2022

Uh oh!

chappjc commented Dec 8, 2022

Uh oh!

chappjc commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

JoeGruffins left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chappjc Dec 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itswisdomagain Dec 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chappjc Dec 19, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chappjc commented Dec 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

itswisdomagain Jan 10, 2023

Choose a reason for hiding this comment

Uh oh!

buck54321 left a comment

Choose a reason for hiding this comment

Uh oh!

buck54321 Jan 27, 2023

Choose a reason for hiding this comment

chappjc left a comment •

edited

Loading

chappjc commented Dec 6, 2022 •

edited

Loading

JoeGruffins commented Dec 8, 2022 •

edited

Loading

chappjc commented Dec 8, 2022 •

edited

Loading

chappjc Dec 16, 2022 •

edited

Loading

itswisdomagain Dec 19, 2022 •

edited

Loading