diff --git a/electrum/commands.py b/electrum/commands.py index 9b5647ed3..2ddd37f2b 100644 --- a/electrum/commands.py +++ b/electrum/commands.py @@ -52,7 +52,7 @@ from .transaction import (Transaction, multisig_script, TxOutput, PartialTransac from . import transaction from .invoices import PR_PAID, PR_UNPAID, PR_UNKNOWN, PR_EXPIRED from .synchronizer import Notifier -from .wallet import Abstract_Wallet, create_new_wallet, restore_wallet_from_text, Deterministic_Wallet +from .wallet import Abstract_Wallet, create_new_wallet, restore_wallet_from_text, Deterministic_Wallet, BumpFeeStrategy from .address_synchronizer import TX_HEIGHT_LOCAL from .mnemonic import Mnemonic from .lnutil import SENT, RECEIVED @@ -819,7 +819,7 @@ class Commands: tx=tx, txid=tx.txid(), coins=coins, - decrease_payment=decrease_payment, + strategy=BumpFeeStrategy.DECREASE_PAYMENT if decrease_payment else BumpFeeStrategy.PRESERVE_PAYMENT, new_fee_rate=new_fee_rate) if not unsigned: wallet.sign_transaction(new_tx, password) diff --git a/electrum/gui/qml/components/RbfBumpFeeDialog.qml b/electrum/gui/qml/components/RbfBumpFeeDialog.qml index c8ed4239b..28229f29a 100644 --- a/electrum/gui/qml/components/RbfBumpFeeDialog.qml +++ b/electrum/gui/qml/components/RbfBumpFeeDialog.qml @@ -62,15 +62,11 @@ ElDialog { ElComboBox { id: bumpMethodComboBox - enabled: rbffeebumper.canChangeBumpMethod textRole: 'text' valueRole: 'value' - model: [ - { text: qsTr('Preserve payment'), value: 'preserve_payment' }, - { text: qsTr('Decrease payment'), value: 'decrease_payment' } - ] + model: rbffeebumper.bumpMethodsAvailable onCurrentValueChanged: { if (activeFocus) rbffeebumper.bumpMethod = currentValue diff --git a/electrum/gui/qml/qetxfinalizer.py b/electrum/gui/qml/qetxfinalizer.py index 1c40fce93..7e4e6938a 100644 --- a/electrum/gui/qml/qetxfinalizer.py +++ b/electrum/gui/qml/qetxfinalizer.py @@ -8,7 +8,7 @@ from electrum.logging import get_logger from electrum.i18n import _ from electrum.transaction import PartialTxOutput, PartialTransaction, Transaction from electrum.util import NotEnoughFunds, profiler -from electrum.wallet import CannotBumpFee, CannotDoubleSpendTx, CannotCPFP +from electrum.wallet import CannotBumpFee, CannotDoubleSpendTx, CannotCPFP, BumpFeeStrategy from electrum.plugin import run_hook from .qewallet import QEWallet @@ -460,13 +460,8 @@ class QETxRbfFeeBumper(TxFeeSlider, TxMonMixin): self._oldfee_rate = 0 self._orig_tx = None self._rbf = True - self._bump_method = 'preserve_payment' - self._can_change_bump_method = True - - canChangeBumpMethodChanged = pyqtSignal() - @pyqtProperty(bool, notify=canChangeBumpMethodChanged) - def canChangeBumpMethod(self): - return self._can_change_bump_method + self._bump_method = BumpFeeStrategy.PRESERVE_PAYMENT.name + self._bump_methods_available = [] oldfeeChanged = pyqtSignal() @pyqtProperty(QEAmount, notify=oldfeeChanged) @@ -496,21 +491,26 @@ class QETxRbfFeeBumper(TxFeeSlider, TxMonMixin): return self._bump_method @bumpMethod.setter - def bumpMethod(self, bumpmethod): - assert self._can_change_bump_method + def bumpMethod(self, bumpmethod: str) -> None: if self._bump_method != bumpmethod: self._bump_method = bumpmethod self.bumpMethodChanged.emit() self.update() + bumpMethodsAvailableChanged = pyqtSignal() + @pyqtProperty('QVariantList', notify=bumpMethodsAvailableChanged) + def bumpMethodsAvailable(self): + return self._bump_methods_available + def get_tx(self): assert self._txid self._orig_tx = self._wallet.wallet.db.get_transaction(self._txid) assert self._orig_tx - if self._wallet.wallet.get_swap_by_funding_tx(self._orig_tx): - self._can_change_bump_method = False - self.canChangeBumpMethodChanged.emit() + strategies, def_strat_idx = self._wallet.wallet.get_bumpfee_strategies_for_tx(tx=self._orig_tx, txid=self._txid) + self._bump_methods_available = [{'value': strat.name, 'text': strat.text()} for strat in strategies] + self.bumpMethodsAvailableChanged.emit() + self.bumpMethod = strategies[def_strat_idx].name if not isinstance(self._orig_tx, PartialTransaction): self._orig_tx = PartialTransaction.from_tx(self._orig_tx) @@ -547,7 +547,7 @@ class QETxRbfFeeBumper(TxFeeSlider, TxMonMixin): tx=self._orig_tx, txid=self._txid, new_fee_rate=new_fee_rate, - decrease_payment=self._bump_method=='decrease_payment' + strategy=BumpFeeStrategy[self._bump_method], ) except CannotBumpFee as e: self._valid = False diff --git a/electrum/gui/qt/rbf_dialog.py b/electrum/gui/qt/rbf_dialog.py index 346e473b2..db04f99f4 100644 --- a/electrum/gui/qt/rbf_dialog.py +++ b/electrum/gui/qt/rbf_dialog.py @@ -15,7 +15,7 @@ from .util import (ColorScheme, WindowModalDialog, Buttons, from electrum.i18n import _ from electrum.transaction import PartialTransaction -from electrum.wallet import CannotRBFTx +from electrum.wallet import CannotRBFTx, BumpFeeStrategy if TYPE_CHECKING: from .main_window import ElectrumWindow @@ -54,19 +54,13 @@ class _BaseRBFDialog(TxEditor): self.feerate_e.setAmount(new_fee_rate) self.update() self.fee_slider.deactivate() - # are we paying max? - invoices = self.wallet.get_relevant_invoices_for_tx(txid) - if len(invoices) == 1 and len(invoices[0].outputs) == 1: - if invoices[0].outputs[0].value == '!': - self.set_decrease_payment() - # do not decrease payment if it is a swap - if self.wallet.get_swap_by_funding_tx(self.old_tx): - self.method_combo.setEnabled(False) def create_grid(self): self.method_label = QLabel(_('Method') + ':') self.method_combo = QComboBox() - self.method_combo.addItems([_('Preserve payment'), _('Decrease payment')]) + self._strategies, def_strat_idx = self.wallet.get_bumpfee_strategies_for_tx(tx=self.old_tx, txid=self.old_txid) + self.method_combo.addItems([strat.text() for strat in self._strategies]) + self.method_combo.setCurrentIndex(def_strat_idx) self.method_combo.currentIndexChanged.connect(self.trigger_update) self.method_combo.setFocusPolicy(Qt.NoFocus) old_size_label = TxSizeLabel() @@ -93,12 +87,6 @@ class _BaseRBFDialog(TxEditor): grid.addWidget(self.locktime_e, 5, 1, 1, 2) return grid - def is_decrease_payment(self): - return self.method_combo.currentIndex() == 1 - - def set_decrease_payment(self): - self.method_combo.setCurrentIndex(1) - def run(self) -> None: if not self.exec_(): return @@ -133,10 +121,12 @@ class _BaseRBFDialog(TxEditor): if not self.tx: return delta = self.tx.get_fee() - self.old_tx.get_fee() - if not self.is_decrease_payment(): + if self._strategies[self.method_combo.currentIndex()] == BumpFeeStrategy.PRESERVE_PAYMENT: msg = _("You will pay {} more.").format(self.main_window.format_amount_and_units(delta)) - else: + elif self._strategies[self.method_combo.currentIndex()] == BumpFeeStrategy.DECREASE_PAYMENT: msg = _("The recipient will receive {} less.").format(self.main_window.format_amount_and_units(delta)) + else: + raise Exception(f"unknown strategy: {self=}") messages.insert(0, msg) return messages @@ -164,7 +154,8 @@ class BumpFeeDialog(_BaseRBFDialog): txid=self.old_txid, new_fee_rate=fee_rate, coins=self.main_window.get_coins(nonlocal_only=True, confirmed_only=confirmed_only), - decrease_payment=self.is_decrease_payment()) + strategy=self._strategies[self.method_combo.currentIndex()], + ) class DSCancelDialog(_BaseRBFDialog): diff --git a/electrum/tests/test_wallet_vertical.py b/electrum/tests/test_wallet_vertical.py index bf5d77a03..841b07762 100644 --- a/electrum/tests/test_wallet_vertical.py +++ b/electrum/tests/test_wallet_vertical.py @@ -12,7 +12,7 @@ from electrum import SimpleConfig from electrum import util 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, CannotBumpFee) + restore_wallet_from_text, Abstract_Wallet, CannotBumpFee, BumpFeeStrategy) from electrum.util import ( bfh, NotEnoughFunds, UnrelatedTransactionException, UserFacingException) @@ -1229,7 +1229,7 @@ class TestWalletSending(ElectrumTestCase): tx = wallet.bump_fee( tx=tx_from_any(orig_rbf_tx.serialize()), new_fee_rate=60, - decrease_payment=True, + strategy=BumpFeeStrategy.DECREASE_PAYMENT, ) tx.locktime = 1936085 tx.version = 2 @@ -1271,7 +1271,7 @@ class TestWalletSending(ElectrumTestCase): tx = wallet.bump_fee( tx=tx_from_any(orig_rbf_tx.serialize()), new_fee_rate=60, - decrease_payment=True, + strategy=BumpFeeStrategy.DECREASE_PAYMENT, ) tx.locktime = 1936095 tx.version = 2 @@ -1313,19 +1313,19 @@ class TestWalletSending(ElectrumTestCase): tx = wallet.bump_fee( tx=tx_from_any(orig_rbf_tx.serialize()), new_fee_rate=99999, - decrease_payment=True, + strategy=BumpFeeStrategy.DECREASE_PAYMENT, ) with self.assertRaises(CannotBumpFee): tx = wallet.bump_fee( tx=tx_from_any(orig_rbf_tx.serialize()), new_fee_rate=99999, - decrease_payment=False, + strategy=BumpFeeStrategy.PRESERVE_PAYMENT, ) tx = wallet.bump_fee( tx=tx_from_any(orig_rbf_tx.serialize()), new_fee_rate=60, - decrease_payment=True, + strategy=BumpFeeStrategy.DECREASE_PAYMENT, ) tx.locktime = 1936085 tx.version = 2 @@ -1647,7 +1647,7 @@ class TestWalletSending(ElectrumTestCase): self.assertEqual((0, 0, 0), wallet.get_balance()) # bump tx - tx = wallet.bump_fee(tx=tx_from_any(tx.serialize()), new_fee_rate=70.0, decrease_payment=True) + tx = wallet.bump_fee(tx=tx_from_any(tx.serialize()), new_fee_rate=70.0, strategy=BumpFeeStrategy.DECREASE_PAYMENT) tx.locktime = 1325500 tx.version = 1 if simulate_moving_txs: diff --git a/electrum/wallet.py b/electrum/wallet.py index 570e395de..76ac6a55b 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -247,6 +247,23 @@ class InternalAddressCorruption(Exception): "Please restore your wallet from seed, and compare the addresses in both files") +class BumpFeeStrategy(enum.Enum): + PRESERVE_PAYMENT = enum.auto() + DECREASE_PAYMENT = enum.auto() + + @classmethod + def all(cls) -> Sequence['BumpFeeStrategy']: + return list(BumpFeeStrategy.__members__.values()) + + def text(self) -> str: + if self == self.PRESERVE_PAYMENT: + return _('Preserve payment') + elif self == self.DECREASE_PAYMENT: + return _('Decrease payment') + else: + raise Exception(f"unknown strategy: {self=}") + + class ReceiveRequestHelp(NamedTuple): # help texts (warnings/errors): address_help: str @@ -1955,6 +1972,28 @@ class Abstract_Wallet(ABC, Logger, EventListener): def can_export(self): return not self.is_watching_only() and hasattr(self.keystore, 'get_private_key') + def get_bumpfee_strategies_for_tx( + self, + *, + tx: Transaction, + txid: str = None, + ) -> Tuple[Sequence[BumpFeeStrategy], int]: + """Returns tuple(list of available strategies, idx of recommended option among those).""" + txid = txid or tx.txid() + assert txid + assert tx.txid() in (None, txid) + all_strats = BumpFeeStrategy.all() + # are we paying max? + invoices = self.get_relevant_invoices_for_tx(txid) + if len(invoices) == 1 and len(invoices[0].outputs) == 1: + if invoices[0].outputs[0].value == '!': + return all_strats, all_strats.index(BumpFeeStrategy.DECREASE_PAYMENT) + # do not decrease payment if it is a swap + if self.get_swap_by_funding_tx(tx): + return [BumpFeeStrategy.PRESERVE_PAYMENT], 0 + # default + return all_strats, all_strats.index(BumpFeeStrategy.PRESERVE_PAYMENT) + def bump_fee( self, *, @@ -1962,7 +2001,7 @@ class Abstract_Wallet(ABC, Logger, EventListener): txid: str = None, new_fee_rate: Union[int, float, Decimal], coins: Sequence[PartialTxInput] = None, - decrease_payment=False, + strategy: BumpFeeStrategy = BumpFeeStrategy.PRESERVE_PAYMENT, ) -> PartialTransaction: """Increase the miner fee of 'tx'. 'new_fee_rate' is the target min rate in sat/vbyte @@ -1991,7 +2030,7 @@ class Abstract_Wallet(ABC, Logger, EventListener): if new_fee_rate <= old_fee_rate: raise CannotBumpFee(_("The new fee rate needs to be higher than the old fee rate.")) - if not decrease_payment: + if strategy == BumpFeeStrategy.PRESERVE_PAYMENT: # FIXME: we should try decreasing change first, # but it requires updating a bunch of unit tests try: @@ -2004,9 +2043,11 @@ class Abstract_Wallet(ABC, Logger, EventListener): except CannotBumpFee as e: tx_new = self._bump_fee_through_decreasing_change( tx=tx, new_fee_rate=new_fee_rate) - else: + elif strategy == BumpFeeStrategy.DECREASE_PAYMENT: tx_new = self._bump_fee_through_decreasing_payment( tx=tx, new_fee_rate=new_fee_rate) + else: + raise Exception(f"unknown strategy: {strategy=}") target_min_fee = new_fee_rate * tx_new.estimated_size() actual_fee = tx_new.get_fee()