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

Transaction Failed: known transaction #3669

Closed
kumavis opened this issue Mar 22, 2018 · 15 comments
Closed

Transaction Failed: known transaction #3669

kumavis opened this issue Mar 22, 2018 · 15 comments
Labels

Comments

@kumavis
Copy link
Member

kumavis commented Mar 22, 2018

https://sentry.io/metamask/metamask/issues/500528076/

Sometimes, on initial send of tx, we get a "Transaction Failed: known transaction" error. Investigation suggests that this is unlikely an error on our side. The transaction does make it onchain.

Hypothesis: Infura's has a sort of broadcast mechanism for sending txs to all of its nodes. Perhaps tx is being gossiped to a node before this broadcast, and the broadcast results in an error, which is returned to us.

Suggestion: Ignore this error on submit, and continue as everything is good. The tx will be confirmed

@kumavis kumavis added type-bug P1-asap area-provider Relating to the provider module. labels Mar 22, 2018
@kumavis kumavis added this to the April 10 Sprint milestone Mar 22, 2018
@kumavis kumavis added the area-nonce Relating to tx nonce logic label Mar 22, 2018
@frankiebee frankiebee self-assigned this Mar 26, 2018
@danjm
Copy link
Contributor

danjm commented Mar 27, 2018

Here are steps that I was able to take to reproduce this error:
(1) switch to mainnet
(2) send a transaction with the lowest possible gas price (0.1 GWEI)
(3) wipe all your data (e.g. remove metamask from your extensions and then re-add it)
(4) restore the account you just sent the transaction from and switch to mainnet
(5) send another transaction with the same gas price

If you do the above steps, but with a different gas in step (5) (that is less than 1.1 * the gas price of the first tx) you'll see an error related the the gas price being too low.

I investigated this when working on the retry tx feature. It seems the source of the errors reproduce by the steps above have to do with that fact that the nonce tracker's getNonceLock() method gets pending txs from state (via getPendingTransactions() -> txStateManager.getPendingTransactions() -> getFilteredTxList() -> getTxsByMetaData() -> getTxList() -> getFullTxList() -> this.store.getState().transactions)

@danfinlay
Copy link
Contributor

We should notify Infura (@maurycy) about this, since they probably shouldn't throw an error when a transaction is successfully broadcast.

@maurycy
Copy link

maurycy commented May 21, 2018

@danfinlay @maurycyp

@danfinlay
Copy link
Contributor

Lol, sorry about that @maurycy!!

@ryanschneider
Copy link

Here are steps that I was able to take to reproduce this error:
(1) switch to mainnet
(2) send a transaction with the lowest possible gas price (0.1 GWEI)
(3) wipe all your data (e.g. remove metamask from your extensions and then re-add it)
(4) restore the account you just sent the transaction from and switch to mainnet
(5) send another transaction with the same gas price

So I think the issue is that the tx in (2) is never mined (at least in the current members pool environment where there are tons of higher priced txs for miners to use instead). So, when you do (5), the account nonce is still that same as it was before sending (2). I assume MM keeps nonce locally for pending txs, but that state is wiped in (3).

So in other words, I don't think those repo steps indicate an issue on our end. Is there another case where this has been repro'd?

We have been seeing some slowdown processing sendRawTx recently, I'm working on a geth patch now to address that. I could see that slowdown leading to race conditions.

IMO geth shouldn't really return an error in this case (since the tx is valid, IMO it should just return the hash), but we want to maintain compatibility with geth behavior. If I get time today I'll open a ticket with them to see what their thoughts are on changing this behavior.

@danfinlay
Copy link
Contributor

Thanks for checking that out, @ryanschneider.

Maybe the problem was just that if the retry has all the same params (and same nonce), it would produce the same tx, and that's why it would be a duplicate.

We should probably just treat a "Duplicate tx" response as a success, since it means this tx has been broadcast, so we can fix this on our end in that way.

@bdresser bdresser added P2-sooner and removed P1-asap labels Jun 6, 2018
@kumavis
Copy link
Member Author

kumavis commented Jun 6, 2018

@bdresser bdresser removed this from the Sprint 03: April 10 Sprint milestone Jun 6, 2018
@kumavis kumavis removed the area-provider Relating to the provider module. label Oct 10, 2018
@nyetwurk
Copy link

I'm seeing this on a geth light node on localhost:8545
It is not restricted to infura

@nyetwurk
Copy link

Transaction never makes it on chain or to pending. It has to be canceled via a non-metamask client (e.g. mycrypto) or metamask stays wedged with pending transactions.

@frankiebee
Copy link
Contributor

@nyetwurk do you try canceling the transaction from within metamask?

@nyetwurk
Copy link

Yes. It doesn't work. I have to cancel it outside of metamask (e.g. myCrypto).

@nyetwurk
Copy link

From what I can tell using a light node as a reference, "Transaction Failed: known transaction" is printed when a transaction is stuck in the node's txpool and MM continuously tries to resend the transaction. Canceling the transaction outside of MM causes MM to notice the transaction is dropped, so it stops trying to resend it.

@nyetwurk
Copy link

this is what it looks like in txpool

> txpool.inspect
{
  pending: {
    0x....: {
      39: "...: 0 wei + 21000 gas × 19000000000 wei",
      40: "...: 0 wei + 21000 gas × 10000000000 wei"
    }
  },
  queued: {}
}
> txpool.status
{
  pending: 2,
  queued: 0
}
> eth.getTransactionCount('0x...')
39
> 

@nyetwurk
Copy link

I think I see the problem. I am using ledger nano with metamask and when I inspect the transaction it has the wrong from address, so the nonce is way off.

@jennypollack
Copy link
Contributor

ledger wrong address, same as #7845
see https://twitter.com/metamask_io/status/1235648375955107842 for more info

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

No branches or pull requests

9 participants