Merge pull request #7026 from SomberNight/20210213_wallet_bumpfee

wallet: refactor bump_fee; add new strategy; change Qt dialog to have "advanced" button

electrum/coinchooser.py

@@ -238,7 +238,7 @@ class CoinChooserBase(Logger):
             assert is_address(change_addrs[0])
 
         # This takes a count of change outputs and returns a tx fee
-        output_weight = 4 * Transaction.estimated_output_size(change_addrs[0])
+        output_weight = 4 * Transaction.estimated_output_size_for_address(change_addrs[0])
         fee_estimator_numchange = lambda count: fee_estimator_w(tx_weight + count * output_weight)
         change = self._change_outputs(tx, change_addrs, fee_estimator_numchange, dust_threshold)
         tx.add_outputs(change)

electrum/gui/qt/main_window.py

@@ -100,6 +100,7 @@ from .update_checker import UpdateCheck, UpdateCheckThread
 from .channels_list import ChannelsList
 from .confirm_tx_dialog import ConfirmTxDialog
 from .transaction_dialog import PreviewTxDialog
+from .rbf_dialog import BumpFeeDialog, DSCancelDialog
 
 if TYPE_CHECKING:
     from . import ElectrumGui
@@ -3264,96 +3265,23 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger):
             return False
         return True
 
-    def _rbf_dialog(self, tx: Transaction, func, title, help_text):
+    def bump_fee_dialog(self, tx: Transaction):
         txid = tx.txid()
-        assert txid
         if not isinstance(tx, PartialTransaction):
             tx = PartialTransaction.from_tx(tx)
         if not self._add_info_to_tx_from_wallet_and_network(tx):
             return
-        fee = tx.get_fee()
-        assert fee is not None
-        tx_label = self.wallet.get_label_for_txid(txid)
-        tx_size = tx.estimated_size()
-        old_fee_rate = fee / tx_size  # sat/vbyte
-        d = WindowModalDialog(self, title)
-        vbox = QVBoxLayout(d)
-        vbox.addWidget(WWLabel(help_text))
-
-        ok_button = OkButton(d)
-        warning_label = WWLabel('\n')
-        warning_label.setStyleSheet(ColorScheme.RED.as_stylesheet())
-        feerate_e = FeerateEdit(lambda: 0)
-        feerate_e.setAmount(max(old_fee_rate * 1.5, old_fee_rate + 1))
-        def on_feerate():
-            fee_rate = feerate_e.get_amount()
-            warning_text = '\n'
-            if fee_rate is not None:
-                try:
-                    new_tx = func(fee_rate)
-                except Exception as e:
-                    new_tx = None
-                    warning_text = str(e).replace('\n',' ')
-            else:
-                new_tx = None
-            ok_button.setEnabled(new_tx is not None)
-            warning_label.setText(warning_text)
-
-        feerate_e.textChanged.connect(on_feerate)
-        def on_slider(dyn, pos, fee_rate):
-            fee_slider.activate()
-            if fee_rate is not None:
-                feerate_e.setAmount(fee_rate / 1000)
-        fee_slider = FeeSlider(self, self.config, on_slider)
-        fee_combo = FeeComboBox(fee_slider)
-        fee_slider.deactivate()
-        feerate_e.textEdited.connect(fee_slider.deactivate)
-
-        grid = QGridLayout()
-        grid.addWidget(QLabel(_('Current Fee') + ':'), 0, 0)
-        grid.addWidget(QLabel(self.format_amount(fee) + ' ' + self.base_unit()), 0, 1)
-        grid.addWidget(QLabel(_('Current Fee rate') + ':'), 1, 0)
-        grid.addWidget(QLabel(self.format_fee_rate(1000 * old_fee_rate)), 1, 1)
-        grid.addWidget(QLabel(_('New Fee rate') + ':'), 2, 0)
-        grid.addWidget(feerate_e, 2, 1)
-        grid.addWidget(fee_slider, 3, 1)
-        grid.addWidget(fee_combo, 3, 2)
-        vbox.addLayout(grid)
-        cb = QCheckBox(_('Final'))
-        vbox.addWidget(cb)
-        vbox.addWidget(warning_label)
-        vbox.addLayout(Buttons(CancelButton(d), ok_button))
-        if not d.exec_():
-            return
-        is_final = cb.isChecked()
-        new_fee_rate = feerate_e.get_amount()
-        try:
-            new_tx = func(new_fee_rate)
-        except Exception as e:
-            self.show_error(str(e))
-            return
-        new_tx.set_rbf(not is_final)
-        self.show_transaction(new_tx, tx_desc=tx_label)
-
-    def bump_fee_dialog(self, tx: Transaction):
-        title = _('Bump Fee')
-        help_text = _("Increase your transaction's fee to improve its position in mempool.")
-        def func(new_fee_rate):
-            return self.wallet.bump_fee(
-                tx=tx,
-                txid=tx.txid(),
-                new_fee_rate=new_fee_rate,
-                coins=self.get_coins())
-        self._rbf_dialog(tx, func, title, help_text)
+        d = BumpFeeDialog(main_window=self, tx=tx, txid=txid)
+        d.run()
 
     def dscancel_dialog(self, tx: Transaction):
-        title = _('Cancel transaction')
-        help_text = _(
-            "Cancel an unconfirmed RBF transaction by double-spending "
-            "its inputs back to your wallet with a higher fee.")
-        def func(new_fee_rate):
-            return self.wallet.dscancel(tx=tx, new_fee_rate=new_fee_rate)
-        self._rbf_dialog(tx, func, title, help_text)
+        txid = tx.txid()
+        if not isinstance(tx, PartialTransaction):
+            tx = PartialTransaction.from_tx(tx)
+        if not self._add_info_to_tx_from_wallet_and_network(tx):
+            return
+        d = DSCancelDialog(main_window=self, tx=tx, txid=txid)
+        d.run()
 
     def save_transaction_into_wallet(self, tx: Transaction):
         win = self.top_level_window()

electrum/gui/qt/rbf_dialog.py

@@ -0,0 +1,213 @@
+# Copyright (C) 2021 The Electrum developers
+# Distributed under the MIT software license, see the accompanying
+# file LICENCE or http://www.opensource.org/licenses/mit-license.php
+
+from typing import TYPE_CHECKING
+
+from PyQt5.QtWidgets import (QCheckBox, QLabel, QVBoxLayout, QGridLayout, QWidget,
+                             QPushButton, QHBoxLayout, QComboBox)
+
+from .amountedit import FeerateEdit
+from .fee_slider import FeeSlider, FeeComboBox
+from .util import (ColorScheme, WindowModalDialog, Buttons,
+                   OkButton, WWLabel, CancelButton)
+
+from electrum.i18n import _
+from electrum.transaction import PartialTransaction
+from electrum.wallet import BumpFeeStrategy
+
+if TYPE_CHECKING:
+    from .main_window import ElectrumWindow
+
+
+class _BaseRBFDialog(WindowModalDialog):
+
+    def __init__(
+            self,
+            *,
+            main_window: 'ElectrumWindow',
+            tx: PartialTransaction,
+            txid: str,
+            title: str,
+            help_text: str,
+    ):
+        WindowModalDialog.__init__(self, main_window, title=title)
+        self.window = main_window
+        self.wallet = main_window.wallet
+        self.tx = tx
+        assert txid
+        self.txid = txid
+
+        fee = tx.get_fee()
+        assert fee is not None
+        tx_size = tx.estimated_size()
+        old_fee_rate = fee / tx_size  # sat/vbyte
+        vbox = QVBoxLayout(self)
+        vbox.addWidget(WWLabel(help_text))
+
+        ok_button = OkButton(self)
+        self.adv_button = QPushButton(_("Show advanced settings"))
+        warning_label = WWLabel('\n')
+        warning_label.setStyleSheet(ColorScheme.RED.as_stylesheet())
+        self.feerate_e = FeerateEdit(lambda: 0)
+        self.feerate_e.setAmount(max(old_fee_rate * 1.5, old_fee_rate + 1))
+
+        def on_feerate():
+            fee_rate = self.feerate_e.get_amount()
+            warning_text = '\n'
+            if fee_rate is not None:
+                try:
+                    new_tx = self.rbf_func(fee_rate)
+                except Exception as e:
+                    new_tx = None
+                    warning_text = str(e).replace('\n', ' ')
+            else:
+                new_tx = None
+            ok_button.setEnabled(new_tx is not None)
+            warning_label.setText(warning_text)
+
+        self.feerate_e.textChanged.connect(on_feerate)
+
+        def on_slider(dyn, pos, fee_rate):
+            fee_slider.activate()
+            if fee_rate is not None:
+                self.feerate_e.setAmount(fee_rate / 1000)
+
+        fee_slider = FeeSlider(self.window, self.window.config, on_slider)
+        fee_combo = FeeComboBox(fee_slider)
+        fee_slider.deactivate()
+        self.feerate_e.textEdited.connect(fee_slider.deactivate)
+
+        grid = QGridLayout()
+        grid.addWidget(QLabel(_('Current Fee') + ':'), 0, 0)
+        grid.addWidget(QLabel(self.window.format_amount(fee) + ' ' + self.window.base_unit()), 0, 1)
+        grid.addWidget(QLabel(_('Current Fee rate') + ':'), 1, 0)
+        grid.addWidget(QLabel(self.window.format_fee_rate(1000 * old_fee_rate)), 1, 1)
+        grid.addWidget(QLabel(_('New Fee rate') + ':'), 2, 0)
+        grid.addWidget(self.feerate_e, 2, 1)
+        grid.addWidget(fee_slider, 3, 1)
+        grid.addWidget(fee_combo, 3, 2)
+        vbox.addLayout(grid)
+        self._add_advanced_options_cont(vbox)
+        vbox.addWidget(warning_label)
+
+        btns_hbox = QHBoxLayout()
+        btns_hbox.addWidget(self.adv_button)
+        btns_hbox.addStretch(1)
+        btns_hbox.addWidget(CancelButton(self))
+        btns_hbox.addWidget(ok_button)
+        vbox.addLayout(btns_hbox)
+
+    def rbf_func(self, fee_rate) -> PartialTransaction:
+        raise NotImplementedError()  # implemented by subclasses
+
+    def _add_advanced_options_cont(self, vbox: QVBoxLayout) -> None:
+        adv_vbox = QVBoxLayout()
+        adv_vbox.setContentsMargins(0, 0, 0, 0)
+        adv_widget = QWidget()
+        adv_widget.setLayout(adv_vbox)
+        adv_widget.setVisible(False)
+        def show_adv_settings():
+            self.adv_button.setEnabled(False)
+            adv_widget.setVisible(True)
+        self.adv_button.clicked.connect(show_adv_settings)
+        self._add_advanced_options(adv_vbox)
+        vbox.addWidget(adv_widget)
+
+    def _add_advanced_options(self, adv_vbox: QVBoxLayout) -> None:
+        self.cb_rbf = QCheckBox(_('Keep Replace-By-Fee enabled'))
+        self.cb_rbf.setChecked(True)
+        adv_vbox.addWidget(self.cb_rbf)
+
+    def run(self) -> None:
+        if not self.exec_():
+            return
+        is_rbf = self.cb_rbf.isChecked()
+        new_fee_rate = self.feerate_e.get_amount()
+        try:
+            new_tx = self.rbf_func(new_fee_rate)
+        except Exception as e:
+            self.window.show_error(str(e))
+            return
+        new_tx.set_rbf(is_rbf)
+        tx_label = self.wallet.get_label_for_txid(self.txid)
+        self.window.show_transaction(new_tx, tx_desc=tx_label)
+        # TODO maybe save tx_label as label for new tx??
+
+
+class BumpFeeDialog(_BaseRBFDialog):
+
+    def __init__(
+            self,
+            *,
+            main_window: 'ElectrumWindow',
+            tx: PartialTransaction,
+            txid: str,
+    ):
+        help_text = _("Increase your transaction's fee to improve its position in mempool.")
+        _BaseRBFDialog.__init__(
+            self,
+            main_window=main_window,
+            tx=tx,
+            txid=txid,
+            title=_('Bump Fee'),
+            help_text=help_text,
+        )
+
+    def rbf_func(self, fee_rate):
+        return self.wallet.bump_fee(
+            tx=self.tx,
+            txid=self.txid,
+            new_fee_rate=fee_rate,
+            coins=self.window.get_coins(),
+            strategies=self.option_index_to_strats[self.strat_combo.currentIndex()],
+        )
+
+    def _add_advanced_options(self, adv_vbox: QVBoxLayout) -> None:
+        self.cb_rbf = QCheckBox(_('Keep Replace-By-Fee enabled'))
+        self.cb_rbf.setChecked(True)
+        adv_vbox.addWidget(self.cb_rbf)
+
+        self.strat_combo = QComboBox()
+        options = [
+            _("decrease change, or add new inputs, or decrease any outputs"),
+            _("decrease change, or decrease any outputs"),
+            _("decrease payment"),
+        ]
+        self.option_index_to_strats = {
+            0: [BumpFeeStrategy.COINCHOOSER, BumpFeeStrategy.DECREASE_CHANGE],
+            1: [BumpFeeStrategy.DECREASE_CHANGE],
+            2: [BumpFeeStrategy.DECREASE_PAYMENT],
+        }
+        self.strat_combo.addItems(options)
+        self.strat_combo.setCurrentIndex(0)
+        strat_hbox = QHBoxLayout()
+        strat_hbox.addWidget(QLabel(_("Strategy") + ":"))
+        strat_hbox.addWidget(self.strat_combo)
+        strat_hbox.addStretch(1)
+        adv_vbox.addLayout(strat_hbox)
+
+
+class DSCancelDialog(_BaseRBFDialog):
+
+    def __init__(
+            self,
+            *,
+            main_window: 'ElectrumWindow',
+            tx: PartialTransaction,
+            txid: str,
+    ):
+        help_text = _(
+            "Cancel an unconfirmed RBF transaction by double-spending "
+            "its inputs back to your wallet with a higher fee.")
+        _BaseRBFDialog.__init__(
+            self,
+            main_window=main_window,
+            tx=tx,
+            txid=txid,
+            title=_('Cancel transaction'),
+            help_text=help_text,
+        )
+
+    def rbf_func(self, fee_rate):
+        return self.wallet.dscancel(tx=self.tx, new_fee_rate=fee_rate)

electrum/tests/test_transaction.py

@@ -126,7 +126,7 @@ class TestTransaction(ElectrumTestCase):
         self.assertEqual(tx.estimated_size(), 193)
 
     def test_estimated_output_size(self):
-        estimated_output_size = transaction.Transaction.estimated_output_size
+        estimated_output_size = transaction.Transaction.estimated_output_size_for_address
         self.assertEqual(estimated_output_size('14gcRovpkCoGkCNBivQBvw7eso7eiNAbxG'), 34)
         self.assertEqual(estimated_output_size('35ZqQJcBQMZ1rsv8aSuJ2wkC7ohUCQMJbT'), 32)
         self.assertEqual(estimated_output_size('bc1q3g5tmkmlvxryhh843v4dz026avatc0zzr6h3af'), 31)

electrum/tests/test_wallet_vertical.py

@@ -10,7 +10,8 @@ from electrum import storage, bitcoin, keystore, bip32, wallet
 from electrum import Transaction
 from electrum import SimpleConfig
 from electrum.address_synchronizer import TX_HEIGHT_UNCONFIRMED, TX_HEIGHT_UNCONF_PARENT
-from electrum.wallet import sweep, Multisig_Wallet, Standard_Wallet, Imported_Wallet, restore_wallet_from_text, Abstract_Wallet
+from electrum.wallet import (sweep, Multisig_Wallet, Standard_Wallet, Imported_Wallet,
+                             restore_wallet_from_text, Abstract_Wallet, BumpFeeStrategy)
 from electrum.util import bfh, bh2u, create_and_start_event_loop
 from electrum.transaction import TxOutput, Transaction, PartialTransaction, PartialTxOutput, PartialTxInput, tx_from_any
 from electrum.mnemonic import seed_type
@@ -937,6 +938,14 @@ class TestWalletSending(TestCaseForTestnet):
                 self._bump_fee_when_not_all_inputs_are_ismine_subcase_all_outputs_are_ismine(
                     simulate_moving_txs=simulate_moving_txs,
                     config=config)
+            with self.subTest(msg="_bump_fee_p2wpkh_decrease_payment", simulate_moving_txs=simulate_moving_txs):
+                self._bump_fee_p2wpkh_decrease_payment(
+                    simulate_moving_txs=simulate_moving_txs,
+                    config=config)
+            with self.subTest(msg="_bump_fee_p2wpkh_decrease_payment_batch", simulate_moving_txs=simulate_moving_txs):
+                self._bump_fee_p2wpkh_decrease_payment_batch(
+                    simulate_moving_txs=simulate_moving_txs,
+                    config=config)
 
     def _bump_fee_p2pkh_when_there_is_a_change_address(self, *, simulate_moving_txs, config):
         wallet = self.create_standard_wallet_from_seed('fold object utility erase deputy output stadium feed stereo usage modify bean',
@@ -1044,6 +1053,89 @@ class TestWalletSending(TestCaseForTestnet):
         wallet.receive_tx_callback(tx.txid(), tx, TX_HEIGHT_UNCONFIRMED)
         self.assertEqual((0, 461600, 0), wallet.get_balance())
 
+    def _bump_fee_p2wpkh_decrease_payment(self, *, simulate_moving_txs, config):
+        wallet = self.create_standard_wallet_from_seed('leader company camera enlist crash sleep insane aware anger hole hammer label',
+                                                       config=config)
+
+        # bootstrap wallet
+        funding_tx = Transaction('020000000001022ea8f7940c2e4bca2f34f21ba15a5c8d5e3c93d9c6deb17983412feefa0f1f6d0100000000fdffffff9d4ba5ab41951d506a7fa8272ef999ce3df166fe28f6f885aa791f012a0924cf0000000000fdffffff027485010000000000160014f80e86af4246960a24cd21c275a8e8842973fbcaa0860100000000001600149c6b743752604b98d30f1a5d27a5d5ce8919f4400247304402203bf6dd875a775f356d4bb8c4e295a2cd506338c100767518f2b31fb85db71c1302204dc4ebca5584fc1cc08bd7f7171135d1b67ca6c8812c3723cd332eccaa7b848101210360bdbd16d9ef390fd3e804c421e6f30e6b065ac314f4d2b9a80d2f0682ad1431024730440220126b442d7988c5883ca17c2429f51ce770e3a57895524c8dfe07b539e483019e02200b50feed4f42f0035c9a9ddd044820607281e45e29e41a29233c2b8be6080bac01210245d47d08915816a5ecc934cff1b17e00071ca06172f51d632ba95392e8aad4fdd38a1d00')
+        funding_txid = funding_tx.txid()
+        self.assertEqual('dd0bf0d1563cd588b4c93cc1a9623c051ddb1c4f4581cf8ef43cfd27f031f246', funding_txid)
+        wallet.receive_tx_callback(funding_txid, funding_tx, TX_HEIGHT_UNCONFIRMED)
+
+        orig_rbf_tx = Transaction('0200000000010146f231f027fd3cf48ecf81454f1cdb1d053c62a9c13cc9b488d53c56d1f00bdd0100000000fdffffff02c8af000000000000160014999a95482213a896c72a251b6cc9f3d137b0a45850c3000000000000160014ea76d391236726af7d7a9c10abe600129154eb5a02473044022076d298537b524a926a8fadad0e9ded5868c8f4cf29246048f76f00eb4afa56310220739ad9e0417e97ce03fad98a454b4977972c2805cef37bfa822c6d6c56737c870121024196fb7b766ac987a08b69a5e108feae8513b7e72bc9e47899e27b36100f2af4d48a1d00')
+        orig_rbf_txid = orig_rbf_tx.txid()
+        self.assertEqual('db2f77709a4a04417b3a45838c21470877fe7c182a4f81005a21ce1315c6a5e6', orig_rbf_txid)
+        wallet.receive_tx_callback(orig_rbf_txid, orig_rbf_tx, TX_HEIGHT_UNCONFIRMED)
+
+        # bump tx
+        tx = wallet.bump_fee(
+            tx=tx_from_any(orig_rbf_tx.serialize()),
+            new_fee_rate=60,
+            strategies=[BumpFeeStrategy.DECREASE_PAYMENT],
+        )
+        tx.locktime = 1936085
+        tx.version = 2
+        if simulate_moving_txs:
+            partial_tx = tx.serialize_as_bytes().hex()
+            self.assertEqual("70736274ff010071020000000146f231f027fd3cf48ecf81454f1cdb1d053c62a9c13cc9b488d53c56d1f00bdd0100000000fdffffff02c8af000000000000160014999a95482213a896c72a251b6cc9f3d137b0a458ccb5000000000000160014ea76d391236726af7d7a9c10abe600129154eb5ad58a1d00000100fd7201020000000001022ea8f7940c2e4bca2f34f21ba15a5c8d5e3c93d9c6deb17983412feefa0f1f6d0100000000fdffffff9d4ba5ab41951d506a7fa8272ef999ce3df166fe28f6f885aa791f012a0924cf0000000000fdffffff027485010000000000160014f80e86af4246960a24cd21c275a8e8842973fbcaa0860100000000001600149c6b743752604b98d30f1a5d27a5d5ce8919f4400247304402203bf6dd875a775f356d4bb8c4e295a2cd506338c100767518f2b31fb85db71c1302204dc4ebca5584fc1cc08bd7f7171135d1b67ca6c8812c3723cd332eccaa7b848101210360bdbd16d9ef390fd3e804c421e6f30e6b065ac314f4d2b9a80d2f0682ad1431024730440220126b442d7988c5883ca17c2429f51ce770e3a57895524c8dfe07b539e483019e02200b50feed4f42f0035c9a9ddd044820607281e45e29e41a29233c2b8be6080bac01210245d47d08915816a5ecc934cff1b17e00071ca06172f51d632ba95392e8aad4fdd38a1d002206024196fb7b766ac987a08b69a5e108feae8513b7e72bc9e47899e27b36100f2af410ce2dd7cb00000080000000000000000000220203ecb63cc22d200c96225671b88a51a71deb053c6445dbd4694f61166e3e5bd05910ce2dd7cb0000008001000000000000000000",
+                             partial_tx)
+            tx = tx_from_any(partial_tx)  # simulates moving partial txn between cosigners
+        self.assertFalse(tx.is_complete())
+
+        wallet.sign_transaction(tx, password=None)
+        self.assertTrue(tx.is_complete())
+        self.assertTrue(tx.is_segwit())
+        tx_copy = tx_from_any(tx.serialize())
+        self.assertEqual('0200000000010146f231f027fd3cf48ecf81454f1cdb1d053c62a9c13cc9b488d53c56d1f00bdd0100000000fdffffff02c8af000000000000160014999a95482213a896c72a251b6cc9f3d137b0a458ccb5000000000000160014ea76d391236726af7d7a9c10abe600129154eb5a024730440220063a2d330f0d659b3f686cc291722a87cc37371d3520c946e74da8dbbd4c57e00220604b0f387754988f71af47db78263698a513173e8ce3b27a696b9e3954ba757b0121024196fb7b766ac987a08b69a5e108feae8513b7e72bc9e47899e27b36100f2af4d58a1d00',
+                         str(tx_copy))
+        self.assertEqual('6b03c00f47cb145ffb632c3ce54dece29b9a980949ef5c574321f7fc83fa2238', tx_copy.txid())
+        self.assertEqual('cb1f123231a3de5b02babddb43208f0273cb0df8addd4275583234eb50c7a87d', tx_copy.wtxid())
+
+        wallet.receive_tx_callback(tx.txid(), tx, TX_HEIGHT_UNCONFIRMED)
+        self.assertEqual((0, 45000, 0), wallet.get_balance())
+
+    def _bump_fee_p2wpkh_decrease_payment_batch(self, *, simulate_moving_txs, config):
+        wallet = self.create_standard_wallet_from_seed('leader company camera enlist crash sleep insane aware anger hole hammer label',
+                                                       config=config)
+
+        # bootstrap wallet
+        funding_tx = Transaction('020000000001022ea8f7940c2e4bca2f34f21ba15a5c8d5e3c93d9c6deb17983412feefa0f1f6d0100000000fdffffff9d4ba5ab41951d506a7fa8272ef999ce3df166fe28f6f885aa791f012a0924cf0000000000fdffffff027485010000000000160014f80e86af4246960a24cd21c275a8e8842973fbcaa0860100000000001600149c6b743752604b98d30f1a5d27a5d5ce8919f4400247304402203bf6dd875a775f356d4bb8c4e295a2cd506338c100767518f2b31fb85db71c1302204dc4ebca5584fc1cc08bd7f7171135d1b67ca6c8812c3723cd332eccaa7b848101210360bdbd16d9ef390fd3e804c421e6f30e6b065ac314f4d2b9a80d2f0682ad1431024730440220126b442d7988c5883ca17c2429f51ce770e3a57895524c8dfe07b539e483019e02200b50feed4f42f0035c9a9ddd044820607281e45e29e41a29233c2b8be6080bac01210245d47d08915816a5ecc934cff1b17e00071ca06172f51d632ba95392e8aad4fdd38a1d00')
+        funding_txid = funding_tx.txid()
+        self.assertEqual('dd0bf0d1563cd588b4c93cc1a9623c051ddb1c4f4581cf8ef43cfd27f031f246', funding_txid)
+        wallet.receive_tx_callback(funding_txid, funding_tx, TX_HEIGHT_UNCONFIRMED)
+
+        orig_rbf_tx = Transaction('0200000000010146f231f027fd3cf48ecf81454f1cdb1d053c62a9c13cc9b488d53c56d1f00bdd0100000000fdffffff05e803000000000000160014a01f6b2a4bdaf3fb61f2a45e5eac92fcc58daee3881300000000000016001470fcde1ed0159ba5af97baec085ceb857098cedb0c49000000000000160014999a95482213a896c72a251b6cc9f3d137b0a458a86100000000000016001440c234c451fbd9ddf7824d6b8f0dc968a220946450c3000000000000160014ea76d391236726af7d7a9c10abe600129154eb5a024730440220782fb75f2398997ac77cd1b5c0d78f30a66b83df1d2d21c7a06cb03eb592d91702200540cf329c4b21e26aaba79a0c0ebdf465c4befb76a61e4eec924bc482cbf2930121024196fb7b766ac987a08b69a5e108feae8513b7e72bc9e47899e27b36100f2af4a58a1d00')
+        orig_rbf_txid = orig_rbf_tx.txid()
+        self.assertEqual('9e0c7d890053c47c7cd653be984bc4b9a5dab8acf9a6ae075a00113d3077ad74', orig_rbf_txid)
+        wallet.receive_tx_callback(orig_rbf_txid, orig_rbf_tx, TX_HEIGHT_UNCONFIRMED)
+
+        # bump tx
+        tx = wallet.bump_fee(
+            tx=tx_from_any(orig_rbf_tx.serialize()),
+            new_fee_rate=60,
+            strategies=[BumpFeeStrategy.DECREASE_PAYMENT],
+        )
+        tx.locktime = 1936095
+        tx.version = 2
+        if simulate_moving_txs:
+            partial_tx = tx.serialize_as_bytes().hex()
+            self.assertEqual("70736274ff0100af020000000146f231f027fd3cf48ecf81454f1cdb1d053c62a9c13cc9b488d53c56d1f00bdd0100000000fdffffff045d0500000000000016001470fcde1ed0159ba5af97baec085ceb857098cedb0c49000000000000160014999a95482213a896c72a251b6cc9f3d137b0a4587d5300000000000016001440c234c451fbd9ddf7824d6b8f0dc968a220946425b5000000000000160014ea76d391236726af7d7a9c10abe600129154eb5adf8a1d00000100fd7201020000000001022ea8f7940c2e4bca2f34f21ba15a5c8d5e3c93d9c6deb17983412feefa0f1f6d0100000000fdffffff9d4ba5ab41951d506a7fa8272ef999ce3df166fe28f6f885aa791f012a0924cf0000000000fdffffff027485010000000000160014f80e86af4246960a24cd21c275a8e8842973fbcaa0860100000000001600149c6b743752604b98d30f1a5d27a5d5ce8919f4400247304402203bf6dd875a775f356d4bb8c4e295a2cd506338c100767518f2b31fb85db71c1302204dc4ebca5584fc1cc08bd7f7171135d1b67ca6c8812c3723cd332eccaa7b848101210360bdbd16d9ef390fd3e804c421e6f30e6b065ac314f4d2b9a80d2f0682ad1431024730440220126b442d7988c5883ca17c2429f51ce770e3a57895524c8dfe07b539e483019e02200b50feed4f42f0035c9a9ddd044820607281e45e29e41a29233c2b8be6080bac01210245d47d08915816a5ecc934cff1b17e00071ca06172f51d632ba95392e8aad4fdd38a1d002206024196fb7b766ac987a08b69a5e108feae8513b7e72bc9e47899e27b36100f2af410ce2dd7cb0000008000000000000000000000220203ecb63cc22d200c96225671b88a51a71deb053c6445dbd4694f61166e3e5bd05910ce2dd7cb000000800100000000000000000000",
+                             partial_tx)
+            tx = tx_from_any(partial_tx)  # simulates moving partial txn between cosigners
+        self.assertFalse(tx.is_complete())
+
+        wallet.sign_transaction(tx, password=None)
+        self.assertTrue(tx.is_complete())
+        self.assertTrue(tx.is_segwit())
+        tx_copy = tx_from_any(tx.serialize())
+        self.assertEqual('0200000000010146f231f027fd3cf48ecf81454f1cdb1d053c62a9c13cc9b488d53c56d1f00bdd0100000000fdffffff045d0500000000000016001470fcde1ed0159ba5af97baec085ceb857098cedb0c49000000000000160014999a95482213a896c72a251b6cc9f3d137b0a4587d5300000000000016001440c234c451fbd9ddf7824d6b8f0dc968a220946425b5000000000000160014ea76d391236726af7d7a9c10abe600129154eb5a024730440220477ff315d3ac58de3bc1ec0b44b90a90da9bc09c440982fd9a1563eae98df0dc0220574033b0e306d388edcc77e4c2b39338fc8f182c747014aef3ce2c99cf9e5e960121024196fb7b766ac987a08b69a5e108feae8513b7e72bc9e47899e27b36100f2af4df8a1d00',
+                         str(tx_copy))
+        self.assertEqual('bc86f4f14fea5305b197c02ae7b0d6b04c5f49144d9ad37c9f64ec0ec6d34594', tx_copy.txid())
+        self.assertEqual('368e4c0429b38e66ac64ac9dbb66145c9f28dfaf2fad60f6424db32c379a12da', tx_copy.wtxid())
+
+        wallet.receive_tx_callback(tx.txid(), tx, TX_HEIGHT_UNCONFIRMED)
+        self.assertEqual((0, 18700, 0), wallet.get_balance())
 
     @mock.patch.object(wallet.Abstract_Wallet, 'save_db')
     def test_cpfp_p2pkh(self, mock_save_db):

electrum/transaction.py

@@ -892,11 +892,18 @@ class Transaction:
         return 4 * input_size + witness_size
 
     @classmethod
-    def estimated_output_size(cls, address):
+    def estimated_output_size_for_address(cls, address: str) -> int:
         """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
+        return cls.estimated_output_size_for_script(script)
+
+    @classmethod
+    def estimated_output_size_for_script(cls, script: str) -> int:
+        """Return an estimate of serialized output size in bytes."""
+        # 8 byte value + varint script len + script
+        script_len = len(script) // 2
+        var_int_len = len(var_int(script_len)) // 2
+        return 8 + var_int_len + script_len
 
     @classmethod
     def virtual_size_from_weight(cls, weight):

electrum/wallet.py

@@ -44,6 +44,7 @@ from typing import TYPE_CHECKING, List, Optional, Tuple, Union, NamedTuple, Sequ
 from abc import ABC, abstractmethod
 import itertools
 import threading
+import enum
 
 from aiorpcx import TaskGroup
 
@@ -97,6 +98,12 @@ TX_STATUS = [
 ]
 
 
+class BumpFeeStrategy(enum.Enum):
+    COINCHOOSER = enum.auto()
+    DECREASE_CHANGE = enum.auto()
+    DECREASE_PAYMENT = enum.auto()
+
+
 async def _append_utxos_to_inputs(*, inputs: List[PartialTxInput], network: 'Network',
                                   pubkey: str, txin_type: str, imax: int) -> None:
     if txin_type in ('p2pkh', 'p2wpkh', 'p2wpkh-p2sh'):
@@ -1439,6 +1446,7 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
             txid: str = None,
             new_fee_rate: Union[int, float, Decimal],
             coins: Sequence[PartialTxInput] = None,
+            strategies: Sequence[BumpFeeStrategy] = None,
     ) -> PartialTransaction:
         """Increase the miner fee of 'tx'.
         'new_fee_rate' is the target min rate in sat/vbyte
@@ -1465,29 +1473,42 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
         old_fee_rate = old_fee / old_tx_size  # sat/vbyte
         if new_fee_rate <= old_fee_rate:
             raise CannotBumpFee(_("The new fee rate needs to be higher than the old fee rate."))
-        try:
-            # method 1: keep all inputs, keep all not is_mine outputs,
-            #           allow adding new inputs
-            tx_new = self._bump_fee_through_coinchooser(
-                tx=tx,
-                txid=txid,
-                new_fee_rate=new_fee_rate,
-                coins=coins,
-            )
-            method_used = 1
-        except CannotBumpFee:
-            # method 2: keep all inputs, no new inputs are added,
-            #           allow decreasing and removing outputs (change is decreased first)
-            # This is less "safe" as it might end up decreasing e.g. a payment to a merchant;
-            # but e.g. if the user has sent "Max" previously, this is the only way to RBF.
-            tx_new = self._bump_fee_through_decreasing_outputs(
-                tx=tx, new_fee_rate=new_fee_rate)
-            method_used = 2
+
+        if not strategies:
+            strategies = [BumpFeeStrategy.COINCHOOSER, BumpFeeStrategy.DECREASE_CHANGE]
+        tx_new = None
+        exc = None
+        for strat in strategies:
+            try:
+                if strat == BumpFeeStrategy.COINCHOOSER:
+                    tx_new = self._bump_fee_through_coinchooser(
+                        tx=tx,
+                        txid=txid,
+                        new_fee_rate=new_fee_rate,
+                        coins=coins,
+                    )
+                elif strat == BumpFeeStrategy.DECREASE_CHANGE:
+                    tx_new = self._bump_fee_through_decreasing_change(
+                        tx=tx, new_fee_rate=new_fee_rate)
+                elif strat == BumpFeeStrategy.DECREASE_PAYMENT:
+                    tx_new = self._bump_fee_through_decreasing_payment(
+                        tx=tx, new_fee_rate=new_fee_rate)
+                else:
+                    raise NotImplementedError(f"unexpected strategy: {strat}")
+            except CannotBumpFee as e:
+                exc = e
+            else:
+                strat_used = strat
+                break
+        if tx_new is None:
+            assert exc
+            raise exc  # all strategies failed, re-raise last exception
+
         target_min_fee = new_fee_rate * tx_new.estimated_size()
         actual_fee = tx_new.get_fee()
         if actual_fee + 1 < target_min_fee:
             raise CannotBumpFee(
-                f"bump_fee fee target was not met (method: {method_used}). "
+                f"bump_fee fee target was not met (strategy: {strat_used}). "
                 f"got {actual_fee}, expected >={target_min_fee}. "
                 f"target rate was {new_fee_rate}")
         tx_new.locktime = get_locktime_for_new_transaction(self.network)
@@ -1503,6 +1524,12 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
             new_fee_rate: Union[int, Decimal],
             coins: Sequence[PartialTxInput] = None,
     ) -> PartialTransaction:
+        """Increase the miner fee of 'tx'.
+
+        - keeps all inputs
+        - keeps all not is_mine outputs,
+        - allows adding new inputs
+        """
         assert txid
         tx = copy.deepcopy(tx)
         tx.add_info_from_wallet(self)
@@ -1549,12 +1576,21 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
         except NotEnoughFunds as e:
             raise CannotBumpFee(e)
 
-    def _bump_fee_through_decreasing_outputs(
+    def _bump_fee_through_decreasing_change(
             self,
             *,
             tx: PartialTransaction,
             new_fee_rate: Union[int, Decimal],
     ) -> PartialTransaction:
+        """Increase the miner fee of 'tx'.
+
+        - keeps all inputs
+        - no new inputs are added
+        - allows decreasing and removing outputs (change is decreased first)
+        This is less "safe" than "coinchooser" method as it might end up decreasing
+        e.g. a payment to a merchant; but e.g. if the user has sent "Max" previously,
+        this is the only way to RBF.
+        """
         tx = copy.deepcopy(tx)
         tx.add_info_from_wallet(self)
         assert tx.get_fee() is not None
@@ -1594,6 +1630,67 @@ class Abstract_Wallet(AddressSynchronizer, ABC):
 
         return PartialTransaction.from_io(inputs, outputs)
 
+    def _bump_fee_through_decreasing_payment(
+            self,
+            *,
+            tx: PartialTransaction,
+            new_fee_rate: Union[int, Decimal],
+    ) -> PartialTransaction:
+        """Increase the miner fee of 'tx'.
+
+        - keeps all inputs
+        - no new inputs are added
+        - decreases payment outputs (not change!). Each non-ismine output is decreased
+          proportionally to their byte-size.
+        """
+        tx = copy.deepcopy(tx)
+        tx.add_info_from_wallet(self)
+        assert tx.get_fee() is not None
+        inputs = tx.inputs()
+        outputs = tx.outputs()
+
+        # select non-ismine outputs
+        s = [(idx, out) for (idx, out) in enumerate(outputs)
+             if not self.is_mine(out.address)]
+        # exempt 2fa fee output if present
+        x_fee = run_hook('get_tx_extra_fee', self, tx)
+        if x_fee:
+            x_fee_address, x_fee_amount = x_fee
+            s = [(idx, out) for (idx, out) in s if out.address != x_fee_address]
+        if not s:
+            raise CannotBumpFee("Cannot find payment output")
+
+        del_out_idxs = set()
+        tx_size = tx.estimated_size()
+        cur_fee = tx.get_fee()
+        # Main loop. Each iteration decreases value of all selected outputs.
+        # The number of iterations is bounded by len(s) as only the final iteration
+        # can *not remove* any output.
+        for __ in range(len(s) + 1):
+            target_fee = int(math.ceil(tx_size * new_fee_rate))
+            delta_total = target_fee - cur_fee
+            if delta_total <= 0:
+                break
+            out_size_total = sum(Transaction.estimated_output_size_for_script(out.scriptpubkey.hex())
+                                 for (idx, out) in s if idx not in del_out_idxs)
+            for idx, out in s:
+                out_size = Transaction.estimated_output_size_for_script(out.scriptpubkey.hex())
+                delta = int(math.ceil(delta_total * out_size / out_size_total))
+                if out.value - delta >= self.dust_threshold():
+                    new_output_value = out.value - delta
+                    assert isinstance(new_output_value, int)
+                    outputs[idx].value = new_output_value
+                    cur_fee += delta
+                else:  # remove output
+                    tx_size -= out_size
+                    cur_fee += out.value
+                    del_out_idxs.add(idx)
+        if delta_total > 0:
+            raise CannotBumpFee(_('Could not find suitable outputs'))
+
+        outputs = [out for (idx, out) in enumerate(outputs) if idx not in del_out_idxs]
+        return PartialTransaction.from_io(inputs, outputs)
+
     def cpfp(self, tx: Transaction, fee: int) -> Optional[PartialTransaction]:
         txid = tx.txid()
         for i, o in enumerate(tx.outputs()):