Merge JoinMarket-Org#1676: Multiple (batch) payment support in `direc…

…t_send()`

f3f4f0a Multiple (batch) payment support in `direct_send()` (Kristaps Kaupe)

Pull request description:

  Work towards JoinMarket-Org#1012. Changes `direct_send()` to instead of single `amount` and `destination` to accept `dest_and_amounts` which is list of tuples of addresses and amounts instead. Haven't yet implemented and tested actual payments to multiple recipients, but tested that this doesn't break existing stuff.

Top commit has no ACKs.

Tree-SHA512: 02195a28d071c9537cb5297e63854ad2571e0ae9b5e06b850d6173c47d53caae953e9d7671ff861a6584a104d7a59da2293781d4440f7db4814f9b2fc4116c46

scripts/joinmarket-qt.py

@@ -801,11 +801,12 @@ def startSingle(self):
             if len(self.changeInput.text().strip()) > 0:
                 custom_change = str(self.changeInput.text().strip())
             try:
-                txid = direct_send(mainWindow.wallet_service, amount, mixdepth,
-                                  destaddr, accept_callback=self.checkDirectSend,
-                                  info_callback=self.infoDirectSend,
-                                  error_callback=self.errorDirectSend,
-                                  custom_change_addr=custom_change)
+                txid = direct_send(mainWindow.wallet_service, mixdepth,
+                                   [(destaddr, amount)],
+                                   accept_callback=self.checkDirectSend,
+                                   info_callback=self.infoDirectSend,
+                                   error_callback=self.errorDirectSend,
+                                   custom_change_addr=custom_change)
             except Exception as e:
                 JMQtMessageBox(self, e.args[0], title="Error", mbtype="warn")
                 return

scripts/sendpayment.py

@@ -270,7 +270,8 @@ def main():
                     sys.exit(EXIT_ARGERROR)
 
     if options.makercount == 0 and not bip78url:
-        tx = direct_send(wallet_service, amount, mixdepth, destaddr,
+        tx = direct_send(wallet_service, mixdepth,
+                         [(destaddr, amount)],
                          options.answeryes,
                          with_final_psbt=options.with_psbt,
                          optin_rbf=not options.no_rbf,

src/jmclient/payjoin.py

@@ -482,8 +482,9 @@ def make_payment_psbt(manager, accept_callback=None, info_callback=None):
     # we can create a standard payment, but have it returned as a PSBT.
     assert isinstance(manager, JMPayjoinManager)
     assert manager.wallet_service.synced
-    payment_psbt = direct_send(manager.wallet_service, manager.amount,
-                               manager.mixdepth, str(manager.destination),
+    payment_psbt = direct_send(manager.wallet_service,
+                               manager.mixdepth,
+                               [(str(manager.destination), manager.amount)],
                                accept_callback=accept_callback,
                                info_callback=info_callback,
                                with_final_psbt=True)

src/jmclient/taker_utils.py

@@ -4,7 +4,7 @@
 import sys
 import time
 import numbers
-from typing import Callable, Optional, Union
+from typing import Callable, List, Optional, Tuple, Union
 
 from jmbase import get_log, jmprint, bintohex, hextobin, \
     cli_prompt_user_yesno
@@ -34,8 +34,10 @@ def get_utxo_scripts(wallet: BaseWallet, utxos: dict) -> list:
         script_types.append(wallet.get_outtype(utxo["address"]))
     return script_types
 
-def direct_send(wallet_service: WalletService, amount: int, mixdepth: int,
-                destination: str, answeryes: bool = False,
+def direct_send(wallet_service: WalletService,
+                mixdepth: int,
+                dest_and_amounts: List[Tuple[str, int]],
+                answeryes: bool = False,
                 accept_callback: Optional[Callable[[str, str, int, int, Optional[str]], bool]] = None,
                 info_callback: Optional[Callable[[str], None]] = None,
                 error_callback: Optional[Callable[[str], None]] = None,
@@ -71,86 +73,110 @@ def direct_send(wallet_service: WalletService, amount: int, mixdepth: int,
     4. The PSBT object if with_final_psbt is True, and in
        this case the transaction is *NOT* broadcast.
     """
+    is_sweep = False
+    outtypes = []
+    total_outputs_val = 0
+
     #Sanity checks
-    assert validate_address(destination)[0] or is_burn_destination(destination)
+    assert isinstance(dest_and_amounts, list)
+    assert len(dest_and_amounts) > 0
     assert custom_change_addr is None or validate_address(custom_change_addr)[0]
-    assert amount > 0 or custom_change_addr is None
     assert isinstance(mixdepth, numbers.Integral)
     assert mixdepth >= 0
-    assert isinstance(amount, numbers.Integral)
-    assert amount >=0
     assert isinstance(wallet_service.wallet, BaseWallet)
 
-    if is_burn_destination(destination):
-        #Additional checks
-        if not isinstance(wallet_service.wallet, FidelityBondMixin):
-            log.error("Only fidelity bond wallets can burn coins")
-            return
-        if answeryes:
-            log.error("Burning coins not allowed without asking for confirmation")
-            return
-        if mixdepth != FidelityBondMixin.FIDELITY_BOND_MIXDEPTH:
-            log.error("Burning coins only allowed from mixdepth " + str(
-                FidelityBondMixin.FIDELITY_BOND_MIXDEPTH))
-            return
-        if amount != 0:
-            log.error("Only sweeping allowed when burning coins, to keep the tx " +
-                "small. Tip: use the coin control feature to freeze utxos")
-            return
+    for target in dest_and_amounts:
+        destination = target[0]
+        amount = target[1]
+        assert validate_address(destination)[0] or \
+            is_burn_destination(destination)
+        if amount == 0:
+            assert custom_change_addr is None and \
+                len(dest_and_amounts) == 1
+            is_sweep = True
+        assert isinstance(amount, numbers.Integral)
+        assert amount >= 0
+        if is_burn_destination(destination):
+            #Additional checks
+            if not isinstance(wallet_service.wallet, FidelityBondMixin):
+                log.error("Only fidelity bond wallets can burn coins")
+                return
+            if answeryes:
+                log.error("Burning coins not allowed without asking for confirmation")
+                return
+            if mixdepth != FidelityBondMixin.FIDELITY_BOND_MIXDEPTH:
+                log.error("Burning coins only allowed from mixdepth " + str(
+                    FidelityBondMixin.FIDELITY_BOND_MIXDEPTH))
+                return
+            if amount != 0:
+                log.error("Only sweeping allowed when burning coins, to keep "
+                    "the tx small. Tip: use the coin control feature to "
+                    "freeze utxos")
+                return
+        # if the output is of a script type not currently
+        # handled by our wallet code, we can't use information
+        # to help us calculate fees, but fall back to default.
+        # This is represented by a return value `None`.
+        # Note that this does *not* imply we accept any nonstandard
+        # output script, because we already called `validate_address`.
+        outtypes.append(wallet_service.get_outtype(destination))
+        total_outputs_val += amount
 
     txtype = wallet_service.get_txtype()
 
-    # if the output is of a script type not currently
-    # handled by our wallet code, we can't use information
-    # to help us calculate fees, but fall back to default.
-    # This is represented by a return value `None`.
-    # Note that this does *not* imply we accept any nonstandard
-    # output script, because we already called `validate_address`.
-    outtype = wallet_service.get_outtype(destination)
-
-    if amount == 0:
+    if is_sweep:
         #doing a sweep
+        destination = dest_and_amounts[0][0]
+        amount = dest_and_amounts[0][1]
         utxos = wallet_service.get_utxos_by_mixdepth()[mixdepth]
         if utxos == {}:
             log.error(
-                "There are no available utxos in mixdepth: " + str(mixdepth) + ", quitting.")
+                f"There are no available utxos in mixdepth {mixdepth}, "
+                 "quitting.")
             return
         total_inputs_val = sum([va['value'] for u, va in utxos.items()])
         script_types = get_utxo_scripts(wallet_service.wallet, utxos)
-        fee_est = estimate_tx_fee(len(utxos), 1, txtype=script_types, outtype=outtype)
-        outs = [{"address": destination, "value": total_inputs_val - fee_est}]
+        fee_est = estimate_tx_fee(len(utxos), 1, txtype=script_types,
+            outtype=outtypes[0])
+        outs = [{"address": destination,
+                 "value": total_inputs_val - fee_est}]
     else:
-        change_type = txtype
         if custom_change_addr:
             change_type = wallet_service.get_outtype(custom_change_addr)
             if change_type is None:
-                # we don't recognize this type; best we can do is revert to default,
-                # even though it may be inaccurate:
+                # we don't recognize this type; best we can do is revert to
+                # default, even though it may be inaccurate:
                 change_type = txtype
-        if outtype is None:
+        else:
+            change_type = txtype
+        if outtypes[0] is None:
             # we don't recognize the destination script type,
             # so set it as the same as the change (which will usually
             # be the same as the spending wallet, but see above for custom)
             # Notice that this is handled differently to the sweep case above,
             # because we must use a list - there is more than one output
-            outtype = change_type
-        outtypes = [change_type, outtype]
+            outtypes[0] = change_type
+        outtypes.append(change_type)
         # not doing a sweep; we will have change.
         # 8 inputs to be conservative; note we cannot account for the possibility
         # of non-standard input types at this point.
-        initial_fee_est = estimate_tx_fee(8, 2, txtype=txtype, outtype=outtypes)
+        initial_fee_est = estimate_tx_fee(8, len(dest_and_amounts) + 1,
+                                          txtype=txtype, outtype=outtypes)
         utxos = wallet_service.select_utxos(mixdepth, amount + initial_fee_est,
                                             includeaddr=True)
         script_types = get_utxo_scripts(wallet_service.wallet, utxos)
         if len(utxos) < 8:
-            fee_est = estimate_tx_fee(len(utxos), 2, txtype=script_types, outtype=outtypes)
+            fee_est = estimate_tx_fee(len(utxos), len(dest_and_amounts) + 1,
+                                      txtype=script_types, outtype=outtypes)
         else:
             fee_est = initial_fee_est
         total_inputs_val = sum([va['value'] for u, va in utxos.items()])
-        changeval = total_inputs_val - fee_est - amount
-        outs = [{"value": amount, "address": destination}]
-        change_addr = wallet_service.get_internal_addr(mixdepth) if custom_change_addr is None \
-                      else custom_change_addr
+        changeval = total_inputs_val - fee_est - total_outputs_val
+        outs = []
+        for out in dest_and_amounts:
+            outs.append({"value": out[1], "address": out[0]})
+        change_addr = wallet_service.get_internal_addr(mixdepth) \
+            if custom_change_addr is None else custom_change_addr
         outs.append({"value": changeval, "address": change_addr})
 
     #compute transaction locktime, has special case for spending timelocked coins
@@ -170,9 +196,10 @@ def direct_send(wallet_service: WalletService, amount: int, mixdepth: int,
 
     #Now ready to construct transaction
     log.info("Using a fee of: " + amount_to_str(fee_est) + ".")
-    if amount != 0:
+    if not is_sweep:
         log.info("Using a change value of: " + amount_to_str(changeval) + ".")
-    tx = make_shuffled_tx(list(utxos.keys()), outs, 2, tx_locktime)
+    tx = make_shuffled_tx(list(utxos.keys()), outs,
+                          version=2, locktime=tx_locktime)
 
     if optin_rbf:
         for inp in tx.vin:

src/jmclient/wallet_rpc.py

@@ -796,9 +796,11 @@ def directsend(self, request, walletname):
 
             try:
                 tx = direct_send(self.services["wallet"],
-                        int(payment_info_json["amount_sats"]),
                         int(payment_info_json["mixdepth"]),
-                        destination=payment_info_json["destination"],
+                        [(
+                            payment_info_json["destination"],
+                            int(payment_info_json["amount_sats"])
+                        )],
                         return_transaction=True, answeryes=True)
                 jm_single().config.set("POLICY", "tx_fees",
                                        self.default_policy_tx_fees)

test/jmclient/test_psbt_wallet.py

@@ -95,10 +95,11 @@ def test_create_and_sign_psbt_with_legacy(setup_psbt_wallet):
     legacy_addr = bitcoin.CCoinAddress.from_scriptPubKey(
         bitcoin.pubkey_to_p2pkh_script(
             bitcoin.privkey_to_pubkey(b"\x01"*33)))
-    tx = direct_send(wallet_service, bitcoin.coins_to_satoshi(0.3), 0,
-                       str(legacy_addr), accept_callback=dummy_accept_callback,
-                       info_callback=dummy_info_callback,
-                       return_transaction=True)
+    tx = direct_send(wallet_service, 0,
+                     [(str(legacy_addr), bitcoin.coins_to_satoshi(0.3))],
+                     accept_callback=dummy_accept_callback,
+                     info_callback=dummy_info_callback,
+                     return_transaction=True)
     assert tx
     # this time we will have one utxo worth <~ 0.7
     my_utxos = wallet_service.select_utxos(0, bitcoin.coins_to_satoshi(0.5))
@@ -277,7 +278,8 @@ def test_payjoin_workflow(setup_psbt_wallet, payment_amt, wallet_cls_sender,
     # **************
 
     # create a normal tx from the sender wallet:
-    payment_psbt = direct_send(wallet_s, payment_amt, 0, destaddr,
+    payment_psbt = direct_send(wallet_s, 0,
+                    [(destaddr, payment_amt)],
                     accept_callback=dummy_accept_callback,
                     info_callback=dummy_info_callback,
                     with_final_psbt=True)

test/jmclient/test_snicker.py

@@ -41,8 +41,11 @@ def test_snicker_e2e(setup_snicker, nw, wallet_structures,
     wallet_p = wallets[1]['wallet']
     # next, create a tx from the receiver wallet
     our_destn_script = wallet_r.get_new_script(1, BaseWallet.ADDRESS_TYPE_INTERNAL)
-    tx = direct_send(wallet_r, btc.coins_to_satoshi(0.3), 0,
-                     wallet_r.script_to_addr(our_destn_script),
+    tx = direct_send(wallet_r, 0,
+                     [(
+                        wallet_r.script_to_addr(our_destn_script),
+                        btc.coins_to_satoshi(0.3)
+                     )],
                      accept_callback=dummy_accept_callback,
                      info_callback=dummy_info_callback,
                      return_transaction=True)    

test/jmclient/test_tx_creation.py

@@ -163,8 +163,8 @@ def test_spend_then_rbf(setup_tx_creation):
     # While `direct_send` usually encapsulates utxo selection
     # for user, here we need to know what was chosen, hence
     # we return the transaction object, not directly broadcast.
-    tx1 = direct_send(wallet_service, amount, 0,
-                      destn, answeryes=True,
+    tx1 = direct_send(wallet_service, 0, [(destn, amount)],
+                      answeryes=True,
                       return_transaction=True)
     assert tx1
     # record the utxos for reuse: