From 8017f11e95ca0162bbda25c34f596eda8f5911eb Mon Sep 17 00:00:00 2001 From: kdmukai Date: Mon, 19 Jun 2023 20:40:12 -0500 Subject: [PATCH 01/11] interim commit --- src/seedsigner/models/settings_definition.py | 32 ++++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/seedsigner/models/settings_definition.py b/src/seedsigner/models/settings_definition.py index 6c2af745..caba7977 100644 --- a/src/seedsigner/models/settings_definition.py +++ b/src/seedsigner/models/settings_definition.py @@ -212,7 +212,6 @@ class SettingsEntry: category: str attr_name: str display_name: str - verbose_name: str = None abbreviated_name: str = None visibility: str = SettingsConstants.VISIBILITY__GENERAL type: str = SettingsConstants.TYPE__ENABLED_DISABLED @@ -228,7 +227,7 @@ class SettingsEntry: self.selection_options = SettingsConstants.OPTIONS__ENABLED_DISABLED_PROMPT elif self.type == SettingsConstants.TYPE__ENABLED_DISABLED_PROMPT_REQUIRED: - self.selection_options = [SettingsConstants.ALL_OPTIONS] + self.selection_options = SettingsConstants.ALL_OPTIONS # Account for List[tuple] and tuple formats as default_value if type(self.default_value) == list and type(self.default_value[0]) == tuple: @@ -247,18 +246,12 @@ class SettingsEntry: def get_selection_option_value(self, i: int): + """ Returns the value of the selection option at index `i` """ value = self.selection_options[i] if type(value) == tuple: value = value[0] return value - - def get_selection_option_display_name(self, i: int) -> str: - value = self.selection_options[i] - if type(value) == tuple: - value = value[1] - return value - def get_selection_option_display_name_by_value(self, value) -> str: for option in self.selection_options: @@ -304,9 +297,8 @@ class SettingsEntry: return { "category": self.category, "attr_name": self.attr_name, - "display_name": self.display_name, - "verbose_name": self.verbose_name, "abbreviated_name": self.abbreviated_name, + "display_name": self.display_name, "visibility": self.visibility, "type": self.type, "help_text": self.help_text, @@ -339,6 +331,7 @@ class SettingsDefinition: # TODO: Full babel multilanguage support! Until then, type == HIDDEN SettingsEntry(category=SettingsConstants.CATEGORY__SYSTEM, attr_name=SettingsConstants.SETTING__LANGUAGE, + abbreviated_name="lang", display_name="Language", type=SettingsConstants.TYPE__SELECT_1, visibility=SettingsConstants.VISIBILITY__HIDDEN, @@ -348,6 +341,7 @@ class SettingsDefinition: # TODO: Support other bip-39 wordlist languages! Until then, type == HIDDEN SettingsEntry(category=SettingsConstants.CATEGORY__SYSTEM, attr_name=SettingsConstants.SETTING__WORDLIST_LANGUAGE, + abbreviated_name="wordlist_lang", display_name="Mnemonic language", type=SettingsConstants.TYPE__SELECT_1, visibility=SettingsConstants.VISIBILITY__HIDDEN, @@ -356,12 +350,14 @@ class SettingsDefinition: SettingsEntry(category=SettingsConstants.CATEGORY__SYSTEM, attr_name=SettingsConstants.SETTING__PERSISTENT_SETTINGS, + abbreviated_name="persistent", display_name="Persistent settings", help_text="Store Settings on SD card.", default_value=SettingsConstants.OPTION__DISABLED), SettingsEntry(category=SettingsConstants.CATEGORY__WALLET, attr_name=SettingsConstants.SETTING__COORDINATORS, + abbreviated_name="coords", display_name="Coordinator software", type=SettingsConstants.TYPE__MULTISELECT, selection_options=SettingsConstants.ALL_COORDINATORS, @@ -374,6 +370,7 @@ class SettingsDefinition: SettingsEntry(category=SettingsConstants.CATEGORY__SYSTEM, attr_name=SettingsConstants.SETTING__BTC_DENOMINATION, + abbreviated_name="denom", display_name="Denomination display", type=SettingsConstants.TYPE__SELECT_1, selection_options=SettingsConstants.ALL_BTC_DENOMINATIONS, @@ -405,6 +402,7 @@ class SettingsDefinition: SettingsEntry(category=SettingsConstants.CATEGORY__FEATURES, attr_name=SettingsConstants.SETTING__SIG_TYPES, + abbreviated_name="sigs", display_name="Sig types", type=SettingsConstants.TYPE__MULTISELECT, visibility=SettingsConstants.VISIBILITY__ADVANCED, @@ -413,6 +411,7 @@ class SettingsDefinition: SettingsEntry(category=SettingsConstants.CATEGORY__FEATURES, attr_name=SettingsConstants.SETTING__SCRIPT_TYPES, + abbreviated_name="scripts", display_name="Script types", type=SettingsConstants.TYPE__MULTISELECT, visibility=SettingsConstants.VISIBILITY__ADVANCED, @@ -435,6 +434,7 @@ class SettingsDefinition: SettingsEntry(category=SettingsConstants.CATEGORY__FEATURES, attr_name=SettingsConstants.SETTING__CAMERA_ROTATION, + abbreviated_name="camera", display_name="Camera rotation", type=SettingsConstants.TYPE__SELECT_1, visibility=SettingsConstants.VISIBILITY__ADVANCED, @@ -449,24 +449,28 @@ class SettingsDefinition: SettingsEntry(category=SettingsConstants.CATEGORY__FEATURES, attr_name=SettingsConstants.SETTING__BIP85_CHILD_SEEDS, + abbreviated_name="bip85", display_name="BIP-85 child seeds", visibility=SettingsConstants.VISIBILITY__ADVANCED, default_value=SettingsConstants.OPTION__DISABLED), SettingsEntry(category=SettingsConstants.CATEGORY__FEATURES, attr_name=SettingsConstants.SETTING__PRIVACY_WARNINGS, + abbreviated_name="priv_warn", display_name="Show privacy warnings", visibility=SettingsConstants.VISIBILITY__ADVANCED, default_value=SettingsConstants.OPTION__ENABLED), SettingsEntry(category=SettingsConstants.CATEGORY__FEATURES, attr_name=SettingsConstants.SETTING__DIRE_WARNINGS, + abbreviated_name="dire_warn", display_name="Show dire warnings", visibility=SettingsConstants.VISIBILITY__ADVANCED, default_value=SettingsConstants.OPTION__ENABLED), SettingsEntry(category=SettingsConstants.CATEGORY__FEATURES, attr_name=SettingsConstants.SETTING__PARTNER_LOGOS, + abbreviated_name="partners", display_name="Show partner logos", visibility=SettingsConstants.VISIBILITY__ADVANCED, default_value=SettingsConstants.OPTION__ENABLED), @@ -482,6 +486,7 @@ class SettingsDefinition: # "Hidden" settings with no UI interaction SettingsEntry(category=SettingsConstants.CATEGORY__SYSTEM, attr_name=SettingsConstants.SETTING__QR_BRIGHTNESS, + abbreviated_name="qr_brightness", display_name="QR background color", type=SettingsConstants.TYPE__FREE_ENTRY, visibility=SettingsConstants.VISIBILITY__HIDDEN, @@ -505,11 +510,6 @@ class SettingsDefinition: return entry - @classmethod - def parse_abbreviated_ini(cls, abbreviated_ini: str) -> dict: - raise Exception("Not implemented, maybe not needed") - - @classmethod def get_defaults(cls) -> dict: as_dict = {} From 99f1cb328b4bb0b98ea699278c94f163939a9579 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Tue, 20 Jun 2023 08:48:35 -0500 Subject: [PATCH 02/11] simplify/improve ingesting SettingsQR --- src/seedsigner/models/decode_qr.py | 121 ++++++------------- src/seedsigner/models/settings.py | 38 ++---- src/seedsigner/models/settings_definition.py | 7 ++ 3 files changed, 54 insertions(+), 112 deletions(-) diff --git a/src/seedsigner/models/decode_qr.py b/src/seedsigner/models/decode_qr.py index 3928ff8e..d35a24ea 100644 --- a/src/seedsigner/models/decode_qr.py +++ b/src/seedsigner/models/decode_qr.py @@ -9,11 +9,11 @@ from embit import psbt, bip39 from pyzbar import pyzbar from pyzbar.pyzbar import ZBarSymbol from urtypes.crypto import PSBT as UR_PSBT -from urtypes.crypto import Account, HDKey, Output, Keypath, PathComponent, SCRIPT_EXPRESSION_TAG_MAP +from urtypes.crypto import Account, Output from urtypes.bytes import Bytes from seedsigner.helpers.ur2.ur_decoder import URDecoder -from seedsigner.models.psbt_parser import PSBTParser +from seedsigner.models.settings_definition import SettingsDefinition from . import QRType, Seed from .settings import SettingsConstants @@ -364,7 +364,7 @@ class DecodeQR: return QRType.BITCOIN_ADDRESS # config data - if "type=settings" in s: + if s.startswith("settings::"): return QRType.SETTINGS # Seed @@ -835,90 +835,49 @@ class SettingsQrDecoder(BaseSingleFrameQrDecoder): def add(self, segment, qr_type=QRType.SETTINGS): - # print(f"SettingsQR:\n{segment}") + print(f"SettingsQR:\n{segment}") try: - self.settings = {} + if not segment.startswith("settings::"): + raise Exception("Invalid SettingsQR data") - # QR Settings format is space-separated key/value pairs, but should also - # parse \n-separated keys. - for entry in segment.split(): - key = entry.split("=")[0].strip() - value = entry.split("=")[1].strip() - self.settings[key] = value - - # Remove values only needed for import - self.settings.pop("type", None) - version = self.settings.pop("version", None) - if not version or int(version) != 1: - raise Exception(f"Settings QR version {version} not supported") - - self.config_name = self.settings.pop("name", None) - if self.config_name: - self.config_name = self.config_name.replace("_", " ") + version = segment.split(" ")[0].split("::")[1] + if version != "v1": + raise Exception(f"Unsupported SettingsQR version: {version}") - # Have to translate the abbreviated settings into the human-readable values - # used in the normal Settings. - map_abbreviated_enable = { - "0": SettingsConstants.OPTION__DISABLED, - "1": SettingsConstants.OPTION__ENABLED, - "2": SettingsConstants.OPTION__PROMPT, - } - map_abbreviated_sig_types = { - "s": SettingsConstants.SINGLE_SIG, - "m": SettingsConstants.MULTISIG, - } - map_abbreviated_scripts = { - "na": SettingsConstants.NATIVE_SEGWIT, - "ne": SettingsConstants.NESTED_SEGWIT, - "tr": SettingsConstants.TAPROOT, - "cu": SettingsConstants.CUSTOM_DERIVATION, - } - map_abbreviated_coordinators = { - "bw": SettingsConstants.COORDINATOR__BLUE_WALLET, - "sw": SettingsConstants.COORDINATOR__SPARROW, - "sd": SettingsConstants.COORDINATOR__SPECTER_DESKTOP, - } + # Start parsing key/value settings at the nth split(" ") index + split_index = 1 - def convert_abbreviated_value(category, key, abbreviation_map, is_list=False, new_key_name=None): - try: - if key not in self.settings: - print(f"'{key}' not found in settings") - return - value = self.settings[key] + # handle optional "name" attr + if " name=" in segment: + self.config_name = segment.split(" name=")[1].split(" ")[0] + split_index += 1 - if not is_list: - new_value = abbreviation_map.get(value) - if not new_value: - logger.error(f"No abbreviation map value for \"{value}\" for setting {key}") - return - else: - # `value` is a comma-separated list; yields list of map matches - values = value.split(",") - new_value = [] - for v in values: - mapped_value = abbreviation_map.get(v) - if not mapped_value: - logger.error(f"No abbreviation map value for \"{v}\" for setting {key}") - return - new_value.append(mapped_value) - del self.settings[key] - if new_key_name: - key = new_key_name - if category not in self.settings: - self.settings[category] = {} - self.settings[category][key] = new_value - except Exception as e: - logger.exception(e) - return + self.settings = {} + for entry in segment.split(" ")[split_index:]: + abbreviated_name, value = entry.split("=") + if "," in value: + value = value.split(",") + elif value.isdigit(): + value = int(value) + + # Replace abbreviated name with full attr_name + settings_entry = SettingsDefinition.get_settings_entry_by_abbreviated_name(abbreviated_name) + if not settings_entry: + logger.warn(f"Ignoring unrecognized attribute: {abbreviated_name}") + continue - convert_abbreviated_value("wallet", "coord", map_abbreviated_coordinators, is_list=True, new_key_name="coordinators") - convert_abbreviated_value("features", "xpub", map_abbreviated_enable, new_key_name="xpub_export") - convert_abbreviated_value("features", "sigs", map_abbreviated_sig_types, is_list=True, new_key_name="sig_types") - convert_abbreviated_value("features", "scripts", map_abbreviated_scripts, is_list=True, new_key_name="script_types") - convert_abbreviated_value("features", "xp_det", map_abbreviated_enable, new_key_name="show_xpub_details") - convert_abbreviated_value("features", "passphrase", map_abbreviated_enable) - convert_abbreviated_value("features", "priv_warn", map_abbreviated_enable, new_key_name="show_privacy_warnings") - convert_abbreviated_value("features", "dire_warn", map_abbreviated_enable, new_key_name="show_dire_warnings") + # Validate value(s) against SettingsDefinition's valid options + if type(value) is not list: + values = [value] + else: + values = value + for v in values: + if v not in [opt[0] for opt in settings_entry.selection_options]: + raise Exception(f"Invalid value for {settings_entry.attr_name}: {v} ({settings_entry.selection_options})") + + self.settings[settings_entry.attr_name] = value + + print(f"new settings: {self.settings}") self.complete = True self.collected_segments = 1 diff --git a/src/seedsigner/models/settings.py b/src/seedsigner/models/settings.py index 97c426f0..1b426e21 100644 --- a/src/seedsigner/models/settings.py +++ b/src/seedsigner/models/settings.py @@ -28,7 +28,7 @@ class Settings(Singleton): # Read persistent settings file, if it exists if os.path.exists(Settings.SETTINGS_FILENAME): with open(Settings.SETTINGS_FILENAME) as settings_file: - settings.update(json.load(settings_file), disable_missing_entries=False) + settings.update(json.load(settings_file)) return cls._instance @@ -47,46 +47,22 @@ class Settings(Singleton): os.fsync(settings_file.fileno()) - def update(self, new_settings: dict, disable_missing_entries: bool = True): + def update(self, new_settings: dict, ): """ - * disable_missing_entries: The SettingsQR Generator omits any multiselect - fields with zero selections or disabled Enabled/Disabled toggles. So if a - field is missing, interpret it as such. But if this is set to False, keep - the existing value for the field; most likely this is a new setting that - the user may not have a value for when loading their persistent settings, - in which case this would preserve the new field's default value. """ for entry in SettingsDefinition.settings_entries: if entry.attr_name not in new_settings: - if not disable_missing_entries: + if entry.visibility == SettingsConstants.VISIBILITY__HIDDEN and entry.attr_name in self._data: + # Preserve existing hidden values + new_settings[entry.attr_name] = self._data[entry.attr_name] + else: # Setting is missing; insert default new_settings[entry.attr_name] = entry.default_value - - elif entry.visibility == SettingsConstants.VISIBILITY__HIDDEN: - # Missing hidden values always get their default - new_settings[entry.attr_name] = entry.default_value - - elif entry.type == SettingsConstants.TYPE__MULTISELECT: - # Clear out the multiselect - new_settings[entry.attr_name] = [] - - elif entry.type in SettingsConstants.ALL_ENABLED_DISABLED_TYPES: - # Set DISABLED for this missing setting - new_settings[entry.attr_name] = SettingsConstants.OPTION__DISABLED - - else: - # Clean the incoming data, if necessary - if entry.type == SettingsConstants.TYPE__MULTISELECT: - if type(new_settings[entry.attr_name]) == str: - # Break comma-separated SettingsQR input into List - new_settings[entry.attr_name] = new_settings[entry.attr_name].split(",") - - # TODO: If value is not in entry.selection_options... - # Can't just merge the _data dict; have to replace keys they have in common # (otherwise list values will be merged instead of replaced). for key, value in new_settings.items(): + print(f"Updating {key} to {value} ({type(value)})") self._data.pop(key, None) self._data[key] = value diff --git a/src/seedsigner/models/settings_definition.py b/src/seedsigner/models/settings_definition.py index caba7977..1e2d0071 100644 --- a/src/seedsigner/models/settings_definition.py +++ b/src/seedsigner/models/settings_definition.py @@ -510,6 +510,13 @@ class SettingsDefinition: return entry + @classmethod + def get_settings_entry_by_abbreviated_name(cls, abbreviated_name: str) -> SettingsEntry: + for entry in cls.settings_entries: + if abbreviated_name in [entry.abbreviated_name, entry.attr_name]: + return entry + + @classmethod def get_defaults(cls) -> dict: as_dict = {} From 5a95818c88201c655b7af5833b7d8f7285bd1bc6 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Wed, 21 Jun 2023 07:19:57 -0500 Subject: [PATCH 03/11] SettingsQR support for any kind of whitespace * more docstrings * misc cleanup --- src/seedsigner/models/decode_qr.py | 37 +++++++++++++++++++----------- src/seedsigner/models/settings.py | 7 +++++- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/seedsigner/models/decode_qr.py b/src/seedsigner/models/decode_qr.py index d35a24ea..5c45f698 100644 --- a/src/seedsigner/models/decode_qr.py +++ b/src/seedsigner/models/decode_qr.py @@ -13,11 +13,10 @@ from urtypes.crypto import Account, Output from urtypes.bytes import Bytes from seedsigner.helpers.ur2.ur_decoder import URDecoder +from seedsigner.models import QRType, Seed +from seedsigner.models.settings import SettingsConstants from seedsigner.models.settings_definition import SettingsDefinition -from . import QRType, Seed -from .settings import SettingsConstants - logger = logging.getLogger(__name__) @@ -578,6 +577,8 @@ class DecodeQR: return descriptor + + class BaseQrDecoder: def __init__(self): self.total_segments = None @@ -673,6 +674,8 @@ class SpecterPsbtQrDecoder(BaseAnimatedQrDecoder): def parse_segment(self, segment) -> str: return segment.split(" ")[-1].strip() + + class Base64PsbtQrDecoder(BaseSingleFrameQrDecoder): """ Decodes single frame base64 encoded qr image. @@ -826,8 +829,10 @@ class SeedQrDecoder(BaseSingleFrameQrDecoder): -# TODO: Refactor this to work with the new SettingsDefinition class SettingsQrDecoder(BaseSingleFrameQrDecoder): + """ + Decodes settings data from the SettingsQR Generator. + """ def __init__(self): super().__init__() self.settings = {} @@ -835,25 +840,31 @@ class SettingsQrDecoder(BaseSingleFrameQrDecoder): def add(self, segment, qr_type=QRType.SETTINGS): - print(f"SettingsQR:\n{segment}") + """ + * Ignores unrecognized settings options. + * Raises an Exception if a settings value is invalid. + + See `Settings.update()` for info on settings validation, especially for + missing settings. + """ try: if not segment.startswith("settings::"): raise Exception("Invalid SettingsQR data") - version = segment.split(" ")[0].split("::")[1] + version = segment.split()[0].split("::")[1] if version != "v1": raise Exception(f"Unsupported SettingsQR version: {version}") - # Start parsing key/value settings at the nth split(" ") index + # Start parsing key/value settings at the nth split() index split_index = 1 # handle optional "name" attr - if " name=" in segment: - self.config_name = segment.split(" name=")[1].split(" ")[0] + if "name=" in segment.split()[1]: + self.config_name = segment.split("name=")[1].split()[0].replace("_", " ") split_index += 1 self.settings = {} - for entry in segment.split(" ")[split_index:]: + for entry in segment.split()[split_index:]: abbreviated_name, value = entry.split("=") if "," in value: value = value.split(",") @@ -1050,11 +1061,11 @@ class GenericWalletQrDecoder(BaseSingleFrameQrDecoder): def get_wallet_descriptor(self): return self.descriptor - + + + class MultiSigConfigFileQRDecoder(GenericWalletQrDecoder): def add(self, segment, qr_type=QRType.WALLET__CONFIGFILE): descriptor = DecodeQR.multisig_setup_file_to_descriptor(segment) return super().add(descriptor,qr_type=QRType.WALLET__CONFIGFILE) - - diff --git a/src/seedsigner/models/settings.py b/src/seedsigner/models/settings.py index 1b426e21..09fdfd2c 100644 --- a/src/seedsigner/models/settings.py +++ b/src/seedsigner/models/settings.py @@ -49,6 +49,11 @@ class Settings(Singleton): def update(self, new_settings: dict, ): """ + Replaces the current settings with the incoming dict. + + If a setting is missing from `new_settings`: + * Hidden settings that have a value remain as-is. + * All other missing settings are set to their default value. """ for entry in SettingsDefinition.settings_entries: if entry.attr_name not in new_settings: @@ -62,7 +67,7 @@ class Settings(Singleton): # Can't just merge the _data dict; have to replace keys they have in common # (otherwise list values will be merged instead of replaced). for key, value in new_settings.items(): - print(f"Updating {key} to {value} ({type(value)})") + # print(f"Updating {key} to {value} ({type(value)})") self._data.pop(key, None) self._data[key] = value From f7c8213f6f150e084b98e77d82cbfe7e5ba1fd8d Mon Sep 17 00:00:00 2001 From: kdmukai Date: Wed, 21 Jun 2023 07:53:01 -0500 Subject: [PATCH 04/11] Debugging cleanup --- src/seedsigner/models/decode_qr.py | 2 -- src/seedsigner/models/settings.py | 1 - 2 files changed, 3 deletions(-) diff --git a/src/seedsigner/models/decode_qr.py b/src/seedsigner/models/decode_qr.py index 5c45f698..e1883386 100644 --- a/src/seedsigner/models/decode_qr.py +++ b/src/seedsigner/models/decode_qr.py @@ -888,8 +888,6 @@ class SettingsQrDecoder(BaseSingleFrameQrDecoder): self.settings[settings_entry.attr_name] = value - print(f"new settings: {self.settings}") - self.complete = True self.collected_segments = 1 return DecodeQRStatus.COMPLETE diff --git a/src/seedsigner/models/settings.py b/src/seedsigner/models/settings.py index 09fdfd2c..e110acec 100644 --- a/src/seedsigner/models/settings.py +++ b/src/seedsigner/models/settings.py @@ -67,7 +67,6 @@ class Settings(Singleton): # Can't just merge the _data dict; have to replace keys they have in common # (otherwise list values will be merged instead of replaced). for key, value in new_settings.items(): - # print(f"Updating {key} to {value} ({type(value)})") self._data.pop(key, None) self._data[key] = value From 4eb90500401de99eab2b72da8ade32f3b9f06e72 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Sun, 30 Jul 2023 15:37:59 -0500 Subject: [PATCH 05/11] `dev` merge, cleanup, added tests --- src/seedsigner/models/decode_qr.py | 2 +- tests/test_flows_seed.py | 8 +-- tests/test_settingsqr_decoder.py | 102 +++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 tests/test_settingsqr_decoder.py diff --git a/src/seedsigner/models/decode_qr.py b/src/seedsigner/models/decode_qr.py index e1883386..b4542722 100644 --- a/src/seedsigner/models/decode_qr.py +++ b/src/seedsigner/models/decode_qr.py @@ -874,7 +874,7 @@ class SettingsQrDecoder(BaseSingleFrameQrDecoder): # Replace abbreviated name with full attr_name settings_entry = SettingsDefinition.get_settings_entry_by_abbreviated_name(abbreviated_name) if not settings_entry: - logger.warn(f"Ignoring unrecognized attribute: {abbreviated_name}") + logger.warning(f"Ignoring unrecognized attribute: {abbreviated_name}") continue # Validate value(s) against SettingsDefinition's valid options diff --git a/tests/test_flows_seed.py b/tests/test_flows_seed.py index 973518d3..935cb5de 100644 --- a/tests/test_flows_seed.py +++ b/tests/test_flows_seed.py @@ -248,10 +248,10 @@ class TestSeedFlows(FlowTest): # exclusively set only one choice for each of sig_types, script_types and coordinators self.settings.update({ - SettingsConstants.SETTING__SIG_TYPES: SettingsConstants.MULTISIG, - SettingsConstants.SETTING__SCRIPT_TYPES: SettingsConstants.NESTED_SEGWIT, - SettingsConstants.SETTING__COORDINATORS: SettingsConstants.COORDINATOR__SPECTER_DESKTOP, - }, disable_missing_entries=False) + SettingsConstants.SETTING__SIG_TYPES: [SettingsConstants.MULTISIG], + SettingsConstants.SETTING__SCRIPT_TYPES: [SettingsConstants.NESTED_SEGWIT], + SettingsConstants.SETTING__COORDINATORS: [SettingsConstants.COORDINATOR__SPECTER_DESKTOP], + }) self.run_sequence( initial_destination_view_args=dict(seed_num=0), diff --git a/tests/test_settingsqr_decoder.py b/tests/test_settingsqr_decoder.py new file mode 100644 index 00000000..c9c60142 --- /dev/null +++ b/tests/test_settingsqr_decoder.py @@ -0,0 +1,102 @@ +from seedsigner.models.decode_qr import DecodeQR, DecodeQRStatus +from seedsigner.models.settings import Settings +from seedsigner.models.settings_definition import SettingsConstants + + + +class TestSettingsQRDecoder: + @classmethod + def setup_class(cls): + cls.settings = Settings.get_instance() + + + def test_decode_settingsqr(self): + """ + Assume the QR reader decodes the SettingsQR content correctly and begin this test + with parsing the result. + """ + settings_name = "Test SettingsQR" + settings_qr_str = f"""settings::v1 name={ settings_name.replace(" ", "_") } persistent=D coords=spa,spd denom=thr network=M qr_density=M xpub_export=E sigs=ss,ms scripts=nat,nes,tr xpub_details=E passphrase=E camera=180 compact_seedqr=E bip85=D priv_warn=E dire_warn=E partners=E""" + + # First explicitly set settings that differ from the settings_qr_str + self.settings.set_value(SettingsConstants.SETTING__COMPACT_SEEDQR, SettingsConstants.OPTION__DISABLED) + self.settings.set_value(SettingsConstants.SETTING__DIRE_WARNINGS, SettingsConstants.OPTION__DISABLED) + self.settings.set_value(SettingsConstants.SETTING__COORDINATORS, [SettingsConstants.COORDINATOR__BLUE_WALLET, SettingsConstants.COORDINATOR__SPARROW]) + + # Now parse the settings_qr_str + decoder = DecodeQR() + status = decoder.add_data(settings_qr_str) + assert(decoder.is_settings) + assert(status == DecodeQRStatus.COMPLETE) + + parsed_name = decoder.get_settings_config_name() + assert(parsed_name == settings_name) + + settings_data = decoder.get_settings_data() + self.settings.update(new_settings=settings_data) + + # Now verify that the settings were updated correctly + assert(self.settings.get_value(SettingsConstants.SETTING__COMPACT_SEEDQR) == SettingsConstants.OPTION__ENABLED) + assert(self.settings.get_value(SettingsConstants.SETTING__DIRE_WARNINGS) == SettingsConstants.OPTION__ENABLED) + + coordinators = self.settings.get_value(SettingsConstants.SETTING__COORDINATORS) + assert(SettingsConstants.COORDINATOR__BLUE_WALLET not in coordinators) + assert(SettingsConstants.COORDINATOR__SPARROW in coordinators) + assert(SettingsConstants.COORDINATOR__SPECTER_DESKTOP in coordinators) + + + def test_settingsqr_version(self): + """ Should accept SettingsQR v1 and reject any others """ + settings_qr_str = "settings::v1 name=Foo" + decoder = DecodeQR() + status = decoder.add_data(settings_qr_str) + assert(decoder.is_settings) + assert(status == DecodeQRStatus.COMPLETE) + + settings_qr_str = "settings::v2 name=Foo" + decoder = DecodeQR() + status = decoder.add_data(settings_qr_str) + assert(decoder.is_settings) + assert(status == DecodeQRStatus.INVALID) + + # Should also fail if version omitted + settings_qr_str = "settings name=Foo" + decoder = DecodeQR() + status = decoder.add_data(settings_qr_str) + assert(decoder.is_settings is False) + assert(status == DecodeQRStatus.INVALID) + + + def test_settingsqr_ignores_unrecognized_setting(self): + """ SettingsQR decoder should ignore unrecognized settings """ + settings_qr_str = "settings::v1 name=Foo favorite_food=bacon xpub_export=D" + decoder = DecodeQR() + status = decoder.add_data(settings_qr_str) + assert(decoder.is_settings) + assert(status == DecodeQRStatus.COMPLETE) + + settings_data = decoder.get_settings_data() + assert("favorite_food" not in settings_data) + assert("xpub_export" in settings_data) + + + def test_settingsqr_fails_unrecognized_option(self): + """ SettingsQR decoder should fail if an unrecognized option is provided """ + settings_qr_str = "settings::v2 name=Foo xpub_export=Yep" + decoder = DecodeQR() + status = decoder.add_data(settings_qr_str) + assert(decoder.is_settings) + assert(status == DecodeQRStatus.INVALID) + + + def test_settingsqr_parses_line_break_separators(self): + """ SettingsQR decoder should read line breaks as acceptable separators """ + settings_qr_str = "settings::v1\nname=Foo\nsigs=ss,ms\nscripts=nat,nes,tr\nxpub_export=E\n" + decoder = DecodeQR() + status = decoder.add_data(settings_qr_str) + assert(decoder.is_settings) + assert(status == DecodeQRStatus.COMPLETE) + + settings_data = decoder.get_settings_data() + assert(len(settings_data.keys()) == 3) + self.settings.update(new_settings=settings_data) From 494872463ba9b0c3037d62373968f0caa9fd6bc8 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Sun, 30 Jul 2023 15:53:46 -0500 Subject: [PATCH 06/11] Exceptions bubble up to UI --- src/seedsigner/models/decode_qr.py | 80 ++++++++++++++---------------- tests/test_settingsqr_decoder.py | 22 ++++---- 2 files changed, 49 insertions(+), 53 deletions(-) diff --git a/src/seedsigner/models/decode_qr.py b/src/seedsigner/models/decode_qr.py index b4542722..b7bbbe2c 100644 --- a/src/seedsigner/models/decode_qr.py +++ b/src/seedsigner/models/decode_qr.py @@ -847,53 +847,49 @@ class SettingsQrDecoder(BaseSingleFrameQrDecoder): See `Settings.update()` for info on settings validation, especially for missing settings. """ - try: - if not segment.startswith("settings::"): - raise Exception("Invalid SettingsQR data") + if not segment.startswith("settings::"): + raise Exception("Invalid SettingsQR data") - version = segment.split()[0].split("::")[1] - if version != "v1": - raise Exception(f"Unsupported SettingsQR version: {version}") + version = segment.split()[0].split("::")[1] + if version != "v1": + raise Exception(f"Unsupported SettingsQR version: {version}") + + # Start parsing key/value settings at the nth split() index + split_index = 1 + + # handle optional "name" attr + if "name=" in segment.split()[1]: + self.config_name = segment.split("name=")[1].split()[0].replace("_", " ") + split_index += 1 + + self.settings = {} + for entry in segment.split()[split_index:]: + abbreviated_name, value = entry.split("=") + if "," in value: + value = value.split(",") + elif value.isdigit(): + value = int(value) - # Start parsing key/value settings at the nth split() index - split_index = 1 + # Replace abbreviated name with full attr_name + settings_entry = SettingsDefinition.get_settings_entry_by_abbreviated_name(abbreviated_name) + if not settings_entry: + logger.warning(f"Ignoring unrecognized attribute: {abbreviated_name}") + continue - # handle optional "name" attr - if "name=" in segment.split()[1]: - self.config_name = segment.split("name=")[1].split()[0].replace("_", " ") - split_index += 1 + # Validate value(s) against SettingsDefinition's valid options + if type(value) is not list: + values = [value] + else: + values = value + for v in values: + if v not in [opt[0] for opt in settings_entry.selection_options]: + raise Exception(f"Invalid value for {settings_entry.attr_name}: {v} ({settings_entry.selection_options})") - self.settings = {} - for entry in segment.split()[split_index:]: - abbreviated_name, value = entry.split("=") - if "," in value: - value = value.split(",") - elif value.isdigit(): - value = int(value) - - # Replace abbreviated name with full attr_name - settings_entry = SettingsDefinition.get_settings_entry_by_abbreviated_name(abbreviated_name) - if not settings_entry: - logger.warning(f"Ignoring unrecognized attribute: {abbreviated_name}") - continue + self.settings[settings_entry.attr_name] = value - # Validate value(s) against SettingsDefinition's valid options - if type(value) is not list: - values = [value] - else: - values = value - for v in values: - if v not in [opt[0] for opt in settings_entry.selection_options]: - raise Exception(f"Invalid value for {settings_entry.attr_name}: {v} ({settings_entry.selection_options})") - - self.settings[settings_entry.attr_name] = value - - self.complete = True - self.collected_segments = 1 - return DecodeQRStatus.COMPLETE - except Exception as e: - logger.exception(e) - return DecodeQRStatus.INVALID + self.complete = True + self.collected_segments = 1 + return DecodeQRStatus.COMPLETE diff --git a/tests/test_settingsqr_decoder.py b/tests/test_settingsqr_decoder.py index c9c60142..5f8cd101 100644 --- a/tests/test_settingsqr_decoder.py +++ b/tests/test_settingsqr_decoder.py @@ -1,3 +1,4 @@ +import pytest from seedsigner.models.decode_qr import DecodeQR, DecodeQRStatus from seedsigner.models.settings import Settings from seedsigner.models.settings_definition import SettingsConstants @@ -55,17 +56,16 @@ class TestSettingsQRDecoder: settings_qr_str = "settings::v2 name=Foo" decoder = DecodeQR() - status = decoder.add_data(settings_qr_str) - assert(decoder.is_settings) - assert(status == DecodeQRStatus.INVALID) - + with pytest.raises(Exception) as e: + status = decoder.add_data(settings_qr_str) + assert("Unsupported SettingsQR version" in str(e.value)) + # Should also fail if version omitted settings_qr_str = "settings name=Foo" decoder = DecodeQR() status = decoder.add_data(settings_qr_str) - assert(decoder.is_settings is False) assert(status == DecodeQRStatus.INVALID) - + def test_settingsqr_ignores_unrecognized_setting(self): """ SettingsQR decoder should ignore unrecognized settings """ @@ -81,12 +81,12 @@ class TestSettingsQRDecoder: def test_settingsqr_fails_unrecognized_option(self): - """ SettingsQR decoder should fail if an unrecognized option is provided """ - settings_qr_str = "settings::v2 name=Foo xpub_export=Yep" + """ SettingsQR decoder should fail if a settings has an unrecognized option """ + settings_qr_str = "settings::v1 name=Foo xpub_export=Yep" decoder = DecodeQR() - status = decoder.add_data(settings_qr_str) - assert(decoder.is_settings) - assert(status == DecodeQRStatus.INVALID) + with pytest.raises(Exception) as e: + decoder.add_data(settings_qr_str) + assert("Invalid value for" in str(e.value)) def test_settingsqr_parses_line_break_separators(self): From d2838a9ec46c92d2adee37c417debefbfc26ee8e Mon Sep 17 00:00:00 2001 From: kdmukai Date: Mon, 31 Jul 2023 10:55:22 -0500 Subject: [PATCH 07/11] Remove BACK button from SettingsUpdatedScreen --- src/seedsigner/gui/screens/scan_screens.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/seedsigner/gui/screens/scan_screens.py b/src/seedsigner/gui/screens/scan_screens.py index 6484fec1..efa27627 100644 --- a/src/seedsigner/gui/screens/scan_screens.py +++ b/src/seedsigner/gui/screens/scan_screens.py @@ -137,6 +137,7 @@ class SettingsUpdatedScreen(ButtonListScreen): def __post_init__(self): # Customize defaults self.button_data = ["Home"] + self.show_back_button = False super().__post_init__() From ffaee40b99a0869179cd355df2873854a225fbd4 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Mon, 31 Jul 2023 10:59:10 -0500 Subject: [PATCH 08/11] Move all SettingsQR validation from `DecodeQR` to `Settings` * Move post-SettingsQR View to settings_views.py * Improve ingestion cleaning to gracefully handle single-value `Settings.update()` entries for multi-select settings. * Improve Exception handling and onscreen error reporting during SettingsQR parsing. * move tests accordingly out of SettingsQR and into Settings. --- src/seedsigner/models/decode_qr.py | 49 ++------------ src/seedsigner/models/settings.py | 69 +++++++++++++++++++ src/seedsigner/views/scan_views.py | 36 ++-------- src/seedsigner/views/settings_views.py | 27 +++++++- tests/test_flows_seed.py | 6 +- tests/test_settings.py | 91 +++++++++++++++++++++++++- tests/test_settingsqr_decoder.py | 78 ++-------------------- 7 files changed, 200 insertions(+), 156 deletions(-) diff --git a/src/seedsigner/models/decode_qr.py b/src/seedsigner/models/decode_qr.py index b7bbbe2c..3ad13910 100644 --- a/src/seedsigner/models/decode_qr.py +++ b/src/seedsigner/models/decode_qr.py @@ -174,12 +174,7 @@ class DecodeQR: def get_settings_data(self): if self.is_settings: - return self.decoder.settings - - - def get_settings_config_name(self): - if self.is_settings: - return self.decoder.config_name + return self.decoder.data def get_address(self): @@ -835,8 +830,7 @@ class SettingsQrDecoder(BaseSingleFrameQrDecoder): """ def __init__(self): super().__init__() - self.settings = {} - self.config_name = None + self.data = None def add(self, segment, qr_type=QRType.SETTINGS): @@ -849,43 +843,10 @@ class SettingsQrDecoder(BaseSingleFrameQrDecoder): """ if not segment.startswith("settings::"): raise Exception("Invalid SettingsQR data") - - version = segment.split()[0].split("::")[1] - if version != "v1": - raise Exception(f"Unsupported SettingsQR version: {version}") - # Start parsing key/value settings at the nth split() index - split_index = 1 - - # handle optional "name" attr - if "name=" in segment.split()[1]: - self.config_name = segment.split("name=")[1].split()[0].replace("_", " ") - split_index += 1 - - self.settings = {} - for entry in segment.split()[split_index:]: - abbreviated_name, value = entry.split("=") - if "," in value: - value = value.split(",") - elif value.isdigit(): - value = int(value) - - # Replace abbreviated name with full attr_name - settings_entry = SettingsDefinition.get_settings_entry_by_abbreviated_name(abbreviated_name) - if not settings_entry: - logger.warning(f"Ignoring unrecognized attribute: {abbreviated_name}") - continue - - # Validate value(s) against SettingsDefinition's valid options - if type(value) is not list: - values = [value] - else: - values = value - for v in values: - if v not in [opt[0] for opt in settings_entry.selection_options]: - raise Exception(f"Invalid value for {settings_entry.attr_name}: {v} ({settings_entry.selection_options})") - - self.settings[settings_entry.attr_name] = value + # Leave any other parsing or validation up to the Settings class itself. + # SettingsQR are just ascii data to hand it over as-is. + self.data = segment self.complete = True self.collected_segments = 1 diff --git a/src/seedsigner/models/settings.py b/src/seedsigner/models/settings.py index e110acec..f892ae0c 100644 --- a/src/seedsigner/models/settings.py +++ b/src/seedsigner/models/settings.py @@ -9,6 +9,10 @@ from .singleton import Singleton +class InvalidSettingsQRData(Exception): + pass + + class Settings(Singleton): HOSTNAME = platform.uname()[1] @@ -33,6 +37,64 @@ class Settings(Singleton): return cls._instance + @classmethod + def parse_settingsqr(cls, data: str) -> tuple[str, dict]: + """ + Parses SettingsQR data and returns a tuple of (config_name, settings_dict). + + The resulting settings config can be applied by calling `Settings.update(settings_dict)`. + """ + if not data.startswith("settings::"): + raise InvalidSettingsQRData() + + version = data.split()[0].split("::")[1] + if version != "v1": + raise InvalidSettingsQRData(f"Unsupported SettingsQR version: {version}") + + # Start parsing key/value settings at the nth split() index + split_index = 1 + + # handle optional "name" attr + config_name = None + if "name=" in data.split()[1]: + config_name = data.split("name=")[1].split()[0].replace("_", " ") + split_index += 1 + + updated_settings = {} + for entry in data.split()[split_index:]: + abbreviated_name, value = entry.split("=") + + # Parse multi-value settings; integer-ize where needed + if "," in value: + values_updated = [] + for v in value.split(","): + if v.isdigit(): + v = int(v) + values_updated.append(v) + value = values_updated + elif value.isdigit(): + value = int(value) + + # Replace abbreviated name with full attr_name + settings_entry = SettingsDefinition.get_settings_entry_by_abbreviated_name(abbreviated_name) + if not settings_entry: + print(f"Ignoring unrecognized attribute: {abbreviated_name}") + continue + + # Validate value(s) against SettingsDefinition's valid options + if type(value) is not list: + values = [value] + else: + values = value + for v in values: + if v not in [opt[0] for opt in settings_entry.selection_options]: + raise InvalidSettingsQRData(f"""{abbreviated_name} = '{v}' is not valid""") + + updated_settings[settings_entry.attr_name] = value + + return (config_name, updated_settings) + + def __str__(self): return json.dumps(self._data, indent=4) @@ -64,6 +126,13 @@ class Settings(Singleton): # Setting is missing; insert default new_settings[entry.attr_name] = entry.default_value + else: + # Clean the incoming data, if necessary + if entry.type == SettingsConstants.TYPE__MULTISELECT: + if type(new_settings[entry.attr_name]) == str: + # Break comma-separated SettingsQR input into List + new_settings[entry.attr_name] = new_settings[entry.attr_name].split(",") + # Can't just merge the _data dict; have to replace keys they have in common # (otherwise list values will be merged instead of replaced). for key, value in new_settings.items(): diff --git a/src/seedsigner/views/scan_views.py b/src/seedsigner/views/scan_views.py index dc81b508..50251267 100644 --- a/src/seedsigner/views/scan_views.py +++ b/src/seedsigner/views/scan_views.py @@ -1,14 +1,12 @@ -import json import re from embit.descriptor import Descriptor -from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON from seedsigner.gui.screens import scan_screens from seedsigner.models import DecodeQR, Seed from seedsigner.models.settings import SettingsConstants - -from .view import BackStackView, MainMenuView, NotYetImplementedView, View, Destination +from seedsigner.views.settings_views import SettingsIngestSettingsQRView +from seedsigner.views.view import MainMenuView, NotYetImplementedView, View, Destination @@ -55,13 +53,8 @@ class ScanView(View): return Destination(PSBTSelectSeedView, skip_current_view=True) elif self.decoder.is_settings: - from seedsigner.models.settings import Settings - settings = self.decoder.get_settings_data() - Settings.get_instance().update(new_settings=settings) - - print(json.dumps(Settings.get_instance()._data, indent=4)) - - return Destination(SettingsUpdatedView, {"config_name": self.decoder.get_settings_config_name()}) + data = self.decoder.get_settings_data() + return Destination(SettingsIngestSettingsQRView, view_args=dict(data=data)) elif self.decoder.is_wallet_descriptor: from seedsigner.views.seed_views import MultisigWalletDescriptorView @@ -113,24 +106,3 @@ class ScanView(View): raise Exception("QRCode not recognized or not yet supported.") return Destination(MainMenuView) - - - -class SettingsUpdatedView(View): - def __init__(self, config_name: str): - super().__init__() - - self.config_name = config_name - - - def run(self): - from seedsigner.gui.screens.scan_screens import SettingsUpdatedScreen - screen = SettingsUpdatedScreen(config_name=self.config_name) - selected_menu_num = screen.display() - - if selected_menu_num == RET_CODE__BACK_BUTTON: - return Destination(BackStackView) - - # Only one exit point - return Destination(MainMenuView) - diff --git a/src/seedsigner/views/settings_views.py b/src/seedsigner/views/settings_views.py index ccb19150..bc02281b 100644 --- a/src/seedsigner/views/settings_views.py +++ b/src/seedsigner/views/settings_views.py @@ -1,9 +1,12 @@ +import logging from seedsigner.gui.components import SeedSignerCustomIconConstants from .view import View, Destination, MainMenuView from seedsigner.gui.screens import (RET_CODE__BACK_BUTTON, ButtonListScreen, settings_screens) -from seedsigner.models.settings import SettingsConstants, SettingsDefinition +from seedsigner.models.settings import Settings, SettingsConstants, SettingsDefinition + +logger = logging.getLogger(__name__) @@ -185,6 +188,28 @@ class SettingsEntryUpdateSelectionView(View): +class SettingsIngestSettingsQRView(View): + def __init__(self, data: str): + super().__init__() + + # May raise an Exception which will bubble up to the Controller to display to the + # user. + self.config_name, settings_update_dict = Settings.parse_settingsqr(data) + self.settings.update(settings_update_dict) + + + def run(self): + from seedsigner.gui.screens.scan_screens import SettingsUpdatedScreen + self.run_screen( + SettingsUpdatedScreen, + config_name=self.config_name + ) + + # Only one exit point + return Destination(MainMenuView) + + + """**************************************************************************** Misc ****************************************************************************""" diff --git a/tests/test_flows_seed.py b/tests/test_flows_seed.py index 935cb5de..fae11c4f 100644 --- a/tests/test_flows_seed.py +++ b/tests/test_flows_seed.py @@ -248,9 +248,9 @@ class TestSeedFlows(FlowTest): # exclusively set only one choice for each of sig_types, script_types and coordinators self.settings.update({ - SettingsConstants.SETTING__SIG_TYPES: [SettingsConstants.MULTISIG], - SettingsConstants.SETTING__SCRIPT_TYPES: [SettingsConstants.NESTED_SEGWIT], - SettingsConstants.SETTING__COORDINATORS: [SettingsConstants.COORDINATOR__SPECTER_DESKTOP], + SettingsConstants.SETTING__SIG_TYPES: SettingsConstants.MULTISIG, + SettingsConstants.SETTING__SCRIPT_TYPES: SettingsConstants.NESTED_SEGWIT, + SettingsConstants.SETTING__COORDINATORS: SettingsConstants.COORDINATOR__SPECTER_DESKTOP, }) self.run_sequence( diff --git a/tests/test_settings.py b/tests/test_settings.py index 891cce1d..7b396b93 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -1,10 +1,16 @@ +import pytest from base import BaseTest -from seedsigner.models.settings import Settings +from seedsigner.models.settings import InvalidSettingsQRData, Settings from seedsigner.models.settings_definition import SettingsConstants + class TestSettings(BaseTest): - + @classmethod + def setup_class(cls): + cls.settings = Settings.get_instance() + + def test_reset_settings(self): """ BaseTest.reset_settings() should wipe out any previous Settings changes """ settings = Settings.get_instance() @@ -14,3 +20,84 @@ class TestSettings(BaseTest): BaseTest.reset_settings() settings = Settings.get_instance() assert settings.get_value(SettingsConstants.SETTING__PERSISTENT_SETTINGS) == SettingsConstants.OPTION__DISABLED + + + def test_parse_settingsqr_data(self): + """ + """ + settings_name = "Test SettingsQR" + settingsqr_data = f"""settings::v1 name={ settings_name.replace(" ", "_") } persistent=D coords=spa,spd denom=thr network=M qr_density=M xpub_export=E sigs=ss,ms scripts=nat,nes,tr xpub_details=E passphrase=E camera=180 compact_seedqr=E bip85=D priv_warn=E dire_warn=E partners=E""" + + # First explicitly set settings that differ from the settingsqr_data + self.settings.set_value(SettingsConstants.SETTING__COMPACT_SEEDQR, SettingsConstants.OPTION__DISABLED) + self.settings.set_value(SettingsConstants.SETTING__DIRE_WARNINGS, SettingsConstants.OPTION__DISABLED) + self.settings.set_value(SettingsConstants.SETTING__COORDINATORS, [SettingsConstants.COORDINATOR__BLUE_WALLET, SettingsConstants.COORDINATOR__SPARROW]) + + # Now parse the settingsqr_data + config_name, settings_update_dict = Settings.parse_settingsqr(settingsqr_data) + assert(config_name == settings_name) + self.settings.update(new_settings=settings_update_dict) + + # Now verify that the settings were updated correctly + assert(self.settings.get_value(SettingsConstants.SETTING__COMPACT_SEEDQR) == SettingsConstants.OPTION__ENABLED) + assert(self.settings.get_value(SettingsConstants.SETTING__DIRE_WARNINGS) == SettingsConstants.OPTION__ENABLED) + + coordinators = self.settings.get_value(SettingsConstants.SETTING__COORDINATORS) + assert(SettingsConstants.COORDINATOR__BLUE_WALLET not in coordinators) + assert(SettingsConstants.COORDINATOR__SPARROW in coordinators) + assert(SettingsConstants.COORDINATOR__SPECTER_DESKTOP in coordinators) + + + def test_settingsqr_version(self): + """ Should accept SettingsQR v1 and reject any others """ + settingsqr_data = "settings::v1 name=Foo" + config_name, settings_update_dict = Settings.parse_settingsqr(settingsqr_data) + + # Accepts update with no Exceptions + self.settings.update(new_settings=settings_update_dict) + + settingsqr_data = "settings::v2 name=Foo" + with pytest.raises(InvalidSettingsQRData) as e: + Settings.parse_settingsqr(settingsqr_data) + assert("Unsupported SettingsQR version" in str(e.value)) + + # Should also fail if version omitted + settingsqr_data = "settings name=Foo" + with pytest.raises(InvalidSettingsQRData) as e: + Settings.parse_settingsqr(settingsqr_data) + + # And if "settings" is omitted entirely + settingsqr_data = "name=Foo" + with pytest.raises(InvalidSettingsQRData) as e: + Settings.parse_settingsqr(settingsqr_data) + + + def test_settingsqr_ignores_unrecognized_setting(self): + """ SettingsQR decoder should ignore unrecognized settings """ + settingsqr_data = "settings::v1 name=Foo favorite_food=bacon xpub_export=D" + config_name, settings_update_dict = Settings.parse_settingsqr(settingsqr_data) + + assert("favorite_food" not in settings_update_dict) + assert("xpub_export" in settings_update_dict) + + # Accepts update with no Exceptions + self.settings.update(new_settings=settings_update_dict) + + + def test_settingsqr_fails_unrecognized_option(self): + """ SettingsQR decoder should fail if a settings has an unrecognized option """ + settingsqr_data = "settings::v1 name=Foo xpub_export=Yep" + with pytest.raises(InvalidSettingsQRData) as e: + Settings.parse_settingsqr(settingsqr_data) + assert("xpub_export" in str(e.value)) + + + def test_settingsqr_parses_line_break_separators(self): + """ SettingsQR decoder should read line breaks as acceptable separators """ + settingsqr_data = "settings::v1\nname=Foo\nsigs=ss,ms\nscripts=nat,nes,tr\nxpub_export=E\n" + config_name, settings_update_dict = Settings.parse_settingsqr(settingsqr_data) + + assert(len(settings_update_dict.keys()) == 3) + + # Accepts update with no Exceptions + self.settings.update(new_settings=settings_update_dict) diff --git a/tests/test_settingsqr_decoder.py b/tests/test_settingsqr_decoder.py index 5f8cd101..8b2a887a 100644 --- a/tests/test_settingsqr_decoder.py +++ b/tests/test_settingsqr_decoder.py @@ -6,11 +6,6 @@ from seedsigner.models.settings_definition import SettingsConstants class TestSettingsQRDecoder: - @classmethod - def setup_class(cls): - cls.settings = Settings.get_instance() - - def test_decode_settingsqr(self): """ Assume the QR reader decodes the SettingsQR content correctly and begin this test @@ -19,84 +14,19 @@ class TestSettingsQRDecoder: settings_name = "Test SettingsQR" settings_qr_str = f"""settings::v1 name={ settings_name.replace(" ", "_") } persistent=D coords=spa,spd denom=thr network=M qr_density=M xpub_export=E sigs=ss,ms scripts=nat,nes,tr xpub_details=E passphrase=E camera=180 compact_seedqr=E bip85=D priv_warn=E dire_warn=E partners=E""" - # First explicitly set settings that differ from the settings_qr_str - self.settings.set_value(SettingsConstants.SETTING__COMPACT_SEEDQR, SettingsConstants.OPTION__DISABLED) - self.settings.set_value(SettingsConstants.SETTING__DIRE_WARNINGS, SettingsConstants.OPTION__DISABLED) - self.settings.set_value(SettingsConstants.SETTING__COORDINATORS, [SettingsConstants.COORDINATOR__BLUE_WALLET, SettingsConstants.COORDINATOR__SPARROW]) - # Now parse the settings_qr_str decoder = DecodeQR() status = decoder.add_data(settings_qr_str) assert(decoder.is_settings) assert(status == DecodeQRStatus.COMPLETE) - parsed_name = decoder.get_settings_config_name() - assert(parsed_name == settings_name) + data = decoder.get_settings_data() + assert(data == settings_qr_str) - settings_data = decoder.get_settings_data() - self.settings.update(new_settings=settings_data) - - # Now verify that the settings were updated correctly - assert(self.settings.get_value(SettingsConstants.SETTING__COMPACT_SEEDQR) == SettingsConstants.OPTION__ENABLED) - assert(self.settings.get_value(SettingsConstants.SETTING__DIRE_WARNINGS) == SettingsConstants.OPTION__ENABLED) - - coordinators = self.settings.get_value(SettingsConstants.SETTING__COORDINATORS) - assert(SettingsConstants.COORDINATOR__BLUE_WALLET not in coordinators) - assert(SettingsConstants.COORDINATOR__SPARROW in coordinators) - assert(SettingsConstants.COORDINATOR__SPECTER_DESKTOP in coordinators) - def test_settingsqr_version(self): - """ Should accept SettingsQR v1 and reject any others """ - settings_qr_str = "settings::v1 name=Foo" - decoder = DecodeQR() - status = decoder.add_data(settings_qr_str) - assert(decoder.is_settings) - assert(status == DecodeQRStatus.COMPLETE) - - settings_qr_str = "settings::v2 name=Foo" - decoder = DecodeQR() - with pytest.raises(Exception) as e: - status = decoder.add_data(settings_qr_str) - assert("Unsupported SettingsQR version" in str(e.value)) - - # Should also fail if version omitted - settings_qr_str = "settings name=Foo" + """ Should fail if the "settings" header is missing """ + settings_qr_str = "name=Foo" decoder = DecodeQR() status = decoder.add_data(settings_qr_str) assert(status == DecodeQRStatus.INVALID) - - - def test_settingsqr_ignores_unrecognized_setting(self): - """ SettingsQR decoder should ignore unrecognized settings """ - settings_qr_str = "settings::v1 name=Foo favorite_food=bacon xpub_export=D" - decoder = DecodeQR() - status = decoder.add_data(settings_qr_str) - assert(decoder.is_settings) - assert(status == DecodeQRStatus.COMPLETE) - - settings_data = decoder.get_settings_data() - assert("favorite_food" not in settings_data) - assert("xpub_export" in settings_data) - - - def test_settingsqr_fails_unrecognized_option(self): - """ SettingsQR decoder should fail if a settings has an unrecognized option """ - settings_qr_str = "settings::v1 name=Foo xpub_export=Yep" - decoder = DecodeQR() - with pytest.raises(Exception) as e: - decoder.add_data(settings_qr_str) - assert("Invalid value for" in str(e.value)) - - - def test_settingsqr_parses_line_break_separators(self): - """ SettingsQR decoder should read line breaks as acceptable separators """ - settings_qr_str = "settings::v1\nname=Foo\nsigs=ss,ms\nscripts=nat,nes,tr\nxpub_export=E\n" - decoder = DecodeQR() - status = decoder.add_data(settings_qr_str) - assert(decoder.is_settings) - assert(status == DecodeQRStatus.COMPLETE) - - settings_data = decoder.get_settings_data() - assert(len(settings_data.keys()) == 3) - self.settings.update(new_settings=settings_data) From 6926f22c135cbbf9c755453b5934e59f7f083269 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Mon, 31 Jul 2023 12:44:24 -0500 Subject: [PATCH 09/11] Imports cleanup --- src/seedsigner/models/decode_qr.py | 1 - src/seedsigner/models/settings.py | 2 +- tests/test_flows_seed.py | 2 +- tests/test_settingsqr_decoder.py | 3 --- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/seedsigner/models/decode_qr.py b/src/seedsigner/models/decode_qr.py index 3ad13910..9c1623b2 100644 --- a/src/seedsigner/models/decode_qr.py +++ b/src/seedsigner/models/decode_qr.py @@ -15,7 +15,6 @@ from urtypes.bytes import Bytes from seedsigner.helpers.ur2.ur_decoder import URDecoder from seedsigner.models import QRType, Seed from seedsigner.models.settings import SettingsConstants -from seedsigner.models.settings_definition import SettingsDefinition logger = logging.getLogger(__name__) diff --git a/src/seedsigner/models/settings.py b/src/seedsigner/models/settings.py index f892ae0c..7bb245ae 100644 --- a/src/seedsigner/models/settings.py +++ b/src/seedsigner/models/settings.py @@ -2,7 +2,7 @@ import json import os import platform -from typing import Any, List +from typing import List from seedsigner.models.settings_definition import SettingsConstants, SettingsDefinition from .singleton import Singleton diff --git a/tests/test_flows_seed.py b/tests/test_flows_seed.py index fae11c4f..9d801884 100644 --- a/tests/test_flows_seed.py +++ b/tests/test_flows_seed.py @@ -1,6 +1,6 @@ # Must import test base before the Controller from base import BaseTest, FlowTest, FlowStep -from base import FlowTestUnexpectedViewException, FlowTestRunScreenNotExecutedException, FlowTestInvalidButtonDataSelectionException +from base import FlowTestRunScreenNotExecutedException, FlowTestInvalidButtonDataSelectionException import pytest from seedsigner.models.settings import SettingsConstants diff --git a/tests/test_settingsqr_decoder.py b/tests/test_settingsqr_decoder.py index 8b2a887a..ac2720b3 100644 --- a/tests/test_settingsqr_decoder.py +++ b/tests/test_settingsqr_decoder.py @@ -1,7 +1,4 @@ -import pytest from seedsigner.models.decode_qr import DecodeQR, DecodeQRStatus -from seedsigner.models.settings import Settings -from seedsigner.models.settings_definition import SettingsConstants From 4ee6557cf8348f37c1ae6d8c50bf4812605bc153 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Mon, 31 Jul 2023 12:54:32 -0500 Subject: [PATCH 10/11] remove errant comma --- src/seedsigner/models/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/seedsigner/models/settings.py b/src/seedsigner/models/settings.py index 7bb245ae..6898474b 100644 --- a/src/seedsigner/models/settings.py +++ b/src/seedsigner/models/settings.py @@ -109,7 +109,7 @@ class Settings(Singleton): os.fsync(settings_file.fileno()) - def update(self, new_settings: dict, ): + def update(self, new_settings: dict): """ Replaces the current settings with the incoming dict. From 56b6e238507c70892516979a42a91a4aed58d587 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Mon, 31 Jul 2023 12:58:34 -0500 Subject: [PATCH 11/11] docstring updates --- tests/test_settings.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_settings.py b/tests/test_settings.py index 7b396b93..38ca68c1 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -24,6 +24,8 @@ class TestSettings(BaseTest): def test_parse_settingsqr_data(self): """ + SettingsQR parser should successfully parse a valid settingsqr input string and + return the resulting config_name and formatted settings_update_dict. """ settings_name = "Test SettingsQR" settingsqr_data = f"""settings::v1 name={ settings_name.replace(" ", "_") } persistent=D coords=spa,spd denom=thr network=M qr_density=M xpub_export=E sigs=ss,ms scripts=nat,nes,tr xpub_details=E passphrase=E camera=180 compact_seedqr=E bip85=D priv_warn=E dire_warn=E partners=E""" @@ -49,7 +51,7 @@ class TestSettings(BaseTest): def test_settingsqr_version(self): - """ Should accept SettingsQR v1 and reject any others """ + """ SettingsQR parser should accept SettingsQR v1 and reject any others """ settingsqr_data = "settings::v1 name=Foo" config_name, settings_update_dict = Settings.parse_settingsqr(settingsqr_data) @@ -73,7 +75,7 @@ class TestSettings(BaseTest): def test_settingsqr_ignores_unrecognized_setting(self): - """ SettingsQR decoder should ignore unrecognized settings """ + """ SettingsQR parser should ignore unrecognized settings """ settingsqr_data = "settings::v1 name=Foo favorite_food=bacon xpub_export=D" config_name, settings_update_dict = Settings.parse_settingsqr(settingsqr_data) @@ -85,7 +87,7 @@ class TestSettings(BaseTest): def test_settingsqr_fails_unrecognized_option(self): - """ SettingsQR decoder should fail if a settings has an unrecognized option """ + """ SettingsQR parser should fail if a settings has an unrecognized option """ settingsqr_data = "settings::v1 name=Foo xpub_export=Yep" with pytest.raises(InvalidSettingsQRData) as e: Settings.parse_settingsqr(settingsqr_data) @@ -93,7 +95,7 @@ class TestSettings(BaseTest): def test_settingsqr_parses_line_break_separators(self): - """ SettingsQR decoder should read line breaks as acceptable separators """ + """ SettingsQR parser should read line breaks as acceptable separators """ settingsqr_data = "settings::v1\nname=Foo\nsigs=ss,ms\nscripts=nat,nes,tr\nxpub_export=E\n" config_name, settings_update_dict = Settings.parse_settingsqr(settingsqr_data)