-
Notifications
You must be signed in to change notification settings - Fork 671
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
Include transactions with insufficient balance #3639
Comments
I think this is a great idea. The current UX of pending transactions with insufficient balance is pretty bad. In the worst case, the transaction actually ends up in the miner/mempool blacklist and the "pending" transaction is just stuck for 48 hours or until it is RBFed. |
I remember hearing in a status call months ago that even successfully mined transactions are not removed from the mempool, so that they'd be available if and when a re-org happened to be mined on the new fork. |
Correct.
Agree in principle, but isn't this a breaking change? To what extent can this be mitigated by better wallet support for e.g. allowing users to seamlessly RBF their transaction with a different one? |
Yep, this would be a breaking change and would need to be included in a consensus change to activate. Regarding whether or not this could be mitigated by better wallet support, the answer there is theoretically yes, but for whatever reason, this has been challenging for wallets to do. Perhaps wallet developers should weigh in on this? |
Tagging @kyranjamie @yknl @markmhx to chime in from wallet perspective |
The majority of users don't understand what is RBF or that you can even replace a transaction that is pending with a completely different one. All of the wallets already support replacing a pending transaction by customizing the nonce. |
The wallet prevents transferring less than your balance, but as Brice says there are plenty of edge cases that might cause this. Do we have any sense how often users run into this problem? Assuming it's infrequent, but debilitating when it does happen. Is there more the wallet can do to prevent this from happening in the first place? As far as fixing the problem, is best thing to do is RBFing to a legit dust transaction to a null/change address? I wonder if this might be a suitable explorer feature? Allowing the user to diagnose stuck transactions, and initiating an extension wallet transaction that'll get it mined. |
What if the node offered an RPC endpoint that lets you query mempool transactions by sender address (or sponsor address), and returned the height of the canonical Stacks tip at the time the transaction was accepted into the mempool? We already track (or can efficiently obtain) this information. From there, the wallet can calculate the "confirmation age" of the transaction, and uses that to deduce that the transaction is stale. The confirmation age of a transaction is the difference between the height of the canonical chain tip when the transaction arrived, and whatever the current canonical chain tip height is. The wallet could use the confirmation age to seamlessly RBF stuck transactions. The wallet would query We'd offer a similar endpoint for sponsors, for the same reason. |
I agree, that the suggested endpoint could be useful to wallets to help avoid this issue, but I would argue for a principle of never leaving a transaction with the next nonce in the mempool unprocessed. The miner is doing work to check whether this transaction is valid or not, so it is perfectly acceptable to take the fee. It's not worth a consensus-change just for this, but it could be included with one of the planned changes. |
Adding context to these two points. The current implementation does this to not only save the user a fee, but also to avoid wasting block space. Once a transaction is mined, it's part of the chainstate forever -- every current and future node must evaluate and process it. I think we should be investigating ways to help miners minimize the work they spend in detecting these kinds of failures. For example, at contract analysis time we could determine and cache the set of potentially-reachable token transfer calls from all public functions. Then, when we process a contract-call transaction, we can check to see if (a) the reachable set of token transfers is reasonably small, and (b) the sender's token balances are all sufficiently high that the entire reachable set of token transfers are guaranteed to succeed. Transactions for which both points are true could be prioritized for block inclusion over those that require subsequent analysis. |
Thanks @obycode for opening up this discussion! Just to be clear here about the general UX if we were to handle things wallet-side:
As for the detection scheme, instead of querying against confirmation age, might it be more effective for the wallet to query mempool transactions and compare their intended transfer amounts to the user's known balance, regardless of age? That seems like perhaps a more direct way to determine the need for RBF (due to incongruent send vs. current balance amounts) than guessing there is a need due to staleness (wherein false positives might arise due to congestion)? I'm also curious if @314159265359879 has seen this scenario much on the support side given the wallet does generally protect users from sending more than their balance. Though I suspect that users who are sending many transactions in a row might find that the wallet doesn't accurately calculate their balance in time for subsequent ones given delays on the API side due to mempool processing? Overall, faced with the need to provide a manual option to RBF here (however better we prompt the user automatically), I'd lean towards having the miners automatically include / clean up failing transactions for the users instead. That would save users the need for manual intervention (which adds friction and risks only partial adoption), even if it comes at the cost of increased block space usage. |
It is especially in cases when a users sends transactions in rapid succession. And some cases that are not yet covered in the wallet to protect the user from double spending. I have long thought that the best solution is to include these transaction in a block and make them fail with an "insufficient funds error". Because:
@jcnelson by not including them you can save the user some money, and you can save the network some blockspace (for now?). Saving block space is likely just a temporary advantage because when the network gets the traction we expect the blocks will be full regardless of how many (potential) double spend transactions are mined to fail. Full blocks lead to a functioning fee market and higher fees that make the mistakes more expensive and users more motivated to prevent them. I do not think this needs a consensus breaking change right away but I am in favor of including it in a future hard fork as @obycode suggested. Some of the work done on the wallet side to prevent these troublesome transactions
That will cover most cases but not all, and the edge cases (sending in quick succession, with another wallet, with a dapp that doesn't specify post conditions exactly, etc) always pop up when there are a lot of new users using the network. I think this solution could help give those new users a better first experience using the Stacks network. |
Is your feature request related to a problem? Please describe.
Transactions that will fail due to an insufficient balance (ex. I transfer 10 STX but I only have 8 STX), are left pending in the mempool until they are dropped after 256 blocks. This causes a lot of confusion from users, because they don't know why their transaction is pending, and also holding up any future transactions (with higher nonces).
Describe the solution you'd like
Instead of just leaving these transactions pending, I think the miner should include them in a block as a failing transaction. I believe the current implementation purposely does not do this so that it does not charge the fee for a transaction that is definitely going to fail, basically trying to save the sender from their mistake. In practice, the pain of this pending transaction is much worse than the pain of the wasted fee. A somewhat related change was implemented in 2.1, #3213.
Describe alternatives you've considered
Leaving it as-is has proven to cause a lot of confusion from users.
Additional context
See discussions in Discord.
The text was updated successfully, but these errors were encountered: