Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions src/seedsigner/helpers/embit_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,12 @@ def parse_derivation_path(derivation_path: str) -> dict:

sections = derivation_path.split("/")

if sections[1] == "48h":
# Paths shorter than m/purpose/network/... are invalid for standard matching,
# but we still parse leniently so callers can handle the result gracefully.
purpose = sections[1] if len(sections) > 1 else None
network = sections[2] if len(sections) > 2 else None

if purpose == "48h":
# So far this helper is only meant for single sig message signing
raise Exception("Not implemented")

Expand All @@ -154,20 +159,23 @@ def parse_derivation_path(derivation_path: str) -> dict:
}

details = dict()
details["script_type"] = lookups["script_types"].get(sections[1])
details["script_type"] = lookups["script_types"].get(purpose)
if not details["script_type"]:
details["script_type"] = SettingsConstants.CUSTOM_DERIVATION
details["network"] = lookups["networks"].get(sections[2])
details["network"] = lookups["networks"].get(network)

change = sections[-2] if len(sections) > 1 else None
index = sections[-1] if len(sections) > 0 else None

# Check if there's a standard change path
if sections[-2] in ["0", "1"]:
details["is_change"] = sections[-2] == "1"
if change in ["0", "1"]:
details["is_change"] = change == "1"
else:
details["is_change"] = None

# Check if there's a standard address index
if sections[-1].isdigit():
details["index"] = int(sections[-1])
if index and index.isdigit():
details["index"] = int(index)
else:
details["index"] = None

Expand Down
43 changes: 33 additions & 10 deletions src/seedsigner/views/tools_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -571,21 +571,31 @@ def __init__(self, seed_num: int = None, script_type: str = None, custom_derivat
self.seed = self.controller.storage.seeds[seed_num]
data["seed_num"] = self.seed
seed_derivation_override = self.seed.derivation_override(sig_type=SettingsConstants.SINGLE_SIG)

custom_derivation_details = None
from seedsigner.helpers import embit_utils
if self.script_type == SettingsConstants.CUSTOM_DERIVATION:
derivation_path = self.custom_derivation
custom_derivation_details = embit_utils.parse_derivation_path(derivation_path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_derivation_path() does sections[2] unconditionally (embit_utils.py). A short custom path like will IndexError here. Could be a follow-up issue since existing callers in seed_views.py have the same problem.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I should fix the parse_derivation_path() then to be safe for length

elif seed_derivation_override:
derivation_path = seed_derivation_override
else:
from seedsigner.helpers import embit_utils
derivation_path = embit_utils.get_standard_derivation_path(
network=self.settings.get_value(SettingsConstants.SETTING__NETWORK),
wallet_type=SettingsConstants.SINGLE_SIG,
script_type=self.script_type,
)

xpub_derivation_path = derivation_path
if custom_derivation_details and custom_derivation_details["wallet_derivation_path"]:
# If the user entered a full path that includes /change/index, derive
# from the wallet-level path so we can still enumerate addresses.
# The receive/change branch is selected later via the UI toggle.
xpub_derivation_path = custom_derivation_details["wallet_derivation_path"]

data["derivation_path"] = derivation_path
data["xpub"] = self.seed.get_xpub(derivation_path, network=network)
if custom_derivation_details:
data["custom_derivation_details"] = custom_derivation_details
data["xpub"] = self.seed.get_xpub(xpub_derivation_path, network=network)

else:
data["wallet_descriptor"] = self.controller.multisig_wallet_descriptor
Expand Down Expand Up @@ -675,14 +685,27 @@ def run(self):
if "xpub" in data:
# Single sig explore from seed
if "script_type" in data and data["script_type"] != SettingsConstants.CUSTOM_DERIVATION:
# Standard derivation path
for i in range(self.start_index, self.start_index + addrs_per_screen):
address = embit_utils.get_single_sig_address(xpub=data["xpub"], script_type=data["script_type"], index=i, is_change=self.is_change, embit_network=data["embit_network"])
addresses.append(address)
data[addr_storage_key].append(address)
script_type = data["script_type"]
index_offset = 0

else:
# TODO: Custom derivation path
raise Exception(_("Custom Derivation address explorer not yet implemented"))
custom_derivation_details = data.get("custom_derivation_details")
if not custom_derivation_details:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fallback can never execute as __init__ always stores custom_derivation_details in data. I think this is dead code. If not, feel free to correct me or some follow up functionality can be there.

raise Exception(_("Routing error: missing custom derivation details"))

script_type = custom_derivation_details["script_type"]
if script_type == SettingsConstants.CUSTOM_DERIVATION:
raise Exception(_("Address Explorer does not support non-standard custom script paths"))

# Keep the user-entered index as the pagination start offset.
# Intentionally ignore parsed `is_change`: branch selection comes
# from the current Receive/Change screen selection (`self.is_change`).
index_offset = custom_derivation_details["index"] if custom_derivation_details["index"] is not None else 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index is used as offset here, but is_change from the parsed path is silently ignored, the UI toggle overrides it. If intentional, a comment explaining this would help.


for i in range(self.start_index + index_offset, self.start_index + index_offset + addrs_per_screen):
address = embit_utils.get_single_sig_address(xpub=data["xpub"], script_type=script_type, index=i, is_change=self.is_change, embit_network=data["embit_network"])
addresses.append(address)
data[addr_storage_key].append(address)

elif "wallet_descriptor" in data:
from embit.descriptor import Descriptor
Expand Down
7 changes: 7 additions & 0 deletions tests/test_embit_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,3 +455,10 @@ def test_parse_derivation_path():
assert actual_result["index"] == expected_result[3]
else:
assert actual_result["index"] == int(derivation_path.split("/")[-1])


def test_parse_derivation_path_short_custom_paths_do_not_crash():
for derivation_path in ["m", "m/9h", "m/9h/78"]:
parsed = embit_utils.parse_derivation_path(derivation_path)
assert parsed["script_type"] == SC.CUSTOM_DERIVATION
assert parsed["clean_match"] is False
34 changes: 34 additions & 0 deletions tests/test_flows_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,40 @@ def test__address_explorer__flow(self):
])


def test__address_explorer__custom_derivation_flow(self):
"""
Address Explorer should support custom derivation paths that still map to a
known single sig script type.
"""
controller = Controller.get_instance()
seed = Seed(mnemonic=["abandon "* 11 + "about"])
controller.storage.set_pending_seed(seed)
controller.storage.finalize_pending_seed()

self.settings.set_value(
SettingsConstants.SETTING__SCRIPT_TYPES,
[SettingsConstants.NATIVE_SEGWIT, SettingsConstants.CUSTOM_DERIVATION],
)

custom_derivation_selection = ButtonOption(
SettingsDefinition.get_settings_entry(SettingsConstants.SETTING__SCRIPT_TYPES).get_selection_option_display_name_by_value(SettingsConstants.CUSTOM_DERIVATION),
return_data=SettingsConstants.CUSTOM_DERIVATION,
)

self.run_sequence([
FlowStep(MainMenuView, button_data_selection=MainMenuView.TOOLS),
FlowStep(tools_views.ToolsMenuView, button_data_selection=tools_views.ToolsMenuView.ADDRESS_EXPLORER),
FlowStep(tools_views.ToolsAddressExplorerSelectSourceView, screen_return_value=0), # ret 1st onboard seed
FlowStep(seed_views.SeedExportXpubScriptTypeView, button_data_selection=custom_derivation_selection),
FlowStep(seed_views.SeedExportXpubCustomDerivationView, screen_return_value="m/44'/1237'/0'/0/0"),
FlowStep(tools_views.ToolsAddressExplorerAddressTypeView, button_data_selection=tools_views.ToolsAddressExplorerAddressTypeView.RECEIVE),
FlowStep(tools_views.ToolsAddressExplorerAddressListView, screen_return_value=10), # ret NEXT page of addrs
FlowStep(tools_views.ToolsAddressExplorerAddressListView, screen_return_value=4), # ret a specific addr from the list
FlowStep(tools_views.ToolsAddressExplorerAddressView), # runs until dismissed; no ret value
FlowStep(tools_views.ToolsAddressExplorerAddressListView),
])


def test__address_explorer__loadseed__sideflow(self):
"""
Finalizing a seed during the Address Explorer flow should return to the next
Expand Down