When the from address is the 0x0 address the fakeKey it generates for signing is invalid

[davidmurdoch]

When from is 0x0

https://github.com/ethereumjs/ethereumjs-vm/blob/51f8af05b2f6486b5c77dfbdea5372d695adb109/packages/tx/src/fake.ts#L56

generates a private key that will cause the d.isZero() check here:

https://github.com/cryptocoinjs/secp256k1-node/blob/024fbdad3fb64499e8db8b35246c2f0a36afa8c8/lib/elliptic.js#L315-L316

    const d = new BN(seckey)
    if (d.cmp(ecparams.n) >= 0 || d.isZero()) return 1

to later throw the error nonce generation function failed or private key is invalid.

A simple solution would be to generate a non-zero private key for the zero address. I believe this change will not be breaking, as it seems to be impossible to generate a hash (with signature) for a transaction "from" the zero address now.

[ryanio]

hey @davidmurdoch, thanks for opening. could you give an example of where you are using this FakeTransaction you are constructing?

I reworked the Transaction class in #812 and removed FakeTransaction (see some details in the readme) so I would like to see if I can support your use here with the new changes.

[davidmurdoch]

Ganache allows users to masquerade as any account. It creates a invalid tries and hashes, of course, but that doesn't matter to users that use this feature.

I actually found this issue because of a related Ganache bug. We also allow users to issue eth_call and eth_estimateGas with any from address, and due to a bug in some error formatting step that attempts to compute the transaction hash things blow up when the from is the zero address because of the "invalid" private key ethereumjs-tx creates.

This linked PR, even with the suggested getSenderAddress hack, will be a breaking change for Ganache, as I think we'll need to fix up hash and sign as well. Which isn't a big deal, just a little inconvenient.

[ryanio]

ah thanks for explaining, that's interesting.

This linked PR, even with the suggested getSenderAddress hack, will be a breaking change for Ganache, as I think we'll need to fix up hash and sign as well. Which isn't a big deal, just a little inconvenient.

good to know, yes that PR has quite a few breaking changes but overall tries to improve the lib with better code and some performance enhancements.

[ryanio]

This should now be resolved with #812.

The FakeTransaction class was refactored out, see the new usage here.