better fees estimates

gui/gtk.py

@@ -164,7 +164,7 @@ def run_settings_dialog(self):
     fee_label.set_size_request(150,10)
     fee_label.show()
     fee.pack_start(fee_label,False, False, 10)
-    fee_entry.set_text( str( Decimal(self.wallet.fee) /100000000 ) )
+    fee_entry.set_text( str( Decimal(self.wallet.fee_per_kb) /100000000 ) )
     fee_entry.connect('changed', numbify, False)
     fee_entry.show()
     fee.pack_start(fee_entry,False,False, 10)
@@ -686,12 +686,13 @@ class ElectrumWindow:
             if not is_fee: fee = None
             if amount is None:
                 return
-            #assume two outputs - one for change
-            inputs, total, fee = self.wallet.choose_tx_inputs( amount, fee, 2 )
+            tx = self.wallet.make_unsigned_transaction([('op_return', 'dummy_tx', amount)], fee)
             if not is_fee:
-                fee_entry.set_text( str( Decimal( fee ) / 100000000 ) )
-                self.fee_box.show()
-            if inputs:
+                if tx:
+                    fee = tx.get_fee()
+                    fee_entry.set_text( str( Decimal( fee ) / 100000000 ) )
+                    self.fee_box.show()
+            if tx:
                 amount_entry.modify_text(Gtk.StateType.NORMAL, Gdk.color_parse("#000000"))
                 fee_entry.modify_text(Gtk.StateType.NORMAL, Gdk.color_parse("#000000"))
                 send_button.set_sensitive(True)

gui/qt/amountedit.py

@@ -14,6 +14,7 @@ class MyLineEdit(QLineEdit):
         self.frozen.emit()
 
 class AmountEdit(MyLineEdit):
+    shortcut = pyqtSignal()
 
     def __init__(self, base_unit, is_int = False, parent=None):
         QLineEdit.__init__(self, parent)
@@ -29,7 +30,8 @@ class AmountEdit(MyLineEdit):
     def numbify(self):
         text = unicode(self.text()).strip()
         if text == '!':
-            self.is_shortcut = True
+            self.shortcut.emit()
+            return
         pos = self.cursorPosition()
         chars = '0123456789'
         if not self.is_int: chars +='.'

gui/qt/main_window.py

@@ -911,51 +911,49 @@ class ElectrumWindow(QMainWindow):
         self.fee_e_help = HelpButton(msg)
         grid.addWidget(self.fee_e_help, 5, 3)
         self.update_fee_edit()
-
         self.send_button = EnterButton(_("Send"), self.do_send)
         grid.addWidget(self.send_button, 6, 1)
-
         b = EnterButton(_("Clear"), self.do_clear)
         grid.addWidget(b, 6, 2)
-
         self.payto_sig = QLabel('')
         grid.addWidget(self.payto_sig, 7, 0, 1, 4)
-
-        #QShortcut(QKeySequence("Up"), w, w.focusPreviousChild)
-        #QShortcut(QKeySequence("Down"), w, w.focusNextChild)
         w.setLayout(grid)
 
-        def entry_changed( is_fee ):
+        def on_shortcut():
+            sendable = self.get_sendable_balance()
+            inputs = self.get_coins()
+            for i in inputs: self.wallet.add_input_info(i)
+            output = ('address', self.payto_e.payto_address, sendable) if self.payto_e.payto_address else ('op_return', 'dummy_tx', sendable)
+            dummy_tx = Transaction(inputs, [output])
+            fee = self.wallet.estimated_fee(dummy_tx)
+            self.amount_e.setAmount(sendable-fee)
+            self.amount_e.textEdited.emit("")
+            self.fee_e.setAmount(fee)
 
-            if self.amount_e.is_shortcut:
-                self.amount_e.is_shortcut = False
-                sendable = self.get_sendable_balance()
-                # there is only one output because we are completely spending inputs
-                inputs, total, fee = self.wallet.choose_tx_inputs( sendable, 0, 1, coins = self.get_coins())
-                fee = self.wallet.estimated_fee(inputs, 1)
-                amount = total - fee
-                self.amount_e.setAmount(amount)
-                self.amount_e.textEdited.emit("")
-                self.fee_e.setAmount(fee)
-                return
+        self.amount_e.shortcut.connect(on_shortcut)
 
-            amount = self.amount_e.get_amount()
-            fee = self.fee_e.get_amount()
+        def text_edited(is_fee):
             outputs = self.payto_e.get_outputs()
-
-            if not is_fee: 
-                fee = None
-
+            amount = self.amount_e.get_amount()
+            fee = self.fee_e.get_amount() if is_fee else None
             if amount is None:
                 self.fee_e.setAmount(None)
-                not_enough_funds = False
+                self.not_enough_funds = False
             else:
-                inputs, total, fee = self.wallet.choose_tx_inputs(amount, fee, len(outputs), coins = self.get_coins())
-                not_enough_funds = len(inputs) == 0
+                if not outputs:
+                    outputs = [('op_return', 'dummy_tx', amount)]
+                tx = self.wallet.make_unsigned_transaction(outputs, fee, coins = self.get_coins())
+                self.not_enough_funds = (tx is None)
                 if not is_fee:
+                    fee = tx.get_fee() if tx else None
                     self.fee_e.setAmount(fee)
-                    
-            if not not_enough_funds:
+
+        self.payto_e.textChanged.connect(lambda:text_edited(False))
+        self.amount_e.textEdited.connect(lambda:text_edited(False))
+        self.fee_e.textEdited.connect(lambda:text_edited(True))
+
+        def entry_changed():
+            if not self.not_enough_funds:
                 palette = QPalette()
                 palette.setColor(self.amount_e.foregroundRole(), QColor('black'))
                 text = ""
@@ -965,13 +963,12 @@ class ElectrumWindow(QMainWindow):
                 text = _( "Not enough funds" )
                 c, u = self.wallet.get_frozen_balance()
                 if c+u: text += ' (' + self.format_amount(c+u).strip() + ' ' + self.base_unit() + ' ' +_("are frozen") + ')'
-
             self.statusBar().showMessage(text)
             self.amount_e.setPalette(palette)
             self.fee_e.setPalette(palette)
 
-        self.amount_e.textChanged.connect(lambda: entry_changed(False) )
-        self.fee_e.textChanged.connect(lambda: entry_changed(True) )
+        self.amount_e.textChanged.connect(entry_changed)
+        self.fee_e.textChanged.connect(entry_changed)
 
         run_hook('create_send_tab', grid)
         return w
@@ -1057,27 +1054,17 @@ class ElectrumWindow(QMainWindow):
                 QMessageBox.warning(self, _('Error'), _('Invalid Amount'), _('OK'))
                 return
 
-        amount = sum(map(lambda x:x[2], outputs))
-
         fee = self.fee_e.get_amount()
         if fee is None:
             QMessageBox.warning(self, _('Error'), _('Invalid Fee'), _('OK'))
             return
 
+        amount = sum(map(lambda x:x[2], outputs))
         confirm_amount = self.config.get('confirm_amount', 100000000)
         if amount >= confirm_amount:
             o = '\n'.join(map(lambda x:x[1], outputs))
             if not self.question(_("send %(amount)s to %(address)s?")%{ 'amount' : self.format_amount(amount) + ' '+ self.base_unit(), 'address' : o}):
                 return
-            
-        if not self.config.get('can_edit_fees', False):
-            if not self.question(_("A fee of %(fee)s will be added to this transaction.\nProceed?")%{ 'fee' : self.format_amount(fee) + ' '+ self.base_unit()}):
-                return
-        else:
-            confirm_fee = self.config.get('confirm_fee', 100000)
-            if fee >= confirm_fee:
-                if not self.question(_("The fee for this transaction seems unusually high.\nAre you really sure you want to pay %(fee)s in fees?")%{ 'fee' : self.format_amount(fee) + ' '+ self.base_unit()}):
-                    return
 
         coins = self.get_coins()
         return outputs, fee, label, coins
@@ -1088,23 +1075,35 @@ class ElectrumWindow(QMainWindow):
         if not r:
             return
         outputs, fee, label, coins = r
-        self.send_tx(outputs, fee, label, coins)
-
-
-    @protected
-    def send_tx(self, outputs, fee, label, coins, password):
-        self.send_button.setDisabled(True)
 
-        # first, create an unsigned tx 
         try:
             tx = self.wallet.make_unsigned_transaction(outputs, fee, None, coins = coins)
             tx.error = None
         except Exception as e:
             traceback.print_exc(file=sys.stdout)
             self.show_message(str(e))
-            self.send_button.setDisabled(False)
             return
 
+        if tx.requires_fee(self.wallet.verifier) and tx.get_fee() < MIN_RELAY_TX_FEE:
+            QMessageBox.warning(self, _('Error'), _("This transaction requires a higher fee, or it will not be propagated by the network."), _('OK'))
+            return
+
+        if not self.config.get('can_edit_fees', False):
+            if not self.question(_("A fee of %(fee)s will be added to this transaction.\nProceed?")%{ 'fee' : self.format_amount(fee) + ' '+ self.base_unit()}):
+                return
+        else:
+            confirm_fee = self.config.get('confirm_fee', 100000)
+            if fee >= confirm_fee:
+                if not self.question(_("The fee for this transaction seems unusually high.\nAre you really sure you want to pay %(fee)s in fees?")%{ 'fee' : self.format_amount(fee) + ' '+ self.base_unit()}):
+                    return
+
+        self.send_tx(tx, label)
+
+
+    @protected
+    def send_tx(self, tx, label, password):
+        self.send_button.setDisabled(True)
+
         # call hook to see if plugin needs gui interaction
         run_hook('send_tx', tx)
 
@@ -1126,10 +1125,6 @@ class ElectrumWindow(QMainWindow):
                 self.show_message(tx.error)
                 self.send_button.setDisabled(False)
                 return
-            if tx.requires_fee(self.wallet.verifier) and fee < MIN_RELAY_TX_FEE:
-                QMessageBox.warning(self, _('Error'), _("This transaction requires a higher fee, or it will not be propagated by the network."), _('OK'))
-                self.send_button.setDisabled(False)
-                return
             if label:
                 self.wallet.set_label(tx.hash(), label)
 
@@ -1274,6 +1269,7 @@ class ElectrumWindow(QMainWindow):
 
 
     def do_clear(self):
+        self.not_enough_funds = False
         self.payto_e.is_pr = False
         self.payto_sig.setVisible(False)
         for e in [self.payto_e, self.message_e, self.amount_e, self.fee_e]:
@@ -2480,7 +2476,7 @@ class ElectrumWindow(QMainWindow):
         if not d.exec_():
             return
 
-        fee = self.wallet.fee
+        fee = self.wallet.fee_per_kb
         tx = Transaction.sweep(get_pk(), self.network, get_address(), fee)
         self.show_transaction(tx)
 
@@ -2568,7 +2564,7 @@ class ElectrumWindow(QMainWindow):
         fee_label = QLabel(_('Transaction fee per kb') + ':')
         fee_help = HelpButton(_('Fee per kilobyte of transaction.') + '\n' + _('Recommended value') + ': ' + self.format_amount(10000) + ' ' + self.base_unit())
         fee_e = BTCAmountEdit(self.get_decimal_point)
-        fee_e.setAmount(self.wallet.fee)
+        fee_e.setAmount(self.wallet.fee_per_kb)
         if not self.config.is_modifiable('fee_per_kb'):
             for w in [fee_e, fee_label]: w.setEnabled(False)
         def on_fee():

lib/transaction.py

@@ -32,6 +32,7 @@ import struct
 import struct
 import StringIO
 import mmap
+import random
 
 NO_SIGNATURE = 'ff'
 
@@ -497,7 +498,7 @@ class Transaction:
 
     def __str__(self):
         if self.raw is None:
-            self.raw = self.serialize(self.inputs, self.outputs, for_sig = None) # for_sig=-1 means do not sign
+            self.raw = self.serialize()
         return self.raw
 
     def __init__(self, inputs, outputs, locktime=0):
@@ -519,7 +520,6 @@ class Transaction:
         self.outputs = map(lambda x: (x['type'], x['address'], x['value']), d['outputs'])
         self.locktime = d['lockTime']
 
-
     @classmethod 
     def sweep(klass, privkeys, network, to_address, fee):
         inputs = []
@@ -594,8 +594,14 @@ class Transaction:
         return script
 
 
-    @classmethod
-    def serialize(klass, inputs, outputs, for_sig = None ):
+    def serialize(self, for_sig=None):
+        # for_sig:
+        #   -1   : do not sign, estimate length
+        #   i>=0 : sign input i
+        #   None : add all signatures
+
+        inputs = self.inputs
+        outputs = self.outputs
 
         s  = int_to_hex(1,4)                                         # version
         s += var_int( len(inputs) )                                  # number of inputs
@@ -613,11 +619,15 @@ class Transaction:
             signatures = filter(lambda x: x is not None, x_signatures)
             is_complete = len(signatures) == num_sig
 
-            if for_sig is None:
+            if for_sig in [-1, None]:
                 # if we have enough signatures, we use the actual pubkeys
                 # use extended pubkeys (with bip32 derivation)
                 sig_list = []
-                if is_complete:
+                if for_sig == -1:
+                    # we assume that signature will be 0x48 bytes long
+                    pubkeys = txin['pubkeys']
+                    sig_list = [ "00"* 0x48 ] * num_sig
+                elif is_complete:
                     pubkeys = txin['pubkeys']
                     for signature in signatures:
                         sig_list.append(signature + '01')
@@ -633,13 +643,14 @@ class Transaction:
                 else:
                     script = '00'                                    # op_0
                     script += sig_list
-                    redeem_script = klass.multisig_script(pubkeys,2)
+                    redeem_script = self.multisig_script(pubkeys,2)
                     script += push_script(redeem_script)
 
             elif for_sig==i:
-                script = txin['redeemScript'] if p2sh else klass.pay_script('address', address)
+                script = txin['redeemScript'] if p2sh else self.pay_script('address', address)
             else:
                 script = ''
+
             s += var_int( len(script)/2 )                            # script length
             s += script
             s += "ffffffff"                                          # sequence
@@ -648,7 +659,7 @@ class Transaction:
         for output in outputs:
             type, addr, amount = output
             s += int_to_hex( amount, 8)                              # amount
-            script = klass.pay_script(type, addr)
+            script = self.pay_script(type, addr)
             s += var_int( len(script)/2 )                           #  script length
             s += script                                             #  script
         s += int_to_hex(0,4)                                        #  lock time
@@ -656,10 +667,8 @@ class Transaction:
             s += int_to_hex(1, 4)                                   #  hash type
         return s
 
-
     def tx_for_sig(self,i):
-        return self.serialize(self.inputs, self.outputs, for_sig = i)
-
+        return self.serialize(for_sig = i)
 
     def hash(self):
         return Hash(self.raw.decode('hex') )[::-1].encode('hex')
@@ -672,8 +681,20 @@ class Transaction:
         txin['signatures'][ii] = sig
         txin['x_pubkeys'][ii] = pubkey
         self.inputs[i] = txin
-        self.raw = self.serialize(self.inputs, self.outputs)
+        self.raw = self.serialize()
+
+    def add_input(self, input):
+        self.inputs.append(input)
+        self.raw = None
 
+    def input_value(self):
+        return sum([x['value'] for x in self.inputs])
+
+    def output_value(self):
+        return sum([ x[2] for x in self.outputs])
+
+    def get_fee(self):
+        return self.input_value() - self.output_value()
 
     def signature_count(self):
         r = 0
@@ -747,9 +768,8 @@ class Transaction:
                     assert public_key.verify_digest( sig, for_sig, sigdecode = ecdsa.util.sigdecode_der)
                     self.add_signature(i, pubkey, sig.encode('hex'))
 
-
         print_error("is_complete", self.is_complete())
-        self.raw = self.serialize( self.inputs, self.outputs )
+        self.raw = self.serialize()
 
 
     def add_pubkey_addresses(self, txlist):
@@ -861,7 +881,7 @@ class Transaction:
     def requires_fee(self, verifier):
         # see https://en.bitcoin.it/wiki/Transaction_fees
         threshold = 57600000
-        size = len(str(self))/2
+        size = len(self.serialize(-1))/2
         if size >= 10000:
             return True
 

lib/wallet.py

@@ -42,6 +42,7 @@ from mnemonic import Mnemonic
 COINBASE_MATURITY = 100
 DUST_THRESHOLD = 5430
 
+
 # internal ID for imported account
 IMPORTED_ACCOUNT = '/x'
 
@@ -163,7 +164,7 @@ class Abstract_Wallet(object):
 
         self.history               = storage.get('addr_history',{})        # address -> list(txid, height)
 
-        self.fee                   = int(storage.get('fee_per_kb', 10000))
+        self.fee_per_kb            = int(storage.get('fee_per_kb', 10000))
 
         self.next_addresses = storage.get('next_addresses',{})
 
@@ -579,60 +580,13 @@ class Abstract_Wallet(object):
                     coins = coins[1:] + [ coins[0] ]
         return [x[1] for x in coins]
 
-    def choose_tx_inputs( self, amount, fixed_fee, num_outputs, domain = None, coins = None ):
-        """ todo: minimize tx size """
-        total = 0
-        fee = self.fee if fixed_fee is None else fixed_fee
-
-        if not coins:
-            if domain is None:
-                domain = self.addresses(True)
-            for i in self.frozen_addresses:
-                if i in domain: domain.remove(i)
-            coins = self.get_unspent_coins(domain)
-
-        inputs = []
-        for item in coins:
-            if item.get('coinbase') and item.get('height') + COINBASE_MATURITY > self.network.get_local_height():
-                continue
-            v = item.get('value')
-            total += v
-            inputs.append(item)
-            fee = self.estimated_fee(inputs, num_outputs) if fixed_fee is None else fixed_fee
-            if total >= amount + fee: break
-        else:
-            inputs = []
-        return inputs, total, fee
 
 
     def set_fee(self, fee):
-        if self.fee != fee:
-            self.fee = fee
-            self.storage.put('fee_per_kb', self.fee, True)
+        if self.fee_per_kb != fee:
+            self.fee_per_kb = fee
+            self.storage.put('fee_per_kb', self.fee_per_kb, True)
 
-    def estimated_fee(self, inputs, num_outputs):
-        estimated_size =  len(inputs) * 180 + num_outputs * 34    # this assumes non-compressed keys
-        fee = self.fee * int(math.ceil(estimated_size/1000.))
-        return fee
-
-    def add_tx_change( self, inputs, outputs, amount, fee, total, change_addr=None):
-        "add change to a transaction"
-        change_amount = total - ( amount + fee )
-        if change_amount > DUST_THRESHOLD:
-            if not change_addr:
-
-                # send change to one of the accounts involved in the tx
-                address = inputs[0].get('address')
-                account, _ = self.get_address_index(address)
-
-                if not self.use_change or account == IMPORTED_ACCOUNT:
-                    change_addr = address
-                else:
-                    change_addr = self.accounts[account].get_addresses(1)[-self.gap_limit_for_change]
-
-            # Insert the change output at a random position in the outputs
-            posn = random.randint(0, len(outputs))
-            outputs[posn:posn] = [( 'address', change_addr,  change_amount)]
 
     def get_history(self, address):
         with self.lock:
@@ -754,22 +708,77 @@ class Abstract_Wallet(object):
 
         return default_label
 
-    def make_unsigned_transaction(self, outputs, fee=None, change_addr=None, domain=None, coins=None ):
+    def estimated_fee(self, tx):
+        estimated_size = len(tx.serialize(-1))/2
+        #print_error('estimated_size', estimated_size)
+        return int(self.fee_per_kb*estimated_size/1024.)
+
+    def make_unsigned_transaction(self, outputs, fixed_fee=None, change_addr=None, domain=None, coins=None ):
+        # check outputs
         for type, data, value in outputs:
             if type == 'op_return':
                 assert len(data) < 41, "string too long"
-                assert value == 0
+                #assert value == 0
             if type == 'address':
                 assert is_address(data), "Address " + data + " is invalid!"
+
+        # get coins
+        if not coins:
+            if domain is None:
+                domain = self.addresses(True)
+            for i in self.frozen_addresses:
+                if i in domain: domain.remove(i)
+            coins = self.get_unspent_coins(domain)
+
         amount = sum( map(lambda x:x[2], outputs) )
-        inputs, total, fee = self.choose_tx_inputs( amount, fee, len(outputs), domain, coins )
-        if not inputs:
-            raise ValueError("Not enough funds")
-        for txin in inputs:
-            self.add_input_info(txin)
-        self.add_tx_change(inputs, outputs, amount, fee, total, change_addr)
-        run_hook('make_unsigned_transaction', inputs, outputs)
-        return Transaction(inputs, outputs)
+        total = 0
+        inputs = []
+        tx = Transaction(inputs, outputs)
+        for item in coins:
+            if item.get('coinbase') and item.get('height') + COINBASE_MATURITY > self.network.get_local_height():
+                continue
+            v = item.get('value')
+            total += v
+            self.add_input_info(item)
+            tx.add_input(item)
+            fee = fixed_fee if fixed_fee is not None else self.estimated_fee(tx)
+            if total >= amount + fee: break
+        else:
+            print_error("Not enough funds", total, amount, fee)
+            return None
+
+        # change address
+        if not change_addr:
+            # send change to one of the accounts involved in the tx
+            address = inputs[0].get('address')
+            account, _ = self.get_address_index(address)
+            if not self.use_change or account == IMPORTED_ACCOUNT:
+                change_addr = address
+            else:
+                change_addr = self.accounts[account].get_addresses(1)[-self.gap_limit_for_change]
+
+        # if change is above dust threshold, add a change output.
+        change_amount = total - ( amount + fee )
+        if change_amount > DUST_THRESHOLD:
+            # Insert the change output at a random position in the outputs
+            posn = random.randint(0, len(tx.outputs))
+            tx.outputs[posn:posn] = [( 'address', change_addr,  change_amount)]
+            # recompute fee including change output
+            fee = fixed_fee if fixed_fee is not None else self.estimated_fee(tx)
+            # remove change output
+            tx.outputs.pop(posn)
+            # if change is still above dust threshold, re-add change output.
+            change_amount = total - ( amount + fee )
+            if change_amount > DUST_THRESHOLD:
+                tx.outputs[posn:posn] = [( 'address', change_addr,  change_amount)]
+                print_error('change', change_amount)
+            else:
+                print_error('not keeping dust', change_amount)
+        else:
+            print_error('not keeping dust', change_amount)
+
+        run_hook('make_unsigned_transaction', tx)
+        return tx
 
     def mktx(self, outputs, password, fee=None, change_addr=None, domain= None, coins = None ):
         tx = self.make_unsigned_transaction(outputs, fee, change_addr, domain, coins)