Merge pull request #3643 from SomberNight/fee_ui_feerounding

fee ui: rounding

gui/qt/fee_slider.py

@@ -18,6 +18,7 @@ class FeeSlider(QSlider):
         self.lock = threading.RLock()
         self.update()
         self.valueChanged.connect(self.moved)
+        self._active = True
 
     def moved(self, pos):
         with self.lock:
@@ -56,9 +57,11 @@ class FeeSlider(QSlider):
             self.setToolTip(tooltip)
 
     def activate(self):
+        self._active = True
         self.setStyleSheet('')
 
     def deactivate(self):
+        self._active = False
         # TODO it would be nice to find a platform-independent solution
         # that makes the slider look as if it was disabled
         self.setStyleSheet(
@@ -79,3 +82,6 @@ class FeeSlider(QSlider):
             }
             """
         )
+
+    def is_active(self):
+        return self._active

gui/qt/main_window.py

@@ -619,7 +619,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
 
     def format_amount_and_units(self, amount):
         text = self.format_amount(amount) + ' '+ self.base_unit()
-        x = self.fx.format_amount_and_units(amount)
+        x = self.fx.format_amount_and_units(amount) if self.fx else None
         if text and x:
             text += ' (%s)'%x
         return text
@@ -1070,6 +1070,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
 
             if fee_rate:
                 self.feerate_e.setAmount(fee_rate // 1000)
+            else:
+                self.feerate_e.setAmount(None)
             self.fee_e.setModified(False)
 
             self.fee_slider.activate()
@@ -1103,6 +1105,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
         self.size_e.setStyleSheet(ColorScheme.DEFAULT.as_stylesheet())
 
         self.feerate_e = FeerateEdit(lambda: 2 if self.fee_unit else 0)
+        self.feerate_e.setAmount(self.config.fee_per_byte())
         self.feerate_e.textEdited.connect(partial(on_fee_or_feerate, self.feerate_e, False))
         self.feerate_e.editingFinished.connect(partial(on_fee_or_feerate, self.feerate_e, True))
 
@@ -1110,6 +1113,18 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
         self.fee_e.textEdited.connect(partial(on_fee_or_feerate, self.fee_e, False))
         self.fee_e.editingFinished.connect(partial(on_fee_or_feerate, self.fee_e, True))
 
+        def feerounding_onclick():
+            text = (_('To somewhat protect your privacy, Electrum tries to create change with similar precision to other outputs.') + ' ' +
+                    _('At most 100 satoshis might be lost due to this rounding.') + '\n' +
+                    _('Also, dust is not kept as change, but added to the fee.'))
+            QMessageBox.information(self, 'Fee rounding', text)
+
+        self.feerounding_icon = QPushButton(QIcon(':icons/info.png'), '')
+        self.feerounding_icon.setFixedWidth(20)
+        self.feerounding_icon.setFlat(True)
+        self.feerounding_icon.clicked.connect(feerounding_onclick)
+        self.feerounding_icon.setVisible(False)
+
         self.connect_fields(self, self.amount_e, self.fiat_send_e, self.fee_e)
 
         vbox_feelabel = QVBoxLayout()
@@ -1123,12 +1138,14 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
         hbox.addWidget(self.feerate_e)
         hbox.addWidget(self.size_e)
         hbox.addWidget(self.fee_e)
+        hbox.addWidget(self.feerounding_icon, Qt.AlignLeft)
+        hbox.addStretch(1)
 
         vbox_feecontrol = QVBoxLayout()
         vbox_feecontrol.addWidget(self.fee_adv_controls)
         vbox_feecontrol.addWidget(self.fee_slider)
 
-        grid.addLayout(vbox_feecontrol, 5, 1, 1, 3)
+        grid.addLayout(vbox_feecontrol, 5, 1, 1, -1)
 
         if not self.config.get('show_fee', False):
             self.fee_adv_controls.setVisible(False)
@@ -1252,15 +1269,22 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
             try:
                 tx = make_tx(fee_estimator)
                 self.not_enough_funds = False
-            except NotEnoughFunds:
-                self.not_enough_funds = True
+            except (NotEnoughFunds, NoDynamicFeeEstimates) as e:
                 if not freeze_fee:
                     self.fee_e.setAmount(None)
-                return
-            except NoDynamicFeeEstimates:
-                tx = make_tx(0)
-                size = tx.estimated_size()
-                self.size_e.setAmount(size)
+                if not freeze_feerate:
+                    self.feerate_e.setAmount(None)
+                self.feerounding_icon.setVisible(False)
+
+                if isinstance(e, NotEnoughFunds):
+                    self.not_enough_funds = True
+                elif isinstance(e, NoDynamicFeeEstimates):
+                    try:
+                        tx = make_tx(0)
+                        size = tx.estimated_size()
+                        self.size_e.setAmount(size)
+                    except BaseException:
+                        pass
                 return
             except BaseException:
                 traceback.print_exc(file=sys.stderr)
@@ -1270,12 +1294,35 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
             self.size_e.setAmount(size)
 
             fee = tx.get_fee()
-            if not freeze_fee:
-                fee = None if self.not_enough_funds else fee
-                self.fee_e.setAmount(fee)
-            if not freeze_feerate:
-                fee_rate = fee // size if fee is not None else None
-                self.feerate_e.setAmount(fee_rate)
+            fee = None if self.not_enough_funds else fee
+
+            # Displayed fee/fee_rate values are set according to user input.
+            # Due to rounding or dropping dust in CoinChooser,
+            # actual fees often differ somewhat.
+            if freeze_feerate or self.fee_slider.is_active():
+                displayed_feerate = self.feerate_e.get_amount()
+                displayed_feerate = displayed_feerate // 1000 if displayed_feerate else 0
+                displayed_fee = displayed_feerate * size
+                self.fee_e.setAmount(displayed_fee)
+            else:
+                if freeze_fee:
+                    displayed_fee = self.fee_e.get_amount()
+                else:
+                    # fallback to actual fee if nothing is frozen
+                    displayed_fee = fee
+                    self.fee_e.setAmount(displayed_fee)
+                displayed_fee = displayed_fee if displayed_fee else 0
+                displayed_feerate = displayed_fee // size if displayed_fee is not None else None
+                self.feerate_e.setAmount(displayed_feerate)
+
+            # show/hide fee rounding icon
+            feerounding = (fee - displayed_fee) if fee else 0
+            if feerounding:
+                self.feerounding_icon.setToolTip(
+                    _('additional {} satoshis will be added').format(feerounding))
+                self.feerounding_icon.setVisible(True)
+            else:
+                self.feerounding_icon.setVisible(False)
 
             if self.is_max:
                 amount = tx.output_value()
@@ -1354,7 +1401,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
             fee_estimator = self.fee_e.get_amount()
         elif self.is_send_feerate_frozen():
             amount = self.feerate_e.get_amount()
-            amount = 0 if amount is None else float(amount)
+            amount = 0 if amount is None else amount
             fee_estimator = partial(
                 simple_config.SimpleConfig.estimate_fee_for_feerate, amount)
         else:
@@ -1440,6 +1487,10 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
             self.show_transaction(tx, tx_desc)
             return
 
+        if not self.network:
+            self.show_error(_("You can't broadcast a transaction without a live network connection."))
+            return
+
         # confirmation dialog
         msg = [
             _("Amount to be sent") + ": " + self.format_amount_and_units(amount),
@@ -1640,7 +1691,9 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
             e.setText('')
             e.setFrozen(False)
         self.fee_slider.activate()
+        self.feerate_e.setAmount(self.config.fee_per_byte())
         self.size_e.setAmount(0)
+        self.feerounding_icon.setVisible(False)
         self.set_pay_from([])
         self.tx_external_keypairs = {}
         self.update_status()
@@ -3028,6 +3081,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
         grid.addWidget(QLabel(_('Output amount') + ':'), 2, 0)
         grid.addWidget(output_amount, 2, 1)
         fee_e = BTCAmountEdit(self.get_decimal_point)
+        # FIXME with dyn fees, without estimates, there are all kinds of crashes here
         def f(x):
             a = max_fee - fee_e.get_amount()
             output_amount.setText((self.format_amount(a) + ' ' + self.base_unit()) if a else '')

icons.qrc

@@ -14,6 +14,7 @@
     <file>icons/electrum_light_icon.png</file>
     <file>icons/electrum_dark_icon.png</file>
     <file>icons/file.png</file>
+    <file>icons/info.png</file>
     <file>icons/keepkey.png</file>
     <file>icons/keepkey_unpaired.png</file>
     <file>icons/key.png</file>

lib/coinchooser.py

@@ -273,6 +273,8 @@ class CoinChooserRandom(CoinChooserBase):
                     candidates.add(tuple(sorted(permutation[:count + 1])))
                     break
             else:
+                # FIXME this assumes that the effective value of any bkt is >= 0
+                # we should make sure not to choose buckets with <= 0 eff. val.
                 raise NotEnoughFunds()
 
         candidates = [[buckets[n] for n in c] for c in candidates]

lib/simple_config.py

@@ -5,7 +5,8 @@ import os
 import stat
 
 from copy import deepcopy
-from .util import user_dir, print_error, print_stderr, PrintError
+from .util import (user_dir, print_error, print_stderr, PrintError,
+                   NoDynamicFeeEstimates)
 
 from .bitcoin import MAX_FEE_RATE, FEE_TARGETS
 
@@ -245,6 +246,9 @@ class SimpleConfig(PrintError):
         return self.get('dynamic_fees', True)
 
     def fee_per_kb(self):
+        """Returns sat/kvB fee to pay for a txn.
+        Note: might return None.
+        """
         dyn = self.is_dynfee()
         if dyn:
             fee_rate = self.dynfee(self.get('fee_level', 2))
@@ -252,8 +256,18 @@ class SimpleConfig(PrintError):
             fee_rate = self.get('fee_per_kb', self.max_fee_rate()/2)
         return fee_rate
 
+    def fee_per_byte(self):
+        """Returns sat/vB fee to pay for a txn.
+        Note: might return None.
+        """
+        fee_per_kb = self.fee_per_kb()
+        return fee_per_kb / 1000 if fee_per_kb is not None else None
+
     def estimate_fee(self, size):
-        return self.estimate_fee_for_feerate(self.fee_per_kb(), size)
+        fee_per_kb = self.fee_per_kb()
+        if fee_per_kb is None:
+            raise NoDynamicFeeEstimates()
+        return self.estimate_fee_for_feerate(fee_per_kb, size)
 
     @classmethod
     def estimate_fee_for_feerate(cls, fee_per_kb, size):

lib/wallet.py

@@ -923,6 +923,7 @@ class Abstract_Wallet(PrintError):
             tx = coin_chooser.make_tx(inputs, outputs, change_addrs[:max_change],
                                       fee_estimator, self.dust_threshold())
         else:
+            # FIXME?? this might spend inputs with negative effective value...
             sendable = sum(map(lambda x:x['value'], inputs))
             _type, data, value = outputs[i_max]
             outputs[i_max] = (_type, data, 0)