Repository: bitcoin-core/HWI (python)

Issue:
Descriptor checksums
https://github.com/bitcoin/bitcoin/pull/15368 adds checksums for descriptors and requires them for using `importmulti`. We need to make sure descriptors that we generate have checksums. It would also be nice for us to check that descriptors being input have valid checksums.

Pull request description:
Implement descriptor checksums
Adds checksums to the Descriptor class. Checksums are optional, but if provided, must be valid. `getkeypool` is also changed to use the descriptor class to return checksums. Tests are updated so that all descriptors used have checksums.

Closes #129 

Patch:
diff --git a/hwilib/commands.py b/hwilib/commands.py
index 8b90cf1..4b66e24 100644
--- a/hwilib/commands.py
+++ b/hwilib/commands.py
@@ -139,15 +139,9 @@ def getkeypool(client, path, start, end, internal=False, keypool=False, account=
     import_data = []
     this_import = {}
 
-    descriptor_open = 'pkh('
-    descriptor_close = ')'
-    if wpkh == True:
-          descriptor_open = 'wpkh('
-    elif sh_wpkh == True:
-          descriptor_open = 'sh(wpkh('
-          descriptor_close = '))'
-
-    this_import['desc'] = descriptor_open + '[' + master_fpr + path_base.replace('m', '') + ']' + base_key + path_suffix + descriptor_close
+    desc = Descriptor(master_fpr, path_base.replace('m', ''), base_key, path_suffix, client.is_testnet, sh_wpkh, wpkh)
+
+    this_import['desc'] = desc.serialize()
     this_import['range'] = {'start': start, 'end': end}
     this_import['timestamp'] = 'now'
     this_import['internal'] = internal
diff --git a/hwilib/descriptor.py b/hwilib/descriptor.py
index b34d5d3..7df62b2 100644
--- a/hwilib/descriptor.py
+++ b/hwilib/descriptor.py
@@ -1,5 +1,54 @@
 import re
 
+# From: https://github.com/bitcoin/bitcoin/blob/master/src/script/descriptor.cpp
+
+def PolyMod(c, val):
+    c0 = c >> 35
+    c = ((c & 0x7ffffffff) << 5) ^ val
+    if (c0 & 1):
+        c ^= 0xf5dee51989
+    if (c0 & 2):
+        c ^= 0xa9fdca3312
+    if (c0 & 4):
+        c ^= 0x1bab10e32d
+    if (c0 & 8):
+        c ^= 0x3706b1677a
+    if (c0 & 16):
+        c ^= 0x644d626ffd
+    return c
+
+def DescriptorChecksum(desc):
+    INPUT_CHARSET = "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ ";
+    CHECKSUM_CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l";
+
+    c = 1
+    cls = 0
+    clscount = 0
+    for ch in desc:
+        pos = INPUT_CHARSET.find(ch)
+        if pos == -1:
+            return ""
+        c = PolyMod(c, pos & 31)
+        cls = cls * 3 + (pos >> 5)
+        clscount += 1
+        if clscount == 3:
+            c = PolyMod(c, cls)
+            cls = 0
+            clscount = 0
+    if clscount > 0:
+        c = PolyMod(c, cls)
+    for j in range (0, 8):
+        c = PolyMod(c, 0)
+    c ^= 1
+
+    ret = [None] * 8
+    for j in range(0, 8):
+        ret[j] = CHECKSUM_CHARSET[(c >> (5 * (7 - j))) & 31]
+    return ''.join(ret)
+
+def AddChecksum(desc):
+    return desc + "#" + DescriptorChecksum(desc)
+
 class Descriptor:
     def __init__(self, origin_fingerprint, origin_path, base_key, path_suffix, testnet, sh_wpkh, wpkh):
         self.origin_fingerprint = origin_fingerprint
@@ -25,6 +74,20 @@ class Descriptor:
         base_key = None
         path_suffix = None
 
+        # Check the checksum
+        check_split = desc.split('#')
+        if len(check_split) > 2:
+            return None
+        if len(check_split) == 2:
+            if len(check_split[1]) != 8:
+                return None
+            checksum = DescriptorChecksum(check_split[0])
+            if not checksum.strip():
+                return None
+            if checksum != check_split[1]:
+                return None
+        desc = check_split[0]
+
         if desc.startswith("sh(wpkh("):
             sh_wpkh = True
         elif desc.startswith("wpkh("):
@@ -74,4 +137,4 @@ class Descriptor:
         if self.path_suffix:
             path_suffix = self.path_suffix
 
-        return descriptor_open + origin + self.base_key + path_suffix + descriptor_close
+        return AddChecksum(descriptor_open + origin + self.base_key + path_suffix + descriptor_close)


Test patch:
diff --git a/test/test_descriptor.py b/test/test_descriptor.py
old mode 100644
new mode 100755
index 907eed4..a479c59
--- a/test/test_descriptor.py
+++ b/test/test_descriptor.py
@@ -62,19 +62,41 @@ class TestDescriptor(unittest.TestCase):
         self.assertEqual(desc.origin_path, "/84'/1'/0'")
 
     def test_serialize_descriptor_with_origin(self):
-        descriptor = "wpkh([00000001/84'/1'/0']tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0/0)"
+        descriptor = "wpkh([00000001/84'/1'/0']tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0/0)#mz20k55p"
         desc = Descriptor.parse(descriptor, True)
         self.assertEqual(desc.serialize(), descriptor)
 
     def test_serialize_descriptor_without_origin(self):
-        descriptor = "wpkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0/0)"
+        descriptor = "wpkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0/0)#ac0p4yhq"
         desc = Descriptor.parse(descriptor, True)
         self.assertEqual(desc.serialize(), descriptor)
 
     def test_serialize_descriptor_with_key_at_end_with_origin(self):
-        descriptor = "wpkh([00000001/84'/1'/0'/0/0]0297dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)"
+        descriptor = "wpkh([00000001/84'/1'/0'/0/0]0297dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#rh7p6vk2"
         desc = Descriptor.parse(descriptor, True)
         self.assertEqual(desc.serialize(), descriptor)
 
+    def test_checksums(self):
+        with self.subTest(msg='Valid checksum'):
+            self.assertIsNotNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#ggrsrxfy"))
+            self.assertIsNotNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#tjg09x5t"))
+            self.assertIsNotNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))"))
+            self.assertIsNotNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))"))
+        with self.subTest(msg="Empty Checksum"):
+            self.assertIsNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#"))
+            self.assertIsNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#"))
+        with self.subTest(msg="Too long Checksum"):
+            self.assertIsNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#ggrsrxfyq"))
+            self.assertIsNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#tjg09x5tq"))
+        with self.subTest(msg="Too Short Checksum"):
+            self.assertIsNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#ggrsrxf"))
+            self.assertIsNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#tjg09x5"))
+        with self.subTest(msg="Error in Payload"):
+            self.assertIsNone(Descriptor.parse("sh(multi(3,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#ggrsrxfy"))
+            self.assertIsNone(Descriptor.parse("sh(multi(3,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#tjg09x5t"))
+        with self.subTest(msg="Error in Checksum"):
+            self.assertIsNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#ggssrxfy"))
+            self.assertIsNone(Descriptor.parse("sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#tjq09x4t"))
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/test/test_device.py b/test/test_device.py
index 8b71635..22d364c 100644
--- a/test/test_device.py
+++ b/test/test_device.py
@@ -319,15 +319,20 @@ class TestSignTx(DeviceTestCase):
 
         # Get origin info/key pair so wallet doesn't forget how to
         # sign with keys post-import
-        pubkeys = [sh_wpkh_info['desc'][8:-2],\
-                wpkh_info['desc'][5:-1],\
-                pkh_info['desc'][4:-1]]
+        pubkeys = [sh_wpkh_info['desc'][8:-11],\
+                wpkh_info['desc'][5:-10],\
+                pkh_info['desc'][4:-10]]
 
-        sh_multi_desc = {'desc':'sh(multi(2,'+pubkeys[0]+','+pubkeys[1]+','+pubkeys[2]+'))', "timestamp":"now", "label":"shmulti"}
-        sh_wsh_multi_desc = {'desc':'sh(wsh(multi(2,'+pubkeys[0]+','+pubkeys[1]+','+pubkeys[2]+')))', "timestamp":"now", "label":"shwshmulti"}
+        # Get the descriptors with their checksums
+        sh_multi_desc = self.wrpc.getdescriptorinfo('sh(multi(2,'+pubkeys[0]+','+pubkeys[1]+','+pubkeys[2]+'))')['descriptor']
+        sh_wsh_multi_desc = self.wrpc.getdescriptorinfo('sh(wsh(multi(2,'+pubkeys[0]+','+pubkeys[1]+','+pubkeys[2]+')))')['descriptor']
+        wsh_multi_desc = self.wrpc.getdescriptorinfo('wsh(multi(2,'+pubkeys[2]+','+pubkeys[1]+','+pubkeys[0]+'))')['descriptor']
+
+        sh_multi_import = {'desc': sh_multi_desc, "timestamp":"now", "label":"shmulti"}
+        sh_wsh_multi_import = {'desc': sh_wsh_multi_desc, "timestamp":"now", "label":"shwshmulti"}
         # re-order pubkeys to allow import without "already have private keys" error
-        wsh_multi_desc = {'desc':'wsh(multi(2,'+pubkeys[2]+','+pubkeys[1]+','+pubkeys[0]+'))', "timestamp":"now", "label":"wshmulti"}
-        multi_result = self.wrpc.importmulti([sh_multi_desc, sh_wsh_multi_desc, wsh_multi_desc])
+        wsh_multi_import = {'desc': wsh_multi_desc, "timestamp":"now", "label":"wshmulti"}
+        multi_result = self.wrpc.importmulti([sh_multi_import, sh_wsh_multi_import, wsh_multi_import])
         self.assertTrue(multi_result[0]['success'])
         self.assertTrue(multi_result[1]['success'])
         self.assertTrue(multi_result[2]['success'])