descriptor.py: sortedmulti to sort .pubkeys already in __init__

electrum/descriptor.py

@@ -278,13 +278,17 @@ class PubkeyProvider(object):
         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:
+        path.extend(self.get_der_suffix_int_list(pos=pos))
-            der_suffix = self.deriv_path
-            assert (wc_count := der_suffix.count("*")) <= 1, wc_count
-            der_suffix = der_suffix.replace("*", str(pos))
-            path.extend(convert_bip32_path_to_list_of_uint32(der_suffix))
         return path
 
+    def get_der_suffix_int_list(self, *, pos: Optional[int] = None) -> List[int]:
+        if not self.deriv_path:
+            return []
+        der_suffix = self.deriv_path
+        assert (wc_count := der_suffix.count("*")) <= 1, wc_count
+        der_suffix = der_suffix.replace("*", str(pos))
+        return convert_bip32_path_to_list_of_uint32(der_suffix)
+
     def __lt__(self, other: 'PubkeyProvider') -> bool:
         return self.pubkey < other.pubkey
 
@@ -606,7 +610,14 @@ class MultisigDescriptor(Descriptor):
         self.thresh = thresh
         self.is_sorted = is_sorted
         if self.is_sorted:
-            self.pubkeys.sort()
+            if not self.is_range():
+                # sort xpubs using the order of pubkeys
+                der_pks = [p.get_pubkey_bytes() for p in self.pubkeys]
+                self.pubkeys = [x[1] for x in sorted(zip(der_pks, self.pubkeys))]
+            else:
+                # not possible to sort according to final order in expanded scripts,
+                # but for easier visual comparison, we do a lexicographical sort
+                self.pubkeys.sort()
 
     def to_string_no_checksum(self) -> str:
         return "{}({},{})".format(self.name, self.thresh, ",".join([p.to_string() for p in self.pubkeys]))

electrum/tests/test_descriptor.py

@@ -337,6 +337,34 @@ class TestDescriptor(ElectrumTestCase):
         with self.assertRaises(ValueError):  # only standard xpub/xprv allowed
             desc = parse_descriptor("wpkh([535e473f/0h]zpub6nAZodjgiMNf9zzX1pTqd6ZVX61ax8azhUDnWRumKVUr1VYATVoqAuqv3qKsb8WJXjxei4wei2p4vnMG9RnpKnen2kmgdhvZUmug2NnHNsr/0/*)")
 
+    @as_testnet
+    def test_sortedmulti_ranged_pubkey_order(self):
+        xpub1 = "tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B"
+        xpub2 = "tpubDFHiBJDeNvqPWNJbzzxqDVXmJZoNn2GEtoVcFhMjXipQiorGUmps3e5ieDGbRrBPTFTh9TXEKJCwbAGW9uZnfrVPbMxxbFohuFzfT6VThty"
+        # if ranged, we sort lexicographically
+        desc = parse_descriptor(f"sh(wsh(sortedmulti(2,[00000001/48h/0h/0h/2h]{xpub1}/0/*,[00000002/48h/0h/0h/2h]{xpub2}/0/*)))")
+        self.assertEqual([xpub1, xpub2], [pk.pubkey for pk in desc.subdescriptors[0].subdescriptors[0].pubkeys])
+        desc = parse_descriptor(f"sh(wsh(sortedmulti(2,[00000002/48h/0h/0h/2h]{xpub2}/0/*,[00000001/48h/0h/0h/2h]{xpub1}/0/*)))")
+        self.assertEqual([xpub1, xpub2], [pk.pubkey for pk in desc.subdescriptors[0].subdescriptors[0].pubkeys])
+        # if unsorted "multi", don't touch order
+        desc = parse_descriptor(f"sh(wsh(multi(2,[00000002/48h/0h/0h/2h]{xpub2}/0/*,[00000001/48h/0h/0h/2h]{xpub1}/0/*)))")
+        self.assertEqual([xpub2, xpub1], [pk.pubkey for pk in desc.subdescriptors[0].subdescriptors[0].pubkeys])
+
+    @as_testnet
+    def test_sortedmulti_unranged_pubkey_order(self):
+        xpub1 = "tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B"
+        xpub2 = "tpubDFHiBJDeNvqPWNJbzzxqDVXmJZoNn2GEtoVcFhMjXipQiorGUmps3e5ieDGbRrBPTFTh9TXEKJCwbAGW9uZnfrVPbMxxbFohuFzfT6VThty"
+        # if not ranged, we sort according to final derived pubkey order
+        desc = parse_descriptor(f"sh(wsh(sortedmulti(2,[00000001/48h/0h/0h/2h]{xpub1}/0/0,[00000002/48h/0h/0h/2h]{xpub2}/0/0)))")
+        self.assertEqual([xpub1, xpub2], [pk.pubkey for pk in desc.subdescriptors[0].subdescriptors[0].pubkeys])
+        desc = parse_descriptor(f"sh(wsh(sortedmulti(2,[00000001/48h/0h/0h/2h]{xpub1}/0/1,[00000002/48h/0h/0h/2h]{xpub2}/0/1)))")
+        self.assertEqual([xpub2, xpub1], [pk.pubkey for pk in desc.subdescriptors[0].subdescriptors[0].pubkeys])
+        desc = parse_descriptor(f"sh(wsh(sortedmulti(2,[00000001/48h/0h/0h/2h]{xpub1}/0/4,[00000002/48h/0h/0h/2h]{xpub2}/0/4)))")
+        self.assertEqual([xpub1, xpub2], [pk.pubkey for pk in desc.subdescriptors[0].subdescriptors[0].pubkeys])
+        # if unsorted "multi", don't touch order
+        desc = parse_descriptor(f"sh(wsh(multi(2,[00000001/48h/0h/0h/2h]{xpub1}/0/1,[00000002/48h/0h/0h/2h]{xpub2}/0/1)))")
+        self.assertEqual([xpub1, xpub2], [pk.pubkey for pk in desc.subdescriptors[0].subdescriptors[0].pubkeys])
+
     def test_pubkey_provider_deriv_path(self):
         xpub = "xpub68W3CJPrQzHhTQcHM6tbCvNVB9ih4tbzsFBLwe7zZUj5uHuhxBUhvnXe1RQhbKCTiTj3D7kXni6yAD88i2xnjKHaJ5NqTtHawKnPFCDnmo4"
         # valid: