Merge #969: Prevent selection of non-standard utxo for podle

8f5998b Prevent selection of non-standard utxo for podle (Adam Gibson)

jmclient/jmclient/taker.py

@@ -744,16 +744,16 @@ class Taker(object):
         This will allow future upgrades to provide different style commitments
         by subclassing Taker and changing the commit_type_byte; existing makers
         will simply not accept this new type of commitment.
-        In case of success, return the commitment and its opening.
-        In case of failure returns (None, None) and constructs a detailed
-        log for the user to read and discern the reason.
+        In case of success, return the (commitment, commitment opening).
+        In case of failure returns (None, None, err) where 'err' is a detailed
+        error string for the user to read and discern the reason.
         """
 
         def filter_by_coin_age_amt(utxos, age, amt):
             results = jm_single().bc_interface.query_utxo_set(utxos,
                                                     includeconf=True)
             newresults = []
-            too_old = []
+            too_new = []
             too_small = []
             for i, r in enumerate(results):
                 #results return "None" if txo is spent; drop this
@@ -762,27 +762,31 @@ class Taker(object):
                 valid_age = r['confirms'] >= age
                 valid_amt = r['value'] >= amt
                 if not valid_age:
-                    too_old.append(utxos[i])
+                    too_new.append(utxos[i])
                 if not valid_amt:
                     too_small.append(utxos[i])
                 if valid_age and valid_amt:
                     newresults.append(utxos[i])
-            return newresults, too_old, too_small
+            return newresults, too_new, too_small
 
         def priv_utxo_pairs_from_utxos(utxos, age, amt):
             #returns pairs list of (priv, utxo) for each valid utxo;
-            #also returns lists "too_old" and "too_small" for any
+            #also returns lists "too_new" and "too_small" for any
             #utxos that did not satisfy the criteria for debugging.
             priv_utxo_pairs = []
-            new_utxos, too_old, too_small = filter_by_coin_age_amt(list(utxos.keys()),
+            new_utxos, too_new, too_small = filter_by_coin_age_amt(list(utxos.keys()),
                                                                    age, amt)
             new_utxos_dict = {k: v for k, v in utxos.items() if k in new_utxos}
             for k, v in new_utxos_dict.items():
+                # filter out any non-standard utxos:
+                path = self.wallet_service.script_to_path(v["script"])
+                if not self.wallet_service.is_standard_wallet_script(path):
+                    continue
                 addr = self.wallet_service.script_to_addr(v["script"])
                 priv = self.wallet_service.get_key_from_addr(addr)
                 if priv:  #can be null from create-unsigned
                     priv_utxo_pairs.append((priv, k))
-            return priv_utxo_pairs, too_old, too_small
+            return priv_utxo_pairs, too_new, too_small
 
         commit_type_byte = "P"
         tries = jm_single().config.getint("POLICY", "taker_utxo_retries")

jmclient/test/test_taker.py

@@ -5,6 +5,7 @@ import jmbitcoin as bitcoin
 import binascii
 import os
 import copy
+import random
 import shutil
 import pytest
 import json
@@ -12,7 +13,7 @@ import struct
 from base64 import b64encode
 from jmbase import utxostr_to_utxo, hextobin
 from jmclient import load_test_config, jm_single, set_commitment_file,\
-    get_commitment_file, SegwitWallet, Taker, VolatileStorage,\
+    get_commitment_file, LegacyWallet, Taker, VolatileStorage,\
     get_network, WalletService, NO_ROUNDING, NotEnoughFundsException,\
     BTC_P2SH_P2WPKH, BTC_P2PKH, BTC_P2WPKH
 from taker_test_data import t_utxos_by_mixdepth, t_orderbook,\
@@ -25,7 +26,7 @@ def convert_utxos(utxodict):
         return_dict[utxostr_to_utxo(uk)[1]] = val
     return return_dict
 
-class DummyWallet(SegwitWallet):
+class DummyWallet(LegacyWallet):
     def __init__(self):
         storage = VolatileStorage()
         super().initialize(storage, get_network(), max_mixdepth=5)
@@ -45,7 +46,7 @@ class DummyWallet(SegwitWallet):
                 self._script_map[script] = path
 
     def add_extra_utxo(self, txid, index, value, md,
-                       address="mrcNu71ztWjAQA6ww9kHiW3zBWSQidHXTQ",
+                       script=None,
                        i=0):
         # note branch and index, path will be ignored in these test cases,
         # the tree is not real.
@@ -53,12 +54,17 @@ class DummyWallet(SegwitWallet):
         # we will need to return a script and an address, although it
         # won't be used; note we can't use base class get_utxos_by_mixdepth
         # because the paths are fake.
+        if not script:
+            address = "mrcNu71ztWjAQA6ww9kHiW3zBWSQidHXTQ"
+            script = self._ENGINE.address_to_script(address)
+        else:
+            address = bitcoin.CCoinAddress.from_scriptPubKey(script)
         if md not in self.ex_utxos:
             self.ex_utxos[md] = {}
         self.ex_utxos[md].update({(txid, index): {"mixdepth": md,
                             "address": address,
                             "value": value,
-                            "script": self._ENGINE.address_to_script(address),
+                            "script": script,
                             "path": (b'dummy', md, i)}})
 
     def remove_extra_utxo(self, txid, index, md):
@@ -135,6 +141,16 @@ class DummyWallet(SegwitWallet):
     def _is_my_bip32_path(self, path):
         return True
 
+    def is_standard_wallet_script(self, path):
+        if path[0] == "nonstandard_path":
+            return False
+        return True
+
+    def script_to_addr(self, script):
+        if self.script_to_path(script)[0] == "nonstandard_path":
+            return "dummyaddr"
+        return super().script_to_addr(script)
+
 
 def dummy_order_chooser():
     return t_chosen_orders
@@ -191,6 +207,12 @@ def test_filter_rejection(setup_taker):
         # make the confs in the spending mixdepth insufficient, while those
         # in another mixdepth are OK; must fail:
         (0, 110000000, False, False, False, 20, 5, {"confchange": {0: 1}}),
+        # add one timelock script in mixdepth 0, must succeed without
+        # trying to use it as PoDLE:
+        (0, 110000000, False, False, True, 20, 5, {"custom-script": {0: [44000000]}}),
+        # add one timelock script in mixdepth 0, must fail because only
+        # the timelocked UTXO is big enough:
+        (0, 1110000000, False, False, False, 20, 5, {"custom-script": {0: [1000000000]}}),
     ])
 def test_make_commitment(setup_taker, mixdepth, cjamt, failquery, external,
                          expected_success, amtpercent, age, mixdepth_extras):
@@ -229,6 +251,22 @@ def test_make_commitment(setup_taker, mixdepth, cjamt, failquery, external,
                 # set the utxos in mixdepth k2 to have confs v2:
                 cdict = taker.wallet_service.get_utxos_by_mixdepth()[k2]
                 jm_single().bc_interface.set_confs({utxo: v2 for utxo in cdict.keys()})
+        elif k == "custom-script":
+            # note: this is inspired by fidelity bonds, and currently
+            # uses scripts of that specific timelock type, but is really
+            # only testing the general concept: that commitments must
+            # not be made on any non-standard script type.
+            for k2, v2 in v.items():
+                priv = os.urandom(32) + b"\x01"
+                tl = random.randrange(1430454400, 1430494400)
+                script_inner = bitcoin.mk_freeze_script(
+                    bitcoin.privkey_to_pubkey(priv), tl)
+                script_outer = bitcoin.redeem_script_to_p2wsh_script(
+                    script_inner)
+                taker.wallet_service.wallet._script_map[
+                    script_outer] = ("nonstandard_path",)
+                taker.wallet_service.add_extra_utxo(os.urandom(32),
+                                    0, v2, k2, script=script_outer)
         else:
             for value in v:
                 taker.wallet_service.add_extra_utxo(