Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check account balance before publishing tx #87

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

brusherru
Copy link
Member

Before allowing to publish transaction it checks for the projected account balance.

image

It checks the sufficient balance for:

  • any tx: to pay a gas fee from current account
  • spend: to have spend amount + gas fee available
  • drain: to have enough funds on vault account (if it is "local" address, otherwise it allows to send a tx)

It also shows a bit different red text on the confirmation page in case if some of the conditions does not pass:

  • Waiting for gas estimation...
  • You have insufficient funds to pay for gas
  • You have insufficient funds to send this amount
  • The vault has insufficient funds to be drained

@brusherru brusherru self-assigned this Sep 16, 2024
Copy link

You can preview the changes at : https://ca2765ac.smapp-lite-prod.pages.dev

@monikasmolarek monikasmolarek linked an issue Sep 17, 2024 that may be closed by this pull request
Copy link
Collaborator

@monikasmolarek monikasmolarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please double check:

fill the details of the tx (exceeding the balance) and hit next,
it shows properly that the tx cannot be processed, all good here
hit back
use delete/backspace to remove input in the gas price field
it throws:
Screenshot 2024-09-17 at 20 35 00

I checked the prod and it's all good there, so it apparently is smth with this PR

Screenshot 2024-09-17 at 20 41 19

Comment on lines +207 to +211
if (BigInt(acc.projected.balance) < fee) {
return 'You have insufficient funds to pay for gas';
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was your intention to display the "insufficient funds for gas" in case the balance is enough to cover the tx amount but the gas fee exceeds the available balance?
Cause now these lines work only in case the gas is the only thing to pay (spawn), but in the regular tx only the "insufficient funds to send this amount" is displayed.
If it's intended to be like that then it's ok, IMHO no need to overcomplicate these alerts, it's just that the PR description left me a bit hesitating about the intent

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got the same after some playing with numbers. Initially it worked.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was your intention to display the "insufficient funds for gas" in case the balance is enough to cover the tx amount but the gas fee exceeds the available balance?

Well, it will be shown for any transaction, when the balance is not enough to pay for a gas. Including the spend tx.
For example, try to send 0 SMH with 10000 Smidge / unit of gas price from the same test account (with 0.001 SMH), and you'll get this error message.

However, for spend tx there is additional condition, that checks that balance is equal or greater than amount + gas. And if it is not — then it will show "insufficient funds to send this amount". Basically it means that you have enough to pay for gas, but you should send less :)

@brusherru brusherru force-pushed the tweak-check-insufficient-funds branch from 8d9e076 to 6900f8f Compare September 18, 2024 17:56
@brusherru
Copy link
Member Author

@monikasmolarek the bug is fixed. Thanks!

Copy link

You can preview the changes at : https://c3b444e5.smapp-lite-prod.pages.dev

Copy link
Collaborator

@monikasmolarek monikasmolarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@brusherru brusherru merged commit 533a26e into main Sep 18, 2024
1 check passed
@brusherru brusherru deleted the tweak-check-insufficient-funds branch September 18, 2024 18:14
@dioexul
Copy link

dioexul commented Sep 18, 2024

Cannot get the same error.

Minor things
There are 2 messages:

  • You have insufficient funds to send this amount
  • You have insufficient funds to pay for gas

And Wallet shows the first one when amount is enough, but gas fee is bigger than the rest of the balance.
Balance 19.7
Funds to send 19
Gas fee: 3.6

And message "insufficient funds to pay for gas" appears only in the case when Gas fee is bigger than balance.
One of the option is to have only the first message for all cases.

Gas price also allows (and keeps) values like 00100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check dynamically tx amount+cost VS available balance
3 participants