Repository: bitcoinjs/bitcoinjs-lib
Pull request #598 (2016-06-22 04:48:26)

Add Transaction.hashForSignature tests (and OP_CODESEPARATOR)
Resolves https://github.com/bitcoinjs/bitcoinjs-lib/issues/436 and finally resolves an inline TODO to add tests for `hashForSignature`.
Granted, it was already under significant integration tests and the Bitcoin core tests,  but we didn't have anything further than that.

I also added 697f2da as a means of cleaning up the code which previously was not-so-straight forward to read.

@jprichardson / @rubensayshi / @fanatid please review :)



Patch:
diff --git a/src/transaction.js b/src/transaction.js
index 378b2127..f645dcac 100644
--- a/src/transaction.js
+++ b/src/transaction.js
@@ -168,6 +168,10 @@ Transaction.prototype.clone = function () {
 
 var ONE = new Buffer('0000000000000000000000000000000000000000000000000000000000000001', 'hex')
 var VALUE_UINT64_MAX = new Buffer('ffffffffffffffff', 'hex')
+var BLANK_OUTPUT = {
+  script: EMPTY_SCRIPT,
+  valueBuffer: VALUE_UINT64_MAX
+}
 
 /**
  * Hash transaction for signing a specific input.
@@ -183,61 +187,55 @@ Transaction.prototype.hashForSignature = function (inIndex, prevOutScript, hashT
   // https://github.com/bitcoin/bitcoin/blob/master/src/test/sighash_tests.cpp#L29
   if (inIndex >= this.ins.length) return ONE
 
-  var txTmp = this.clone()
-
-  // in case concatenating two scripts ends up with two codeseparators,
-  // or an extra one at the end, this prevents all those possible incompatibilities.
-  var hashScript = bscript.compile(bscript.decompile(prevOutScript).filter(function (x) {
+  // ignore OP_CODESEPARATOR
+  var ourScript = bscript.compile(bscript.decompile(prevOutScript).filter(function (x) {
     return x !== opcodes.OP_CODESEPARATOR
   }))
-  var i
 
-  // blank out other inputs' signatures
-  txTmp.ins.forEach(function (input) { input.script = EMPTY_SCRIPT })
-  txTmp.ins[inIndex].script = hashScript
+  var txTmp = this.clone()
 
-  // blank out some of the inputs
+  // SIGHASH_NONE: ignore all outputs? (wildcard payee)
   if ((hashType & 0x1f) === Transaction.SIGHASH_NONE) {
-    // wildcard payee
     txTmp.outs = []
 
-    // let the others update at will
+    // ignore sequence numbers (except at inIndex)
     txTmp.ins.forEach(function (input, i) {
-      if (i !== inIndex) {
-        input.sequence = 0
-      }
+      if (i === inIndex) return
+
+      input.sequence = 0
     })
-  } else if ((hashType & 0x1f) === Transaction.SIGHASH_SINGLE) {
-    var nOut = inIndex
 
-    // only lock-in the txOut payee at same index as txIn
+  // SIGHASH_SINGLE: ignore all outputs, except at the same index?
+  } else if ((hashType & 0x1f) === Transaction.SIGHASH_SINGLE) {
     // https://github.com/bitcoin/bitcoin/blob/master/src/test/sighash_tests.cpp#L60
-    if (nOut >= this.outs.length) return ONE
+    if (inIndex >= this.outs.length) return ONE
 
-    txTmp.outs = txTmp.outs.slice(0, nOut + 1)
+    // truncate outputs after
+    txTmp.outs.length = inIndex + 1
 
-    // blank all other outputs (clear scriptPubKey, value === -1)
-    var stubOut = {
-      script: EMPTY_SCRIPT,
-      valueBuffer: VALUE_UINT64_MAX
+    // "blank" outputs before
+    for (var i = 0; i < inIndex; i++) {
+      txTmp.outs[i] = BLANK_OUTPUT
     }
 
-    for (i = 0; i < nOut; i++) {
-      txTmp.outs[i] = stubOut
-    }
-
-    // let the others update at will
+    // ignore sequence numbers (except at inIndex)
     txTmp.ins.forEach(function (input, i) {
-      if (i !== inIndex) {
-        input.sequence = 0
-      }
+      if (i === inIndex) return
+
+      input.sequence = 0
     })
   }
 
-  // blank out other inputs completely, not recommended for open transactions
+  // SIGHASH_ANYONECANPAY: ignore inputs entirely?
   if (hashType & Transaction.SIGHASH_ANYONECANPAY) {
-    txTmp.ins[0] = txTmp.ins[inIndex]
-    txTmp.ins = txTmp.ins.slice(0, 1)
+    txTmp.ins = [txTmp.ins[inIndex]]
+    txTmp.ins[0].script = ourScript
+
+  // SIGHASH_ALL: only ignore input scripts
+  } else {
+    // "blank" others input scripts
+    txTmp.ins.forEach(function (input) { input.script = EMPTY_SCRIPT })
+    txTmp.ins[inIndex].script = ourScript
   }
 
   // serialize and hash


Test patch:
diff --git a/test/bitcoin.core.js b/test/bitcoin.core.js
index 0833e3dc..571db0e8 100644
--- a/test/bitcoin.core.js
+++ b/test/bitcoin.core.js
@@ -201,9 +201,7 @@ describe('Bitcoin-core', function () {
       var scriptHex = f[1]
       var inIndex = f[2]
       var hashType = f[3]
-
-      // reverse because test data is reversed
-      var expectedHash = [].reverse.call(new Buffer(f[4], 'hex'))
+      var expectedHash = f[4]
 
       var hashTypes = []
       if ((hashType & 0x1f) === bitcoin.Transaction.SIGHASH_NONE) hashTypes.push('SIGHASH_NONE')
@@ -222,7 +220,9 @@ describe('Bitcoin-core', function () {
         assert.strictEqual(bitcoin.script.compile(scriptChunks).toString('hex'), scriptHex)
 
         var hash = transaction.hashForSignature(inIndex, script, hashType)
-        assert.deepEqual(hash, expectedHash)
+
+        // reverse because test data is reversed
+        assert.equal([].reverse.call(hash).toString('hex'), expectedHash)
       })
     })
   })
diff --git a/test/fixtures/transaction.json b/test/fixtures/transaction.json
index 214b1150..20de9a11 100644
--- a/test/fixtures/transaction.json
+++ b/test/fixtures/transaction.json
@@ -231,6 +231,45 @@
       "coinbase": false
     }
   ],
+  "hashForSignature": [
+    {
+      "description": "Out of range inIndex",
+      "txHex": "010000000200000000000000000000000000000000000000000000000000000000000000000000000000ffffffff00000000000000000000000000000000000000000000000000000000000000000000000000ffffffff01e8030000000000000000000000",
+      "inIndex": 2,
+      "script": "OP_0",
+      "type": 0,
+      "hash": "0000000000000000000000000000000000000000000000000000000000000001"
+    },
+    {
+      "description": "inIndex > nOutputs (SIGHASH_SINGLE)",
+      "txHex": "010000000200000000000000000000000000000000000000000000000000000000000000000000000000ffffffff00000000000000000000000000000000000000000000000000000000000000000000000000ffffffff01e8030000000000000000000000",
+      "inIndex": 2,
+      "script": "OP_0",
+      "type": 3,
+      "hash": "0000000000000000000000000000000000000000000000000000000000000001"
+    },
+    {
+      "txHex": "010000000200000000000000000000000000000000000000000000000000000000000000000000000000ffffffff00000000000000000000000000000000000000000000000000000000000000000000000000ffffffff01e8030000000000000000000000",
+      "inIndex": 0,
+      "script": "OP_0 OP_3",
+      "type": 0,
+      "hash": "3d56a632462b9fc9b89eeddcad7dbe476297f34aff7e5f9320e2a99fb5e97136"
+    },
+    {
+      "txHex": "010000000200000000000000000000000000000000000000000000000000000000000000000000000000ffffffff00000000000000000000000000000000000000000000000000000000000000000000000000ffffffff01e8030000000000000000000000",
+      "inIndex": 0,
+      "script": "OP_0 OP_CODESEPARATOR OP_3",
+      "type": 0,
+      "hash": "3d56a632462b9fc9b89eeddcad7dbe476297f34aff7e5f9320e2a99fb5e97136"
+    },
+    {
+      "txHex": "010000000200000000000000000000000000000000000000000000000000000000000000000000000000ffffffff00000000000000000000000000000000000000000000000000000000000000000000000000ffffffff01e8030000000000000000000000",
+      "inIndex": 0,
+      "script": "OP_0 OP_CODESEPARATOR OP_4",
+      "type": 0,
+      "hash": "fa075877cb54916236806a6562e4a8cdad48adf1268e73d72d1f9fdd867df463"
+    }
+  ],
   "invalid": {
     "addInput": [
       {
diff --git a/test/transaction.js b/test/transaction.js
index 4aa7a27d..d11ab294 100644
--- a/test/transaction.js
+++ b/test/transaction.js
@@ -172,6 +172,14 @@ describe('Transaction', function () {
     })
   })
 
-  // TODO:
-  //  hashForSignature: [Function],
+  describe('hashForSignature', function () {
+    fixtures.hashForSignature.forEach(function (f) {
+      it('should return ' + f.hash + ' for ' + (f.description ? ('case "' + f.description + '"') : f.script), function () {
+        var tx = Transaction.fromHex(f.txHex)
+        var script = bscript.fromASM(f.script)
+
+        assert.strictEqual(tx.hashForSignature(f.inIndex, script, f.type).toString('hex'), f.hash)
+      })
+    })
+  })
 })