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
(ab14c3e138).

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

@@ -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

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 []

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)