"Invalid number" (no gas limit is set by default) instead of insufficient balance error at attempt to register ENS name

[churik]

Description

Type: Bug
Summary: if you don't have enough SNT to register new name - instead of appropriate error application couldn't set Network fee (Gas Limit is always empty)
If you set it Gas Limit manually, you can proceed and sign transaction - "Intrisic gas too low" error.
Theoretically, it can be reproduced in any DApp if token balance is not validated on DApp side (tried in AirSwap and UniSwap, balance is validated.)

Expected behavior

"Insufficient balance" error or button Register is disabled when you don't have enough SNT (so you can't open transaction screen)

Actual behavior

Reproduction

• Open Status
• Create new account
• Try to register new ENS name

Additional Information

• Status version: nightly 01.08.2019
• Operating System: Android, iOS

[asemiankevich]

@rachelhamlin

[flexsurfer]

@rachelhamlin @hesterbruikman @andmironov @errorists in the current case there is an error in the contract because there is not enough SNT, and contract can't be run, and gas can't be estimated, so we need to show a proper message when gas can't be estimated and there is an error in the contract and we know transaction will be failed even if user provide gas manually

[hesterbruikman]

@flexsurfer thanks for tagging!

What can the user do in this situation?

It sounds like:
The issue might resolved if the user somehow buys more ETH
Does this solve the issue or might the contract error also occur when there is enough ETH for gas available?

[flexsurfer]

@hesterbruikman this current case not related to ETH it's about a contract execution, contract can't be executed, and we don't know the exact reason, yeah for this current case we could guess that there is not enought SNT and if user buys more SNT it will work, but we need more general message for all cases, something like "gas cannot be estimated because of an error during contract execution"

[hesterbruikman]

Thanks for clarifying. Sounds like we're looking for a bandaid:/ In this case the user also can't change the amount as it's specified by the contract.

What causes the contract error?

For a message I suggest:
"Something's wrong with the contract. Make sure you have enough assets and try again"

[flexsurfer]

What causes the contract error?

it may be some condition in the contract, for example, permissions, or balance check, or smth else, wrong parameters passed to contract from dapp for example etc

[hesterbruikman]

If you think the above error message works, I suggest we go with that for now. Next to that I'll make a note in our design backlog on Wrike. I think eventually we want a different pattern/screen in case of a full blown contract error. Like an error state version of the Send transaction overview screen. Now it seems the error is gas estimate, but that's a symptom, the problem is the contract itself not performing required steps to produce an accurate/useful overview screen.

[rachelhamlin]

Is it possible that the user can try again and succeed, or is the contract going to fail repeatedly if this error is hit @flexsurfer? It sounds like a really variable underlying cause; ditto Hester.

Also, is @3esmit maybe able to look into why this is happening?

[flexsurfer]

@rachelhamlin the contract going to fail repeatedly if this error is hit, and we know why, it's normal behaviour, we just need to show a proper error message to a user instead of "invalid number"

[hesterbruikman]

fyi: https://www.wrike.com/open.htm?id=389379190

[andmironov]

Hey! What about informing about a "special" transaction like this?

On the left – not enough ETH to pay the network fee error. On the right – a special transaction with enough ETH to pay the network fee.

Also, I'm not sure I've understood the problem 100% :-)

[andmironov]

UPD

1. Not enough ETH to pay for the network fee;
2. Same as 1 but user taps on the (!) icon;
3. A special transaction

[andmironov]

Ok, had a chat with @flexsurfer and he explained it to me. So the error while calculating the transaction fee is an indicator of a possible error in the contract (we can't know what exactly is the error) and means that the user most likely should not send this transaction. But they still can. So this is what I suggest:

wdyt?

[flexsurfer]

@andmironov looks good, but to sign anyway gas should be set first manually

[andmironov]

ok got it! Also, made a few changes and have a question:
since it takes time to check if the network fee is possible to calculate, should there be a loading state? Take a loot, from left to right.

[flexsurfer]

looks good, but complex :)

[andmironov]

just as life is...

[andmironov]

UPD

I just realized after talking to Simon and discussing the transaction overview from another perspective, that a transaction interaction should be a special type of transaction, with not too many fields. This makes it look cleaner also

[rachelhamlin]

Love the gate keeping with Set custom fee requiring action @andmironov. And I understand the problem much more clearly now too. :) Edit: Also prefer the pared back fields for this special Tx.

Might want to suggest a copy tweak, but first...

@flexsurfer can you help me to better understand the risk involved in signing? Assuming the possible range of errors is mostly buggy things like balance check not working, wrong parameters passed, etc...is the risk simply that the user wastes gas on a Tx that is likely to fail? Or more seriously, perhaps the contract has holes in it that could lead to bigger losses?

[andmironov]

Also, the network will be shown only if it's not Mainnet, which means one less field.
And another thing: when it's a transaction interaction, it might better be "Reject" instead of "Cancel"

[3esmit]

Seems like the issue is no ETH in user selected/active account?

[3esmit]

I tested this and I get the error com.facebook.react.bridge.NoSuchKeyException:backgroundColor when I tap the "invalid number" error.

[3esmit]

The "invalid number" might be a side effect of trying to make an invalid transaction, this can have multiple causes:

• no ether in balance
• infinite gas (failing) transaction

In the case of ENS Usernams seems like we have a problem as the dapp is trying a transaction against 0xDB5ac1a559b02E12F29fC0eC0e37Be8E046DEF49 (UsernameRegistrar) instead of 0x744d70FDBE2Ba4CF95131626614a1763DF805B9E (SNT)

This is the correct usage of UsernameRegistrar:
SNT.approveAndCall(UsernameRegistrar,10*10^18,UsernameRegistrar.methods.register(...params).encodeABI())
See:
https://github.com/status-im/ens-usernames/blob/77d9394d21a5b6213902473b7a16d62a41d9cd09/test/usernameregistrar.spec.js#L342-L444

Some checks must be done before allowing the button to be pressed:

• user have 10 SNT in account
• user have allowed/entered public key (why register username with no pubkey? perhaps only warn but allow?)

Note: input filed for address (payment address?) dont need to be showed, because its optional (not used?), perhaps show only in an advanced form?

[3esmit]

Sorry, my mistake, its actually using the correct call. I sucessfully registered an username: https://etherscan.io/tx/0x7dade245d085889da1f1b62f72472e09e089c25181838168a4cc275691a4e402

So to get rid off the "Invalid Number" error I needed to have 10 SNT on my account.

So perhaps we need only to threat errors on the case user dont have SNT or sufficient ETH/GAS before allowing the button to be pressed.

@andmironov for the signing UI suggested I also agree, it should show appropriate error. "could not calculate fee" means that something is really bad about that transaction. so I think that the appropriate error message should be what it actually means: "transaction always fails or exceeds gas limit". Advanced users should be able to force sign/submit a transaction that wasn't able to calculate the gas limit, by manually specifying the transaction gas limit - as you suggested.

[rachelhamlin]

@3esmit what about these potential failures mentioned by Andrey?

it may be some condition in the contract, for example, permissions, or balance check, or smth else, wrong parameters passed to contract from dapp for example etc

There are more options than just insufficient ETH/SNT, no?

[3esmit]

@rachelhamlin Yes, but all them render the same result: not possible o calculate gas, transaction fails if included in a block.

All these errors (some condition in the contract) will be shown as "unable to calculate gas fee".

In case ETH transfer, a transaction with no enough balance will be ignored by nodes (and if included in a block would render an invalid block), but in case of ERC20, a transaction with no enough erc20 will not able to calculate gas, and in case included in a block, will render a valid block but will fail the individual transaction with "invalid jump" or "no enough gas".

[rachelhamlin]

Okay, got it. So this solution looks pretty good.

Perhaps a more accurate + succinct error message flow would be:

The contract can't calculate the network fee. Set a custom network fee to sign at your own risk. --> Set custom fee --> This contract has an error in it. Sign at your own risk using custom network fee.

@3esmit it's accurate to say "the contract has an error in it" right?

@andmironov wdyt?

[andmironov]

looks good to me! @rachelhamlin will update the screens with new copy

[andmironov]

Note: the tooltip next to the (!) icon is displayed when the icon is tapped. It disappears when the icon is tapped again. Here's how the animation could look https://www.dropbox.com/s/o21lf4uppizzr7e/err.mov?dl=0

[andmironov]

~ Figma: https://www.figma.com/file/XUehMnhyD1FGcWzvGz6SXqvh/Mobile-wallet?node-id=3864%3A33253

[3esmit]

it's accurate to say "the contract has an error in it" right?

In some cases yes, but for all would be accurate to say "transaction probably will fail", or if you want to say that it has an error, would be always on the transaction, because we cannot test/check if the error is because of a badly programmed contract logic or because of badly set parameters in the transaction.

This message could also appear if someone sets the gas limit of transaction to a value lower then the gas estimated (if it was able to estimate in first place).

[rachelhamlin]

Got it! I think "transaction will probably fail" is a much better message for the second error screen then. Left a comment to that effect in Figma, @andmironov - let me know if you disagree.

[rachelhamlin]

Closing this issue in favor of #9037, which combines this and #8485 into one.