Metamask-extension: Incorrectly high pending nonces can prevent any valid txs from being composed

Created on 24 Aug 2017 · 13Comments · Source: MetaMask/metamask-extension

If nonces are non-sequentially higher than the network confirmed nonce, we should ignore them when composing new transactions.

Really, they should fail in that case when composed.

Source

danfinlay

Most helpful comment

Furthermore, it turns out our new nonce-tracker was parsing network nonces by calling parseInt() on the result of an ethjs response, expecting it to be hex instead of a BN. This meant we were sometimes generating very wrong nonces.

We actually had a lot of tests covering this code, but somehow when it generated wrong nonces, they were within ranges that passed all of our tests.

This seems like a pretty far-fetched and unlikely event, so I hope the community can forgive us for an evening where many users are going without the ability to send.

danfinlay on 24 Aug 2017

👍5

All 13 comments

We actually had a lot of tests covering this code, but somehow when it generated wrong nonces, they were within ranges that passed all of our tests.

This seems like a pretty far-fetched and unlikely event, so I hope the community can forgive us for an evening where many users are going without the ability to send.

danfinlay on 24 Aug 2017

👍5

Types in JavaScript would've prevented this, but Aaron pointed out TypeScript requires you to write type files for all your dependencies, that sounds hard to adopt. Then I joked: TypeScript lite is if you just add type assertions at the top of every function, and then @kumavis pointed out you could probably auto-generate those type assertions from JSDoc comments, and that's how JSDoc Types was born. Someone please implement, it's a great idea that would've prevented this bug.

Does anyone think that's actually a good idea? It's kindof interesting, at least.

danfinlay on 24 Aug 2017

😄1

ive never actually used TS so shrug emoji

kumavis on 24 Aug 2017

Thanks for looking into this, flyswatter. I'm one of those who's experiencing a very high nonce with my transactions this evening. I believe my transaction numbers were originally in the high 400's and it recently shifted to the 1100's which is likely due to your aforementioned explanation concerning hex/BN mixup. Most transactions will immediately fail but others stack up in the queue. If I shift networks they then read as failed. Restored vault to try to resolve but didn't seem to work.

My understanding of programming is very limited but if it's of any assistance I can provide a state log. Thanks again to you and your team for your hard work and great program.

dwm256 on 24 Aug 2017

That's kind of you @dwm256, but I think we have this one handled, and should have a fix shipped soon. I'll probably have to write a script to analyze the odds of getting weird nonces later on, because it's such a quirky bug.

Anyways, all you can do is stay patient and cool, and we'll have it going again soon.

danfinlay on 24 Aug 2017

By the way, I do not want to hijack the thread but we use Haxe (https://haxe.org) for Etherplay. It is a typed language with nice features and a cool community. We are writing "externs" (equivalent of type file) manually for web3 and other lib we use. It is actually not too bad as you only need to write what you use.

Our web3 extern is there https://github.com/etherplay/web3.hx

It is far from complete or polished though. Haxe externs are also a different beast. The "abstract" feature can provide really cool API.

I am going to present our work to the Haxe summit next month : http://summit.haxe.org

In the Haxe community there were few attempts at generating externs automatically and some are used for specific libs, like nodejs I think but no general one as manual ones turned out to be better. I guess typescript must have similar tools. For typescript they also have a good selection of type file already written : https://github.com/DefinitelyTyped/DefinitelyTyped

wighawag on 24 Aug 2017

👍2

Haxe would be fun to explore, those haxe files don't look bad at all.

We did a little analysis, and the result of the parsing bug was that any account with more than 10 transactions would fail to send, because calling parseInt() on a BN coerses it as a string, which returns a decimal, but we were then parsing it as hex, so any nonce that spilled into multiple digits had its additional digits interpreted as base 16.

danfinlay on 24 Aug 2017

👍1

Switched to localhost 8545 to clear the cache and transaction number corrected itself. Thanks a bunch!

dwm256 on 25 Aug 2017

https://flow.org/