Repository: bitcoinjs/bitcoinjs-lib
Pull request #401 (2015-04-28 02:07:15)

Remove instantiable Address
~~WIP pull request.~~ Ready to merge

weilu: @dcousens Let's finish this.
dcousens: @weilu sounds good,  I'll get back on this today.
dcousens: Tests failing for unrelated reasons.  I intend to fix that.

@weilu please review.
Should be quickest to review commit-by-commit.

The API could maybe change,  but lets do that in a different PR,  I want to get this merged and leave the API bike shedding separate. 

@jprichardson feel free to review also. :)
dcousens: Intend to merge ASAP.  Any comments/qualms?
jprichardson: Most of it looks good. I like this change. My only thought is not removing the tests (you'd still keep this commit as is https://github.com/bitcoinjs/bitcoinjs-lib/commit/16a711ba3309e0a4b8f0751cab5b1ab6e7366c40 i.e. remove these tests as you have). But by not removing the other tests and maybe just changing them to errors helps to communicate how the expectations have changed. I'm fine with it as is, just a thought.

Also, what's with bundling the commit to add the network into TxBuilder into this PR? 

Patch:
diff --git a/src/address.js b/src/address.js
index 4b29a1fb..1b419a6f 100644
--- a/src/address.js
+++ b/src/address.js
@@ -1,4 +1,3 @@
-var assert = require('assert')
 var base58check = require('bs58check')
 var typeForce = require('typeforce')
 var networks = require('./networks')
@@ -13,50 +12,55 @@ function findScriptTypeByVersion (version) {
   }
 }
 
-function Address (hash, version) {
-  typeForce('Buffer', hash)
-
-  assert.strictEqual(hash.length, 20, 'Invalid hash length')
-  assert.strictEqual(version & 0xff, version, 'Invalid version byte')
-
-  this.hash = hash
-  this.version = version
-}
-
-Address.fromBase58Check = function (string) {
+function fromBase58Check (string) {
   var payload = base58check.decode(string)
+  if (payload.length !== 21) throw new TypeError('Invalid address length')
+
   var version = payload.readUInt8(0)
   var hash = payload.slice(1)
 
-  return new Address(hash, version)
+  return { hash: hash, version: version }
 }
 
-Address.fromOutputScript = function (script, network) {
+function fromOutputScript (script, network) {
   network = network || networks.bitcoin
 
-  if (scripts.isPubKeyHashOutput(script)) return new Address(script.chunks[2], network.pubKeyHash)
-  if (scripts.isScriptHashOutput(script)) return new Address(script.chunks[1], network.scriptHash)
+  if (scripts.isPubKeyHashOutput(script)) return toBase58Check(script.chunks[2], network.pubKeyHash)
+  if (scripts.isScriptHashOutput(script)) return toBase58Check(script.chunks[1], network.scriptHash)
 
   throw new Error(script.toASM() + ' has no matching Address')
 }
 
-Address.prototype.toBase58Check = function () {
+function toBase58Check (hash, version) {
+  typeForce('Buffer', hash)
+
+  if (hash.length !== 20) throw new TypeError('Invalid hash length')
+  if (version & ~0xff) throw new TypeError('Invalid version byte')
+
   var payload = new Buffer(21)
-  payload.writeUInt8(this.version, 0)
-  this.hash.copy(payload, 1)
+  payload.writeUInt8(version, 0)
+  hash.copy(payload, 1)
 
   return base58check.encode(payload)
 }
 
-Address.prototype.toOutputScript = function () {
-  var scriptType = findScriptTypeByVersion(this.version)
+function toOutputScript (address) {
+  var payload = base58check.decode(address)
+  if (payload.length !== 21) throw new TypeError('Invalid hash length')
 
-  if (scriptType === 'pubkeyhash') return scripts.pubKeyHashOutput(this.hash)
-  if (scriptType === 'scripthash') return scripts.scriptHashOutput(this.hash)
+  var version = payload.readUInt8(0)
+  var hash = payload.slice(1)
+  var scriptType = findScriptTypeByVersion(version)
 
-  throw new Error(this.toString() + ' has no matching Script')
-}
+  if (scriptType === 'pubkeyhash') return scripts.pubKeyHashOutput(hash)
+  if (scriptType === 'scripthash') return scripts.scriptHashOutput(hash)
 
-Address.prototype.toString = Address.prototype.toBase58Check
+  throw new Error(address + ' has no matching Script')
+}
 
-module.exports = Address
+module.exports = {
+  fromBase58Check: fromBase58Check,
+  fromOutputScript: fromOutputScript,
+  toBase58Check: toBase58Check,
+  toOutputScript: toOutputScript
+}
diff --git a/src/ecpair.js b/src/ecpair.js
index a8042c6f..c6298354 100644
--- a/src/ecpair.js
+++ b/src/ecpair.js
@@ -1,5 +1,5 @@
 var assert = require('assert')
-var base58check = require('bs58check')
+var bs58check = require('bs58check')
 var bcrypto = require('./crypto')
 var ecdsa = require('./ecdsa')
 var ecurve = require('ecurve')
@@ -7,7 +7,6 @@ var networks = require('./networks')
 var randomBytes = require('randombytes')
 var typeForce = require('typeforce')
 
-var Address = require('./address')
 var BigInteger = require('bigi')
 
 function findNetworkByWIFVersion (version) {
@@ -69,7 +68,7 @@ ECPair.fromPublicKeyBuffer = function (buffer, network) {
 }
 
 ECPair.fromWIF = function (string) {
-  var payload = base58check.decode(string)
+  var payload = bs58check.decode(string)
   var version = payload.readUInt8(0)
   var compressed
 
@@ -125,13 +124,18 @@ ECPair.prototype.toWIF = function () {
     buffer.writeUInt8(0x01, 33)
   }
 
-  return base58check.encode(buffer)
+  return bs58check.encode(buffer)
 }
 
 ECPair.prototype.getAddress = function () {
   var pubKey = this.getPublicKeyBuffer()
+  var pubKeyHash = bcrypto.hash160(pubKey)
 
-  return new Address(bcrypto.hash160(pubKey), this.network.pubKeyHash)
+  var payload = new Buffer(21)
+  payload.writeUInt8(this.network.pubKeyHash, 0)
+  pubKeyHash.copy(payload, 1)
+
+  return bs58check.encode(payload)
 }
 
 ECPair.prototype.getPublicKeyBuffer = function () {
diff --git a/src/message.js b/src/message.js
index f3791fea..b2c584c4 100644
--- a/src/message.js
+++ b/src/message.js
@@ -47,7 +47,7 @@ function verify (address, signature, message, network) {
     network: network
   })
 
-  return keyPair.getAddress().toString() === address.toString()
+  return keyPair.getAddress() === address
 }
 
 module.exports = {
diff --git a/src/transaction_builder.js b/src/transaction_builder.js
index 7ddb6a6b..186c93db 100644
--- a/src/transaction_builder.js
+++ b/src/transaction_builder.js
@@ -1,4 +1,5 @@
 var assert = require('assert')
+var bcrypto = require('./crypto')
 var bufferutils = require('./bufferutils')
 var ops = require('./opcodes')
 var scripts = require('./scripts')
@@ -36,7 +37,7 @@ function extractInput (txIn) {
       hashType = parsed.hashType
       pubKeys = scriptSig.chunks.slice(1)
       signatures = [parsed.signature]
-      prevOutScript = ECPair.fromPublicKeyBuffer(pubKeys[0]).getAddress().toOutputScript()
+      prevOutScript = Address.toOutputScript(ECPair.fromPublicKeyBuffer(pubKeys[0]).getAddress())
 
       break
     }
@@ -187,12 +188,7 @@ TransactionBuilder.prototype.addOutput = function (scriptPubKey, value) {
 
   // Attempt to get a valid address if it's a base58 address string
   if (typeof scriptPubKey === 'string') {
-    scriptPubKey = Address.fromBase58Check(scriptPubKey)
-  }
-
-  // Attempt to get a valid script if it's an Address object
-  if (scriptPubKey instanceof Address) {
-    scriptPubKey = scriptPubKey.toOutputScript()
+    scriptPubKey = Address.toOutputScript(scriptPubKey)
   }
 
   return this.tx.addOutput(scriptPubKey, value)
@@ -330,7 +326,7 @@ TransactionBuilder.prototype.sign = function (index, keyPair, redeemScript, hash
 
         case 'pubkeyhash': {
           var pkh1 = redeemScript.chunks[2]
-          var pkh2 = keyPair.getAddress().hash
+          var pkh2 = bcrypto.hash160(keyPair.getPublicKeyBuffer())
 
           assert.deepEqual(pkh1, pkh2, 'privateKey cannot sign for this input')
           pubKeys = [kpPubKey]
@@ -362,7 +358,7 @@ TransactionBuilder.prototype.sign = function (index, keyPair, redeemScript, hash
 
       // we know nothin' Jon Snow, assume pubKeyHash
       } else {
-        input.prevOutScript = keyPair.getAddress().toOutputScript()
+        input.prevOutScript = Address.toOutputScript(keyPair.getAddress())
         input.prevOutType = 'pubkeyhash'
         input.pubKeys = [kpPubKey]
         input.scriptType = input.prevOutType


Test patch:
diff --git a/test/address.js b/test/address.js
index 5259fdc0..666832c7 100644
--- a/test/address.js
+++ b/test/address.js
@@ -9,25 +9,13 @@ var Script = require('../src/script')
 var fixtures = require('./fixtures/address.json')
 
 describe('Address', function () {
-  describe('Constructor', function () {
-    it('does not mutate the input', function () {
-      fixtures.valid.forEach(function (f) {
-        var hash = new Buffer(f.hash, 'hex')
-        var addr = new Address(hash, f.version)
-
-        assert.strictEqual(addr.version, f.version)
-        assert.strictEqual(addr.hash.toString('hex'), f.hash)
-      })
-    })
-  })
-
   describe('fromBase58Check', function () {
     fixtures.valid.forEach(function (f) {
-      it('imports ' + f.script + ' (' + f.network + ') correctly', function () {
-        var addr = Address.fromBase58Check(f.base58check)
+      it('decodes ' + f.base58check, function () {
+        var decode = Address.fromBase58Check(f.base58check)
 
-        assert.strictEqual(addr.version, f.version)
-        assert.strictEqual(addr.hash.toString('hex'), f.hash)
+        assert.strictEqual(decode.version, f.version)
+        assert.strictEqual(decode.hash.toString('hex'), f.hash)
       })
     })
 
@@ -42,12 +30,11 @@ describe('Address', function () {
 
   describe('fromOutputScript', function () {
     fixtures.valid.forEach(function (f) {
-      it('imports ' + f.script + ' (' + f.network + ') correctly', function () {
+      it('parses ' + f.script + ' (' + f.network + ')', function () {
         var script = Script.fromASM(f.script)
-        var addr = Address.fromOutputScript(script, networks[f.network])
+        var address = Address.fromOutputScript(script, networks[f.network])
 
-        assert.strictEqual(addr.version, f.version)
-        assert.strictEqual(addr.hash.toString('hex'), f.hash)
+        assert.strictEqual(address, f.base58check)
       })
     })
 
@@ -64,20 +51,18 @@ describe('Address', function () {
 
   describe('toBase58Check', function () {
     fixtures.valid.forEach(function (f) {
-      it('exports ' + f.script + ' (' + f.network + ') correctly', function () {
-        var addr = Address.fromBase58Check(f.base58check)
-        var result = addr.toBase58Check()
+      it('formats ' + f.hash + ' (' + f.network + ')', function () {
+        var address = Address.toBase58Check(new Buffer(f.hash, 'hex'), f.version)
 
-        assert.strictEqual(result, f.base58check)
+        assert.strictEqual(address, f.base58check)
       })
     })
   })
 
   describe('toOutputScript', function () {
     fixtures.valid.forEach(function (f) {
-      it('imports ' + f.script + ' (' + f.network + ') correctly', function () {
-        var addr = Address.fromBase58Check(f.base58check)
-        var script = addr.toOutputScript()
+      it('exports ' + f.script + '(' + f.network + ')', function () {
+        var script = Address.toOutputScript(f.base58check)
 
         assert.strictEqual(script.toASM(), f.script)
       })
@@ -85,10 +70,8 @@ describe('Address', function () {
 
     fixtures.invalid.toOutputScript.forEach(function (f) {
       it('throws when ' + f.description, function () {
-        var addr = new Address(new Buffer(f.hash, 'hex'), f.version)
-
         assert.throws(function () {
-          addr.toOutputScript()
+          Address.toOutputScript(f.address)
         }, new RegExp(f.description))
       })
     })
diff --git a/test/bitcoin.core.js b/test/bitcoin.core.js
index a2d5b524..2c058b58 100644
--- a/test/bitcoin.core.js
+++ b/test/bitcoin.core.js
@@ -47,15 +47,15 @@ describe('Bitcoin-core', function () {
   })
 
   // base58_keys_valid
-  describe('Address.formBase58Check', function () {
+  describe('Address.toBase58Check', function () {
     var typeMap = {
       'pubkey': 'pubKeyHash',
       'script': 'scriptHash'
     }
 
     base58_keys_valid.forEach(function (f) {
-      var string = f[0]
-      var hex = f[1]
+      var expected = f[0]
+      var hash = new Buffer(f[1], 'hex')
       var params = f[2]
       var network = networks.bitcoin
 
@@ -64,11 +64,10 @@ describe('Bitcoin-core', function () {
         network = networks.testnet
       }
 
-      it('can import ' + string, function () {
-        var address = Address.fromBase58Check(string)
+      var version = network[typeMap[params.addrType]]
 
-        assert.strictEqual(address.hash.toString('hex'), hex)
-        assert.strictEqual(address.version, network[typeMap[params.addrType]])
+      it('can export ' + expected, function () {
+        assert.strictEqual(Address.toBase58Check(hash, version), expected)
       })
     })
   })
@@ -90,7 +89,7 @@ describe('Bitcoin-core', function () {
           var address = Address.fromBase58Check(string)
 
           assert.notEqual(allowedNetworks.indexOf(address.version), -1, 'Invalid network')
-        }, /Invalid (checksum|hash length|network)/)
+        }, /Invalid (checksum|address length|network)/)
       })
     })
   })
diff --git a/test/ecpair.js b/test/ecpair.js
index 5137c526..4408ca18 100644
--- a/test/ecpair.js
+++ b/test/ecpair.js
@@ -171,7 +171,7 @@ describe('ECPair', function () {
       it('returns ' + f.address + ' for ' + f.WIF, function () {
         var keyPair = ECPair.fromWIF(f.WIF)
 
-        assert.strictEqual(keyPair.getAddress().toString(), f.address)
+        assert.strictEqual(keyPair.getAddress(), f.address)
       })
     })
   })
diff --git a/test/fixtures/address.json b/test/fixtures/address.json
index fff8d9a5..02c36a14 100644
--- a/test/fixtures/address.json
+++ b/test/fixtures/address.json
@@ -34,12 +34,12 @@
       {
         "description": "hash too short",
         "base58check": "7SeEnXWPaCCALbVrTnszCVGfRU8cGfx",
-        "exception": "Invalid hash length"
+        "exception": "Invalid address length"
       },
       {
         "description": "hash too long",
         "base58check": "j9ywUkWg2fTQrouxxh5rSZhRvrjMkEUfuiKe",
-        "exception": "Invalid hash length"
+        "exception": "Invalid address length"
       }
     ],
     "fromOutputScript": [
@@ -59,8 +59,7 @@
     "toOutputScript": [
       {
         "description": "24kPZCmVgzfkpGdXExy56234MRHrsqQxNWE has no matching Script",
-        "hash": "751e76e8199196d454941c45d1b3a323f1433bd6",
-        "version": 153
+        "address": "24kPZCmVgzfkpGdXExy56234MRHrsqQxNWE"
       }
     ]
   }
diff --git a/test/integration/advanced.js b/test/integration/advanced.js
index 81998c44..23629a7d 100644
--- a/test/integration/advanced.js
+++ b/test/integration/advanced.js
@@ -29,7 +29,7 @@ describe('bitcoinjs-lib (advanced)', function () {
     var keyPair = bitcoin.ECPair.makeRandom({
       network: bitcoin.networks.testnet
     })
-    var address = keyPair.getAddress().toString()
+    var address = keyPair.getAddress()
 
     faucetWithdraw(address, 2e4, function (err) {
       if (err) return done(err)
diff --git a/test/integration/basic.js b/test/integration/basic.js
index 4c90dcd4..981ffb66 100644
--- a/test/integration/basic.js
+++ b/test/integration/basic.js
@@ -11,7 +11,7 @@ describe('bitcoinjs-lib (basic)', function () {
 
     // generate random keyPair
     var keyPair = bitcoin.ECPair.makeRandom({ rng: rng })
-    var address = keyPair.getAddress().toString()
+    var address = keyPair.getAddress()
 
     assert.strictEqual(address, '1F5VhMHukdnUES9kfXqzPzMeF1GPHKiF64')
   })
@@ -21,7 +21,7 @@ describe('bitcoinjs-lib (basic)', function () {
     var d = bigi.fromBuffer(hash)
 
     var keyPair = new bitcoin.ECPair(d)
-    var address = keyPair.getAddress().toString()
+    var address = keyPair.getAddress()
 
     assert.strictEqual(address, '1C7zdTfnkzmr13HfA2vNm5SJYRK6nEKyq8')
   })
@@ -34,7 +34,7 @@ describe('bitcoinjs-lib (basic)', function () {
 
     var keyPair = bitcoin.ECPair.makeRandom({ network: litecoin, rng: rng })
     var wif = keyPair.toWIF()
-    var address = keyPair.getAddress().toString()
+    var address = keyPair.getAddress()
 
     assert.strictEqual(address, 'LZJSxZbjqJ2XVEquqfqHg1RQTDdfST5PTn')
     assert.strictEqual(wif, 'T7A4PUSgTDHecBxW1ZiYFrDNRih2o7M8Gf9xpoCgudPF9gDiNvuS')
@@ -42,7 +42,7 @@ describe('bitcoinjs-lib (basic)', function () {
 
   it('can import an address via WIF', function () {
     var keyPair = bitcoin.ECPair.fromWIF('Kxr9tQED9H44gCmp6HAdmemAzU3n84H3dGkuWTKvE23JgHMW8gct')
-    var address = keyPair.getAddress().toString()
+    var address = keyPair.getAddress()
 
     assert.strictEqual(address, '19AAjaTUbRjQCMuVczepkoPswiZRhjtg31')
   })
diff --git a/test/integration/crypto.js b/test/integration/crypto.js
index 3d8f3654..f70047ea 100644
--- a/test/integration/crypto.js
+++ b/test/integration/crypto.js
@@ -35,7 +35,7 @@ describe('bitcoinjs-lib (crypto)', function () {
     assert.deepEqual(QprimeR.getEncoded(), QprimeS.getEncoded())
 
     // derived shared-secret address
-    var address = new bitcoin.ECPair(null, QprimeS).getAddress().toString()
+    var address = new bitcoin.ECPair(null, QprimeS).getAddress()
 
     assert.strictEqual(address, '1EwCNJNZM5q58YPPTnjR1H5BvYRNeyZi47')
   })
diff --git a/test/integration/multisig.js b/test/integration/multisig.js
index 15d5a737..7de1aa0b 100644
--- a/test/integration/multisig.js
+++ b/test/integration/multisig.js
@@ -58,7 +58,7 @@ describe('bitcoinjs-lib (multisig)', function () {
         // make a random destination address
         var targetAddress = bitcoin.ECPair.makeRandom({
           network: bitcoin.networks.testnet
-        }).getAddress().toString()
+        }).getAddress()
 
         var txb = new bitcoin.TransactionBuilder()
         txb.addInput(unspent.txId, unspent.vout)
diff --git a/test/message.js b/test/message.js
index b478a663..fb116b0d 100644
--- a/test/message.js
+++ b/test/message.js
@@ -4,7 +4,6 @@ var assert = require('assert')
 var message = require('../src/message')
 var networks = require('../src/networks')
 
-var Address = require('../src/address')
 var BigInteger = require('bigi')
 var ECPair = require('../src/ecpair')
 
@@ -23,14 +22,6 @@ describe('message', function () {
   })
 
   describe('verify', function () {
-    it('accepts an Address object', function () {
-      var f = fixtures.valid.verify[0]
-      var network = networks[f.network]
-
-      var address = Address.fromBase58Check(f.address)
-      assert(message.verify(address, f.signature, f.message, network))
-    })
-
     fixtures.valid.verify.forEach(function (f) {
       it('verifies a valid signature for "' + f.message + '" (' + f.network + ')', function () {
         var network = networks[f.network]
diff --git a/test/scripts.js b/test/scripts.js
index 4e0f6441..f89c44a0 100644
--- a/test/scripts.js
+++ b/test/scripts.js
@@ -1,10 +1,10 @@
 /* global describe, it */
 
 var assert = require('assert')
+var bcrypto = require('../src/crypto')
 var ops = require('../src/opcodes')
 var scripts = require('../src/scripts')
 
-var ECPair = require('../src/ecpair')
 var Script = require('../src/script')
 
 var fixtures = require('./fixtures/scripts.json')
@@ -177,10 +177,10 @@ describe('Scripts', function () {
       if (f.type !== 'pubkeyhash') return
 
       var pubKey = new Buffer(f.pubKey, 'hex')
-      var address = ECPair.fromPublicKeyBuffer(pubKey).getAddress()
+      var pubKeyHash = bcrypto.hash160(pubKey)
 
       it('returns ' + f.scriptPubKey, function () {
-        var scriptPubKey = scripts.pubKeyHashOutput(address.hash)
+        var scriptPubKey = scripts.pubKeyHashOutput(pubKeyHash)
         assert.strictEqual(scriptPubKey.toASM(), f.scriptPubKey)
       })
     })
diff --git a/test/transaction_builder.js b/test/transaction_builder.js
index ccb3ae0b..09dcb9a0 100644
--- a/test/transaction_builder.js
+++ b/test/transaction_builder.js
@@ -65,13 +65,13 @@ describe('TransactionBuilder', function () {
     txb = new TransactionBuilder()
 
     prevTx = new Transaction()
-    prevTx.addOutput(Address.fromBase58Check('1BgGZ9tcN4rm9KBzDn7KprQz87SZ26SAMH').toOutputScript(), 0)
-    prevTx.addOutput(Address.fromBase58Check('1cMh228HTCiwS8ZsaakH8A8wze1JR5ZsP').toOutputScript(), 1)
+    prevTx.addOutput(Address.toOutputScript('1BgGZ9tcN4rm9KBzDn7KprQz87SZ26SAMH'), 0)
+    prevTx.addOutput(Address.toOutputScript('1cMh228HTCiwS8ZsaakH8A8wze1JR5ZsP'), 1)
     prevTxHash = prevTx.getHash()
 
     keyPair = new ECPair(BigInteger.ONE)
     privAddress = keyPair.getAddress()
-    privScript = privAddress.toOutputScript()
+    privScript = Address.toOutputScript(privAddress)
   })
 
   describe('addInput', function () {
@@ -125,15 +125,6 @@ describe('TransactionBuilder', function () {
 
   describe('addOutput', function () {
     it('accepts an address string and value', function () {
-      var vout = txb.addOutput(privAddress.toBase58Check(), 1000)
-      assert.strictEqual(vout, 0)
-
-      var txout = txb.tx.outs[0]
-      assert.deepEqual(txout.script, privScript)
-      assert.strictEqual(txout.value, 1000)
-    })
-
-    it('accepts an Address object and value', function () {
       var vout = txb.addOutput(privAddress, 1000)
       assert.strictEqual(vout, 0)