diff --git a/gui/qt/main_window.py b/gui/qt/main_window.py index 51c9f0e06..e73b0ec26 100644 --- a/gui/qt/main_window.py +++ b/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)) diff --git a/lib/coinchooser.py b/lib/coinchooser.py index 37b975c55..d6dee1cc6 100644 --- a/lib/coinchooser.py +++ b/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): diff --git a/lib/tests/test_transaction.py b/lib/tests/test_transaction.py index 5cf85b382..99c6d001b 100644 --- a/lib/tests/test_transaction.py +++ b/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) diff --git a/lib/transaction.py b/lib/transaction.py index 88314f466..cf9bd3b54 100644 --- a/lib/transaction.py +++ b/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)