tx.hash vs keccak256

[rajeshsubhankar]

const Transaction = require('ethereumjs-tx');
const keccak256 = require('js-sha3').keccak256;

// RLP encoded hex
const hexData = 'eb8084b2d05e008261a894e36ea790bc9d7ab70c55260c66d52b1eca985f8488016345785d8a0000801c8080';
console.log('tx hash: ', keccak256(hexData)); // 1f0a103ad8cb3df6a28a654b5bf5bbb26f60ec89857cb07118f1915b03aec7cd
const tx = new Transaction(hexData);
console.log('tx hash 2: ', tx.hash(false).toString('hex')); // 1a7390591440368a685688f5e37e23efd66c9c42e53bccb881a7a585de8724d2

I was expecting both the log output to be the same. Am I missing something?

[alcuadrado]

Hey @rajeshsubhankar! There are multiple issues with your code:

1. If you pass a string to keccak256, it won't interpret at hexa, you have to do that yourself.
2. Same happens with Transaction's constructor unless you have a 0x prefix.
3. The RLP encoded data you have, hexData, includes values for the tx signature (a default v, and empty r and s byte arrays). You are signing the whole thing manually, but telling ethereumjs-tx to exclude the signatures.

I fixed the three issues here:

const Transaction = require('ethereumjs-tx');
const keccak256 = require('js-sha3').keccak256;

// RLP encoded hex
const hexData = 'eb8084b2d05e008261a894e36ea790bc9d7ab70c55260c66d52b1eca985f8488016345785d8a0000801c8080';
const data = Buffer.from(hexData, "hex")
console.log('tx hash: ', keccak256(data));
const tx = new Transaction(data);
console.log('tx hash 2: ', tx.hash(true).toString('hex'));

I'm closing this issue, but feel free to open a new issue or comment in here.

[holgerd77]

Can we eventually transform experiences from this issue into some better overall or code documentation?

[alcuadrado]

There's not much we can do about (1).

(2) will be better once ethereumjs/ethereumjs-util#197 (comment) is merged and available in all the libraries.

(3) is more complex. Let me explain why.

tx.hash(false) shouldn't be part of the public API. It's used internally, but it makes no sense to use it externally.

A refactor transforming tx.hash(true) into tx.hash() and tx.hash(false) into tx.getHashForSigning() may make sense. It would improve the lib a little, but would introduce a breaking change without significally improving it.

https://github.com/ethereumjs/ethereumjs-tx/issues/140#issuecomment-491079633 has more info about the main issue I see with this library.

[alcuadrado]

Maybe we should concentrate this discussion in #151