tMerge pull request #6867 from SomberNight/202012_wallet_rbf_fixes - electrum - Electrum Bitcoin wallet
HTML git clone https://git.parazyd.org/electrum
DIR Log
DIR Files
DIR Refs
DIR Submodules
---
DIR commit 7ab1a4552b579cd7b9a2197be12da4f02f396629
DIR parent b28b3994c7c879d43325faa8db8f4691be1f920f
HTML Author: ghost43 <somber.night@protonmail.com>
Date: Sun, 20 Dec 2020 14:31:47 +0000
Merge pull request #6867 from SomberNight/202012_wallet_rbf_fixes
wallet: fix bump_fee and dscancel for "not all inputs ismine" case
Diffstat:
M electrum/gui/kivy/uix/dialogs/tx_d… | 77 ++++++++++++++++++++++++-------
M electrum/gui/qt/history_list.py | 5 ++---
M electrum/gui/qt/main_window.py | 44 ++++++++++++++++++++++++-------
M electrum/tests/test_wallet_vertica… | 202 ++++++++++++++++++++++++++++++-
M electrum/transaction.py | 22 +++++++++++++++++-----
M electrum/wallet.py | 110 ++++++++++++++++++++++---------
6 files changed, 394 insertions(+), 66 deletions(-)
---
DIR diff --git a/electrum/gui/kivy/uix/dialogs/tx_dialog.py b/electrum/gui/kivy/uix/dialogs/tx_dialog.py
t@@ -1,6 +1,7 @@
import copy
from datetime import datetime
from typing import NamedTuple, Callable, TYPE_CHECKING
+from functools import partial
from kivy.app import App
from kivy.factory import Factory
t@@ -18,6 +19,7 @@ from electrum.util import InvalidPassword
from electrum.address_synchronizer import TX_HEIGHT_LOCAL
from electrum.wallet import CannotBumpFee, CannotDoubleSpendTx
from electrum.transaction import Transaction, PartialTransaction
+from electrum.network import NetworkException
from ...util import address_colors
if TYPE_CHECKING:
t@@ -137,6 +139,7 @@ class TxDialog(Factory.Popup):
# As a result, e.g. we might learn an imported address tx is segwit,
# or that a beyond-gap-limit address is is_mine.
# note: this might fetch prev txs over the network.
+ # note: this is a no-op for complete txs
tx.add_info_from_wallet(self.wallet)
def on_open(self):
t@@ -231,22 +234,51 @@ class TxDialog(Factory.Popup):
action_button = self.ids.action_button
self._action_button_fn(action_button)
+ def _add_info_to_tx_from_wallet_and_network(self, tx: PartialTransaction) -> bool:
+ """Returns whether successful."""
+ # note side-effect: tx is being mutated
+ assert isinstance(tx, PartialTransaction)
+ try:
+ # note: this might download input utxos over network
+ # FIXME network code in gui thread...
+ tx.add_info_from_wallet(self.wallet, ignore_network_issues=False)
+ except NetworkException as e:
+ self.app.show_error(repr(e))
+ return False
+ return True
+
def do_rbf(self):
from .bump_fee_dialog import BumpFeeDialog
- fee = self.wallet.get_wallet_delta(self.tx).fee
- if fee is None:
- self.app.show_error(_("Can't bump fee: unknown fee for original transaction."))
+ tx = self.tx
+ 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
- size = self.tx.estimated_size()
- d = BumpFeeDialog(self.app, fee, size, self._do_rbf)
+ fee = tx.get_fee()
+ assert fee is not None
+ size = tx.estimated_size()
+ cb = partial(self._do_rbf, tx=tx, txid=txid)
+ d = BumpFeeDialog(self.app, fee, size, cb)
d.open()
- def _do_rbf(self, new_fee_rate, is_final):
+ def _do_rbf(
+ self,
+ new_fee_rate,
+ is_final,
+ *,
+ tx: PartialTransaction,
+ txid: str,
+ ):
if new_fee_rate is None:
return
try:
- new_tx = self.wallet.bump_fee(tx=self.tx,
- new_fee_rate=new_fee_rate)
+ new_tx = self.wallet.bump_fee(
+ tx=tx,
+ txid=txid,
+ new_fee_rate=new_fee_rate,
+ )
except CannotBumpFee as e:
self.app.show_error(str(e))
return
t@@ -258,20 +290,33 @@ class TxDialog(Factory.Popup):
def do_dscancel(self):
from .dscancel_dialog import DSCancelDialog
- fee = self.wallet.get_wallet_delta(self.tx).fee
- if fee is None:
- self.app.show_error(_('Cannot cancel transaction') + ': ' + _('unknown fee for original transaction'))
+ tx = self.tx
+ 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
- size = self.tx.estimated_size()
- d = DSCancelDialog(self.app, fee, size, self._do_dscancel)
+ fee = tx.get_fee()
+ assert fee is not None
+ size = tx.estimated_size()
+ cb = partial(self._do_dscancel, tx=tx)
+ d = DSCancelDialog(self.app, fee, size, cb)
d.open()
- def _do_dscancel(self, new_fee_rate):
+ def _do_dscancel(
+ self,
+ new_fee_rate,
+ *,
+ tx: PartialTransaction,
+ ):
if new_fee_rate is None:
return
try:
- new_tx = self.wallet.dscancel(tx=self.tx,
- new_fee_rate=new_fee_rate)
+ new_tx = self.wallet.dscancel(
+ tx=tx,
+ new_fee_rate=new_fee_rate,
+ )
except CannotDoubleSpendTx as e:
self.app.show_error(str(e))
return
DIR diff --git a/electrum/gui/qt/history_list.py b/electrum/gui/qt/history_list.py
t@@ -691,14 +691,13 @@ class HistoryList(MyTreeView, AcceptFileDragDrop):
if channel_id:
menu.addAction(_("View Channel"), lambda: self.parent.show_channel(bytes.fromhex(channel_id)))
if is_unconfirmed and tx:
- # note: the current implementation of RBF *needs* the old tx fee
- if tx_details.can_bump and tx_details.fee is not None:
+ if tx_details.can_bump:
menu.addAction(_("Increase fee"), lambda: self.parent.bump_fee_dialog(tx))
else:
child_tx = self.wallet.cpfp(tx, 0)
if child_tx:
menu.addAction(_("Child pays for parent"), lambda: self.parent.cpfp(tx, child_tx))
- if tx_details.can_dscancel and tx_details.fee is not None:
+ if tx_details.can_dscancel:
menu.addAction(_("Cancel (double-spend)"), lambda: self.parent.dscancel_dialog(tx))
invoices = self.wallet.get_relevant_invoices_for_tx(tx)
if len(invoices) == 1:
DIR diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py
t@@ -70,7 +70,8 @@ from electrum.wallet import (Multisig_Wallet, CannotBumpFee, Abstract_Wallet,
sweep_preparations, InternalAddressCorruption,
CannotDoubleSpendTx)
from electrum.version import ELECTRUM_VERSION
-from electrum.network import Network, TxBroadcastError, BestEffortRequestFailed, UntrustedServerReturnedError
+from electrum.network import (Network, TxBroadcastError, BestEffortRequestFailed,
+ UntrustedServerReturnedError, NetworkException)
from electrum.exchange_rate import FxThread
from electrum.simple_config import SimpleConfig
from electrum.logging import Logger
t@@ -90,7 +91,7 @@ from .util import (read_QIcon, ColorScheme, text_dialog, icon_path, WaitingDialo
import_meta_gui, export_meta_gui,
filename_field, address_field, char_width_in_lineedit, webopen,
TRANSACTION_FILE_EXTENSION_FILTER_ANY, MONOSPACE_FONT,
- getOpenFileName, getSaveFileName)
+ getOpenFileName, getSaveFileName, BlockingWaitingDialog)
from .util import ButtonsTextEdit, ButtonsLineEdit
from .installwizard import WIF_HELP_TEXT
from .history_list import HistoryList, HistoryModel
t@@ -3189,13 +3190,31 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger):
new_tx.set_rbf(True)
self.show_transaction(new_tx)
+ def _add_info_to_tx_from_wallet_and_network(self, tx: PartialTransaction) -> bool:
+ """Returns whether successful."""
+ # note side-effect: tx is being mutated
+ assert isinstance(tx, PartialTransaction)
+ try:
+ # note: this might download input utxos over network
+ BlockingWaitingDialog(
+ self,
+ _("Adding info to tx, from wallet and network..."),
+ lambda: tx.add_info_from_wallet(self.wallet, ignore_network_issues=False),
+ )
+ except NetworkException as e:
+ self.show_error(repr(e))
+ return False
+ return True
+
def bump_fee_dialog(self, tx: Transaction):
txid = tx.txid()
assert txid
- fee = self.wallet.get_tx_fee(txid)
- if fee is None:
- self.show_error(_("Can't bump fee: unknown fee for original transaction."))
+ 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
t@@ -3236,7 +3255,12 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger):
is_final = cb.isChecked()
new_fee_rate = feerate_e.get_amount()
try:
- new_tx = self.wallet.bump_fee(tx=tx, new_fee_rate=new_fee_rate, coins=self.get_coins())
+ new_tx = self.wallet.bump_fee(
+ tx=tx,
+ txid=txid,
+ new_fee_rate=new_fee_rate,
+ coins=self.get_coins(),
+ )
except CannotBumpFee as e:
self.show_error(str(e))
return
t@@ -3247,10 +3271,12 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger):
def dscancel_dialog(self, tx: Transaction):
txid = tx.txid()
assert txid
- fee = self.wallet.get_tx_fee(txid)
- if fee is None:
- self.show_error(_('Cannot cancel transaction') + ': ' + _('unknown fee for original transaction'))
+ 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_size = tx.estimated_size()
old_fee_rate = fee / tx_size # sat/vbyte
d = WindowModalDialog(self, _('Cancel transaction'))
DIR diff --git a/electrum/tests/test_wallet_vertical.py b/electrum/tests/test_wallet_vertical.py
t@@ -11,7 +11,7 @@ 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.util import bfh, bh2u
+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
t@@ -575,11 +575,11 @@ class TestWalletSending(TestCaseForTestnet):
super().setUp()
self.config = SimpleConfig({'electrum_path': self.electrum_path})
- def create_standard_wallet_from_seed(self, seed_words, *, config=None):
+ def create_standard_wallet_from_seed(self, seed_words, *, config=None, gap_limit=2):
if config is None:
config = self.config
ks = keystore.from_seed(seed_words, '', False)
- return WalletIntegrityHelper.create_standard_wallet(ks, gap_limit=2, config=config)
+ return WalletIntegrityHelper.create_standard_wallet(ks, gap_limit=gap_limit, config=config)
@mock.patch.object(wallet.Abstract_Wallet, 'save_db')
def test_sending_between_p2wpkh_and_compressed_p2pkh(self, mock_save_db):
t@@ -929,6 +929,14 @@ class TestWalletSending(TestCaseForTestnet):
self._rbf_batching(
simulate_moving_txs=simulate_moving_txs,
config=config)
+ with self.subTest(msg="_bump_fee_when_not_all_inputs_are_ismine_subcase_some_outputs_are_ismine_but_not_all", simulate_moving_txs=simulate_moving_txs):
+ self._bump_fee_when_not_all_inputs_are_ismine_subcase_some_outputs_are_ismine_but_not_all(
+ simulate_moving_txs=simulate_moving_txs,
+ config=config)
+ with self.subTest(msg="_bump_fee_when_not_all_inputs_are_ismine_subcase_all_outputs_are_ismine", simulate_moving_txs=simulate_moving_txs):
+ self._bump_fee_when_not_all_inputs_are_ismine_subcase_all_outputs_are_ismine(
+ 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',
t@@ -1134,6 +1142,130 @@ class TestWalletSending(TestCaseForTestnet):
wallet.receive_tx_callback(tx.txid(), tx, TX_HEIGHT_UNCONFIRMED)
self.assertEqual((0, 7490060, 0), wallet.get_balance())
+ def _bump_fee_when_not_all_inputs_are_ismine_subcase_some_outputs_are_ismine_but_not_all(self, *, simulate_moving_txs, config):
+ class NetworkMock:
+ relay_fee = 1000
+ async def get_transaction(self, txid, timeout=None):
+ if txid == "597098f9077cd2a7bf5bb2a03c9ae5fcd9d1f07c0891cb42cbb129cf9eaf57fd":
+ return "02000000000102a5883f3de780d260e6f26cf85144403c7744a65a44cd38f9ff45aecadf010c540000000000fdffffffbdeb0175b1c51c96843d1952f7e1c49c1703717d7d020048d4de0a8eed94dad50000000000fdffffff03b2a00700000000001600140cd6c9f8ce0aa73d77fcf7f156c74f5cbec6906bb2a00700000000001600146435504ddc95e6019a90bb7dfc7ca81a88a8633106d790000000000016001444bd3017ee214370abf683abaa7f6204c9f40210024730440220652a04a2a301d9a031a034f3ae48174e204e17acf7bfc27f0dcab14243f73e2202207b29e964c434dfb2c515232d36566a40dccd4dd93ccb7fd15260ecbda10f0d9801210231994e564a0530068d17a9b0f85bec58d1352517a2861ea99e5b3070d2c5dbda02473044022072186473874919019da0e3d92b6e0aa4f88cba448ed5434615e5a3c8e2b7c42a02203ec05cef66960d5bc45d0f3d25675190cf8035b11a05ed4b719fd9c3a894899b012102f5fdca8c4e30ba0a1babf9cf9ebe62519b08aead351c349ed1ffc8316c24f542d7f61c00"
+ else:
+ raise Exception("unexpected txid")
+ def has_internet_connection(self):
+ return True
+ def run_from_another_thread(self, coro, *, timeout=None):
+ loop, stop_loop, loop_thread = create_and_start_event_loop()
+ fut = asyncio.run_coroutine_threadsafe(coro, loop)
+ try:
+ return fut.result(timeout)
+ finally:
+ loop.call_soon_threadsafe(stop_loop.set_result, 1)
+ loop_thread.join(timeout=1)
+ def get_local_height(self):
+ return 0
+ def blockchain(self):
+ class BlockchainMock:
+ def is_tip_stale(self):
+ return True
+ return BlockchainMock()
+
+ wallet = self.create_standard_wallet_from_seed('mix total present junior leader live state athlete mistake crack wall valve',
+ config=config)
+ wallet.network = NetworkMock()
+
+ # bootstrap wallet
+ funding_tx = Transaction('02000000000101a5883f3de780d260e6f26cf85144403c7744a65a44cd38f9ff45aecadf010c540100000000fdffffff0220a1070000000000160014db44724ac632ae47ee5765954d64796dd5fec72708de3c000000000016001424b32aadb42a89016c4de8f11741c3b29b15f21c02473044022045cc6c1cc875cbb0c0d8fe323dc1de9716e49ed5659741b0fb3dd9a196894066022077c242640071d12ec5763c5870f482a4823d8713e4bd14353dd621ed29a7f96d012102aea8d439a0f79d8b58e8d7bda83009f587e1f3da350adaa484329bf47cd03465fef61c00')
+ funding_txid = funding_tx.txid()
+ self.assertEqual('08557327673db61cc921e1a30826608599b86457836be3021105c13940d9a9a3', funding_txid)
+ wallet.receive_tx_callback(funding_txid, funding_tx, TX_HEIGHT_UNCONFIRMED)
+
+ orig_rbf_tx = Transaction('02000000000102a3a9d94039c1051102e36b835764b89985602608a3e121c91cb63d67277355080000000000fdfffffffd57af9ecf29b1cb42cb91087cf0d1d9fce59a3ca0b25bbfa7d27c07f99870590200000000fdffffff03b2a00700000000001600145dc80fd43eb70fd21a6c4446e3ce043df94f100cb2a00700000000001600147db4ab480b7d2218fba561ff304178f4afcbc972be358900000000001600149d91f0053172fab394d277ae27e9fa5c5a49210902473044022003999f03be8b9e299b2cd3bc7bce05e273d5d9ce24fc47af8754f26a7a13e13f022004e668499a67061789f6ebd2932c969ece74417ae3f2307bf696428bbed4fe36012102a1c9b25b37aa31ccbb2d72caaffce81ec8253020a74017d92bbfc14a832fc9cb0247304402207121358a66c0e716e2ba2be928076736261c691b4fbf89ea8d255449a4f5837b022042cadf9fe1b4f3c03ede3cef6783b42f0ba319f2e0273b624009cd023488c4c1012103a5ba95fb1e0043428ed70680fc17db254b3f701dfccf91e48090aa17c1b7ea40fef61c00')
+ orig_rbf_txid = orig_rbf_tx.txid()
+ self.assertEqual('6057690010ddac93a371629e1f41866400623e13a9cd336d280fc3239086a983', 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=70)
+ tx.locktime = 1898268
+ tx.version = 2
+ if simulate_moving_txs:
+ partial_tx = tx.serialize_as_bytes().hex()
parazyd.org:70 /git/electrum/commit/7ab1a4552b579cd7b9a2197be12da4f02f396629.gph:346: line too long