fix(change): variable change addr types

blockchain · May 25, 2021 · 1403ce8 · 1403ce8
1 parent 4a00974
commit 1403ce8
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 33 deletions.
diff --git a/packages/blockchain-wallet-v4/src/coinSelection/coin.js b/packages/blockchain-wallet-v4/src/coinSelection/coin.js
@@ -1,11 +1,12 @@
+/* eslint-disable */ 
 import { inputComparator, sortOutputs } from 'bip69'
 import * as Bitcoin from 'bitcoinjs-lib'
 import { clamp, curry, is, length, sort, split } from 'ramda'
 import { over, view } from 'ramda-lens'
 
 import Type from '../types/Type'
 import { addressToScript, scriptToAddress } from '../utils/btc'
-import { IO_TYPES } from './'
+import { IO_TYPES } from './index'
 
 export const TX_EMPTY_SIZE = 4 + 1 + 1 + 4
 export const TX_INPUT_BASE = 32 + 4 + 1 + 4
@@ -101,25 +102,25 @@ export const selectPath = view(path)
 
 export const fromJS = (o, network) => {
   return new Coin({
-    value: parseInt(o.value),
-    script: o.script ? o.script : addressToScript(o.address, network),
-    txHash: o.tx_hash_big_endian,
-    index: o.tx_output_n,
+    address: o.address ? o.address : scriptToAddress(o.script, network),
     change: o.change || false,
-    priv: o.priv,
+    index: o.tx_output_n,
     path: o.path,
-    xpub: o.xpub,
-    address: o.address ? o.address : scriptToAddress(o.script, network)
+    priv: o.priv,
+    script: o.script ? o.script : addressToScript(o.address, network),
+    txHash: o.tx_hash_big_endian,
+    value: parseInt(o.value),
+    xpub: o.xpub
   })
 }
 
 export const empty = new Coin({ value: 0 })
 
-export const inputBytes = input => {
+export const inputBytes = (input) => {
   return IO_TYPES.inputs[input.type ? input.type() : 'P2PKH']
 }
 
-export const outputBytes = output => {
+export const outputBytes = (output) => {
   return IO_TYPES.outputs[output.type ? output.type() : 'P2PKH']
 }
 

diff --git a/packages/blockchain-wallet-v4/src/coinSelection/coinSelection.spec.js b/packages/blockchain-wallet-v4/src/coinSelection/coinSelection.spec.js
@@ -1,3 +1,4 @@
+/* eslint-disable */
 import { map } from 'ramda'
 
 import * as Coin from './coin.js'
@@ -278,14 +279,15 @@ describe('Coin Selection', () => {
       const feePerByte = 55
       const selection = cs.findTarget(targets, feePerByte, inputs)
 
-      const estimatedSize = 10 + 2 * 148 + 2 * 34 // 374
-      const estimatedFee = estimatedSize * feePerByte // 205700
+      const estimatedSize = 10 + 2 * 148 + 34 * 2 // 374
+      const estimatedFee = estimatedSize * feePerByte // 20570
+      const feeForAdditionalChangeOutput = cs.IO_TYPES.outputs.P2PKH * feePerByte
 
       expect(selection.fee).toEqual(estimatedFee)
       expect(selection.inputs.map(x => x.value)).toEqual([20000, 300000])
       expect(selection.outputs.map(x => x.value)).toEqual([
         10000,
-        300000 + 20000 - 10000 - estimatedFee
+        300000 + 20000 - 10000 - estimatedFee + feeForAdditionalChangeOutput
       ])
     })
   })
@@ -333,16 +335,18 @@ describe('Coin Selection', () => {
       ])
       const targets = map(Coin.fromJS, [{ value: 100000 }])
       const selection = cs.descentDraw(targets, 55, inputs, 'change-address')
+      const feeForAdditionalChangeOutput = cs.IO_TYPES.outputs.P2PKH * 55
 
       expect(selection.inputs.map(x => x.value)).toEqual([20000, 300000])
 
       // overhead + inputs + outputs
       // 55 * (10 + 148 * 2 + 34 * 2) = 20570
       expect(selection.fee).toEqual(20570)
 
-      // change = inputs - outputs - fee
-      // 20000 + 300000 - 100000 - 20570 = 199430
-      expect(selection.outputs.map(x => x.value)).toEqual([100000, 199430])
+      console.log(selection.inputs.map((val) => val.value))
+      // change = inputs - outputs - fee + feeForAdditionalChangeOutput
+      // 20000 + 300000 - 100000 - 20570 + 1870 = 201300
+      expect(selection.outputs.map(x => x.value)).toEqual([100000, 201300])
     })
   })
 
@@ -359,15 +363,16 @@ describe('Coin Selection', () => {
       ])
       const targets = map(Coin.fromJS, [{ value: 100000 }])
       const selection = cs.ascentDraw(targets, 55, inputs, 'change-address')
+      const feeForAdditionalChangeOutput = cs.IO_TYPES.outputs.P2PKH * 55
       expect(selection.inputs.map(x => x.value)).toEqual([20000, 300000])
 
       // overhead + inputs + outputs
       // 55 * (10 + 148 * 2 + 34 * 2) = 20570
       expect(selection.fee).toEqual(20570)
 
       // change = inputs - outputs - fee
-      // 20000 + 300000 - 100000 - 20570 = 199430
-      expect(selection.outputs.map(x => x.value)).toEqual([100000, 199430])
+      // 20000 + 300000 - 100000 - 20570 + 1870 = 201300
+      expect(selection.outputs.map(x => x.value)).toEqual([100000, 201300])
     })
   })
 

diff --git a/packages/blockchain-wallet-v4/src/coinSelection/index.js b/packages/blockchain-wallet-v4/src/coinSelection/index.js
@@ -1,3 +1,4 @@
+/* eslint-disable */
 import memoize from 'fast-memoize'
 import shuffle from 'fisher-yates'
 import { List } from 'immutable-ext'
@@ -30,8 +31,8 @@ export const isFromAccount = selection =>
 export const isFromLegacy = selection =>
   selection.inputs[0] ? selection.inputs[0].isFromLegacy() : false
 
-export const dustThreshold = feeRate =>
-  Math.ceil((Coin.inputBytes({}) + Coin.outputBytes({})) * feeRate)
+export const dustThreshold = (feeRate, change) =>
+  Math.ceil((Coin.inputBytes(change) + Coin.outputBytes(change)) * feeRate)
 
 export const transactionBytes = (inputs, outputs) => {
   const coinTypeReducer = (acc, coin) => {
@@ -46,13 +47,13 @@ export const transactionBytes = (inputs, outputs) => {
   return getByteCount(inputTypeCollection, outputTypeCollection)
 }
 
+export const changeBytes = (type) => IO_TYPES.outputs[type]
+
 export const DEPRECATED_transactionBytes = (inputs, outputs) =>
   Coin.TX_EMPTY_SIZE +
   inputs.reduce((a, c) => a + Coin.inputBytes(c), 0) +
   outputs.reduce((a, c) => a + Coin.outputBytes(c), 0)
 
-export const changeBytes = () => Coin.TX_OUTPUT_BASE + Coin.TX_OUTPUT_PUBKEYHASH
-
 export const effectiveBalance = curry((feePerByte, inputs, outputs = [{}]) =>
   List(inputs)
     .fold(Coin.empty)
@@ -102,22 +103,20 @@ const ft = (targets, feePerByte, coins, changeAddress) => {
       return { fee: fee, inputs: [], outputs: targets }
     } else {
       const extra = maxBalance - target - fee
-      const feeChange = changeBytes() * feePerByte
-      const extraWithChangeFee = extra - feeChange
-      if (extraWithChangeFee >= dustThreshold(feePerByte)) {
-        // add change
-        const change = Coin.fromJS({
-          value: extraWithChangeFee,
-          address: changeAddress,
-          change: true
-        })
+      const change = Coin.fromJS({
+        address: changeAddress,
+        change: true,
+        value: extra,
+      })
+      // should we add change?
+      if (extra >= dustThreshold(feePerByte, change)) {
+        const feeForAdditionalChangeOutput = changeBytes(change.type()) * feePerByte
         return {
-          fee: fee + feeChange,
+          fee: fee + feeForAdditionalChangeOutput,
           inputs: selectedCoins,
           outputs: [...targets, change]
         }
       } else {
-        // TODO: SEGWIT update burn logic?
         // burn change
         return { fee: fee + extra, inputs: selectedCoins, outputs: targets }
       }