From d01e6b81018df669da091a8f0da365274a0740fd Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 5 Sep 2025 19:17:23 +0000 Subject: [PATCH] qt confirm_tx_dialog: fix wallet.get_candidates_for_batching I don't understand what the "coins not used" comment meant here. It was added in the change away from the old config.WALLET_BATCH_RBF option (https://github.com/spesmilo/electrum/commit/ab14c3e1382c1af48baff73b790aecfbd069eb8a). The `coins` param *is used* in wallet.get_candidates_for_batching. Without setting that, the returned set of candidates was restricted to only base txs that had a large enough change output to cover *all* the newly added outputs. Instead, it is desirable to allow adding new inputs. --- electrum/gui/qt/send_tab.py | 9 ++++++++- electrum/wallet.py | 11 ++++++++++- tests/test_wallet_vertical.py | 2 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/electrum/gui/qt/send_tab.py b/electrum/gui/qt/send_tab.py index 1550e85fd..3c1adcced 100644 --- a/electrum/gui/qt/send_tab.py +++ b/electrum/gui/qt/send_tab.py @@ -327,7 +327,14 @@ class SendTab(QWidget, MessageBoxMixin, Logger): is_max = any(parse_max_spend(outval) for outval in output_values) output_value = '!' if is_max else sum(output_values) - candidates = self.wallet.get_candidates_for_batching(outputs, []) # coins not used + # To find batching candidates, we need to know our available UTXOs. + # Ideally should use same set of coins make_tx() will use. + # note: - prone to races: coins set might change due to new txs between now and make_tx() call + # - make_tx() might pass different params to get_coins() + # - to mitigate, we prefer to be more restrictive. hence confirmed_only=True + coins_conservative = get_coins(nonlocal_only=True, confirmed_only=True) + candidates = self.wallet.get_candidates_for_batching(outputs, coins=coins_conservative) + tx, is_preview = self.window.confirm_tx_dialog(make_tx, output_value, batching_candidates=candidates) if tx is None: # user cancelled diff --git a/electrum/wallet.py b/electrum/wallet.py index 843d521ab..e0ef00641 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -1788,7 +1788,16 @@ class Abstract_Wallet(ABC, Logger, EventListener): def dust_threshold(self): return dust_threshold(self.network) - def get_candidates_for_batching(self, outputs, coins) -> Sequence[Transaction]: + def get_candidates_for_batching( + self, + outputs: Sequence[PartialTxOutput], + *, + coins: Sequence[PartialTxInput], + ) -> Sequence[Transaction]: + """ + coins: utxos available to add as inputs into the final tx. If empty, the set of candidates is restricted to + base txs with large enough change outputs to cover paying for all the `outputs`. + """ # do not batch if we spend max (not supported by make_unsigned_transaction) if any([parse_max_spend(o.value) is not None for o in outputs]): return [] diff --git a/tests/test_wallet_vertical.py b/tests/test_wallet_vertical.py index 1a415629c..ffe737add 100644 --- a/tests/test_wallet_vertical.py +++ b/tests/test_wallet_vertical.py @@ -2162,7 +2162,7 @@ class TestWalletSending(ElectrumTestCase): coins = wallet.get_spendable_coins(domain=None) self.assertEqual(2, len(coins)) - candidates = wallet.get_candidates_for_batching(outputs, coins) + candidates = wallet.get_candidates_for_batching(outputs, coins=coins) self.assertEqual(candidates, []) with self.assertRaises(NotEnoughFunds): wallet.make_unsigned_transaction(coins=coins, outputs=outputs, fee_policy=FixedFeePolicy(1000), base_tx=toself_tx)