Fixes #30: Calculate fee with only used unspents, warn about default …

…fee (#32)

Before this commit there was an issue where combine=False send()'s
would calculate the fee with all unspents. Now it will recalculate
the fee as it goes along adding unspents to use the minimum number
of unspents and the smallest fee.

Note that is this is very unoptimized and some unspents add more
fees than they are worth.

Adds one test case that was not broken before this point and another
for validating that additional, unnecessary unspents do not raise
the fee.

Also introduce logging, add warnings if the fee API cannot be reached
and the default or cached is used.

The warnings will only be seen in some cases depending on the logging
level set, but are always caught with failing test cases and can
further aid in debugging.

.gitignore

@@ -1,5 +1,6 @@
 *.log
 *.pyc
+*.orig
 /.cache
 /.idea
 /.coverage

bit/network/fees.py

@@ -1,3 +1,4 @@
+import logging
 from functools import wraps
 from time import time
 
@@ -54,7 +55,12 @@ def wrapper(fast=True):
                     cached_fee_fast = request.json()['fastestFee']
                     fast_last_update = now
                 except (ConnectionError, HTTPError, Timeout):  # pragma: no cover
-                    return cached_fee_fast or DEFAULT_FEE_FAST
+                    if cached_fee_fast is None:
+                        logging.warning('Connection to fee API failed, returning default fee (fast) of {}'.format(DEFAULT_FEE_FAST))
+                        return DEFAULT_FEE_FAST
+                    else:
+                        logging.warning('Connection to fee API failed, returning cached fee (fast).')
+                        return cached_fee_fast
 
             return cached_fee_fast
 
@@ -71,7 +77,12 @@ def wrapper(fast=True):
                     cached_fee_hour = request.json()['hourFee']
                     hour_last_update = now
                 except (ConnectionError, HTTPError, Timeout):  # pragma: no cover
-                    return cached_fee_hour or DEFAULT_FEE_HOUR
+                    if cached_fee_hour is None:
+                        logging.warning('Connection to fee API failed, returning default fee (hour) of {}'.format(DEFAULT_FEE_HOUR))
+                        return DEFAULT_FEE_HOUR
+                    else:
+                        logging.warning('Connection to fee API failed, returning cached fee (hour).')
+                        return cached_fee_hour
 
             return cached_fee_hour
 

bit/transaction.py

@@ -1,3 +1,4 @@
+import logging
 from collections import namedtuple
 from itertools import islice
 
@@ -70,10 +71,19 @@ def estimate_tx_fee(n_in, n_out, satoshis, compressed):
         + 8
     )
 
-    return estimated_size * satoshis
+    estimated_fee = estimated_size * satoshis
+
+    logging.debug('Estimated fee: {} satoshis for {} bytes'.format(estimated_fee, estimated_size))
+
+    return estimated_fee
 
 
 def sanitize_tx_data(unspents, outputs, fee, leftover, combine=True, message=None, compressed=True):
+    """
+    sanitize_tx_data()
+
+    fee is in satoshis per byte.
+    """
 
     outputs = outputs.copy()
 
@@ -94,12 +104,15 @@ def sanitize_tx_data(unspents, outputs, fee, leftover, combine=True, message=Non
             messages.append((message, 0))
 
     # Include return address in fee estimate.
-    fee = estimate_tx_fee(len(unspents), len(outputs) + len(messages) + 1, fee, compressed)
-    total_out = sum(out[1] for out in outputs) + fee
 
     total_in = 0
+    num_outputs = len(outputs) + len(messages) + 1
+    sum_outputs = sum(out[1] for out in outputs)
 
     if combine:
+        # calculated_fee is in total satoshis.
+        calculated_fee = estimate_tx_fee(len(unspents), num_outputs, fee, compressed)
+        total_out = sum_outputs + calculated_fee
         unspents = unspents.copy()
         total_in += sum(unspent.amount for unspent in unspents)
 
@@ -110,6 +123,8 @@ def sanitize_tx_data(unspents, outputs, fee, leftover, combine=True, message=Non
 
         for index, unspent in enumerate(unspents):
             total_in += unspent.amount
+            calculated_fee = estimate_tx_fee(len(unspents[:index + 1]), num_outputs, fee, compressed)
+            total_out = sum_outputs + calculated_fee
 
             if total_in >= total_out:
                 break

tests/test_transaction.py

@@ -172,6 +172,48 @@ def test_no_combine_remaining(self):
         assert outputs[1][0] == RETURN_ADDRESS
         assert outputs[1][1] == 1000
 
+    def test_no_combine_remaining_small_inputs(self):
+        unspents_original = [Unspent(1500, 0, '', '', 0),
+                             Unspent(1600, 0, '', '', 0),
+                             Unspent(1700, 0, '', '', 0)]
+        outputs_original = [(RETURN_ADDRESS, 2000, 'satoshi')]
+
+        unspents, outputs = sanitize_tx_data(
+            unspents_original, outputs_original, fee=0, leftover=RETURN_ADDRESS,
+            combine=False, message=None
+        )
+        assert unspents == [Unspent(1500, 0, '', '', 0), Unspent(1600, 0, '', '', 0)]
+        assert len(outputs) == 2
+        assert outputs[1][0] == RETURN_ADDRESS
+        assert outputs[1][1] == 1100
+
+    def test_no_combine_with_fee(self):
+        """
+        Verify that unused unspents do not increase fee.
+        """
+        unspents_single = [Unspent(5000, 0, '', '', 0)]
+        unspents_original = [Unspent(5000, 0, '', '', 0),
+                             Unspent(5000, 0, '', '', 0)]
+        outputs_original = [(RETURN_ADDRESS, 1000, 'satoshi')]
+
+        unspents, outputs = sanitize_tx_data(
+            unspents_original, outputs_original, fee=1, leftover=RETURN_ADDRESS,
+            combine=False, message=None
+        )
+
+        unspents_single, outputs_single = sanitize_tx_data(
+            unspents_single, outputs_original, fee=1, leftover=RETURN_ADDRESS,
+            combine=False, message=None
+        )
+
+        assert unspents == [Unspent(5000, 0, '', '', 0)]
+        assert unspents_single == [Unspent(5000, 0, '', '', 0)]
+        assert len(outputs) == 2
+        assert len(outputs_single) == 2
+        assert outputs[1][0] == RETURN_ADDRESS
+        assert outputs_single[1][0] == RETURN_ADDRESS
+        assert outputs[1][1] == outputs_single[1][1]
+
     def test_no_combine_insufficient_funds(self):
         unspents_original = [Unspent(1000, 0, '', '', 0),
                              Unspent(1000, 0, '', '', 0)]