Add form when failing to register with low balance #1092

vctt94 · 2021-05-17T18:03:25Z

Resolves #768

JoeGruffins

I get Looks like you do not have enough funds even if the failure is because I put in the wrong app password. The popup looks good to me, but it would really be better if it didn't have the possibility of showing misleading information.

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

vctt94 · 2021-05-18T12:35:13Z

Thanks for the review joe, fixed it.

JoeGruffins

It seems I only get the new form if I have exactly 0 in the account. If I have, say, .01 and the dex want's 1, I do not see the form, although I do have insufficient funds.

vctt94 · 2021-05-19T13:24:47Z

Are you trying to register to dex-test.ssgen.io:7232? Because if that is the case, the fee is 0.1 dcr (10000000 atoms), so you have sufficient funds, but no sufficient funds for the tx fee.

I added an insuficient funds into the message, so the form also shows in this case.

JoeGruffins · 2021-05-20T05:52:36Z

client/asset/dcr/dcr.go

@@ -2444,7 +2444,7 @@ func (dcr *ExchangeWallet) sendRegFee(addr dcrutil.Address, regFee, netFeeRate u
 	}
 	coins, _, _, _, err := dcr.fund(enough)
 	if err != nil {
-		return nil, 0, fmt.Errorf("unable to pay registration fee of %s DCR with fee rate of %d atoms/byte: %w",
+		return nil, 0, fmt.Errorf("Insufficient funds for fee rate. Unable to pay registration fee of %s DCR with fee rate of %d atoms/byte: %w",


I think must be lowercase i in order to match the string.

But really, would be good to have an error code or something, as matching strings is very fragile.

Also, one day fees may be payable by different coins. So, other assets must also pay attention to their error messages. Also, this may cause problems with internationalization.

client/webserver/site/src/js/register.js

chappjc

Yup, this approach is what I had in mind. Just a little clean-up and docs, and maybe consider something for js not to fail if code is undefined.

client/webserver/site/src/js/register.js

chappjc · 2021-05-27T18:13:37Z

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

+      <div class="bg2 px-2 py-1 text-center fs18">Insuficient funds</div>
+      <div class="p-4">
+        <div class="fs16">
+          Looks like you do not have enough funds for paying the fee in the
+          selected account. Please fund the account and try again.
+        </div>
+        <div class="fs15 pt-3 text-center d-hide errcolor" id="regFundsErr"></div>
+        <hr class="dashed mt-4">
+      </div>


This is still just a guess that it's insufficient funds, but really it's any error from wallet.PayFee, which causes (*Core).Register to return a feeSendErr-coded error.

It could be many things including a bad fee address, or an error from sendWithReturn such as failure to sign the tx or failure to broadcast with sendrawtransaction.

Thus, I think it's warranted to soften the language in this dialog to indicate an error, but suggest the most likely cause of insufficient funds, and ofc show the whole error message as you are doing.

We could make it that specific with changes to the each asset's PayFee implementation to return a new error defined in client/asset/interface.go, which (*Core).Register recognizes and replaces with a more specific error code. But the most likely scenario is balance, and the form does show the full error, so I'm OK with simply framing this dialog as a suggestion rather than definitively saying it is balance.

vctt94 · 2021-06-07T13:31:13Z

This PR is ready for another review.

chappjc · 2021-06-08T19:47:33Z

client/webserver/types.go

-	Msg string `json:"msg,omitempty"`
+	OK   bool   `json:"ok"`
+	Msg  string `json:"msg,omitempty"`
+	Code int    `json:"code,omitempty"`


Since walletErr = iota (0), this first code in the enumeration will not be marshalled. Nothing breaks in this PR, but it should allow 0.

- Code int `json:"code,omitempty"` + Code *int `json:"code,omitempty"`

- Code: code, + Code: &code,

In retrospect, it would have been better to define walletErr = iota + 1, but I'm hesitant to make that change now.

client/core/errors.go

client/asset/dcr/dcr.go

client/webserver/site/src/js/constants.js

client/core/errors.go

client/webserver/api.go

vctt94 · 2021-06-09T16:07:37Z

Thanks for the review chapp. Addressed your comments

client/webserver/api.go

chappjc · 2021-06-09T18:24:18Z

I've revised the PR description so it doesn't close #768 when merged.

This PR will recognize an error from PayFee and then show the following:

However, because the above comes after confirming the following dialog, the issue isn't quite resolved.

In #768, @buck54321 was pointing out that instead of showing them this dialog when the frontend already knows the balance is insufficient, it should show a different dialog that prevents the registration and fee payment attempt entirely. (or modifies this Confirm Registration dialog with a clear note about balance and a disabled Register button).

Detecting a certain fee payment error code and displaying an appropriate dialog, as done in this PR, makes complete sense, but it does not quite resolve #768. Note that instead of getfee, we are now using getdexinfo

vctt94 · 2021-06-14T15:25:01Z

okay, now it goes directly to the registration failed form, if there are not enough funds.

If available funds are equal the reg fee, than the process occurs and only fails in the end, as we do not check for the network fee.

JoeGruffins

Working well for me now.

chappjc

Working well.

UI after entering dex address:

(doesn't have you try to confirm payment first -- good)

dexcctl register:

error paying registration fee: Unable to pay registration fee of 1 DCR with fee rate of 11 atoms/byte: insufficient funds. 0 DCR available to spend in "broke" account

chappjc · 2021-07-06T14:44:00Z

client/webserver/api.go

-		Msg: errMsg,
+		OK:   false,
+		Msg:  err.Error(),
+		Code: code,


Good. Code is omitted when there is no error (just OK=true).

JoeGruffins reviewed May 18, 2021

View reviewed changes

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

JoeGruffins reviewed May 19, 2021

View reviewed changes

JoeGruffins reviewed May 20, 2021

View reviewed changes

chappjc requested changes May 25, 2021

View reviewed changes

client/webserver/site/src/js/register.js Outdated Show resolved Hide resolved

vctt94 changed the title ~~Add form when failing to register with low balance~~ [wip] Add form when failing to register with low balance May 27, 2021

vctt94 force-pushed the add-no-funds-form branch from 88151c9 to fcf3b47 Compare May 27, 2021 17:48

chappjc requested changes May 27, 2021

View reviewed changes

client/webserver/site/src/js/register.js Outdated Show resolved Hide resolved

client/webserver/site/src/js/register.js Outdated Show resolved Hide resolved

chappjc reviewed May 27, 2021

View reviewed changes

POC: change error to receive err instead of format

661577e

vctt94 force-pushed the add-no-funds-form branch from fcf3b47 to 661577e Compare May 28, 2021 14:28

vctt94 added 2 commits May 28, 2021 11:45

review fixes

21d8189

fix test

ab50be3

vctt94 changed the title ~~[wip] Add form when failing to register with low balance~~ Add form when failing to register with low balance May 28, 2021

chappjc requested changes Jun 8, 2021

View reviewed changes

chappjc reviewed Jun 9, 2021

View reviewed changes

client/webserver/api.go Outdated Show resolved Hide resolved

chappjc reviewed Jun 9, 2021

View reviewed changes

client/webserver/api.go Outdated Show resolved Hide resolved

vctt94 added 2 commits June 14, 2021 12:14

chapp review fixes

26e7016

go directly to registration failed if no funds

22b4aad

vctt94 force-pushed the add-no-funds-form branch from f41c32a to 22b4aad Compare June 14, 2021 15:16

JoeGruffins approved these changes Jun 15, 2021

View reviewed changes

chappjc approved these changes Jul 6, 2021

View reviewed changes

chappjc merged commit cb0e295 into decred:master Jul 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add form when failing to register with low balance #1092

Add form when failing to register with low balance #1092

vctt94 commented May 17, 2021 •

edited by chappjc

Loading

JoeGruffins left a comment

vctt94 commented May 18, 2021

JoeGruffins left a comment

vctt94 commented May 19, 2021

JoeGruffins May 20, 2021

chappjc left a comment

chappjc May 27, 2021 •

edited

Loading

vctt94 commented Jun 7, 2021

chappjc Jun 8, 2021

vctt94 commented Jun 9, 2021

chappjc commented Jun 9, 2021 •

edited

Loading

vctt94 commented Jun 14, 2021

JoeGruffins left a comment

chappjc left a comment

chappjc Jul 6, 2021

Add form when failing to register with low balance #1092

Add form when failing to register with low balance #1092

Conversation

vctt94 commented May 17, 2021 • edited by chappjc Loading

JoeGruffins left a comment

Choose a reason for hiding this comment

vctt94 commented May 18, 2021

JoeGruffins left a comment

Choose a reason for hiding this comment

vctt94 commented May 19, 2021

JoeGruffins May 20, 2021

Choose a reason for hiding this comment

chappjc left a comment

Choose a reason for hiding this comment

chappjc May 27, 2021 • edited Loading

Choose a reason for hiding this comment

vctt94 commented Jun 7, 2021

chappjc Jun 8, 2021

Choose a reason for hiding this comment

vctt94 commented Jun 9, 2021

chappjc commented Jun 9, 2021 • edited Loading

vctt94 commented Jun 14, 2021

JoeGruffins left a comment

Choose a reason for hiding this comment

chappjc left a comment

Choose a reason for hiding this comment

chappjc Jul 6, 2021

Choose a reason for hiding this comment

vctt94 commented May 17, 2021 •

edited by chappjc

Loading

chappjc May 27, 2021 •

edited

Loading

chappjc commented Jun 9, 2021 •

edited

Loading