Repository: bitcoinjs/bitcoinjs-lib
Pull request #507 (2015-11-26 01:49:00)

txbuilder: add setLockTime
WIP, needs tests.

Open to feedback, not the biggest fan, but, its simple enough.

Maybe I should use a property instead?

dcousens: Resolves https://github.com/bitcoinjs/bitcoinjs-lib/issues/437

dcousens: ping @jprichardson thoughts on API?
jprichardson: `setLockTime()` is fine. 

An aside, builders are commonly used in conjunction with a fluent interface. Since it doesn't, what's the rationalization for splitting up `Transaction` / `TransactionBuilder`? (Really doesn't have to be answered here)
dcousens: `Transaction` is just the data structure,  no state,  capable of being manually mutated and it will still serialize correctly.
To which, yes, it probably does make sense to remove `addOutput` on it and just have `transaction.outputs.push`  etc.
dcousens: > An aside, builders are commonly used in conjunction with a fluent interface. 

Any ideas on how we could improve this interface?
weilu: The API looks ok to me.
dcousens: @jprichardson not going to merge this without status checks as tests have been added.
Please verify manually and merge when ready. :+1: 

Patch:
diff --git a/src/transaction_builder.js b/src/transaction_builder.js
index 73fd12b9..64069f2b 100644
--- a/src/transaction_builder.js
+++ b/src/transaction_builder.js
@@ -4,6 +4,8 @@ var bscript = require('./script')
 var bufferEquals = require('buffer-equals')
 var networks = require('./networks')
 var ops = require('./opcodes')
+var typeforce = require('typeforce')
+var types = require('./types')
 
 var ECPair = require('./ecpair')
 var ECSignature = require('./ecsignature')
@@ -135,6 +137,21 @@ function TransactionBuilder (network) {
   this.tx = new Transaction()
 }
 
+TransactionBuilder.prototype.setLockTime = function (locktime) {
+  typeforce(types.UInt32, locktime)
+
+  // if any signatures exist, throw
+  if (this.inputs.some(function (input) {
+    if (!input.signatures) return false
+
+    return input.signatures.some(function (s) { return s })
+  })) {
+    throw new Error('No, this would invalidate signatures')
+  }
+
+  this.tx.locktime = locktime
+}
+
 TransactionBuilder.fromTransaction = function (transaction, network) {
   var txb = new TransactionBuilder(network)
 


Test patch:
diff --git a/test/fixtures/transaction_builder.json b/test/fixtures/transaction_builder.json
index 2ccda67a..f17cecaa 100644
--- a/test/fixtures/transaction_builder.json
+++ b/test/fixtures/transaction_builder.json
@@ -263,6 +263,28 @@
             "value": 10000
           }
         ]
+      },
+      {
+        "description": "Transaction w/ nLockTime, pubKeyHash -> pubKeyHash",
+        "txHex": "0100000001ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff000000006b483045022100a3b254e1c10b5d039f36c05f323995d6e5a367d98dd78a13d5bbc3991b35720e022022fccea3897d594de0689601fbd486588d5bfa6915be2386db0397ee9a6e80b601210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798ffffffff0110270000000000001976a914aa4d7985c57e011a8b3dd8e0e5a73aaef41629c588acffff0000",
+        "locktime": 65535,
+        "inputs": [
+          {
+            "txId": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+            "vout": 0,
+            "signs": [
+              {
+                "keyPair": "KwDiBf89QgGbjEhKnhXJuH7LrciVrZi3qYjgd9M7rFU73sVHnoWn"
+              }
+            ]
+          }
+        ],
+        "outputs": [
+          {
+            "script": "OP_DUP OP_HASH160 aa4d7985c57e011a8b3dd8e0e5a73aaef41629c5 OP_EQUALVERIFY OP_CHECKSIG",
+            "value": 10000
+          }
+        ]
       }
     ],
     "fromTransaction": [
diff --git a/test/integration/advanced.js b/test/integration/advanced.js
index c28e78d3..16c09cfc 100644
--- a/test/integration/advanced.js
+++ b/test/integration/advanced.js
@@ -95,13 +95,11 @@ describe('bitcoinjs-lib (advanced)', function () {
       this.timeout(30000)
 
       var tx2 = new bitcoin.TransactionBuilder(network)
-//       tx2.setLockTime(threeHoursAgo) // TODO
+      tx2.setLockTime(threeHoursAgo)
       tx2.addInput(txId, 0, 0xfffffffe)
       tx2.addOutput(alice.getAddress(), 1000)
 
       var tx2Raw = tx2.buildIncomplete()
-      tx2Raw.locktime = threeHoursAgo // TODO
-
       var hashType = bitcoin.Transaction.SIGHASH_ALL
       var signatureHash = tx2Raw.hashForSignature(0, redeemScript, hashType)
       var signature = alice.sign(signatureHash)
@@ -120,13 +118,11 @@ describe('bitcoinjs-lib (advanced)', function () {
       this.timeout(30000)
 
       var tx2 = new bitcoin.TransactionBuilder(network)
-//       tx2.setLockTime(threeHoursAgo) // TODO
+      tx2.setLockTime(threeHoursAgo)
       tx2.addInput(txId, 0, 0xfffffffe)
       tx2.addOutput(alice.getAddress(), 1000)
 
       var tx2Raw = tx2.buildIncomplete()
-      tx2Raw.locktime = threeHoursAgo // TODO
-
       var hashType = bitcoin.Transaction.SIGHASH_ALL
       var signatureHash = tx2Raw.hashForSignature(0, redeemScript, hashType)
       var signatureA = alice.sign(signatureHash)
diff --git a/test/transaction_builder.js b/test/transaction_builder.js
index 5720d625..afbb6d9a 100644
--- a/test/transaction_builder.js
+++ b/test/transaction_builder.js
@@ -17,13 +17,13 @@ function construct (f, sign) {
   var network = NETWORKS[f.network]
   var txb = new TransactionBuilder(network)
 
-  // FIXME: add support for locktime/version in TransactionBuilder API
+  // FIXME: add support for version in TransactionBuilder API
   if (f.version !== undefined) {
     txb.tx.version = f.version
   }
 
   if (f.locktime !== undefined) {
-    txb.tx.locktime = f.locktime
+    txb.setLockTime(f.locktime)
   }
 
   f.inputs.forEach(function (input) {
@@ -241,6 +241,17 @@ describe('TransactionBuilder', function () {
     })
   })
 
+  describe('setLockTime', function () {
+    it('throws if if there exist any scriptSigs', function () {
+      txb.addInput(txHash, 0)
+      txb.sign(0, keyPair)
+
+      assert.throws(function () {
+        txb.setLockTime(65535)
+      }, /No, this would invalidate signatures/)
+    })
+  })
+
   describe('sign', function () {
     fixtures.invalid.sign.forEach(function (f) {
       it('throws on ' + f.exception + (f.description ? ' (' + f.description + ')' : ''), function () {