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

Increasing fees doesn't check account balance/pending fees/pending STX transfers #3109

Closed
314159265359879 opened this issue Feb 13, 2023 · 9 comments
Assignees
Labels
bug Functionality broken bug-p2 Critical functionality broken for few users, with no clear workarounds effort:small Expected to take up to 1 day of integration work

Comments

@314159265359879
Copy link
Contributor

We are seeing an increased number of forever pending transactions such as this (screenshot):

I think a scenario such as the following plays out:
New users sends three transactions. A pre-order for a domain. A transfer of STX and something else. After waiting two minutes, they believe it is taking to long so they increase the fee on the first transaction to something like 2 STX. Now there is not enough left to cover the fee and transfer of STX (second transaction). The users sends more transaction and they all remain pending for hours... and hours eventually they ask for help.

I am wondering if we can do more to check balance and include the current fees needed to cover pending transactions as well as STX to cover pending transfers out of the wallet so we can error "not enough STX balance to increase the fee to that value" or similar. Instead of getting wallets stuck like this.

image

I think this is a P2 because it especially happens to new users. More experienced Stackers will know transaction times on Stacks are (currently) longer than on Ethereum. Although in case of real congestion this can affect experienced users too.

@314159265359879 314159265359879 added bug Functionality broken bug-p2 Critical functionality broken for few users, with no clear workarounds labels Feb 13, 2023
@314159265359879
Copy link
Contributor Author

314159265359879 commented Feb 15, 2023

Scenario checked
image
I have transaction in microblocks that reduce the wallet balance to less then 1.0 STX.
I was still able to send a new transaction with a fee of 1 STX. I believe this should also fail.

The max button doesn't take into account pending transfers, I believe it should at least take into account pending transfers out (reduced actually available balance) as well as fees pending:
image

@314159265359879
Copy link
Contributor Author

Currently there are a lot of transactions for BNS names. We may even want to consider taking into account the most common 2 STX needed for a pre-order of a BNS domain in the .btc namespace. I am not sure if it is viable to cover all namespaces or pending transactions more generally, perhaps if we go off the post-conditions of these pending transactions?
image

image

Or would this set us up for other issues?

@Hero-Gamer
Copy link

Still happening: #3338 (comment)
please fix this..

@harrybasra95
Copy link

Hi @314159265359879, I did some research on the issue.

We are using a hook called useCalculateMaxBitcoinSpend in the file use-calculate-max-spend to get the max spending amount and we are not subtracting the pending amount from that.

There is another hook called useBitcoinPendingTransactionsBalance transactions-by-address which we can use to get the balance of the pending transactions and then subtract it.

One more thing I would like to add, we also have a list of submitted transactions, I think their balance should also be taken into account while calculating the max amount.

@markmhendrickson
Copy link
Collaborator

Assigning @alter-eggo to take a look since I believe he's worked on related code recently

@markmhendrickson markmhendrickson moved this from Bugs backlog to Assigned in Hiro Wallet (DEPRECATED) May 16, 2023
@harrybasra95
Copy link

@harrybasra95 Sure

@alter-eggo alter-eggo added the effort:small Expected to take up to 1 day of integration work label Jun 21, 2023
@alter-eggo alter-eggo moved this from Assigned to WIP in Hiro Wallet (DEPRECATED) Jun 29, 2023
@alter-eggo
Copy link
Contributor

@314159265359879 wonder if this is still relevant. seems like previous work on available balances fixed this

@314159265359879
Copy link
Contributor Author

@alter-eggo, yes #3525 (pending outgoing STX's transfers) and another fix to subtract pending fees, prevent a lot of cases already.

There is still one issue open that should be addressed to catch all of the most common cases (subtract STX transfers due to contract calls based on exact post-conditions):
#3568

Lets proceed there?

@alter-eggo
Copy link
Contributor

yeah, let's proceed there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Functionality broken bug-p2 Critical functionality broken for few users, with no clear workarounds effort:small Expected to take up to 1 day of integration work
Projects
None yet
Development

No branches or pull requests

5 participants