URI: 
       tRBF batching: some fixes - electrum - Electrum Bitcoin wallet
  HTML git clone https://git.parazyd.org/electrum
   DIR Log
   DIR Files
   DIR Refs
   DIR Submodules
       ---
   DIR commit 71ac3bb305c6622f8224323c56cba6d3a88a1dc6
   DIR parent f55db2f90b7ad0114deea178ac9d3fc2dee4fe1a
  HTML Author: SomberNight <somber.night@protonmail.com>
       Date:   Fri,  9 Nov 2018 17:56:42 +0100
       
       RBF batching: some fixes
       
       Diffstat:
         M electrum/address_synchronizer.py    |      10 ++--------
         M electrum/coinchooser.py             |       9 +++++----
         M electrum/gui/qt/main_window.py      |       2 +-
         M electrum/transaction.py             |       6 +-----
         M electrum/wallet.py                  |      29 ++++++++++++++++++++++++++---
       
       5 files changed, 35 insertions(+), 21 deletions(-)
       ---
   DIR diff --git a/electrum/address_synchronizer.py b/electrum/address_synchronizer.py
       t@@ -25,7 +25,7 @@ import threading
        import asyncio
        import itertools
        from collections import defaultdict
       -from typing import TYPE_CHECKING
       +from typing import TYPE_CHECKING, Dict
        
        from . import bitcoin
        from .bitcoin import COINBASE_MATURITY, TYPE_ADDRESS, TYPE_PUBKEY
       t@@ -457,7 +457,7 @@ class AddressSynchronizer(PrintError):
                        self.spent_outpoints = defaultdict(dict)
                        self.history = {}
                        self.verified_tx = {}
       -                self.transactions = {}
       +                self.transactions = {}  # type: Dict[str, Transaction]
                        self.save_transactions()
        
            def get_txpos(self, tx_hash):
       t@@ -484,12 +484,6 @@ class AddressSynchronizer(PrintError):
                        self.threadlocal_cache.local_height = orig_val
                return f
        
       -    def get_unconfirmed_tx(self):
       -        for tx_hash, tx_mined_status, delta, balance in self.get_history():
       -            if tx_mined_status.conf <= 0 and delta < 0:
       -                tx = self.transactions.get(tx_hash)
       -                return tx
       -
            @with_local_height_cached
            def get_history(self, domain=None):
                # get domain
   DIR diff --git a/electrum/coinchooser.py b/electrum/coinchooser.py
       t@@ -84,8 +84,8 @@ def strip_unneeded(bkts, sufficient_funds):
            for i in range(len(bkts)):
                if not sufficient_funds(bkts[i + 1:]):
                    return bkts[i:]
       -    # Shouldn't get here
       -    return bkts
       +    # none of the buckets are needed
       +    return []
        
        class CoinChooserBase(PrintError):
        
       t@@ -203,12 +203,13 @@ class CoinChooserBase(PrintError):
        
                # Copy the outputs so when adding change we don't modify "outputs"
                tx = Transaction.from_io(inputs[:], outputs[:])
       -        v = tx.input_value()
       +        input_value = tx.input_value()
        
                # Weight of the transaction with no inputs and no change
                # Note: this will use legacy tx serialization as the need for "segwit"
                # would be detected from inputs. The only side effect should be that the
                # marker and flag are excluded, which is compensated in get_tx_weight()
       +        # FIXME calculation will be off by this (2 wu) in case of RBF batching
                base_weight = tx.estimated_weight()
                spent_amount = tx.output_value()
        
       t@@ -232,7 +233,7 @@ class CoinChooserBase(PrintError):
                def sufficient_funds(buckets):
                    '''Given a list of buckets, return True if it has enough
                    value to pay for the transaction'''
       -            total_input = v + sum(bucket.value for bucket in buckets)
       +            total_input = input_value + sum(bucket.value for bucket in buckets)
                    total_weight = get_tx_weight(buckets)
                    return total_input >= spent_amount + fee_estimator_w(total_weight)
        
   DIR diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py
       t@@ -2763,7 +2763,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
                batch_rbf_cb.setChecked(self.config.get('batch_rbf', False))
                batch_rbf_cb.setEnabled(use_rbf)
                batch_rbf_cb.setToolTip(
       -            _('If you check this box, your unconfirmed transactios will be consolidated in a single transaction') + '\n' + \
       +            _('If you check this box, your unconfirmed transactions will be consolidated into a single transaction.') + '\n' + \
                    _('This will save fees.'))
                def on_batch_rbf(x):
                    self.config.set_key('batch_rbf', bool(x))
   DIR diff --git a/electrum/transaction.py b/electrum/transaction.py
       t@@ -864,7 +864,7 @@ class Transaction:
            @classmethod
            def serialize_witness(self, txin, estimate_size=False):
                _type = txin['type']
       -        if not self.is_segwit_input(txin) and not self.is_input_value_needed(txin):
       +        if not self.is_segwit_input(txin) and not txin['type'] == 'address':
                    return '00'
                if _type == 'coinbase':
                    return txin['witness']
       t@@ -903,10 +903,6 @@ class Transaction:
                return txin_type in ('p2wpkh', 'p2wpkh-p2sh', 'p2wsh', 'p2wsh-p2sh')
        
            @classmethod
       -    def is_input_value_needed(cls, txin):
       -        return cls.is_segwit_input(txin) or txin['type'] == 'address'
       -
       -    @classmethod
            def guess_txintype_from_address(cls, addr):
                # It's not possible to tell the script type in general
                # just from an address.
   DIR diff --git a/electrum/wallet.py b/electrum/wallet.py
       t@@ -536,6 +536,29 @@ class Abstract_Wallet(AddressSynchronizer):
            def dust_threshold(self):
                return dust_threshold(self.network)
        
       +    def get_unconfirmed_base_tx_for_batching(self) -> Optional[Transaction]:
       +        candidate = None
       +        for tx_hash, tx_mined_status, delta, balance in self.get_history():
       +            # tx should not be mined yet
       +            if tx_mined_status.conf > 0: continue
       +            # tx should be "outgoing" from wallet
       +            if delta >= 0: continue
       +            tx = self.transactions.get(tx_hash)
       +            if not tx: continue
       +            # is_mine outputs should not be spent yet
       +            # to avoid cancelling our own dependent transactions
       +            for output_idx, o in enumerate(tx.outputs()):
       +                if self.is_mine(o.address) and self.spent_outpoints[tx.txid()].get(output_idx):
       +                    continue
       +            # prefer txns already in mempool (vs local)
       +            if tx_mined_status.height == TX_HEIGHT_LOCAL:
       +                candidate = tx
       +                continue
       +            # tx must have opted-in for RBF
       +            if tx.is_final(): continue
       +            return tx
       +        return candidate
       +
            def make_unsigned_transaction(self, coins, outputs, config, fixed_fee=None,
                                          change_addr=None, is_sweep=False):
                # check outputs
       t@@ -592,8 +615,8 @@ class Abstract_Wallet(AddressSynchronizer):
                    max_change = self.max_change_outputs if self.multiple_change else 1
                    coin_chooser = coinchooser.get_coin_chooser(config)
                    # If there is an unconfirmed RBF tx, merge with it
       -            base_tx = self.get_unconfirmed_tx()
       -            if config.get('batch_rbf', False) and base_tx and not base_tx.is_final():
       +            base_tx = self.get_unconfirmed_base_tx_for_batching()
       +            if config.get('batch_rbf', False) and base_tx:
                        base_tx = Transaction(base_tx.serialize())
                        base_tx.deserialize(force_full_parse=True)
                        base_tx.remove_signatures()
       t@@ -602,7 +625,7 @@ class Abstract_Wallet(AddressSynchronizer):
                        fee_per_byte = Decimal(base_fee) / base_tx.estimated_size()
                        fee_estimator = lambda size: base_fee + round(fee_per_byte * size)
                        txi = base_tx.inputs()
       -                txo = list(filter(lambda x: not self.is_change(x[1]), base_tx.outputs()))
       +                txo = list(filter(lambda o: not self.is_change(o.address), base_tx.outputs()))
                    else:
                        txi = []
                        txo = []