Metamask-extension: Improve nonce tracking

[danfinlay]

During the melonport launch, a user transmitted several txs that failed for different reasons at once. The result was that their MetaMask account was put into an incorrect nonce state.

We need to improve our nonce updating logic. Maybe querying the network for an updated nonce before each tx transmission?

[kumavis]

Maybe querying the network for an updated nonce before each tx transmission?

I wish it were that easy. infura switching to single node will likely open up more options for us here

[danfinlay]

Is that just an issue when emitting several txs in series, where the nodes don't have time to synchronize?

In the short term, I guess we have some bug in how we decrement the nonce after tx failures.

[kumavis]

yeah need more tests around the nonce tracker
one reason why we have fewer tests than we would like is bc the damn block polling is built in and so makes writing tests a pain. gah. need to attack that beast

[danfinlay]

the getTransactionCount method we use to retrieve a nonce accepts a pending parameter, which factors in the number of pending transactions! That's kindof interesting.

I wonder if a single over-nonced tx would then throw off the whole flow, tricking the client into incrementing the nonce to the wrong place in the series...

[tomusdrw]

For Parity it would be best to use parity_nextNonce, the definition of pending state as implemented in Parity doesn't take the whole queue into consideration (just transactions in the pending block and only if the node is actually sealing blocks).

[danfinlay]

Is there a chance this feature will be implemented by other clients with a common name? Our users like being able to point at whatever RPC they are running, @tomusdrw.

[tomusdrw]

Some discussions on RPC API governance and cross-client tests suites / specs is ongoing here https://github.com/ethereum/EIPs/issues/217 I really hope that it will be the case in a near future.

I've also created an issue in Parity repo to discuss different definitions/exceptions for pending state: https://github.com/paritytech/parity/issues/5014

[danfinlay]

Glad you're thinking about the same things.

Yeah the RPC API governance will be nice to help coordinate, but we shouldn't use it as a reason to avoid coordinating in the meanwhile. It could take a while to have something good working there.

[danfinlay]

If we added nonces to the tx history, it might be easier to visually debug these issues.

[danfinlay]

I just helped debug a user on reddit who had another sort of nonce issue:

They had the same account on both metamask and MEW, and after sending a tx on MEW, the next metamask TX failed, because it had the same nonce.

Maybe if we polled for updated nonces more frequently, we could alleviate his issue.

[rickfeiner]

flyswatter, I am experiencing the same issue that you just described. I get a "Nonce too low" error when trying to submit a transaction from metamask after submitting transactions from the same account in geth.

Here are the reproduction steps:

1. Import a "loose" account into Metamask via JSON import using geth keys.
2. Make a transaction with geth using the account you just imported into Metamask
3. Attempt to make a Metamask transaction using the imported account

Result: "Nonce too low" error.

[danfinlay]

Thanks for the report, @rickfeiner, I hope you found that you can restart your browser to fix the issue. If not, at worst, you might need to uninstall & reinstall MetaMask.

[rickfeiner]

I've found that switching to test-net from main-net (or vise-versa) and then back again, resolves the problem.

[danfinlay]

Good tip! Maybe we should just add a reset nonce button until we have this well sorted out.

[kumavis]

nonce tracking plan:

• [ ] reduce possibility of unreasonable txs
  
  
  
  • [x] low gasPrice
  
  
  • [ ] high gasLimit P1
  
  
  • disallow txs with gas limit higher than 95% of the block gas limit
  
  
  • validation message should indicate current % of block gas limit and/or max gasLimit
  
  
  • validation message should indicate that its due to "fluxuating block gas limits"
  
  
  • [ ] low gasLimit P2
  
  
  • i think the lowest possible gasLimit is 21000?
  
  
• [ ] track txs
  
  
  
  • [ ] incomming external txs P3
  
  
  • here by external i mean key reused by other app like 2nd metamask or mew
  
  
  • [ ] known pending txs
  
  
• [ ] handle failure
  
  
  
  • [ ] consider pending txs unknown to node as failed P3
  
  
  • [ ] provide "retry" button P3
  
  
• [ ] nonce tracker
  
  
  
  • [ ] should calculate nonce based on all pending txs
  
  

[danfinlay]

Retry button = edit gas settings and resubmit?

[kumavis]

Retry button = edit gas settings and resubmit?

yes and with a fresh nonce. should just drop you on the conf tx page with the same details filled in.

[danfinlay]

This problem could have been simplified/avoided sooner if we'd shown support for this Parity feature:

https://github.com/paritytech/parity/issues/5014

Head down, chime in, and then hopefully we can get the Geth team to do the same.

It's probably a lot slower to wait for that feature than to just fix this without it, though.

[danfinlay]

Here are some action steps me and Frankie came up with regarding this issue:

step 0:
remove retry limitation on pending txs, retry on block.

step 1:
People have a big ugly queue of failed txs.
Those error’d txs should have the error icon & show failed.

step 2:
calculate nonce based on local pending txs w/o error state.

step 2.5:
Reset nonce switch to cancel pending: #1668

step 3:
Clicking a pending tx goes to tx approval screen,
change “ACCEPT” to “SUBMIT” or “SEND”.

step 4: allow editing nonce via an advanced thing on the tx approval screen.

[arcalinea]

A reset nonce button would be great. I just created this problem for myself on testrpc while developing a dapp that uses metamask, now have a pile of failed txs and would like to just clear the state.

Update: Switching back and forth between networks seems to reset

[kumavis]

Nonce tracking has been significantly improved via the new tx-controller and its nonce-tracker
closing this ticket for now

[doowb]

I ran into this issue using a local ganache instance. I had closed it down and started it up again which resets all of the accounts. It seems like MetaMask doesn't get the new nonce so I'm not able to submit any new transactions with the test accounts that I've already used.

Posting here since I'm not sure if this should be a new issue or not.

[danfinlay]

Not a new issue. Two ways around it:

• Restart ganache with a different network ID
• Uninstall & reinstall MetaMask.

[doowb]

Thanks! Reinstalling MetaMask worked for me.