Merge pull request #448 from kdmukai/signmessage_addr_fix

[bugfix] Derive correct "signmessage" address

src/seedsigner/helpers/embit_utils.py

@@ -146,8 +146,24 @@ def parse_derivation_path(derivation_path: str) -> dict:
     if not details["script_type"]:
         details["script_type"] = SettingsConstants.CUSTOM_DERIVATION
     details["network"] = lookups["networks"].get(sections[2])
-    details["is_change"] = sections[-2] == "1"
-    details["index"] = int(sections[-1])
+
+    # Check if there's a standard change path
+    if sections[-2] in ["0", "1"]:
+        details["is_change"] = sections[-2] == "1"
+    else:
+        details["is_change"] = None
+
+    # Check if there's a standard address index
+    if sections[-1].isdigit():
+        details["index"] = int(sections[-1])
+    else:
+        details["index"] = None
+
+    if details["is_change"] is not None and details["index"] is not None:
+        # standard change and addr index; safe to truncate to the wallet level
+        details["wallet_derivation_path"] = "/".join(sections[:-2])
+    else:
+        details["wallet_derivation_path"] = None
 
     details["clean_match"] = True
     for k, v in details.items():

src/seedsigner/views/seed_views.py

@@ -1997,14 +1997,37 @@ class SeedSignMessageConfirmAddressView(View):
     def __init__(self):
         super().__init__()
         data = self.controller.sign_message_data
-        seed = self.controller.storage.seeds[data.get("seed_num")]
+        seed_num = data.get("seed_num")
         self.derivation_path = data.get("derivation_path")
+
+        if seed_num is None or not self.derivation_path:
+            raise Exception("Routing error: sign_message_data hasn't been set")
+
+        seed = self.controller.storage.seeds[seed_num]
         addr_format = data.get("addr_format")
 
-        # Current settings will differentiate TESTNET and REGTEST (since derivation path
-        # alone doesn't specify which one we're using).
-        xpub = seed.get_xpub(wallet_path=self.derivation_path, network=self.settings.get_value(SettingsConstants.SETTING__NETWORK))
-        embit_network = embit_utils.get_embit_network_name(self.settings.get_value(SettingsConstants.SETTING__NETWORK))
+        # calculate the actual receive address
+        seed = self.controller.storage.seeds[seed_num]
+        addr_format = embit_utils.parse_derivation_path(self.derivation_path)
+        if not addr_format["clean_match"] or addr_format["script_type"] == SettingsConstants.CUSTOM_DERIVATION:
+            raise Exception("Signing messages for custom derivation paths not supported")
+
+        if addr_format["network"] != SettingsConstants.MAINNET:
+            # We're in either Testnet or Regtest or...?
+            if self.settings.get_value(SettingsConstants.SETTING__NETWORK) in [SettingsConstants.TESTNET, SettingsConstants.REGTEST]:
+                addr_format["network"] = self.settings.get_value(SettingsConstants.SETTING__NETWORK)
+            else:
+                from seedsigner.views.view import NetworkMismatchErrorView
+                self.set_redirect(Destination(NetworkMismatchErrorView, view_args=dict(text=f"Current network setting ({self.settings.get_value_display_name(SettingsConstants.SETTING__NETWORK)}) doesn't match {self.derivation_path}")))
+
+                # cleanup. Note: We could leave this in place so the user can resume the
+                # flow, but for now we avoid complications and keep things simple.
+                self.controller.resume_main_flow = None
+                self.controller.sign_message_data = None
+                return
+
+        xpub = seed.get_xpub(wallet_path=addr_format["wallet_derivation_path"], network=addr_format["network"])
+        embit_network = embit_utils.get_embit_network_name(addr_format["network"])
         self.address = embit_utils.get_single_sig_address(xpub=xpub, script_type=addr_format["script_type"], index=addr_format["index"], is_change=addr_format["is_change"], embit_network=embit_network)
 
 

tests/test_embit_utils.py

@@ -361,7 +361,7 @@ def test_parse_derivation_path():
         (SC.TESTNET, SC.TAPROOT, True): "m/86'/1'/0'/1/5",
         (SC.REGTEST, SC.TAPROOT, True): "m/86'/1'/0'/1/5",
 
-        # Try a typical custom derivation path (Unchained Capital)
+        # Try a typical custom derivation path (Unchained vault keys)
         (SC.MAINNET, SC.CUSTOM_DERIVATION, False): "m/45'/0'/0'/0/5",
         (SC.TESTNET, SC.CUSTOM_DERIVATION, False): "m/45'/1'/0'/0/5",
         (SC.REGTEST, SC.CUSTOM_DERIVATION, False): "m/45'/1'/0'/0/5",
@@ -369,8 +369,14 @@ def test_parse_derivation_path():
         (SC.TESTNET, SC.CUSTOM_DERIVATION, True): "m/45'/1'/0'/1/5",
         (SC.REGTEST, SC.CUSTOM_DERIVATION, True): "m/45'/1'/0'/1/5",
 
-        # CRAZY custom derivation path
-        (None, SC.CUSTOM_DERIVATION, False): "m/879345978543'/908327034508534983495'/9085098430894380959043'/0/5",
+        # CRAZY custom derivation paths
+        (None, SC.CUSTOM_DERIVATION, False, 5): "m/123'/9083270/9083270/9083270/9083270/0/5",
+
+        # non-standard change and/or index
+        (None, SC.CUSTOM_DERIVATION, None, 5): "m/9'/78/5",
+        (None, SC.CUSTOM_DERIVATION, None, 5): "m/9'/78'/5",
+        (None, SC.CUSTOM_DERIVATION, None, None): "m/9'/78'/5'",
+        (None, SC.CUSTOM_DERIVATION, False, None): "m/9'/0/5'",
     }
 
     for expected_result, derivation_path in vectors_args.items():
@@ -389,4 +395,8 @@ def test_parse_derivation_path():
 
         assert(actual_result["script_type"] == expected_result[1])
         assert(actual_result["is_change"] == expected_result[2])
-        assert(actual_result["index"] == int(derivation_path.split("/")[-1]))
+
+        if len(expected_result) == 4:
+            assert(actual_result["index"] == expected_result[3])
+        else:
+            assert(actual_result["index"] == int(derivation_path.split("/")[-1]))