descriptor.py: clean-up and test PubkeyProvider.get_full_derivation_*

electrum/bip32.py

@@ -334,14 +334,18 @@ def convert_bip32_path_to_list_of_uint32(n: str) -> List[int]:
             # makes concatenating paths easier
             continue
         prime = 0
-        if x.endswith("'") or x.endswith("h"):
+        if x.endswith("'") or x.endswith("h"):  # note: some implementations also accept "H", "p", "P"
             x = x[:-1]
             prime = BIP32_PRIME
         if x.startswith('-'):
             if prime:
                 raise ValueError(f"bip32 path child index is signalling hardened level in multiple ways")
             prime = BIP32_PRIME
-        child_index = abs(int(x)) | prime
+        try:
+            x_int = int(x)
+        except ValueError as e:
+            raise ValueError(f"failed to parse bip32 path: {(str(e))}") from None
+        child_index = abs(x_int) | prime
         if child_index > UINT32_MAX:
             raise ValueError(f"bip32 path child index too large: {child_index} > {UINT32_MAX}")
         path.append(child_index)

electrum/descriptor.py

@@ -16,7 +16,7 @@
 
 import enum
 
-from .bip32 import convert_bip32_path_to_list_of_uint32, BIP32Node, KeyOriginInfo
+from .bip32 import convert_bip32_path_to_list_of_uint32, BIP32Node, KeyOriginInfo, BIP32_PRIME
 from . import bitcoin
 from .bitcoin import construct_script, opcodes, construct_witness
 from . import constants
@@ -254,35 +254,31 @@ class PubkeyProvider(object):
             assert not self.is_range()
             return unhexlify(self.pubkey)
 
-    def get_full_derivation_path(self, pos: int) -> str:
+    def get_full_derivation_path(self, *, pos: Optional[int] = None) -> str:
         """
         Returns the full derivation path at the given position, including the origin
         """
-        path = self.origin.get_derivation_path() if self.origin is not None else "m/"
+        if self.is_range() and pos is None:
+            raise ValueError("pos must be set for ranged descriptor")
+        path = self.origin.get_derivation_path() if self.origin is not None else "m"
         path += self.deriv_path if self.deriv_path is not None else ""
         if path[-1] == "*":
             path = path[:-1] + str(pos)
         return path
 
-    def get_full_derivation_int_list(self, pos: int) -> List[int]:
+    def get_full_derivation_int_list(self, *, pos: Optional[int] = None) -> List[int]:
         """
         Returns the full derivation path as an integer list at the given position.
         Includes the origin and master key fingerprint as an int
         """
+        if self.is_range() and pos is None:
+            raise ValueError("pos must be set for ranged descriptor")
         path: List[int] = self.origin.get_full_int_list() if self.origin is not None else []
         if self.deriv_path is not None:
-            der_split = self.deriv_path.split("/")
+            der_suffix = self.deriv_path
-            for p in der_split:
+            assert (wc_count := der_suffix.count("*")) <= 1, wc_count
-                if not p:
+            der_suffix = der_suffix.replace("*", str(pos))
-                    continue
+            path.extend(convert_bip32_path_to_list_of_uint32(der_suffix))
-                if p == "*":
-                    i = pos
-                elif p[-1] in "'phHP":
-                    assert len(p) >= 2
-                    i = int(p[:-1]) | 0x80000000
-                else:
-                    i = int(p)
-                path.append(i)
         return path
 
     def __lt__(self, other: 'PubkeyProvider') -> bool:

electrum/tests/test_descriptor.py

@@ -34,6 +34,8 @@ class TestDescriptor(ElectrumTestCase):
         self.assertEqual(desc.pubkeys[0].origin.get_derivation_path(), "m/84h/1h/0h")
         self.assertEqual(desc.pubkeys[0].pubkey, "tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B")
         self.assertEqual(desc.pubkeys[0].deriv_path, "/0/0")
+        self.assertEqual(desc.pubkeys[0].get_full_derivation_path(), "m/84h/1h/0h/0/0")
+        self.assertEqual(desc.pubkeys[0].get_full_derivation_int_list(), [16777216, 2147483732, 2147483649, 2147483648, 0, 0])
         self.assertEqual(desc.to_string_no_checksum(), d)
         e = desc.expand()
         self.assertEqual(e.output_script, unhexlify("0014d95fc47eada9e4c3cf59a2cbf9e96517c3ba2efa"))
@@ -51,6 +53,8 @@ class TestDescriptor(ElectrumTestCase):
         self.assertEqual(desc.subdescriptors[0].pubkeys[0].origin.get_derivation_path(), "m/48h/0h/0h/2h")
         self.assertEqual(desc.subdescriptors[0].pubkeys[0].pubkey, "tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B")
         self.assertEqual(desc.subdescriptors[0].pubkeys[0].deriv_path, "/0/0")
+        self.assertEqual(desc.subdescriptors[0].pubkeys[0].get_full_derivation_path(), "m/48h/0h/0h/2h/0/0")
+        self.assertEqual(desc.subdescriptors[0].pubkeys[0].get_full_derivation_int_list(), [16777216, 2147483696, 2147483648, 2147483648, 2147483650, 0, 0])
 
         self.assertEqual(desc.subdescriptors[0].pubkeys[1].origin.fingerprint.hex(), "00000002")
         self.assertEqual(desc.subdescriptors[0].pubkeys[1].origin.get_derivation_path(), "m/48h/0h/0h/2h")
@@ -109,6 +113,8 @@ class TestDescriptor(ElectrumTestCase):
         self.assertEqual(desc.pubkeys[0].origin, None)
         self.assertEqual(desc.pubkeys[0].pubkey, "tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B")
         self.assertEqual(desc.pubkeys[0].deriv_path, "/0/0")
+        self.assertEqual(desc.pubkeys[0].get_full_derivation_path(), "m/0/0")
+        self.assertEqual(desc.pubkeys[0].get_full_derivation_int_list(), [0, 0])
         self.assertEqual(desc.to_string_no_checksum(), d)
         e = desc.expand()
         self.assertEqual(e.output_script, unhexlify("0014d95fc47eada9e4c3cf59a2cbf9e96517c3ba2efa"))
@@ -138,6 +144,8 @@ class TestDescriptor(ElectrumTestCase):
         self.assertEqual(desc.pubkeys[0].origin.get_derivation_path(), "m/84h/1h/0h/0/0")
         self.assertEqual(desc.pubkeys[0].pubkey, "02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7")
         self.assertEqual(desc.pubkeys[0].deriv_path, None)
+        self.assertEqual(desc.pubkeys[0].get_full_derivation_path(), "m/84h/1h/0h/0/0")
+        self.assertEqual(desc.pubkeys[0].get_full_derivation_int_list(), [16777216, 2147483732, 2147483649, 2147483648, 0, 0])
         self.assertEqual(desc.to_string_no_checksum(), d)
         e = desc.expand()
         self.assertEqual(e.output_script, unhexlify("0014d95fc47eada9e4c3cf59a2cbf9e96517c3ba2efa"))
@@ -164,6 +172,8 @@ class TestDescriptor(ElectrumTestCase):
         self.assertEqual(desc.pubkeys[0].origin, None)
         self.assertEqual(desc.pubkeys[0].pubkey, "02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7")
         self.assertEqual(desc.pubkeys[0].deriv_path, None)
+        self.assertEqual(desc.pubkeys[0].get_full_derivation_path(), "m")
+        self.assertEqual(desc.pubkeys[0].get_full_derivation_int_list(), [])
         self.assertEqual(desc.to_string_no_checksum(), d)
 
     def test_parse_empty_descriptor(self):
@@ -176,6 +186,13 @@ class TestDescriptor(ElectrumTestCase):
         self.assertIsNotNone(desc)
         self.assertEqual(desc.pubkeys[0].origin.get_derivation_path(), "m/84h/1h/0h")
 
+    @as_testnet
+    def test_parse_descriptor_unknown_notation_for_hardened_derivation(self):
+        with self.assertRaises(ValueError):
+            desc = parse_descriptor("wpkh([00000001/84x/1x/0x]tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0/0)")
+        with self.assertRaises(ValueError):
+            desc = parse_descriptor("wpkh([00000001/84h/1h/0h]tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/0x)")
+
     def test_checksums(self):
         with self.subTest(msg="Valid checksum"):
             self.assertIsNotNone(parse_descriptor("sh(multi(2,[00000000/111h/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#5js07kwj"))