davide/purple-electrumwallet
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

wallet: make_unsigned_tx: fix base_tx for GUI simple-send batching

Browse Source 
fixes regression from https://github.com/spesmilo/electrum/commit/a9f20e4d3de9848247a9079b5b164d8006cf80db :
For the GUI / manual new tx constructions usecase, the flow relies on `base_tx.add_info_from_wallet(self)` being called before `base_tx_fee = base_tx.get_fee()`.

fixes https://github.com/spesmilo/electrum/issues/10587
maybe fixes https://github.com/spesmilo/electrum/issues/8876
    probably not a full fix: base_tx could have inputs for which add_info_from_wallet is not sufficient
This commit is contained in:
SomberNight
2026-04-19 22:19:00 +00:00
parent 16c8cb50e3
commit febe95e642
4 changed files with 74 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
electrum/gui/qt/confirm_tx_dialog.py
+5 -5
View File
@@ -26,7 +26,7 @@
import asyncio
from decimal import Decimal
from functools import partial
from typing import TYPE_CHECKING, Optional, Union
from typing import TYPE_CHECKING, Optional, Union, Sequence
from concurrent.futures import Future
from enum import Enum, auto
@@ -39,7 +39,7 @@ from electrum.i18n import _
from electrum.util import (UserCancelled, quantize_feerate, profiler, NotEnoughFunds, NoDynamicFeeEstimates,
get_asyncio_loop, wait_for2, UserFacingException)
from electrum.plugin import run_hook
from electrum.transaction import PartialTransaction, PartialTxOutput
from electrum.transaction import PartialTransaction, PartialTxOutput, Transaction
from electrum.wallet import InternalAddressCorruption
from electrum.bitcoin import DummyAddress
from electrum.fee_policy import FeePolicy, FixedFeePolicy, FeeMethod
@@ -82,7 +82,7 @@ class TxEditor(WindowModalDialog, QtEventListener, Logger):
output_value: Union[int, str],
payee_outputs: Optional[list[PartialTxOutput]] = None,
context: TxEditorContext = TxEditorContext.PAYMENT,
batching_candidates=None,
batching_candidates: Sequence[Transaction] = None,
):
WindowModalDialog.__init__(self, window, title=title)
@@ -106,7 +106,7 @@ class TxEditor(WindowModalDialog, QtEventListener, Logger):
self.needs_update = False
self.context = context
self.is_preview = False
self._base_tx = None # for batching
self._base_tx = None # type: Optional[Transaction] # for batching
self.batching_candidates = batching_candidates
self.swap_manager = self.wallet.lnworker.swap_manager if self.wallet.has_lightning() else None
@@ -1123,7 +1123,7 @@ class ConfirmTxDialog(TxEditor):
output_value: Union[int, str],
payee_outputs: Optional[list[PartialTxOutput]] = None,
context: TxEditorContext = TxEditorContext.PAYMENT,
batching_candidates=None,
batching_candidates: Sequence[Transaction] = None,
):
TxEditor.__init__(
electrum/gui/qt/main_window.py
+1 -1
View File
@@ -1507,7 +1507,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener):
output_value, *,
payee_outputs: Optional[list[TxOutput]] = None,
context: TxEditorContext = TxEditorContext.PAYMENT,
batching_candidates=None,
batching_candidates: Sequence[Transaction] = None,
) -> tuple[Optional[PartialTransaction], bool, bool]:
d = ConfirmTxDialog(
window=self,
electrum/wallet.py
+7 -5
View File
@@ -1811,6 +1811,7 @@ class Abstract_Wallet(ABC, Logger, EventListener):
domain = self.get_addresses()
for hist_item in self.adb.get_history(domain):
# tx should not be mined yet
if hist_item.tx_mined_status.conf is None: continue
if hist_item.tx_mined_status.conf > 0: continue
# conservative future proofing of code: only allow known unconfirmed types
if hist_item.tx_mined_status.height() not in (
@@ -2016,20 +2017,21 @@ class Abstract_Wallet(ABC, Logger, EventListener):
coin_chooser = coinchooser.get_coin_chooser(self.config)
# If there is an unconfirmed RBF tx, merge with it
if base_tx:
assert base_tx.txid() is not None # pre-segwit and incomplete?
# make sure we don't try to spend change from the tx-to-be-replaced:
coins = [c for c in coins if c.prevout.txid.hex() != base_tx.txid()]
is_local = self.adb.get_tx_height(base_tx.txid()).height() == TX_HEIGHT_LOCAL
# estimate base tx fee before stripping tx for more accurate estimate
base_tx_fee = base_tx.get_fee()
base_feerate = Decimal(base_tx_fee)/base_tx.estimated_size()
relayfeerate = Decimal(self.relayfee()) / 1000
original_fee_estimator = fee_estimator
base_tx_size = base_tx.estimated_size() # estimate before stripping tx for more accurate estimate
if not isinstance(base_tx, PartialTransaction):
base_tx = PartialTransaction.from_tx(base_tx)
base_tx.add_info_from_wallet(self)
else:
# don't cast PartialTransaction, because it removes make_witness
base_tx.remove_signatures()
base_tx_fee = base_tx.get_fee() # FIXME could be None if some inputs are non-ismine
base_feerate = Decimal(base_tx_fee) / base_tx_size
relayfeerate = Decimal(self.relayfee()) / 1000
original_fee_estimator = fee_estimator
def fee_estimator(size: Union[int, float, Decimal]) -> int:
size = Decimal(size)
lower_bound_relayfee = int(base_tx_fee + round(size * relayfeerate)) if not is_local else 0
tests/test_wallet_vertical.py
+61 -1
View File
@@ -2159,7 +2159,7 @@ class TestWalletSending(ElectrumTestCase):
tx2_feerate_sat_vb = tx2.get_fee() / tx2.estimated_size()
self.assertGreater(tx2_feerate_sat_vb, tx1_feerate_sat_vb, msg="tx2 feerate lower than tx1")
def _create_cause_carbon_wallet(self):
def _create_cause_carbon_wallet(self) -> tuple[Standard_Wallet, str, str]:
data = read_test_vector('cause_carbon_wallet.json')
ks = keystore.from_seed(data['seed'], passphrase='', for_multisig=False)
wallet = WalletIntegrityHelper.create_standard_wallet(ks, gap_limit=2, gap_limit_for_change=2, config=self.config)
@@ -2356,6 +2356,66 @@ class TestWalletSending(ElectrumTestCase):
wallet.adb.receive_tx_callback(tx3, tx_height=TX_HEIGHT_UNCONFIRMED)
self.assertEqual((0, 197_000, 0), wallet.get_balance())
async def test_rbf_batching__happypath_using_get_candidates_for_batching(self):
"""Test GUI simple-send RBF 'batch with' functionality:
- while there is already an unconfirmed outgoing tx1 in the wallet history,
- try making another payment, and batch it with tx1.
"""
wallet_faucet = self._create_cause_carbon_wallet()[0]
mywallet = self.create_standard_wallet_from_seed(
'response era cable net spike again observe dumb wage wonder sail tortoise',
config=self.config)
# fund mywallet with 2 UTXOs
def fund_mywallet_from_faucet():
addr = mywallet.get_receiving_address()
assert not mywallet.adb.is_used(addr)
funding_tx1 = wallet_faucet.make_unsigned_transaction(
outputs=[PartialTxOutput.from_address_and_value(addr, 40_000)],
fee_policy=FixedFeePolicy(200), locktime=4_000_000, rbf=True,
)
wallet_faucet.sign_transaction(funding_tx1, password=None)
wallet_faucet.adb.receive_tx_callback(funding_tx1, tx_height=TX_HEIGHT_UNCONFIRMED)
mywallet.adb.receive_tx_callback(funding_tx1, tx_height=TX_HEIGHT_UNCONFIRMED)
assert mywallet.adb.is_used(addr)
fund_mywallet_from_faucet()
fund_mywallet_from_faucet()
external_addr1 = wallet_faucet.get_receiving_addresses()[0]
external_addr2 = wallet_faucet.get_receiving_addresses()[1]
assert external_addr1 and external_addr2 and (external_addr1 != external_addr2)
# create outgoing tx1
output1 = PartialTxOutput.from_address_and_value(external_addr1, 30_000)
candidates1 = mywallet.get_candidates_for_batching([output1], coins=mywallet.get_spendable_coins(domain=None))
self.assertEqual(candidates1, [])
outgoing_tx1 = mywallet.make_unsigned_transaction(
outputs=[output1],
fee_policy=FixedFeePolicy(200), locktime=4_000_001, rbf=True,
)
mywallet.sign_transaction(outgoing_tx1, password=None)
mywallet.adb.receive_tx_callback(outgoing_tx1, tx_height=TX_HEIGHT_UNCONFIRMED)
# create outgoing tx2, and try to batch it with tx1
output2 = PartialTxOutput.from_address_and_value(external_addr2, 30_000)
candidates2 = mywallet.get_candidates_for_batching([output2], coins=mywallet.get_spendable_coins(domain=None))
self.assertEqual(1, len(candidates2))
base_tx = candidates2[0]
self.assertEqual(outgoing_tx1.txid(), base_tx.txid())
outgoing_tx2 = mywallet.make_unsigned_transaction(
outputs=[output2],
fee_policy=FixedFeePolicy(200), locktime=4_000_001, rbf=True,
base_tx=base_tx,
)
mywallet.sign_transaction(outgoing_tx2, password=None)
mywallet.adb.receive_tx_callback(outgoing_tx2, tx_height=TX_HEIGHT_UNCONFIRMED)
self.assertEqual(3, len(outgoing_tx2.outputs()))
self.assertIn(output1, outgoing_tx2.outputs())
self.assertIn(output2, outgoing_tx2.outputs())
self.assertEqual('02000000000102ab99cf51f3e219735c49491a9ecf354517a215a2b462227df7e7624188a7ff830000000000fdffffff417bbd802768510a0c641fc79af6e691a01fd10f37b84162f253b594e5cb87c50100000000fdffffff032c4c0000000000001600144e1b662f616fe134430054e29295ea6e5c18f17330750000000000001600142266c890fad71396f106319368107d5b2a1146fe307500000000000016001476efaaa243327bf3a2c0f5380cb3914099448cec02473044022033f8ddcb07ad7e06cef417208a5244507157cccdd6817029e968ec60a2395add02200720eb6a7c8cea86b470398ce75d69abcc66624a2253b18ea81cd1566eb5132c0121029b1a61d66896486ab893741b38dbafb9673b91a82237d6e4ca0da3cda7cbeb7c0247304402205e00156d74bcd85ed26ee9d0bdbd72890656881e25c04b2ac94f1c6b91f1176f02205f94d055e6385b48fbd3ac1a2dc2f57a640f4b33740cd9fb9e0fc20eb0cf5dcb012102c1ed648e71f15643950b444b864ab784b9d0e31e6ca6ec7d849d3dda4d98da0501093d00',
str(outgoing_tx2))
async def test_join_psbts__merge_duplicate_outputs(self):
"""txos paying to the same address might be merged into a single output with a larger value"""
rawtx1 = "70736274ff01007102000000019264597cffcce8f0c17b16a02adca7a95ae90f2ea51bd4b4df60c76dfe86686e0000000000fdffffff02400d03000000000016001458aee0f1201d1ae12dbd241209bbe92ed45e39b6108c0400000000001600144e1b662f616fe134430054e29295ea6e5c18f173c2ad26000001011f20a1070000000000160014542266519a44eb9b903761d40c6fe1055d33fa050100de02000000000101013548c9019890e27ce9e58766de05f18ea40ede70751fb6cd7a3a1715ece0a30100000000fdffffff0220a1070000000000160014542266519a44eb9b903761d40c6fe1055d33fa05485a080000000000160014bc69f7d82c403a9f35dfb6d1a4531d6b19cab0e3024730440220346b200f21c3024e1d51fb4ecddbdbd68bd24ae7b9dfd501519f6dcbeb7c052402200617e3ce7b0eb308e30caf23894fb0388b68fb1c15dd0681dd13ae5e735f148101210360d0c9ef15b8b6a16912d341ad218a4e4e4e07e9347f4a2dbc7ca8d974f8bc9ec1ad26002206029b1a61d66896486ab893741b38dbafb9673b91a82237d6e4ca0da3cda7cbeb7c101f1b48320000008000000000000000000000220203db4846ec1841f48484590e67fcd7d1039f124a04410c5794f38ec8625329ea23101f1b483200000080010000000000000000"