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

question about "hash(false)" when "chainId" > 0 #712

Closed
warren-bank opened this issue Jul 26, 2017 · 5 comments
Closed

question about "hash(false)" when "chainId" > 0 #712

warren-bank opened this issue Jul 26, 2017 · 5 comments

Comments

@warren-bank
Copy link

hello. please correct me if I'm wrong, but my understanding of EIP 155 is that the raw data Array that gets hashed should contain the value 0 for the fields: r and s.

in looking at your implementation of the hash() function, it also appears that's what is intended.

however, in testing the result.. I'm seeing that these fields are being set to empty (unallocated, zero-length) Buffers.

here's a demo (based mostly on the example in the README)..

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

const txParams = {
  nonce: '0x00',
  gasPrice: '0x09184e72a000', 
  gasLimit: '0x2710',
  to: '0x0000000000000000000000000000000000000000', 
  value: '0x00', 
  data: '0x7f7465737432000000000000000000000000000000000000000000000000000000600057',
  // EIP 155 chainId - mainnet: 1, ropsten: 3
  chainId: 3
}

const tx = new EthereumTx(txParams)
tx.sign(privateKey)

let raw = tx.raw
tx.hash(false)  // bit of a hack: "raw" contains updated {v,r,s} from "items"

console.log(raw.slice(6))  // print the last 3 fields: [v,r,s]

output:

[ <Buffer 03>, <Buffer >, <Buffer > ]

What's really odd (to me) is that both of these fields are marked "allowZero". So, without tracing the code in much depth, it would seem that the setter for these fields should allow the assignment to 0.

Is this a bug?

@warren-bank
Copy link
Author

warren-bank commented Jul 26, 2017

quick bit of tracing later..

the setter calls ethereumjs-util.toBuffer(0).. which returns ethjs-util.intToBuffer(0).. which return new Buffer('00', 'hex').. which looks good

in the setter itself, there's some odd logic going on:
if (field.allowLess && field.length) v = exports.stripZeros(v)
..which strips the 0 out of the Buffer
..leaving it empty (zero-length)

fwiw, you might want to compare the logic in this setter to here

@fanatid
Copy link
Contributor

fanatid commented Jun 10, 2018

When checked code, I asked myself about same question. Also if chainId is not zero and tx already have signatures they will be lost. Ping @holgerd77

@davidmurdoch
Copy link
Contributor

Related to the behavior of utils.stripZero and this issue: https://github.com/ethereumjs/ethereumjs-util/issues/141

@danjm
Copy link
Contributor

danjm commented Jan 4, 2019

This will likely be resolved when we resolve https://github.com/ethereumjs/ethereumjs-tx/issues/112

@ryanio
Copy link
Contributor

ryanio commented Sep 22, 2020

This should now be resolved with #812. Please use the new tx.getMessageToSign() in place of tx.hash(false).

@ryanio ryanio closed this as completed Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants