Signing data and signing hash are not according to EIP-155

[tryice3]

I tried to reproduce the example from EIP-155 and the results are not the same.

My code is:

const EthereumTx = require('ethereumjs-tx')
const privateKey = Buffer.from('4646464646464646464646464646464646464646464646464646464646464646', 'hex')

const txParams = {
  nonce: 9,
  gasPrice: 20 * 10**9, 
  gasLimit: 21000,
  to: '0x3535353535353535353535353535353535353535', 
  value: 10**18, 
  data: '',
  chainId: 1 // EIP 155 chainId - mainnet: 1, ropsten: 3
}

const tx = new EthereumTx(txParams)
console.log(tx)
console.log('Signing data: 0x' + tx.serialize().toString('hex'))

txHash = tx.hash();
console.log('Signing hash: 0x' + txHash.toString('hex'))

tx.sign(privateKey)
console.log('Signed tx: 0x' + tx.serialize().toString('hex'))

The signing data in RLC-Encoding is:

0xec098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a7640000801c8080

Instead of:

0xec098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a764000080018080

The difference is that the v is not replaced by chainId.

The signing hash is also different because the signing data are different.

The signed transaction remains the same because it uses the hard coded hash(false).

[davidmurdoch]

@fibice3, I'm looking into ensuring correct transaction hashes in ganache-core and came across this issue. I can't reproduce your findings.

Running the code above with ethereumjs-tx@1.3.7 I get:

Signing data: 0xec098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a764000080018080
Signing hash: 0xdaf5a779ae972f972197303d7b574746c7ef83eadac0f2791ad23db92e4c8e53
Signed tx: 0xf86c098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a76400008025a028ef61340bd939bc2195fe537567866003e1a15d3c71ff63e1590620aa636276a067cbe9d8997f761aecb703304b3800ccf555c9f3dc64214b297fb1966a3b6d83

which matches the example output in EIP-155.

[tryice3]

@davidmurdoch It's strange. Running the code above with ethereumjs-tx version 1.3.7 I get:

Transaction {
  raw:
   [ <Buffer 09>,
     <Buffer 04 a8 17 c8 00>,
     <Buffer 52 08>,
     <Buffer 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35>,
     <Buffer 0d e0 b6 b3 a7 64 00 00>,
     <Buffer >,
     <Buffer 1c>,
     <Buffer >,
     <Buffer > ],
  _fields:
   [ 'nonce',
     'gasPrice',
     'gasLimit',
     'to',
     'value',
     'data',
     'v',
     'r',
     's' ],
  toJSON: [Function],
  serialize: [Function: serialize],
  nonce: [Getter/Setter],
  gasPrice: [Getter/Setter],
  gasLimit: [Getter/Setter],
  to: [Getter/Setter],
  value: [Getter/Setter],
  data: [Getter/Setter],
  v: [Getter/Setter],
  r: [Getter/Setter],
  s: [Getter/Setter],
  from: [Getter],
  _chainId: 1,
  _homestead: true }
Signing data: 0xec098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a7640000801c8080
Signing hash: 0xcde7ce101bc69ad5c9ca811e2c20df9df642c6900a7bc7e50454a270f3cde816
Signed tx: 0xf86c098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a76400008025a028ef61340bd939bc2195fe537567866003e1a15d3c71ff63e1590620aa636276a067cbe9d8997f761aecb703304b3800ccf555c9f3dc64214b297fb1966a3b6d83

[davidmurdoch]

@fibice3, It seems I ran the code incorrectly yesterday. I ca, in fact, reproduce your results.

tx.hash() will always produce a hash that includes the signature parameters by default. If you want to not include the signature in the hash use tx.hash(false), which will give you the signing hash; tx.sign(key) uses tx.hash(false) internally.

tx.hash() and tx.hash(true) will create a hash of the transaction. tx.hash(false) creates a hash of the transaction for use in the signature.

[tryice3]

@davidmurdoch, the issue is that the signing data are always wrong.

0xec098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a7640000801c8080

Instead of:

0xec098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a764000080018080

[davidmurdoch]

Ah, I see it now. Thanks for pointing it out! I'll look into this for ganache-core's implementation.

[davidmurdoch]

@fibice3 I'm currently monkey-patching ethereumjs-tx in a ganache-core branch. The new hash method that seems to pass EIP-155 is here: https://github.com/trufflesuite/ganache-core/blob/3483bbc8d73d84efe5ed3f766e7ae9ce8b1beb42/lib/utils/transaction.js#L300-L329

Another key bit of monkey-patching specific to a Transaction created from a signedTransaction is https://github.com/trufflesuite/ganache-core/blob/3483bbc8d73d84efe5ed3f766e7ae9ce8b1beb42/lib/utils/transaction.js#L36-L53

[danjm]

The issue here is that the tx.serialize() method uses the raw data and does not correctly apply the rules from EIP155. Those rules are, however, properly applied in other methods (e.g. hash())

This should be resolved by https://github.com/ethereumjs/ethereumjs-tx/pull/124

I will look into adding a failing test for this case:

• [ ] Add failing test for tx.serialize() when the transaction is initialized with a chainId

[danjm]

@davidmurdoch Are those monkey patches necessary for ganache-core specific requirements? Any thoughts on whether the handling of the updated v values should be supported by ethereumjs-tx?

[davidmurdoch]

I think ethereumjs-tx should be able to handle setting the v and chainId values at any time, not just at initialization, which is part of what Ganache's monkey patching enables here. I don't particularly like how I've handled the chainId/v value swap in the hash method in the ganache-core monkey-patch, as it seems very fragile and is rather confusing code.

It is tricky to update this in ejs-tx because the field getters and setters have been abstracted away into ethereumjs-utils, and the v and _chainId values need to behave in an unusual way (as EIP-155 requires v to be replaced by chainId).