coinchooser fee calculation now uses tx weights. more precise fee calculation for segwit and mixed-segwit txns.

lib/coinchooser.py

@@ -68,7 +68,13 @@ class PRNG:
             x[i], x[j] = x[j], x[i]
 
 
-Bucket = namedtuple('Bucket', ['desc', 'size', 'value', 'coins', 'min_height'])
+Bucket = namedtuple('Bucket',
+                    ['desc',
+                     'weight',      # as in BIP-141
+                     'value',       # in satoshis
+                     'coins',       # UTXOs
+                     'min_height',  # min block height where a coin was confirmed
+                     'witness'])    # whether any coin uses segwit
 
 def strip_unneeded(bkts, sufficient_funds):
     '''Remove buckets that are unnecessary in achieving the spend amount'''
@@ -91,12 +97,14 @@ class CoinChooserBase(PrintError):
             buckets[key].append(coin)
 
         def make_Bucket(desc, coins):
-            weight = sum(Transaction.estimated_input_weight(coin)
-                       for coin in coins)
-            size = Transaction.virtual_size_from_weight(weight)
+            witness = any(Transaction.is_segwit_input(coin) for coin in coins)
+            # note that we're guessing whether the tx uses segwit based
+            # on this single bucket
+            weight = sum(Transaction.estimated_input_weight(coin, witness)
+                         for coin in coins)
             value = sum(coin['value'] for coin in coins)
             min_height = min(coin['height'] for coin in coins)
-            return Bucket(desc, size, value, coins, min_height)
+            return Bucket(desc, weight, value, coins, min_height, witness)
 
         return list(map(make_Bucket, buckets.keys(), buckets.values()))
 
@@ -169,10 +177,13 @@ class CoinChooserBase(PrintError):
 
     def make_tx(self, coins, outputs, change_addrs, fee_estimator,
                 dust_threshold):
-        '''Select unspent coins to spend to pay outputs.  If the change is
+        """Select unspent coins to spend to pay outputs.  If the change is
         greater than dust_threshold (after adding the change output to
         the transaction) it is kept, otherwise none is sent and it is
-        added to the transaction fee.'''
+        added to the transaction fee.
+
+        Note: fee_estimator expects virtual bytes
+        """
 
         # Deterministic randomness from coins
         utxos = [c['prevout_hash'] + str(c['prevout_n']) for c in coins]
@@ -180,16 +191,36 @@ class CoinChooserBase(PrintError):
 
         # Copy the ouputs so when adding change we don't modify "outputs"
         tx = Transaction.from_io([], outputs[:])
-        # Size of the transaction with no inputs and no change
-        base_size = tx.estimated_size()
+        # Weight of the transaction with no inputs and no change
+        # Note: this will use legacy tx serialization as the need for "segwit"
+        # would be detected from inputs. The only side effect should be that the
+        # marker and flag are excluded, which is compensated in get_tx_weight()
+        base_weight = tx.estimated_weight()
         spent_amount = tx.output_value()
 
+        def fee_estimator_w(weight):
+            return fee_estimator(Transaction.virtual_size_from_weight(weight))
+
+        def get_tx_weight(buckets):
+            total_weight = base_weight + sum(bucket.weight for bucket in buckets)
+            is_segwit_tx = any(bucket.witness for bucket in buckets)
+            if is_segwit_tx:
+                total_weight += 2  # marker and flag
+                # non-segwit inputs were previously assumed to have
+                # a witness of '' instead of '00' (hex)
+                # note that mixed legacy/segwit buckets are already ok
+                num_legacy_inputs = sum((not bucket.witness) * len(bucket.coins)
+                                        for bucket in buckets)
+                total_weight += num_legacy_inputs
+
+            return total_weight
+
         def sufficient_funds(buckets):
             '''Given a list of buckets, return True if it has enough
             value to pay for the transaction'''
             total_input = sum(bucket.value for bucket in buckets)
-            total_size = sum(bucket.size for bucket in buckets) + base_size
-            return total_input >= spent_amount + fee_estimator(total_size)
+            total_weight = get_tx_weight(buckets)
+            return total_input >= spent_amount + fee_estimator_w(total_weight)
 
         # Collect the coins into buckets, choose a subset of the buckets
         buckets = self.bucketize_coins(coins)
@@ -197,11 +228,11 @@ class CoinChooserBase(PrintError):
                                       self.penalty_func(tx))
 
         tx.add_inputs([coin for b in buckets for coin in b.coins])
-        tx_size = base_size + sum(bucket.size for bucket in buckets)
+        tx_weight = get_tx_weight(buckets)
 
         # This takes a count of change outputs and returns a tx fee
-        output_size = Transaction.estimated_output_size(change_addrs[0])
-        fee = lambda count: fee_estimator(tx_size + count * output_size)
+        output_weight = 4 * Transaction.estimated_output_size(change_addrs[0])
+        fee = lambda count: fee_estimator_w(tx_weight + count * output_weight)
         change = self.change_outputs(tx, change_addrs, fee, dust_threshold)
         tx.add_outputs(change)
 

lib/transaction.py

@@ -863,17 +863,16 @@ class Transaction:
         return self.virtual_size_from_weight(weight)
 
     @classmethod
-    def estimated_input_weight(cls, txin):
+    def estimated_input_weight(cls, txin, is_segwit_tx):
         '''Return an estimate of serialized input weight in weight units.'''
         script = cls.input_script(txin, True)
         input_size = len(cls.serialize_input(txin, script)) // 2
 
-        # note: we should actually branch based on tx.is_segwit()
-        # only if none of the inputs have a witness, is the size actually 0
         if cls.is_segwit_input(txin):
+            assert is_segwit_tx
             witness_size = len(cls.serialize_witness(txin, True)) // 2
         else:
-            witness_size = 0
+            witness_size = 1 if is_segwit_tx else 0
 
         return 4 * input_size + witness_size