Repository: bitcoinjs/bitcoinjs-lib
Pull request #898 (2017-09-22 02:06:24)

scriptHash.check should be agnostic of classification order
Alternative to https://github.com/bitcoinjs/bitcoinjs-lib/pull/897

Thanks and credit to @arik-so for reporting the bug,  and providing a test case!

The issue is that classifications aren't unique and aren't guaranteed to be un-ambiguous.
`scriptHash.check` has assumed that through using `classify*` internally.

Instead,  lets match 1 for 1,  and if there is a double match,  then we can say OK,  it is probably script hash.


Patch:
diff --git a/src/templates/scripthash/input.js b/src/templates/scripthash/input.js
index ac34149a..f89cb3d3 100644
--- a/src/templates/scripthash/input.js
+++ b/src/templates/scripthash/input.js
@@ -20,12 +20,23 @@ function check (script, allowIncomplete) {
   // is redeemScriptSig push only?
   if (!bscript.isPushOnly(scriptSigChunks)) return false
 
-  var inputType = bscript.classifyInput(scriptSigChunks, allowIncomplete)
-  var outputType = bscript.classifyOutput(redeemScriptChunks)
+  // is witness?
   if (chunks.length === 1) {
-    return outputType === bscript.types.P2WSH || outputType === bscript.types.P2WPKH
+    return bscript.witnessScriptHash.output.check(redeemScriptChunks) ||
+      bscript.witnessPubKeyHash.output.check(redeemScriptChunks)
   }
-  return inputType === outputType
+
+  // match types
+  if (bscript.pubKeyHash.input.check(scriptSigChunks) &&
+    bscript.pubKeyHash.output.check(redeemScriptChunks)) return true
+
+  if (bscript.multisig.input.check(scriptSigChunks, allowIncomplete) &&
+    bscript.multisig.output.check(redeemScriptChunks)) return true
+
+  if (bscript.pubKey.input.check(scriptSigChunks) &&
+    bscript.pubKey.output.check(redeemScriptChunks)) return true
+
+  return false
 }
 check.toJSON = function () { return 'scriptHash input' }
 


Test patch:
diff --git a/test/fixtures/templates.json b/test/fixtures/templates.json
index e2404277..2623684f 100644
--- a/test/fixtures/templates.json
+++ b/test/fixtures/templates.json
@@ -153,6 +153,15 @@
         ""
       ]
     },
+    {
+      "type": "scripthash",
+      "redeemScript": "OP_2 0327e023a353d111808f61d554c2e1934721eaf87f33b7a771e807006908a49372 03251670bb6a179a0d43b75476c7e580c0ba274378a18077e8de0832c870e5381f 02cca7f9a64425a0466d26d5c7e9eb3ad6b64cd48ea89edb38bc08f58a792dde47 OP_3 OP_CHECKMULTISIG",
+      "redeemScriptSig": "OP_0 304502210093efc26facedc5f51e304aa270a7b4f1a911b2d912c3674e5c6e2ad4ac7a410402201cf0b62c240461902f9f16d8a0bc3a210b7bfcd2c06523dd4b4b63be22e8525201",
+      "input": "OP_0 304502210093efc26facedc5f51e304aa270a7b4f1a911b2d912c3674e5c6e2ad4ac7a410402201cf0b62c240461902f9f16d8a0bc3a210b7bfcd2c06523dd4b4b63be22e8525201 52210327e023a353d111808f61d554c2e1934721eaf87f33b7a771e807006908a493722103251670bb6a179a0d43b75476c7e580c0ba274378a18077e8de0832c870e5381f2102cca7f9a64425a0466d26d5c7e9eb3ad6b64cd48ea89edb38bc08f58a792dde4753ae",
+      "inputHex": "0048304502210093efc26facedc5f51e304aa270a7b4f1a911b2d912c3674e5c6e2ad4ac7a410402201cf0b62c240461902f9f16d8a0bc3a210b7bfcd2c06523dd4b4b63be22e85252014c6952210327e023a353d111808f61d554c2e1934721eaf87f33b7a771e807006908a493722103251670bb6a179a0d43b75476c7e580c0ba274378a18077e8de0832c870e5381f2102cca7f9a64425a0466d26d5c7e9eb3ad6b64cd48ea89edb38bc08f58a792dde4753ae",
+      "output": "OP_HASH160 fcc42dd4aa770d75cb6796bbd7853a325e746659 OP_EQUAL",
+      "outputHex": "a914fcc42dd4aa770d75cb6796bbd7853a325e74665987"
+    },
     {
       "type": "nonstandard",
       "typeIncomplete": "scripthash",
@@ -191,9 +200,7 @@
       "outputHex": "00"
     },
     {
-      "type": "scripthash",
-      "redeemScript": "OP_0",
-      "redeemScriptSig": "OP_0",
+      "type": "nonstandard",
       "input": "OP_0 00",
       "inputHex": "000100",
       "inputStack": [
diff --git a/test/fixtures/transaction_builder.json b/test/fixtures/transaction_builder.json
index ad1cb912..9fcb2cb1 100644
--- a/test/fixtures/transaction_builder.json
+++ b/test/fixtures/transaction_builder.json
@@ -1446,6 +1446,9 @@
         ]
       }
     ],
+    "classification": {
+      "hex": "01000000059c06fb641a8cd69be81ca91e68d8a115cb698396876ecd77120ec1e4ab9002279f000000b500483045022100d58f828ab39cfac592f89fe372fb520992975218698c683a893f29e39cf0080302207cc0485dab5ce621089bdd15e1f15db0ecbde8dd4bb661bcf0e3af6ecab075e6014c6952210327e023a353d111808f61d554c2e1934721eaf87f33b7a771e807006908a493722103251670bb6a179a0d43b75476c7e580c0ba274378a18077e8de0832c870e5381f2102cca7f9a64425a0466d26d5c7e9eb3ad6b64cd48ea89edb38bc08f58a792dde4753aeffffffff0821dc00213d2b7993f8f2a1553800c6f2f31106da176505d0ade467b68401d795000000b400473044022028e937a7bba888fe3428f442f6e22d92ce2ddba01548c38780d40890fa6cc305022043204d0bcfb1150b045d54cf9b13462e44e2ef47fee03d3cea08e84a8060fc30014c6952210327e023a353d111808f61d554c2e1934721eaf87f33b7a771e807006908a493722103251670bb6a179a0d43b75476c7e580c0ba274378a18077e8de0832c870e5381f2102cca7f9a64425a0466d26d5c7e9eb3ad6b64cd48ea89edb38bc08f58a792dde4753aeffffffffaa997ac385dc666af1f5947ef615431024eb314cac2308d5e1b903e28ca466f499000000b50048304502210093efc26facedc5f51e304aa270a7b4f1a911b2d912c3674e5c6e2ad4ac7a410402201cf0b62c240461902f9f16d8a0bc3a210b7bfcd2c06523dd4b4b63be22e85252014c6952210327e023a353d111808f61d554c2e1934721eaf87f33b7a771e807006908a493722103251670bb6a179a0d43b75476c7e580c0ba274378a18077e8de0832c870e5381f2102cca7f9a64425a0466d26d5c7e9eb3ad6b64cd48ea89edb38bc08f58a792dde4753aeffffffffd9f61bf98a021ee144f33ba5a6b04274de8fcb5c05f1ff7c12367fb7a608b2dd9e000000b4004730440220456e1201c1fa727288cba7fa0054dc02d8dd6c7418cae1e97006ef0652891c9202201192d0fbf3a9c00afb99a415f2bf515509e1150805acd8de95c496c27cb6570f014c6952210327e023a353d111808f61d554c2e1934721eaf87f33b7a771e807006908a493722103251670bb6a179a0d43b75476c7e580c0ba274378a18077e8de0832c870e5381f2102cca7f9a64425a0466d26d5c7e9eb3ad6b64cd48ea89edb38bc08f58a792dde4753aeffffffff1f8119e3bc7c2f451feaa79f42ec5a63502afb425c253c935e43d217d5c29bdea1000000b500483045022100f669004f770490093eba4ac4903cb7581f7d18ea9245c538585ef5367e520e4702205485fafe0be178563a599d41e0cc172bb01314ed65d0e48df19a5258f17bdbc4014c6952210327e023a353d111808f61d554c2e1934721eaf87f33b7a771e807006908a493722103251670bb6a179a0d43b75476c7e580c0ba274378a18077e8de0832c870e5381f2102cca7f9a64425a0466d26d5c7e9eb3ad6b64cd48ea89edb38bc08f58a792dde4753aeffffffff0380f0fa02000000001976a91439692085cf9d27e8c1cf63e76bd32d9bd15cab8b88ac50c300000000000017a9147204bb26950ce1595255897f63d205779f033f3e875b5409000000000017a9142538893d984a4b5695e4bfde1a90a9f02fabf8e38700000000"
+    },
     "multisig": [
       {
         "description": "P2SH 2-of-2 multisig, signed in correct order",
diff --git a/test/script.js b/test/script.js
index 4adf2935..07f907f0 100644
--- a/test/script.js
+++ b/test/script.js
@@ -5,6 +5,7 @@ var bscript = require('../src/script')
 var minimalData = require('minimaldata')
 
 var fixtures = require('./fixtures/script.json')
+var fixtures2 = require('./fixtures/templates.json')
 
 describe('script', function () {
   // TODO
@@ -23,8 +24,8 @@ describe('script', function () {
   describe('fromASM/toASM', function () {
     fixtures.valid.forEach(function (f) {
       it('encodes/decodes ' + f.asm, function () {
-        var scriptSig = bscript.fromASM(f.asm)
-        assert.strictEqual(bscript.toASM(scriptSig), f.asm)
+        var script = bscript.fromASM(f.asm)
+        assert.strictEqual(bscript.toASM(script), f.asm)
       })
     })
 
@@ -37,6 +38,28 @@ describe('script', function () {
     })
   })
 
+  describe('fromASM/toASM (templates)', function () {
+    fixtures2.valid.forEach(function (f) {
+      if (f.inputHex) {
+        var ih = bscript.toASM(Buffer.from(f.inputHex, 'hex'))
+
+        it('encodes/decodes ' + ih, function () {
+          var script = bscript.fromASM(f.input)
+          assert.strictEqual(script.toString('hex'), f.inputHex)
+          assert.strictEqual(bscript.toASM(script), f.input)
+        })
+      }
+
+      if (f.outputHex) {
+        it('encodes/decodes ' + f.output, function () {
+          var script = bscript.fromASM(f.output)
+          assert.strictEqual(script.toString('hex'), f.outputHex)
+          assert.strictEqual(bscript.toASM(script), f.output)
+        })
+      }
+    })
+  })
+
   describe('isPushOnly', function () {
     fixtures.valid.forEach(function (f) {
       it('returns ' + !!f.stack + ' for ' + f.asm, function () {
diff --git a/test/transaction_builder.js b/test/transaction_builder.js
index fbe78425..2ad4c931 100644
--- a/test/transaction_builder.js
+++ b/test/transaction_builder.js
@@ -125,6 +125,16 @@ describe('TransactionBuilder', function () {
       })
     })
 
+    it('correctly classifies transaction inputs', function () {
+      var tx = Transaction.fromHex(fixtures.valid.classification.hex)
+      var txb = TransactionBuilder.fromTransaction(tx)
+      txb.inputs.forEach(function (i) {
+        assert.strictEqual(i.prevOutType, 'scripthash')
+        assert.strictEqual(i.redeemScriptType, 'multisig')
+        assert.strictEqual(i.signType, 'multisig')
+      })
+    })
+
     fixtures.invalid.fromTransaction.forEach(function (f) {
       it('throws ' + f.exception, function () {
         var tx = Transaction.fromHex(f.txHex)