coinchooser: "privacy" policy now prefers confirmed coins. removed "priority" policy.

gui/qt/main_window.py

@@ -2668,19 +2668,20 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
             return '\n'.join([key, "", " ".join(lines)])
 
         choosers = sorted(coinchooser.COIN_CHOOSERS.keys())
-        chooser_name = coinchooser.get_name(self.config)
-        msg = _('Choose coin (UTXO) selection method.  The following are available:\n\n')
-        msg += '\n\n'.join(fmt_docs(*item) for item in coinchooser.COIN_CHOOSERS.items())
-        chooser_label = HelpLabel(_('Coin selection') + ':', msg)
-        chooser_combo = QComboBox()
-        chooser_combo.addItems(choosers)
-        i = choosers.index(chooser_name) if chooser_name in choosers else 0
-        chooser_combo.setCurrentIndex(i)
-        def on_chooser(x):
-            chooser_name = choosers[chooser_combo.currentIndex()]
-            self.config.set_key('coin_chooser', chooser_name)
-        chooser_combo.currentIndexChanged.connect(on_chooser)
-        tx_widgets.append((chooser_label, chooser_combo))
+        if len(choosers) > 1:
+            chooser_name = coinchooser.get_name(self.config)
+            msg = _('Choose coin (UTXO) selection method.  The following are available:\n\n')
+            msg += '\n\n'.join(fmt_docs(*item) for item in coinchooser.COIN_CHOOSERS.items())
+            chooser_label = HelpLabel(_('Coin selection') + ':', msg)
+            chooser_combo = QComboBox()
+            chooser_combo.addItems(choosers)
+            i = choosers.index(chooser_name) if chooser_name in choosers else 0
+            chooser_combo.setCurrentIndex(i)
+            def on_chooser(x):
+                chooser_name = choosers[chooser_combo.currentIndex()]
+                self.config.set_key('coin_chooser', chooser_name)
+            chooser_combo.currentIndexChanged.connect(on_chooser)
+            tx_widgets.append((chooser_label, chooser_combo))
 
         def on_unconf(x):
             self.config.set_key('confirmed_only', bool(x))

lib/coinchooser.py

@@ -68,7 +68,7 @@ class PRNG:
             x[i], x[j] = x[j], x[i]
 
 
-Bucket = namedtuple('Bucket', ['desc', 'size', 'value', 'coins'])
+Bucket = namedtuple('Bucket', ['desc', 'size', 'value', 'coins', 'min_height'])
 
 def strip_unneeded(bkts, sufficient_funds):
     '''Remove buckets that are unnecessary in achieving the spend amount'''
@@ -95,7 +95,8 @@ class CoinChooserBase(PrintError):
                        for coin in coins)
             size = Transaction.virtual_size_from_weight(weight)
             value = sum(coin['value'] for coin in coins)
-            return Bucket(desc, size, value, coins)
+            min_height = min(coin['height'] for coin in coins)
+            return Bucket(desc, size, value, coins, min_height)
 
         return list(map(make_Bucket, buckets.keys(), buckets.values()))
 
@@ -198,9 +199,9 @@ class CoinChooserBase(PrintError):
         tx.add_inputs([coin for b in buckets for coin in b.coins])
         tx_size = base_size + sum(bucket.size for bucket in buckets)
 
-        # This takes a count of change outputs and returns a tx fee;
-        # each pay-to-bitcoin-address output serializes as 34 bytes
-        fee = lambda count: fee_estimator(tx_size + count * 34)
+        # 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)
         change = self.change_outputs(tx, change_addrs, fee, dust_threshold)
         tx.add_outputs(change)
 
@@ -212,35 +213,14 @@ class CoinChooserBase(PrintError):
     def choose_buckets(self, buckets, sufficient_funds, penalty_func):
         raise NotImplemented('To be subclassed')
 
-class CoinChooserOldestFirst(CoinChooserBase):
-    '''Maximize transaction priority. Select the oldest unspent
-    transaction outputs in your wallet, that are sufficient to cover
-    the spent amount. Then, remove any unneeded inputs, starting with
-    the smallest in value.
-    '''
-
-    def keys(self, coins):
-        return [coin['prevout_hash'] + ':' + str(coin['prevout_n'])
-                for coin in coins]
-
-    def choose_buckets(self, buckets, sufficient_funds, penalty_func):
-        '''Spend the oldest buckets first.'''
-        # Unconfirmed coins are young, not old
-        adj_height = lambda height: 99999999 if height <= 0 else height
-        buckets.sort(key = lambda b: max(adj_height(coin['height'])
-                                         for coin in b.coins))
-        selected = []
-        for bucket in buckets:
-            selected.append(bucket)
-            if sufficient_funds(selected):
-                return strip_unneeded(selected, sufficient_funds)
-        else:
-            raise NotEnoughFunds()
 
 class CoinChooserRandom(CoinChooserBase):
 
-    def bucket_candidates(self, buckets, sufficient_funds):
+    def bucket_candidates_any(self, buckets, sufficient_funds):
         '''Returns a list of bucket sets.'''
+        if not buckets:
+            raise NotEnoughFunds()
+
         candidates = set()
 
         # Add all singletons
@@ -267,8 +247,42 @@ class CoinChooserRandom(CoinChooserBase):
         candidates = [[buckets[n] for n in c] for c in candidates]
         return [strip_unneeded(c, sufficient_funds) for c in candidates]
 
+    def bucket_candidates_prefer_confirmed(self, buckets, sufficient_funds):
+        """Returns a list of bucket sets preferring confirmed coins.
+
+        Any bucket can be:
+        1. "confirmed" if it only contains confirmed coins; else
+        2. "unconfirmed" if it does not contain coins with unconfirmed parents
+        3. "unconfirmed parent" otherwise
+
+        This method tries to only use buckets of type 1, and if the coins there
+        are not enough, tries to use the next type but while also selecting
+        all buckets of all previous types.
+        """
+        conf_buckets = [bkt for bkt in buckets if bkt.min_height > 0]
+        unconf_buckets = [bkt for bkt in buckets if bkt.min_height == 0]
+        unconf_par_buckets = [bkt for bkt in buckets if bkt.min_height == -1]
+
+        bucket_sets = [conf_buckets, unconf_buckets, unconf_par_buckets]
+        already_selected_buckets = []
+
+        for bkts_choose_from in bucket_sets:
+            try:
+                def sfunds(bkts):
+                    return sufficient_funds(already_selected_buckets + bkts)
+
+                candidates = self.bucket_candidates_any(bkts_choose_from, sfunds)
+                break
+            except NotEnoughFunds:
+                already_selected_buckets += bkts_choose_from
+        else:
+            raise NotEnoughFunds()
+
+        candidates = [(already_selected_buckets + c) for c in candidates]
+        return [strip_unneeded(c, sufficient_funds) for c in candidates]
+
     def choose_buckets(self, buckets, sufficient_funds, penalty_func):
-        candidates = self.bucket_candidates(buckets, sufficient_funds)
+        candidates = self.bucket_candidates_prefer_confirmed(buckets, sufficient_funds)
         penalties = [penalty_func(cand) for cand in candidates]
         winner = candidates[penalties.index(min(penalties))]
         self.print_error("Bucket sets:", len(buckets))
@@ -276,14 +290,15 @@ class CoinChooserRandom(CoinChooserBase):
         return winner
 
 class CoinChooserPrivacy(CoinChooserRandom):
-    '''Attempts to better preserve user privacy.  First, if any coin is
-    spent from a user address, all coins are.  Compared to spending
-    from other addresses to make up an amount, this reduces
+    """Attempts to better preserve user privacy.
+    First, if any coin is spent from a user address, all coins are.
+    Compared to spending from other addresses to make up an amount, this reduces
     information leakage about sender holdings.  It also helps to
     reduce blockchain UTXO bloat, and reduce future privacy loss that
-    would come from reusing that address' remaining UTXOs.  Second, it
-    penalizes change that is quite different to the sent amount.
-    Third, it penalizes change that is too big.'''
+    would come from reusing that address' remaining UTXOs.
+    Second, it penalizes change that is quite different to the sent amount.
+    Third, it penalizes change that is too big.
+    """
 
     def keys(self, coins):
         return [coin['address'] for coin in coins]
@@ -296,6 +311,7 @@ class CoinChooserPrivacy(CoinChooserRandom):
         def penalty(buckets):
             badness = len(buckets) - 1
             total_input = sum(bucket.value for bucket in buckets)
+            # FIXME "change" here also includes fees
             change = float(total_input - spent_amount)
             # Penalize change not roughly in output range
             if change < min_change:
@@ -309,13 +325,14 @@ class CoinChooserPrivacy(CoinChooserRandom):
         return penalty
 
 
-COIN_CHOOSERS = {'Priority': CoinChooserOldestFirst,
-                 'Privacy': CoinChooserPrivacy}
+COIN_CHOOSERS = {
+    'Privacy': CoinChooserPrivacy,
+}
 
 def get_name(config):
     kind = config.get('coin_chooser')
     if not kind in COIN_CHOOSERS:
-        kind = 'Priority'
+        kind = 'Privacy'
     return kind
 
 def get_coin_chooser(config):

lib/tests/test_transaction.py

@@ -135,6 +135,13 @@ class TestTransaction(unittest.TestCase):
         self.assertEqual(tx.estimated_weight(), 772)
         self.assertEqual(tx.estimated_size(), 193)
 
+    def test_estimated_output_size(self):
+        estimated_output_size = transaction.Transaction.estimated_output_size
+        self.assertEqual(estimated_output_size('14gcRovpkCoGkCNBivQBvw7eso7eiNAbxG'), 34)
+        self.assertEqual(estimated_output_size('35ZqQJcBQMZ1rsv8aSuJ2wkC7ohUCQMJbT'), 32)
+        self.assertEqual(estimated_output_size('bc1q3g5tmkmlvxryhh843v4dz026avatc0zzr6h3af'), 31)
+        self.assertEqual(estimated_output_size('bc1qnvks7gfdu72de8qv6q6rhkkzu70fqz4wpjzuxjf6aydsx7wxfwcqnlxuv3'), 43)
+
     # TODO other tests for segwit tx
     def test_tx_signed_segwit(self):
         tx = transaction.Transaction(signed_segwit_blob)

lib/transaction.py

@@ -877,6 +877,13 @@ class Transaction:
 
         return 4 * input_size + witness_size
 
+    @classmethod
+    def estimated_output_size(cls, address):
+        """Return an estimate of serialized output size in bytes."""
+        script = bitcoin.address_to_script(address)
+        # 8 byte value + 1 byte script len + script
+        return 9 + len(script) // 2
+
     @classmethod
     def virtual_size_from_weight(cls, weight):
         return weight // 4 + (weight % 4 > 0)