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

Swap marked as dropped but it was successful #2942

Open
jagodarybacka opened this issue Jan 30, 2023 · 4 comments
Open

Swap marked as dropped but it was successful #2942

jagodarybacka opened this issue Jan 30, 2023 · 4 comments
Labels
Priority: Medium Medium monetary / growth impact, long time frame for completion Status: Pending Needs further attention and categorization Type: Bug Something isn't working

Comments

@jagodarybacka
Copy link
Contributor

Swapped on Optimism, CRV -> AAVE
https://optimistic.etherscan.io/tx/0xf5abee267fe5e4940b574ccd08ea0bf91bd8ec45b48a01afb70036a3fb8e673e
Pasted Graphic 1

@jagodarybacka jagodarybacka added Type: Bug Something isn't working Status: Pending Needs further attention and categorization Priority: Medium Medium monetary / growth impact, long time frame for completion labels Jan 30, 2023
@jagodarybacka
Copy link
Contributor Author

another one: in-wallet approval on Polygon
image

@tallysam tallysam added this to the Nonce management milestone Feb 15, 2023
@tallysam
Copy link

tallysam commented Mar 1, 2023

this is difficult to reproduce.

@andreachapman
Copy link
Contributor

I've also experienced this issue where Taho is reporting that the trxn was dropped but if you follow the scan link, it shows as successful. My latest example, with the evidence following, was not a swap though, it was a send.

Here's the trxn on the scan site and it says it was successful: https://snowtrace.io//tx/0x8f14e338a70e5f9bbd496778b1bda4400839bdce7ac2be42e2f55646d5f111e1

Here's a screenshot from the extension:
Screenshot 2023-08-10 at 9 45 26 AM

Here's a warning in console that corresponded to the time of the trxn:
Screenshot 2023-08-10 at 9 39 55 AM

And here's the log file I exported from the extension right after this happened (13:39 is the time):
DroppedTxnLogs2023-08-10T09_39_59-04_00.txt

@Shadowfiend
Copy link
Contributor

What seems to be happening here is that after submission, we poll the remote node for our transaction and don't find it. Since we don't find it, we assume it was dropped—but we do that immediately, instead of retrying a handful of times or checking some other way.

I think part of the issue here is that eth_getTransaction is not pushed directly to Alchemy, while eth_send*Transaction calls are, which means that we won't check the node we submitted to—which should definitely have the transaction if it wasn't dropped—but instead we check a different node in the network. Since pending transactions can take time to propagate, and sometimes won't propagate at all until they're actually included in a block, we end up in trouble.

A couple of ways we can approach here:

  • We can retry some number of times. Downside is we don't show transactions that actually were dropped as quickly; also, depending on how long the transaction takes to get confirmed, etc, we could still end up marking some as dropped.
  • We can mark as dropped immediately, but continue to check dropped transactions that are the latest nonce periodically (e.g. with the regular transaction alarms) until there is a confirmed transaction with the same nonce OR we find that this “dropped” transaction was actually confirmed.
  • We always use Alchemy for eth_getTransaction. Depending on how many of these calls we do, could get expensive.
  • We always use Alchemy for these particular lookups (through some additional functionality in SerialFallbackProvider). I hate adding more functionality there though, that class is gnarly.
  • If we fail to find it normally, we fall back on forcing Alchemy usage (again would require more code in SerialFallbackProvider).
  • If we fail to find it normally, we do a direct Alchemy API call using their mempool stuff (either needs more code in SerialFallbackProvider or some separate mechanism that might not work for every chain).

The “continuing to check” approach seems most straightforward, as I think it's just a matter of adding an additional conditional before the cleanup at https://github.com/tahowallet/extension/blob/main/background/services/chain/index.ts#L1375-L1396 and re-enqueueing. If the nonce on the transaction is ahead of or same as the latest nonce, we don't stop querying for it. If the transaction is older than X (say 2 minutes or something), we mark it as dropped and release the nonce but continue querying. If we see a transaction with the same or greater nonce show up, we stop querying this transaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium Medium monetary / growth impact, long time frame for completion Status: Pending Needs further attention and categorization Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants