Detailed tests across all 3 wallet types for psbt

Now supports PSBT for all 3 of Legacy, SegwitLegacy and
Segwit wallets. Tests cover also mixed inputs of different
types and owned/unowned. Direct send now exported to be used
in tests rather than only script usage, also supports returning
a tx object rather than only a txid.

jmclient/jmclient/__init__.py

@@ -40,7 +40,7 @@
 from .commitment_utils import get_utxo_info, validate_utxo_data, quit
 from .taker_utils import (tumbler_taker_finished_update, restart_waiter,
                              restart_wait, get_tumble_log, direct_send,
-                             tumbler_filter_orders_callback)
+                             tumbler_filter_orders_callback, direct_send)
 from .cli_options import (add_base_options, add_common_options,
                           get_tumbler_parser, get_max_cj_fee_values,
                           check_regtest, get_sendpayment_parser,

jmclient/jmclient/taker_utils.py

@@ -20,7 +20,8 @@
 """
 
 def direct_send(wallet_service, amount, mixdepth, destaddr, answeryes=False,
-                accept_callback=None, info_callback=None):
+                accept_callback=None, info_callback=None,
+                return_transaction=False):
     """Send coins directly from one mixdepth to one destination address;
     does not need IRC. Sweep as for normal sendpayment (set amount=0).
     If answeryes is True, callback/command line query is not performed.
@@ -37,7 +38,8 @@ def direct_send(wallet_service, amount, mixdepth, destaddr, answeryes=False,
     pushed), and returns nothing.
 
     This function returns:
-    The txid if transaction is pushed, False otherwise
+    The txid if transaction is pushed, False otherwise,
+    or the full CMutableTransaction if return_transaction is True.
     """
     #Sanity checks
     assert validate_address(destaddr)[0]
@@ -107,7 +109,8 @@ def direct_send(wallet_service, amount, mixdepth, destaddr, answeryes=False,
     successmsg = "Transaction sent: " + txid
     cb = log.info if not info_callback else info_callback
     cb(successmsg)
-    return txid
+    txinfo = txid if not return_transaction else tx
+    return txinfo
 
 
 def sign_tx(wallet_service, tx, utxos):

jmclient/jmclient/wallet.py

@@ -1031,16 +1031,20 @@ def create_psbt_from_tx(self, tx, spent_outs=None):
         # we now insert redeemscripts where that is possible and necessary:
         for i, txinput in enumerate(new_psbt.inputs):
             if isinstance(txinput.utxo, btc.CMutableTxOut):
-                # witness; TODO: native case, possibly p2sh legacy case
-                try:
-                    path = self.script_to_path(txinput.utxo.scriptPubKey)
-                except AssertionError:
-                    # this happens when an input is provided but it's not in
-                    # this wallet; in this case, we cannot set the redeem script.
+                # witness
+                if txinput.utxo.scriptPubKey.is_witness_scriptpubkey():
+                    # nothing needs inserting; the scriptSig is empty.
                     continue
-                privkey, _ = self._get_priv_from_path(path)
-                txinput.redeem_script = btc.pubkey_to_p2wpkh_script(
-                    btc.privkey_to_pubkey(privkey))
+                elif txinput.utxo.scriptPubKey.is_p2sh():
+                    try:
+                        path = self.script_to_path(txinput.utxo.scriptPubKey)
+                    except AssertionError:
+                        # this happens when an input is provided but it's not in
+                        # this wallet; in this case, we cannot set the redeem script.
+                        continue
+                    privkey, _ = self._get_priv_from_path(path)
+                    txinput.redeem_script = btc.pubkey_to_p2wpkh_script(
+                        btc.privkey_to_pubkey(privkey))
         return new_psbt
 
     def sign_psbt(self, in_psbt, with_sign_result=False):
@@ -1619,7 +1623,7 @@ def get_details(self, path):
         return self._get_mixdepth_from_path(path), path[-2], path[-1]
 
 
-class LegacyWallet(ImportWalletMixin, BIP32Wallet):
+class LegacyWallet(ImportWalletMixin, PSBTWalletMixin, BIP32Wallet):
     TYPE = TYPE_P2PKH
     _ENGINE = ENGINES[TYPE_P2PKH]
 

jmclient/test/test_psbt_wallet.py

@@ -9,21 +9,75 @@
 import time
 import binascii
 import struct
-from commontest import make_wallets, make_sign_and_push
+import copy
+from commontest import make_wallets
 
 import jmbitcoin as bitcoin
 import pytest
 from jmbase import get_log, bintohex, hextobin
-from jmclient import load_test_config, jm_single,\
-     get_p2pk_vbyte
+from jmclient import (load_test_config, jm_single, direct_send,
+                      SegwitLegacyWallet, SegwitWallet, LegacyWallet)
 
 log = get_log()
 
-@pytest.mark.parametrize('unowned_utxo', [
-    True,
-    False,
+def dummy_accept_callback(tx, destaddr, actual_amount, fee_est):
+    return True
+def dummy_info_callback(msg):
+    pass
+
+def test_create_and_sign_psbt_with_legacy(setup_psbt_wallet):
+    """ The purpose of this test is to check that we can create and
+    then partially sign a PSBT where we own one input and the other input
+    is of legacy p2pkh type.
+    """
+    wallet_service = make_wallets(1, [[1,0,0,0,0]], 1)[0]['wallet']
+    wallet_service.sync_wallet(fast=True)
+    utxos = wallet_service.select_utxos(0, bitcoin.coins_to_satoshi(0.5))
+    assert len(utxos) == 1
+    # create a legacy address and make a payment into it
+    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)
+    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))
+    assert len(my_utxos) == 1
+    # find the outpoint for the legacy address we're spending
+    n = -1
+    for i, t in enumerate(tx.vout):
+        if bitcoin.CCoinAddress.from_scriptPubKey(t.scriptPubKey) == legacy_addr:
+            n = i
+    assert n > -1
+    utxos = copy.deepcopy(my_utxos)
+    utxos[(tx.GetTxid()[::-1], n)] ={"script": legacy_addr.to_scriptPubKey(),
+              "value": bitcoin.coins_to_satoshi(0.3)}
+    outs = [{"value": bitcoin.coins_to_satoshi(0.998),
+                 "address": wallet_service.get_addr(0,0,0)}]
+    tx2 = bitcoin.mktx(list(utxos.keys()), outs)
+    spent_outs = wallet_service.witness_utxos_to_psbt_utxos(my_utxos)
+    spent_outs.append(tx)
+    new_psbt = wallet_service.create_psbt_from_tx(tx2, spent_outs)
+    signed_psbt_and_signresult, err = wallet_service.sign_psbt(
+        new_psbt.serialize(), with_sign_result=True)
+    assert err is None
+    signresult, signed_psbt = signed_psbt_and_signresult
+    assert signresult.num_inputs_signed == 1
+    assert signresult.num_inputs_final == 1
+    assert not signresult.is_final
+
+@pytest.mark.parametrize('unowned_utxo, wallet_cls', [
+    (True, SegwitLegacyWallet),
+    (False, SegwitLegacyWallet),
+    (True, SegwitWallet),
+    (False, SegwitWallet),
+    (True, LegacyWallet),
+    (False, LegacyWallet),
 ])
-def test_create_psbt_and_sign(setup_psbt_wallet, unowned_utxo):
+def test_create_psbt_and_sign(setup_psbt_wallet, unowned_utxo, wallet_cls):
     """ Plan of test:
     1. Create a wallet and source 3 destination addresses.
     2. Make, and confirm, transactions that fund the 3 addrs.
@@ -39,28 +93,47 @@ def test_create_psbt_and_sign(setup_psbt_wallet, unowned_utxo):
     8. In case where whole psbt is finalized, attempt to broadcast the tx.
     """
     # steps 1 and 2:
-    wallet_service = make_wallets(1, [[3,0,0,0,0]], 1)[0]['wallet']
+    wallet_service = make_wallets(1, [[3,0,0,0,0]], 1,
+                                  wallet_cls=wallet_cls)[0]['wallet']
     wallet_service.sync_wallet(fast=True)
     utxos = wallet_service.select_utxos(0, bitcoin.coins_to_satoshi(1.5))
+    # for legacy wallets, psbt creation requires querying for the spending
+    # transaction:
+    if wallet_cls == LegacyWallet:
+        fulltxs = []
+        for utxo, v in utxos.items():
+            fulltxs.append(jm_single().bc_interface.get_deser_from_gettransaction(
+                jm_single().bc_interface.get_transaction(utxo[0])))
+
     assert len(utxos) == 2
+    u_utxos = {}
     if unowned_utxo:
         # note: tx creation uses the key only; psbt creation uses the value,
         # which can be fake here; we do not intend to attempt to fully
         # finalize a psbt with an unowned input. See
         # https://github.com/Simplexum/python-bitcointx/issues/30
-        # for whether this redeemscript construction can be avoided.
+        # the redeem script creation (which is artificial) will be
+        # avoided in future.
         priv = b"\xaa"*32 + b"\x01"
         pub = bitcoin.privkey_to_pubkey(priv)
         script = bitcoin.pubkey_to_p2sh_p2wpkh_script(pub)
         redeem_script = bitcoin.pubkey_to_p2wpkh_script(pub)
-        utxos[(b"\xaa"*32, 12)] = {"value": 1000, "script": script}
+        u_utxos[(b"\xaa"*32, 12)] = {"value": 1000, "script": script}
+    utxos.update(u_utxos)
     # outputs aren't interesting for this test (we selected 1.5 but will get 2):
     outs = [{"value": bitcoin.coins_to_satoshi(1.999),
              "address": wallet_service.get_addr(0,0,0)}]
     tx = bitcoin.mktx(list(utxos.keys()), outs)
 
-    newpsbt = wallet_service.create_psbt_from_tx(tx,
-                    wallet_service.witness_utxos_to_psbt_utxos(utxos))
+    if wallet_cls != LegacyWallet:
+        spent_outs = wallet_service.witness_utxos_to_psbt_utxos(utxos)
+    else:
+        spent_outs = fulltxs
+        # the extra input is segwit:
+        if unowned_utxo:
+            spent_outs.extend(
+                wallet_service.witness_utxos_to_psbt_utxos(u_utxos))
+    newpsbt = wallet_service.create_psbt_from_tx(tx, spent_outs)
     # see note above
     if unowned_utxo:
         newpsbt.inputs[-1].redeem_script = redeem_script
@@ -73,8 +146,9 @@ def test_create_psbt_and_sign(setup_psbt_wallet, unowned_utxo):
     # note: redeem_script field is a CScript which is a bytes instance,
     # so checking length is best way to check for existence (comparison
     # with None does not work):
-    assert len(newpsbt.inputs[0].redeem_script) != 0
-    assert len(newpsbt.inputs[1].redeem_script) != 0
+    if wallet_cls == SegwitLegacyWallet:
+        assert len(newpsbt.inputs[0].redeem_script) != 0
+        assert len(newpsbt.inputs[1].redeem_script) != 0
     if unowned_utxo:
         assert newpsbt.inputs[2].redeem_script == redeem_script