TxBuilder: fix failing test for non-standard/multisig inputs

Instead of failing in `fromTransaction`, TxBuilder will now only fail in `sign` if you attempt to sign a non-standard input. Transactions with non-standard inputs can only be built with buildIncomplete() (for now).
10 years ago · 4c9fd6010e
3 changed files with 188 additions and 104 deletions
--- a/src/transaction_builder.js
+++ b/src/transaction_builder.js
@ -13,31 +13,39 @@ function isCoinbase(txHash) {
  })
 }

-function extractSignature(txIn) {
+function extractInput(txIn) {
  var redeemScript
  var scriptSig = txIn.script
-  var scriptType = scripts.classifyInput(scriptSig, true)
+  var prevOutScript
+  var prevOutType = scripts.classifyInput(scriptSig, true)
+  var scriptType

  // Re-classify if P2SH
-  if (scriptType === 'scripthash') {
+  if (prevOutType === 'scripthash') {
    redeemScript = Script.fromBuffer(scriptSig.chunks.slice(-1)[0])
-    scriptSig = Script.fromChunks(scriptSig.chunks.slice(0, -1))
+    prevOutScript = scripts.scriptHashOutput(redeemScript.getHash())

+    scriptSig = Script.fromChunks(scriptSig.chunks.slice(0, -1))
    scriptType = scripts.classifyInput(scriptSig, true)
    assert.equal(scripts.classifyOutput(redeemScript), scriptType, 'Non-matching scriptSig and scriptPubKey in input')
+
+  } else {
+    scriptType = prevOutType
  }

  // Extract hashType, pubKeys and signatures
-  var hashType, parsed, pubKeys, signatures
+  var hashType, initialized, parsed, pubKeys, signatures

  switch (scriptType) {
    case 'pubkeyhash':
      parsed = ECSignature.parseScriptSignature(scriptSig.chunks[0])
      hashType = parsed.hashType
-
      pubKeys = [ECPubKey.fromBuffer(scriptSig.chunks[1])]
      signatures = [parsed.signature]

+      initialized = true
+      prevOutScript = pubKeys[0].getAddress().toOutputScript()
+
      break

    case 'multisig':
@ -46,8 +54,8 @@ function extractSignature(txIn) {
      }).map(ECSignature.parseScriptSignature)

      hashType = parsed[0].hashType
-      pubKeys = []
      signatures = parsed.map(function(p) { return p.signature })
+      initialized = true

      if (redeemScript) {
        pubKeys = redeemScript.chunks.slice(1, -2).map(ECPubKey.fromBuffer)
@ -57,11 +65,11 @@ function extractSignature(txIn) {

    case 'pubkey':
      parsed = ECSignature.parseScriptSignature(scriptSig.chunks[0])
-
      hashType = parsed.hashType
-      pubKeys = []
      signatures = [parsed.signature]

+      initialized = true
+
      if (redeemScript) {
        pubKeys = [ECPubKey.fromBuffer(redeemScript.chunks[0])]
      }
@ -69,11 +77,18 @@ function extractSignature(txIn) {
      break

    default:
-      assert(false, scriptType + ' inputs not supported')
+      if (redeemScript) {
+        initialized = true
+      }
+
+      break
  }

  return {
    hashType: hashType,
+    initialized: initialized,
+    prevOutScript: prevOutScript,
+    prevOutType: prevOutType,
    pubKeys: pubKeys,
    redeemScript: redeemScript,
    scriptType: scriptType,
@ -86,7 +101,7 @@ function TransactionBuilder() {
  this.prevOutScripts = {}
  this.prevOutTypes = {}

-  this.signatures = []
+  this.inputs = []
  this.tx = new Transaction()
 }

@ -108,14 +123,16 @@ TransactionBuilder.fromTransaction = function(transaction) {
  })

  // Extract/add signatures
-  txb.signatures = transaction.ins.map(function(txIn) {
-    // TODO: remove me after testcase added
-    assert(!isCoinbase(txIn.hash), 'coinbase inputs not supported')
+  txb.inputs = transaction.ins.map(function(txIn) {
+    // Coinbase inputs not supported
+    assert(!Array.prototype.every.call(txIn.hash, function(x) {
+      return x === 0
+    }), 'coinbase inputs not supported')

    // Ignore empty scripts
    if (txIn.script.buffer.length === 0) return

-    return extractSignature(txIn)
+    return extractInput(txIn)
  })

  return txb
@ -139,31 +156,51 @@ TransactionBuilder.prototype.addInput = function(prevTx, index, sequence, prevOu

  }

-  var prevOutType
-  if (prevOutScript !== undefined) {
-    prevOutType = scripts.classifyOutput(prevOutScript)
+  var input = {}
+  if (prevOutScript) {
+    var prevOutType = scripts.classifyOutput(prevOutScript)
+
+    // if we can, extract pubKey information
+    switch (prevOutType) {
+      case 'multisig':
+        input.pubKeys = prevOutScript.chunks.slice(1, -2).map(ECPubKey.fromBuffer)
+        break
+
+      case 'pubkey':
+        input.pubKeys = prevOutScript.chunks.slice(0, 1).map(ECPubKey.fromBuffer)
+        break
+    }
+
+    if (prevOutType !== 'scripthash') {
+      input.scriptType = prevOutType
+    }

-    assert.notEqual(prevOutType, 'nonstandard', 'PrevOutScript not supported (nonstandard)')
+    input.prevOutScript = prevOutScript
+    input.prevOutType = prevOutType
  }

-  assert(this.signatures.every(function(input) {
-    return input.hashType & Transaction.SIGHASH_ANYONECANPAY
+  assert(this.inputs.every(function(input2) {
+    if (input2.hashType === undefined) return true
+
+    return input2.hashType & Transaction.SIGHASH_ANYONECANPAY
  }), 'No, this would invalidate signatures')

  var prevOut = prevOutHash.toString('hex') + ':' + index
  assert(!(prevOut in this.prevTxMap), 'Transaction is already an input')

  var vout = this.tx.addInput(prevOutHash, index, sequence)
+
  this.prevTxMap[prevOut] = true
-  this.prevOutScripts[vout] = prevOutScript
-  this.prevOutTypes[vout] = prevOutType
+  this.inputs[vout] = input

  return vout
 }

 TransactionBuilder.prototype.addOutput = function(scriptPubKey, value) {
-  assert(this.signatures.every(function(signature) {
-    return (signature.hashType & 0x1f) === Transaction.SIGHASH_SINGLE
+  assert(this.inputs.every(function(input) {
+    if (input.hashType === undefined) return true
+
+    return (input.hashType & 0x1f) === Transaction.SIGHASH_SINGLE
  }), 'No, this would invalidate signatures')

  return this.tx.addOutput(scriptPubKey, value)
@ -175,41 +212,50 @@ TransactionBuilder.prototype.__build = function(allowIncomplete) {
  if (!allowIncomplete) {
    assert(this.tx.ins.length > 0, 'Transaction has no inputs')
    assert(this.tx.outs.length > 0, 'Transaction has no outputs')
-    assert(this.signatures.length > 0, 'Transaction has no signatures')
-    assert.equal(this.signatures.length, this.tx.ins.length, 'Transaction is missing signatures')
  }

  var tx = this.tx.clone()

  // Create script signatures from signature meta-data
-  this.signatures.forEach(function(input, index) {
-    var scriptSig
-    var scriptType = input.scriptType
+  this.inputs.forEach(function(input, index) {
+    if (!allowIncomplete) {
+      assert(input.initialized, 'Transaction is not complete')
+    }

-    var signatures = input.signatures.map(function(signature) {
-      return signature.toScriptSignature(input.hashType)
-    })
+    var scriptSig

-    switch (scriptType) {
+    switch (input.scriptType) {
      case 'pubkeyhash':
-        var pubKey = input.pubKeys[0]
-        scriptSig = scripts.pubKeyHashInput(signatures[0], pubKey)
+        assert(input.signatures, 'Transaction is missing signatures')
+        assert.equal(input.signatures.length, 1, 'Transaction is missing signatures')

+        var pkhSignature = input.signatures[0].toScriptSignature(input.hashType)
+        scriptSig = scripts.pubKeyHashInput(pkhSignature, input.pubKeys[0])
        break

      case 'multisig':
+        assert(input.signatures, 'Transaction is missing signatures')
+
+        var signatures = input.signatures.map(function(signature) {
+          return signature.toScriptSignature(input.hashType)
+        }).filter(function(signature) { return !!signature })
+
        var redeemScript = allowIncomplete ? undefined : input.redeemScript
        scriptSig = scripts.multisigInput(signatures, redeemScript)
-
        break

      case 'pubkey':
-        scriptSig = scripts.pubKeyInput(signatures[0])
+        assert(input.signatures, 'Transaction is missing signatures')
+        assert.equal(input.signatures.length, 1, 'Transaction is missing signatures')

+        var pkSignature = input.signatures[0].toScriptSignature(input.hashType)
+        scriptSig = scripts.pubKeyInput(pkSignature)
        break

      default:
-        assert(false, scriptType + ' not supported')
+        if (allowIncomplete) return
+
+        assert(false, input.scriptType + ' not supported')
    }

    if (input.redeemScript) {
@ -223,73 +269,116 @@ TransactionBuilder.prototype.__build = function(allowIncomplete) {
 }

 TransactionBuilder.prototype.sign = function(index, privKey, redeemScript, hashType) {
-  assert(this.tx.ins.length >= index, 'No input at index: ' + index)
+  assert(index in this.inputs, 'No input at index: ' + index)
  hashType = hashType || Transaction.SIGHASH_ALL

-  var prevOutScript = this.prevOutScripts[index]
-  var prevOutType = this.prevOutTypes[index]
+  var input = this.inputs[index]

-  var scriptType, hash
-  if (redeemScript) {
-    prevOutScript = prevOutScript || scripts.scriptHashOutput(redeemScript.getHash())
-    prevOutType = prevOutType || 'scripthash'
+  if (input.hashType !== undefined) {
+    assert.equal(input.hashType, hashType, 'Inconsistent hashType')
+  }

-    assert.equal(prevOutType, 'scripthash', 'PrevOutScript must be P2SH')
+  // are we already initialized?
+  if (input.initialized) {
+    if (input.prevOutType === 'scripthash') {
+      assert(input.redeemScript, 'PrevOutScript is P2SH, missing redeemScript')

-    scriptType = scripts.classifyOutput(redeemScript)
+    } else {
+      assert(!input.redeemScript, 'PrevOutScript must be P2SH')
+    }

-    assert.notEqual(scriptType, 'scripthash', 'RedeemScript can\'t be P2SH')
-    assert.notEqual(scriptType, 'nulldata', 'RedeemScript not supported (nulldata)')
-    assert.notEqual(scriptType, 'nonstandard', 'RedeemScript not supported (nonstandard)')
+    // redeemScript only needed to initialize, but if provided again, enforce consistency
+    if (redeemScript) {
+      assert.deepEqual(input.redeemScript, redeemScript, 'Inconsistent redeemScript')
+    }

-    hash = this.tx.hashForSignature(index, redeemScript, hashType)
+    // if signatures already exist, enforce multisig scriptType
+    if (input.signatures.length > 0) {
+      assert.equal(input.scriptType, 'multisig', input.scriptType + ' doesn\'t support multiple signatures')
+    }

+  // initialize it
  } else {
-    prevOutScript = prevOutScript || privKey.pub.getAddress().toOutputScript()
-    prevOutType = prevOutType || 'pubkeyhash'
+    if (redeemScript) {
+      if (input.prevOutScript) {
+        assert.equal(input.prevOutType, 'scripthash', 'PrevOutScript must be P2SH')

-    assert.notEqual(prevOutType, 'scripthash', 'PrevOutScript is P2SH, missing redeemScript')
+        var scriptHash = input.prevOutScript.chunks[1]
+        assert.deepEqual(scriptHash, redeemScript.getHash(), 'RedeemScript does not match ' + scriptHash.toString('hex'))

-    scriptType = prevOutType
+      } else {
+        input.prevOutScript = scripts.scriptHashOutput(redeemScript.getHash())
+        input.prevOutType = 'scripthash'
+      }

-    hash = this.tx.hashForSignature(index, prevOutScript, hashType)
-  }
+      var scriptType = scripts.classifyOutput(redeemScript)
+
+      switch (scriptType) {
+        case 'multisig':
+          input.pubKeys = redeemScript.chunks.slice(1, -2).map(ECPubKey.fromBuffer)
+          break
+
+        case 'pubkeyhash':
+          var pkh1 = redeemScript.chunks[2]
+          var pkh2 = privKey.pub.getAddress().hash
+
+          assert.deepEqual(pkh1, pkh2, 'privateKey cannot sign for this input')
+          input.pubKeys = [privKey.pub]
+          break
+
+        case 'pubkey':
+          input.pubKeys = redeemScript.chunks.slice(0, 1).map(ECPubKey.fromBuffer)
+          break

-  var input = this.signatures[index]
-  if (!input) {
-    var pubKeys = []
+        default:
+          assert(false, 'RedeemScript not supported (' + scriptType + ')')
+      }

-    if (redeemScript && scriptType === 'multisig') {
-      pubKeys = redeemScript.chunks.slice(1, -2).map(ECPubKey.fromBuffer)
+      input.redeemScript = redeemScript
+      input.scriptType = scriptType

    } else {
-      pubKeys.push(privKey.pub)
-    }
+      assert.notEqual(input.prevOutType, 'scripthash', 'PrevOutScript is P2SH, missing redeemScript')

-    input = {
-      hashType: hashType,
-      pubKeys: pubKeys,
-      redeemScript: redeemScript,
-      scriptType: scriptType,
-      signatures: []
+      if (!input.scriptType) {
+        input.prevOutScript = privKey.pub.getAddress().toOutputScript()
+        input.prevOutType = 'pubkeyhash'
+        input.pubKeys = [privKey.pub]
+        input.scriptType = input.prevOutType
+      }
    }

-    this.signatures[index] = input
-    this.prevOutScripts[index] = prevOutScript
-    this.prevOutTypes[index] = prevOutType
+    input.hashType = hashType
+    input.initialized = true
+    input.signatures = input.signatures || []
+  }
+
+  switch (input.scriptType) {
+    case 'pubkeyhash':
+    case 'multisig':
+    case 'pubkey':
+      break
+
+    default:
+      assert(false, input.scriptType + ' not supported')
+  }
+
+  var signatureHash
+  if (input.redeemScript) {
+    signatureHash = this.tx.hashForSignature(index, input.redeemScript, hashType)

  } else {
-    assert.equal(scriptType, 'multisig', scriptType + ' doesn\'t support multiple signatures')
-    assert.equal(input.hashType, hashType, 'Inconsistent hashType')
-    assert.deepEqual(input.redeemScript, redeemScript, 'Inconsistent redeemScript')
+    signatureHash = this.tx.hashForSignature(index, input.prevOutScript, hashType)
  }

+  var signature = privKey.sign(signatureHash)
+
  // enforce signing in order of public keys
  assert(input.pubKeys.some(function(pubKey, i) {
-    if (!privKey.pub.Q.equals(pubKey.Q)) return false // FIXME: could be better?
+    if (!privKey.pub.Q.equals(pubKey.Q)) return false

    assert(!input.signatures[i], 'Signature already exists')
-    input.signatures[i] = privKey.sign(hash)
+    input.signatures[i] = signature

    return true
  }), 'privateKey cannot sign for this input')
--- a/test/fixtures/transaction_builder.json
+++ b/test/fixtures/transaction_builder.json
@ -150,7 +150,8 @@
        "outputs": []
      },
      {
-        "exception": "Transaction has no signatures",
+        "description": "Incomplete transaction, nothing assumed",
+        "exception": "Transaction is not complete",
        "inputs": [
          {
            "index": 0,
@ -166,7 +167,8 @@
        ]
      },
      {
-        "exception": "Transaction is missing signatures",
+        "description": "Incomplete transaction w/ prevTxScript defined",
+        "exception": "Transaction is not complete",
        "inputs": [
          {
            "index": 0,
@ -178,6 +180,7 @@
          {
            "index": 1,
            "prevTx": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+            "prevTxScript": "OP_DUP OP_HASH160 aa4d7985c57e011a8b3dd8e0e5a73aaef41629c5 OP_EQUALVERIFY OP_CHECKSIG",
            "privKeys": []
          }
        ],
@ -251,7 +254,7 @@
        ]
      },
      {
-        "exception": "RedeemScript can\\'t be P2SH",
+        "exception": "RedeemScript not supported \\(scripthash\\)",
        "inputs": [
          {
            "index": 1,
@ -313,13 +316,14 @@
        ]
      },
      {
+        "description": "Wrong private key for multisig redeemScript",
        "exception": "privateKey cannot sign for this input",
        "inputs": [
          {
            "index": 1,
            "prevTx": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
            "privKeys": [
-              "91avARGdfge8E4tZfYLoxeJ5sGBdNJQH4kvjJoQFacbgwmaKkrx"
+              "KzrA86mCVMGWnLGBQu9yzQa32qbxb5dvSK4XhyjjGAWSBKYX4rHx"
            ],
            "redeemScript": "OP_1 0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8 OP_1 OP_CHECKMULTISIG",
            "throws": 0
@ -356,11 +360,7 @@
    "fromTransaction": [
      {
        "exception": "coinbase inputs not supported",
-        "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000000000006b483045022100a3b254e1c10b5d039f36c05f323995d6e5a367d98dd78a13d5bbc3991b35720e022022fccea3897d594de0689601fbd486588d5bfa6915be2386db0397ee9a6e80b601210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798ffffffff0110270000000000001976a914aa4d7985c57e011a8b3dd8e0e5a73aaef41629c588ac00000000"
-      },
-      {
-        "exception": "nonstandard inputs not supported",
-        "hex": "0100000001ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0000000023aa206fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d619000000000087ffffffff0110270000000000001976a914aa4d7985c57e011a8b3dd8e0e5a73aaef41629c588ac00000000"
+        "txHex": "01000000010000000000000000000000000000000000000000000000000000000000000000000000006b483045022100a3b254e1c10b5d039f36c05f323995d6e5a367d98dd78a13d5bbc3991b35720e022022fccea3897d594de0689601fbd486588d5bfa6915be2386db0397ee9a6e80b601210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798ffffffff0110270000000000001976a914aa4d7985c57e011a8b3dd8e0e5a73aaef41629c588ac00000000"
      }
    ]
  }
--- a/test/transaction_builder.js
+++ b/test/transaction_builder.js
@ -37,7 +37,7 @@ describe('TransactionBuilder', function() {
      assert.equal(txin.hash, prevTxHash)
      assert.equal(txin.index, 1)
      assert.equal(txin.sequence, 54)
-      assert.equal(txb.prevOutScripts[0], undefined)
+      assert.equal(txb.inputs[0].prevOutScript, undefined)
    })

    it('accepts a txHash, index [, sequence number and scriptPubKey]', function() {
@ -48,7 +48,7 @@ describe('TransactionBuilder', function() {
      assert.equal(txin.hash, prevTxHash)
      assert.equal(txin.index, 1)
      assert.equal(txin.sequence, 54)
-      assert.equal(txb.prevOutScripts[0], prevTx.outs[1].script)
+      assert.equal(txb.inputs[0].prevOutScript, prevTx.outs[1].script)
    })

    it('accepts a prevTx, index [and sequence number]', function() {
@ -59,7 +59,7 @@ describe('TransactionBuilder', function() {
      assert.deepEqual(txin.hash, prevTxHash)
      assert.equal(txin.index, 1)
      assert.equal(txin.sequence, 54)
-      assert.equal(txb.prevOutScripts[0], prevTx.outs[1].script)
+      assert.equal(txb.inputs[0].prevOutScript, prevTx.outs[1].script)
    })

    it('returns the input index', function() {
@ -67,12 +67,6 @@ describe('TransactionBuilder', function() {
      assert.equal(txb.addInput(prevTxHash, 1), 1)
    })

-    it('throws if prevOutScript is not supported', function() {
-      assert.throws(function() {
-        txb.addInput(prevTxHash, 0, undefined, Script.EMPTY)
-      }, /PrevOutScript not supported \(nonstandard\)/)
-    })
-
    it('throws if SIGHASH_ALL has been used to sign any existing scriptSigs', function() {
      txb.addInput(prevTxHash, 0)
      txb.sign(0, privKey)
@ -101,8 +95,8 @@ describe('TransactionBuilder', function() {
        txb.addInput(prevTxHash, 0)
        txb.sign(0, privKey)

-        assert.strictEqual(txb.signatures[0].redeemScript, undefined)
-        assert.equal(txb.signatures[0].scriptType, 'pubkeyhash')
+        assert.equal(txb.inputs[0].scriptType, 'pubkeyhash')
+        assert.equal(txb.inputs[0].redeemScript, undefined)
      })
    })

@ -111,7 +105,8 @@ describe('TransactionBuilder', function() {
        txb.addInput(prevTxHash, 0)
        txb.sign(0, privKey, privScript)

-        assert.equal(txb.signatures[0].redeemScript, privScript)
+        assert.equal(txb.inputs[0].prevOutType, 'scripthash')
+        assert.equal(txb.inputs[0].redeemScript, privScript)
      })

      it('throws if hashType is inconsistent', function() {
@ -139,7 +134,7 @@ describe('TransactionBuilder', function() {
    })

    fixtures.invalid.sign.forEach(function(f) {
-      it('throws on ' + f.exception, function() {
+      it('throws on ' + f.exception + ' (' + f.description + ')', function() {
        f.inputs.forEach(function(input) {
          var prevTxScript

@ -218,13 +213,12 @@ describe('TransactionBuilder', function() {
        if (f.locktime !== undefined) txb.tx.locktime = f.locktime

        var tx = txb.build()
-
        assert.equal(tx.toHex(), f.txhex)
      })
    })

    fixtures.invalid.build.forEach(function(f) {
-      it('throws on ' + f.exception, function() {
+      it('throws on ' + f.exception + ' (' + f.description + ')', function() {
        f.inputs.forEach(function(input) {
          var prevTxScript

@ -274,7 +268,7 @@ describe('TransactionBuilder', function() {

    fixtures.invalid.fromTransaction.forEach(function(f) {
      it('throws on ' + f.exception, function() {
-        var tx = Transaction.fromHex(f.hex)
+        var tx = Transaction.fromHex(f.txHex)

        assert.throws(function() {
          TransactionBuilder.fromTransaction(tx)
@ -282,6 +276,7 @@ describe('TransactionBuilder', function() {
      })
    })

+    // TODO: test for reverse order signing
    it('works for the P2SH multisig case', function() {
      var privKeys = [
        "91avARGdfge8E4tZfYLoxeJ5sGBdNJQH4kvjJoQFacbgwmaKkrx",