commit 936ee47d3a07bbb28651751b303328b223cf22ae
parent bc7051372fd80c78a5ac70a5a540ce23bea00ea8
Author: ThomasV <thomasv@electrum.org>
Date: Tue, 12 Dec 2017 17:19:19 +0100
Merge pull request #3496 from SomberNight/coinchooser1
CoinChooser: privacy prefers confirmed and is default
Diffstat:
4 files changed, 85 insertions(+), 53 deletions(-)
diff --git a/gui/qt/main_window.py b/gui/qt/main_window.py
@@ -2674,19 +2674,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
@@ -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
@@ -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
@@ -878,6 +878,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)
