Merge pull request #134 from liquality/fix-address-retrieval

Fix BitcoinLedgerProvider retrieving public key from address not using cache

Author: monokh

src/Client.js

@@ -323,15 +323,6 @@ export default class Client {
     return addresses
   }
 
-  async getAddressExtendedPubKeys (startingIndex = 0, numAddresses = 1) {
-    const xpubkey = await this.getMethod('getAddressExtendedPubKeys')(startingIndex, numAddresses)
-
-    if (!isArray(xpubkey)) {
-      throw new InvalidProviderResponseError('Provider returned an invalid response')
-    }
-
-    return xpubkey
-  }
   /**
    * Check if an address has been used or not.
    * @param {!string|Address} addresses - An address to check for.
@@ -434,7 +425,7 @@ export default class Client {
    * @return {Promise<string>} Resolves with secret
    */
   async generateSecret (message) {
-    const address = (await this.getMethod('getAddresses')())[0]
+    const address = (await this.getMethod('getAddresses')())[0].address
     const signedMessage = await this.signMessage(message, address)
     const secret = sha256(signedMessage)
     return secret

src/providers/LedgerProvider.js

@@ -41,7 +41,7 @@ export default class LedgerProvider extends Provider {
     return this._baseDerivationPath + changePath + index
   }
 
-  async getDerivationPathFromAddress (address) {
+  async getWalletAddress (address) {
     let index = 0
     let change = false
 
@@ -54,7 +54,7 @@ export default class LedgerProvider extends Provider {
       const addrs = await this.getAddresses(index, addressesPerCall)
       const addr = addrs.find(addr => addr.address === address)
       if (addr) {
-        return addr.derivationPath
+        return addr
       }
       index += addressesPerCall
       if (index === maxAddresses && change === false) {
@@ -76,18 +76,6 @@ export default class LedgerProvider extends Provider {
     return address
   }
 
-  async getAddressExtendedPubKeys (startingIndex = 0, numAddresses = 1) {
-    const xpubkeys = []
-    const lastIndex = startingIndex + numAddresses
-
-    for (let currentIndex = startingIndex; currentIndex < lastIndex; currentIndex++) {
-      const xpubkey = await this.getAddressExtendedPubKey(currentIndex)
-      xpubkeys.push(xpubkey)
-    }
-
-    return xpubkeys
-  }
-
   async getAddresses (startingIndex = 0, numAddresses = 1, change = false) {
     return this.getAddresses(startingIndex, numAddresses, change)
   }

src/providers/bitcoin/BitcoinJsLibSwapProvider.js

@@ -1,5 +1,5 @@
 import Provider from '../../Provider'
-import { addressToPubKeyHash, compressPubKey, pubKeyToAddress, reverseBuffer, scriptNumEncode } from './BitcoinUtil'
+import { addressToPubKeyHash, pubKeyToAddress, reverseBuffer, scriptNumEncode } from './BitcoinUtil'
 import { sha256, padHexStart } from '../../crypto'
 import networks from './networks'
 import bitcoin from 'bitcoinjs-lib'
@@ -102,42 +102,7 @@ export default class BitcoinJsLibSwapProvider extends Provider {
   }
 
   async refundSwap (initiationTxHash, recipientAddress, refundAddress, secretHash, expiration) {
-    return this._redeemSwap(initiationTxHash, recipientAddress, refundAddress, secretHash, expiration, false)
-  }
-
-  async _redeemSwap (initiationTxHash, recipientAddress, refundAddress, secretParam, expiration, isClaim) {
-    const secretHash = isClaim ? sha256(secretParam) : secretParam
-    const lockTime = isClaim ? 0 : expiration + 100
-    const lockTimeHex = isClaim ? padHexStart('0', 8) : padHexStart(scriptNumEncode(lockTime).toString('hex'), 8)
-    const to = isClaim ? recipientAddress : refundAddress
-    const script = this.createSwapScript(recipientAddress, refundAddress, secretHash, expiration)
-    const scriptPubKey = padHexStart(script)
-    const p2shAddress = pubKeyToAddress(scriptPubKey, this._network.name, 'scriptHash')
-    const sendScript = this.getMethod('createScript')(p2shAddress)
-    const initiationTxRaw = await this.getMethod('getRawTransactionByHash')(initiationTxHash)
-    const initiationTx = await this.getMethod('splitTransaction')(initiationTxRaw, true)
-    const voutIndex = initiationTx.outputs.findIndex((output) => output.script.toString('hex') === sendScript)
-    const txHashLE = Buffer.from(initiationTxHash, 'hex').reverse().toString('hex') // TX HASH IN LITTLE ENDIAN
-    const newTxInput = this.generateSigTxInput(txHashLE, voutIndex, script)
-    const newTx = this.generateRawTx(initiationTx, voutIndex, to, newTxInput, lockTimeHex)
-    const splitNewTx = await this.getMethod('splitTransaction')(newTx, true)
-    const outputScriptObj = await this.getMethod('serializeTransactionOutputs')(splitNewTx)
-    const outputScript = outputScriptObj.toString('hex')
-    const addressPath = await this.getMethod('getDerivationPathFromAddress')(to)
-    const signature = await this.getMethod('signP2SHTransaction')(
-      [[initiationTx, 0, script, 0]],
-      [addressPath],
-      outputScript,
-      lockTime
-    )
-    const pubKeyInfo = await this.getMethod('getPubKey')(isClaim ? recipientAddress : refundAddress)
-    const pubKey = compressPubKey(pubKeyInfo.publicKey)
-    const spendSwap = this._spendSwap(signature[0], pubKey, isClaim, secretParam)
-    const spendSwapInput = this._spendSwapInput(spendSwap, script)
-    const rawClaimTxInput = this.generateRawTxInput(txHashLE, spendSwapInput)
-    const rawClaimTx = this.generateRawTx(initiationTx, voutIndex, to, rawClaimTxInput, lockTimeHex)
-
-    return this.getMethod('sendRawTransaction')(rawClaimTx)
+    throw new Error('BitcoinJsLibSwapProvider: Refunding not implemented')
   }
 
   _spendSwap (signature, pubKey, isClaim, secret) {

src/providers/bitcoin/BitcoinLedgerProvider.js

@@ -2,8 +2,8 @@ import LedgerProvider from '../LedgerProvider'
 import Bitcoin from '@ledgerhq/hw-app-btc'
 
 import { BigNumber } from 'bignumber.js'
-import { base58, padHexStart, sha256, ripemd160 } from '../../crypto'
-import { pubKeyToAddress, addressToPubKeyHash, compressPubKey, createXPUB, encodeBase58Check, parseHexString } from './BitcoinUtil'
+import { base58, padHexStart } from '../../crypto'
+import { pubKeyToAddress, addressToPubKeyHash, compressPubKey } from './BitcoinUtil'
 import Address from '../../Address'
 import networks from './networks'
 import bip32 from 'bip32'
@@ -16,58 +16,22 @@ export default class BitcoinLedgerProvider extends LedgerProvider {
     this._bjsnetwork = chain.network.name.replace('bitcoin_', '') // for bitcoin js
     this._segwit = chain.segwit
     this._coinType = chain.network.coinType
-    this._extendedPubKeyCache = {}
+    this._walletPublicKeyCache = {}
   }
 
-  async getPubKey (from) {
+  async _getWalletPublicKey (path) {
     const app = await this.getApp()
-    const derivationPath = from.derivationPath ||
-    await this.getDerivationPathFromAddress(from)
-    return app.getWalletPublicKey(derivationPath)
+    return app.getWalletPublicKey(path, undefined, this._segwit)
   }
 
-  async _getAddressExtendedPubKey (path) {
-    const app = await this.getApp()
-    var parts = path.split('/')
-    var prevPath = parts[0] + '/' + parts[1]
-    var account = parseInt(parts[2])
-    var segwit = this._segwit
-    var network = this._network.bip32.public
-    const finalize = async fingerprint => {
-      // var path = prevPath + '/' + account
-      let nodeData = await app.getWalletPublicKey(path, undefined, segwit)
-      var publicKey = compressPubKey(nodeData.publicKey)
-      var childnum = (0x80000000 | account) >>> 0
-      var xpub = createXPUB(
-        3,
-        fingerprint,
-        childnum,
-        nodeData.chainCode,
-        publicKey,
-        network
-      )
-      return encodeBase58Check(xpub)
+  async getWalletPublicKey (path) {
+    if (path in this._walletPublicKeyCache) {
+      return this._walletPublicKeyCache[path]
     }
 
-    let nodeData = await app.getWalletPublicKey(prevPath, undefined, segwit)
-    var publicKey = compressPubKey(nodeData.publicKey)
-    publicKey = parseHexString(publicKey)
-    var result = sha256(Buffer.from(publicKey, 'hex'))
-    result = ripemd160(result)
-    var fingerprint =
-      ((result[0] << 24) | (result[1] << 16) | (result[2] << 8) | result[3]) >>>
-      0
-    return finalize(fingerprint)
-  }
-
-  async getAddressExtendedPubKey (path) {
-    if (path in this._extendedPubKeyCache) {
-      return this._extendedPubKeyCache[path]
-    }
-
-    const extendedPubKey = this._getAddressExtendedPubKey(path)
-    this._extendedPubKeyCache[path] = extendedPubKey
-    return extendedPubKey
+    const walletPublicKey = this._getWalletPublicKey(path)
+    this._walletPublicKeyCache[path] = walletPublicKey
+    return walletPublicKey
   }
 
   async getAddressFromDerivationPath (path) {
@@ -78,11 +42,9 @@ export default class BitcoinLedgerProvider extends LedgerProvider {
 
   async signMessage (message, from) {
     const app = await this.getApp()
-    const derivationPath = from.derivationPath ||
-    await this.getDerivationPathFromAddress(from)
-
+    const address = await this.getWalletAddress(from)
     const hex = Buffer.from(message).toString('hex')
-    return app.signMessageNew(derivationPath, hex)
+    return app.signMessageNew(address.derivationPath, hex)
   }
 
   // async getUnusedAddress (from = {}) {
@@ -432,16 +394,25 @@ export default class BitcoinLedgerProvider extends LedgerProvider {
   }
 
   async getLedgerAddresses (startingIndex, numAddresses, change = false) {
+    const walletPubKey = await this.getWalletPublicKey(this._baseDerivationPath)
+    const compressedPubKey = compressPubKey(walletPubKey.publicKey)
+    const node = bip32.fromPublicKey(
+      Buffer.from(compressedPubKey, 'hex'),
+      Buffer.from(walletPubKey.chainCode, 'hex'),
+      this._network
+    )
+
     const addresses = []
     const lastIndex = startingIndex + numAddresses
     const changeVal = change ? '1' : '0'
-    const xpubkeys = await this.getAddressExtendedPubKeys(this._baseDerivationPath)
-    const node = bip32.fromBase58(xpubkeys[0], this._network)
     for (let currentIndex = startingIndex; currentIndex < lastIndex; currentIndex++) {
-      const address = pubKeyToAddress(node.derivePath(changeVal + '/' + currentIndex).__Q, this._network.name, 'pubKeyHash')
-      const path = this._baseDerivationPath + changeVal + '/' + currentIndex
+      const subPath = changeVal + '/' + currentIndex
+      const publicKey = node.derivePath(subPath).publicKey
+      const address = pubKeyToAddress(publicKey, this._network.name, 'pubKeyHash')
+      const path = this._baseDerivationPath + subPath
       addresses.push({
         address,
+        publicKey: publicKey.toString('hex'),
         derivationPath: path,
         index: currentIndex
       })

src/providers/bitcoin/BitcoinSwapProvider.js

@@ -1,5 +1,5 @@
 import Provider from '../../Provider'
-import { addressToPubKeyHash, compressPubKey, pubKeyToAddress, reverseBuffer, scriptNumEncode } from './BitcoinUtil'
+import { addressToPubKeyHash, pubKeyToAddress, reverseBuffer, scriptNumEncode } from './BitcoinUtil'
 import { BigNumber } from 'bignumber.js'
 import { sha256, padHexStart } from '../../crypto'
 import networks from './networks'
@@ -79,18 +79,16 @@ export default class BitcoinSwapProvider extends Provider {
     const outputScriptObj = await this.getMethod('serializeTransactionOutputs')(splitNewTx)
     const outputScript = outputScriptObj.toString('hex')
 
-    const addressPath = await this.getMethod('getDerivationPathFromAddress')(to)
+    const walletAddress = await this.getMethod('getWalletAddress')(to)
 
     const signature = await this.getMethod('signP2SHTransaction')(
       [[initiationTx, 0, script, 0]],
-      [addressPath],
+      [walletAddress.derivationPath],
       outputScript,
       lockTime
     )
 
-    const pubKeyInfo = await this.getMethod('getPubKey')(isClaim ? recipientAddress : refundAddress)
-    const pubKey = compressPubKey(pubKeyInfo.publicKey)
-    const spendSwap = this._spendSwap(signature[0], pubKey, isClaim, secretParam)
+    const spendSwap = this._spendSwap(signature[0], walletAddress.publicKey, isClaim, secretParam)
     const spendSwapInput = this._spendSwapInput(spendSwap, script)
     const rawClaimTxInput = this.generateRawTxInput(txHashLE, spendSwapInput)
     const rawClaimTx = await this.generateRawTx(initiationTx, voutIndex, to, rawClaimTxInput, lockTimeHex, feePerByte)

src/providers/bitcoin/BitcoinUtil.js

@@ -5,10 +5,6 @@ import {
   base58
 } from '../../crypto'
 
-import bitcoin from 'bitcoinjs-lib'
-import bs58 from 'bs58'
-
-import padStart from 'lodash/padStart'
 import networks from './networks'
 
 /**
@@ -103,24 +99,6 @@ function scriptNumEncode (number) {
   return buffer
 }
 
-function parseHexString (str) {
-  var result = []
-  while (str.length >= 2) {
-    result.push(parseInt(str.substring(0, 2), 16))
-    str = str.substring(2, str.length)
-  }
-  return result
-}
-
-function encodeBase58Check (vchIn) {
-  vchIn = parseHexString(vchIn.toString())
-  var chksum = bitcoin.crypto.sha256(Buffer.from(vchIn, 'hex'))
-  chksum = bitcoin.crypto.sha256(chksum)
-  chksum = chksum.slice(0, 4)
-  var hash = vchIn.concat(Array.from(chksum))
-  return bs58.encode(hash)
-}
-
 function toHexDigit (number) {
   var digits = '0123456789abcdef'
   return digits.charAt(number >> 4) + digits.charAt(number & 0x0f)
@@ -135,20 +113,7 @@ function toHexInt (number) {
   )
 }
 
-function createXPUB (depth, fingerprint, childnum, chaincode, publicKey, network) {
-  var xpub = toHexInt(network)
-  xpub = xpub + padStart(depth.toString(16), 2, '0')
-  xpub = xpub + padStart(fingerprint.toString(16), 8, '0')
-  xpub = xpub + padStart(childnum.toString(16), 8, '0')
-  xpub = xpub + chaincode
-  xpub = xpub + publicKey
-  return xpub
-}
-
 export {
-  encodeBase58Check,
-  createXPUB,
-  parseHexString,
   toHexInt,
   compressPubKey,
   pubKeyToAddress,

src/providers/bitcoin/BitcoreRPCProvider.js

@@ -75,7 +75,7 @@ export default class BitcoreRPCProvider extends BitcoinRPCProvider {
 
     for (let currentIndex = startingIndex; currentIndex < lastIndex; currentIndex++) {
       const address = await this.getNewAddress()
-      addresses.push(address)
+      addresses.push({ address })
     }
 
     return addresses

src/providers/ethereum/EthereumLedgerProvider.js

@@ -17,10 +17,8 @@ export default class EthereumLedgerProvider extends LedgerProvider {
 
   async signMessage (message, from) {
     const app = await this.getApp()
-    const derivationPath = from.derivationPath ||
-      await this.getDerivationPathFromAddress(from)
-
+    const address = await this.getWalletAddress(from)
     const hex = Buffer.from(message).toString('hex')
-    return app.signPersonalMessage(derivationPath, hex)
+    return app.signPersonalMessage(address.derivationPath, hex)
   }
 }

src/providers/ethereum/EthereumRPCProvider.js

@@ -11,7 +11,8 @@ export default class EthereumRPCProvider extends JsonRpcProvider {
   }
 
   async getAddresses () {
-    return this.jsonrpc('eth_accounts')
+    const addresses = await this.jsonrpc('eth_accounts')
+    return addresses.map(address => ({ address }))
   }
 
   async generateBlock (numberOfBlocks) {
@@ -21,7 +22,7 @@ export default class EthereumRPCProvider extends JsonRpcProvider {
 
   async getUnusedAddress () {
     var addresses = await this.getAddresses()
-    return { address: addresses[0] }
+    return addresses[0]
   }
 
   async sendTransaction (to, value, data, from = null) {
@@ -31,7 +32,8 @@ export default class EthereumRPCProvider extends JsonRpcProvider {
 
     if (from == null) {
       const addresses = await this.getAddresses()
-      from = ensureHexEthFormat(('address' in addresses) ? addresses[0].address : addresses[0])
+      const address = addresses[0].address
+      from = ensureHexEthFormat(address)
     }
     value = BigNumber(value).toString(16)