bitcoinjs / bitcoinjs-lib

A javascript Bitcoin library for node.js and browsers.
MIT License
5.6k stars 2.08k forks source link

Transaction.fromBuffer().getHash() giving incorrect results #1103

Closed RyanZim closed 6 years ago

RyanZim commented 6 years ago

If I do:

require('bitcoinjs-lib').Transaction
  .fromHex('01000000012a6809b66bbe22bf25bdbb2a02648b0f38c4507c1ceadf2ad0b11f49d19fdea7010000006a473044022072d7baa5f01765dce54c54db69ce6bedf82cbe0bf627f88a145822ac10170b2f0220552e7c4c7612d3cd99e605e8df8aa4c8b56672e5760c2f7650d1750d92bedd1d0121022b8cdc633443c361d16a8b2c90a1af4f0de3fa795d4411c21091e94dacde0c8efeffffff040958aa05000000001976a9146113ec56708a8f589456c39ad68ad938e2a76e4788acfac39b00000000001976a91408eecea991b5ef86287bb0c5bee75924807b8a8d88ac20bd6838000000001976a91402826485624dd23b4d01e6fdf07227ed30b445bf88accbdf2a01000000001976a914a2eddc9ce23507d041838b8fff01af132ba3d54c88acacb10600')
  .getHash()
  .toString('hex')

(runkit) I get:

8de6461ef98236992d567b21f608123b4d4243bb025242cbf42ee3f29aceb2f9

This actual transaction is on the blockchain with a transaction ID of:

f9b2ce9af2e32ef4cb425202bb43424d3b1208f6217b562d993682f91e46e68d

You can verify this with https://blockchain.info/decode-tx

What's wrong?

dcousens commented 6 years ago

@RyanZim getHash provides the cryptographic hash as used in parts of the code (signing, etc). You want the reverse-byte order transaction id, using getId. This is what is used by block explorers and most GUIs. It is confusing.

Quick way to see this, is by the looking at what you provided:

8d e6 46 1ef98236992d567b21f608123b4d4243bb025242cbf42ee3f29aceb2f9
     -------------------------------------------------------->
f9b2ce9af2e32ef4cb425202bb43424d3b1208f6217b562d993682f91e 46 e6 8d
RyanZim commented 6 years ago

OK, thanks! My bad for just assuming that's what getHash did.

dcousens commented 6 years ago

My bad for just assuming that's what getHash did.

It wasn't a bad assumption! I banged my head against a wall for a long time trying to think of a way out of this hell. Let me know if you have any ideas on how we could make it clearer.

RyanZim commented 6 years ago

Is getHash intended for use by end users? If not, rename it to _getHash. If you want it external, how about getHash256 or getDoubleSha256?

dcousens commented 6 years ago

@RyanZim it is used externally for many modules too, especially if dealing with custom transaction creation etc.

how about getHash256 or getDoubleSha256?

getDoubleSha256 isn't too bad, getHash256 I dont think would help any.