From ddba341f67ec5c00b8785de8fbd05e4bf63c178d Mon Sep 17 00:00:00 2001 From: kdmukai Date: Sat, 1 Feb 2025 17:04:57 -0600 Subject: [PATCH 1/4] Proper View separation; allows for more screenshots; button_data bugfix --- src/seedsigner/views/seed_views.py | 152 ++++++++++++++++-------- tests/base.py | 14 ++- tests/screenshot_generator/generator.py | 3 + tests/test_flows_seed.py | 43 ++++++- 4 files changed, 158 insertions(+), 54 deletions(-) diff --git a/src/seedsigner/views/seed_views.py b/src/seedsigner/views/seed_views.py index ae369587..6ce7e163 100644 --- a/src/seedsigner/views/seed_views.py +++ b/src/seedsigner/views/seed_views.py @@ -850,7 +850,6 @@ class SeedExportXpubCoordinatorView(View): - class SeedExportXpubWarningView(View): def __init__(self, seed_num: int, sig_type: str, script_type: str, coordinator: str, custom_derivation: str): super().__init__() @@ -1196,6 +1195,7 @@ class SeedBIP85SelectChildIndexView(View): ) + class SeedBIP85InvalidChildIndexView(View): def __init__(self, seed_num: int, num_words: int): super().__init__() @@ -1419,6 +1419,14 @@ class SeedWordsBackupTestSuccessView(View): Export as SeedQR ****************************************************************************""" class SeedTranscribeSeedQRFormatView(View): + # SeedQR dims for 12-word seeds + STANDARD_12 = ButtonOption("Standard: 25x25", return_data=25) + COMPACT_12 = ButtonOption("Compact: 21x21", return_data=21) + + # SeedQR dims for 24-word seeds + STANDARD_24 = ButtonOption("Standard: 29x29", return_data=29) + COMPACT_24 = ButtonOption("Compact: 25x25", return_data=25) + def __init__(self, seed_num: int): super().__init__() self.seed_num = seed_num @@ -1426,12 +1434,6 @@ class SeedTranscribeSeedQRFormatView(View): def run(self): seed = self.controller.get_seed(self.seed_num) - if len(seed.mnemonic_list) == 12: - STANDARD = ButtonOption("Standard: 25x25", return_data=25) - COMPACT = ButtonOption("Compact: 21x21", return_data=21) - else: - STANDARD = ButtonOption("Standard: 29x29", return_data=29) - COMPACT = ButtonOption("Compact: 25x25", return_data=25) if self.settings.get_value(SettingsConstants.SETTING__COMPACT_SEEDQR) != SettingsConstants.OPTION__ENABLED: # Only configured for standard SeedQR @@ -1440,22 +1442,26 @@ class SeedTranscribeSeedQRFormatView(View): view_args={ "seed_num": self.seed_num, "seedqr_format": QRType.SEED__SEEDQR, - "num_modules": STANDARD.return_data, + "num_modules": self.STANDARD_12.return_data, }, skip_current_view=True, ) - button_data = [STANDARD, COMPACT] + if len(seed.mnemonic_list) == 12: + button_data = [self.STANDARD_12, self.COMPACT_12] + else: + button_data = [self.STANDARD_24, self.COMPACT_24] - selected_menu_num = seed_screens.SeedTranscribeSeedQRFormatScreen( + selected_menu_num = self.run_screen( + seed_screens.SeedTranscribeSeedQRFormatScreen, title=_("SeedQR Format"), button_data=button_data, - ).display() + ) if selected_menu_num == RET_CODE__BACK_BUTTON: return Destination(BackStackView) - if button_data[selected_menu_num] == STANDARD: + if button_data[selected_menu_num] in [self.STANDARD_12, self.STANDARD_24]: seedqr_format = QRType.SEED__SEEDQR else: seedqr_format = QRType.SEED__COMPACTSEEDQR @@ -1496,10 +1502,11 @@ class SeedTranscribeSeedQRWarningView(View): # Forward straight to transcribing the SeedQR return destination - selected_menu_num = DireWarningScreen( + selected_menu_num = self.run_screen( + DireWarningScreen, status_headline=_("SeedQR is your private key!"), text=_("Never photograph or scan it into a device that connects to the internet."), - ).display() + ) if selected_menu_num == RET_CODE__BACK_BUTTON: return Destination(BackStackView) @@ -1529,10 +1536,11 @@ class SeedTranscribeSeedQRWholeQRView(View): data = e.next_part() - ret = seed_screens.SeedTranscribeSeedQRWholeQRScreen( + ret = self.run_screen( + seed_screens.SeedTranscribeSeedQRWholeQRScreen, qr_data=data, num_modules=self.num_modules, - ).display() + ) if ret == RET_CODE__BACK_BUTTON: return Destination(BackStackView) @@ -1607,10 +1615,11 @@ class SeedTranscribeSeedQRConfirmQRPromptView(View): def run(self): button_data = [self.SCAN, self.DONE] - selected_menu_option = seed_screens.SeedTranscribeSeedQRConfirmQRPromptScreen( + selected_menu_option = self.run_screen( + seed_screens.SeedTranscribeSeedQRConfirmQRPromptScreen, title=_("Confirm SeedQR?"), button_data=button_data, - ).display() + ) if selected_menu_option == RET_CODE__BACK_BUTTON: return Destination(BackStackView) @@ -1625,61 +1634,100 @@ class SeedTranscribeSeedQRConfirmQRPromptView(View): class SeedTranscribeSeedQRConfirmScanView(View): def __init__(self, seed_num: int): + from seedsigner.models.decode_qr import DecodeQR super().__init__() self.seed_num = seed_num self.seed = self.controller.get_seed(seed_num) + wordlist_language_code = self.settings.get_value(SettingsConstants.SETTING__WORDLIST_LANGUAGE) + self.decoder = DecodeQR(wordlist_language_code=wordlist_language_code) def run(self): from seedsigner.gui.screens.scan_screens import ScanScreen - from seedsigner.models.decode_qr import DecodeQR # Run the live preview and QR code capture process # TODO: Does this belong in its own BaseThread? - wordlist_language_code = self.settings.get_value(SettingsConstants.SETTING__WORDLIST_LANGUAGE) - self.decoder = DecodeQR(wordlist_language_code=wordlist_language_code) - ScanScreen( + self.run_screen( + ScanScreen, decoder=self.decoder, instructions_text=_("Scan your SeedQR") - ).display() + ) if self.decoder.is_complete: if self.decoder.is_seed: seed_mnemonic = self.decoder.get_seed_phrase() # Found a valid mnemonic seed! But does it match? if seed_mnemonic != self.seed.mnemonic_list: - DireWarningScreen( - title=_("Confirm SeedQR"), - status_headline=_("Error!"), - text=_("Your transcribed SeedQR does not match your original seed!"), - show_back_button=False, - button_data=[_("Review SeedQR")], - ).display() - - return Destination(BackStackView, skip_current_view=True) - + return Destination(SeedTranscribeSeedQRConfirmWrongSeedView, skip_current_view=True) else: - from seedsigner.gui.screens.screen import LargeIconStatusScreen - LargeIconStatusScreen( - title=_("Confirm SeedQR"), - status_headline=_("Success!"), - text=_("Your transcribed SeedQR successfully scanned and yielded the same seed."), - show_back_button=False, - button_data=[_("OK")], - ).display() + return Destination(SeedTranscribeSeedQRConfirmSuccessView, view_args={"seed_num": self.seed_num}) - return Destination(SeedOptionsView, view_args={"seed_num": self.seed_num}) + else: + # Will this case ever happen? Will trigger if a different kind of QR code is scanned + return Destination(SeedTranscribeSeedQRConfirmInvalidQRView, skip_current_view=True) - else: - # Will this case ever happen? Will trigger if a different kind of QR code is scanned - DireWarningScreen( - title=_("Confirm SeedQR"), - status_headline=_("Error!"), - text=_("Your transcribed SeedQR could not be read!"), - show_back_button=False, - button_data=[_("Review SeedQR")], - ).display() - return Destination(BackStackView, skip_current_view=True) + +class SeedTranscribeSeedQRConfirmWrongSeedView(View): + """ + A valid SeedQR was scanned but it did NOT match the one we just transcribed! + """ + def run(self): + self.run_screen( + DireWarningScreen, + title=_("Confirm SeedQR"), + status_headline=_("Error!"), + text=_("Your transcribed SeedQR does not match your original seed!"), + show_back_button=False, + button_data=[ButtonOption("Review SeedQR")], + ) + + # Skip BACK to the zoomed in transcription view + return Destination(BackStackView, skip_current_view=True) + + + +class SeedTranscribeSeedQRConfirmInvalidQRView(View): + """ + A QR code was scanned but it was not a SeedQR and certainly not the SeedQR we just + transcribed! + """ + def run(self): + # TODO: A better error message would be something like: "The QR code you scanned does not contain a valid SeedQR." + self.run_screen( + DireWarningScreen, + title=_("Confirm SeedQR"), + status_headline=_("Error!"), + text=_("Your transcribed SeedQR could not be read!"), + show_back_button=False, + button_data=[ButtonOption("Review SeedQR")], + ) + + # Skip BACK to the zoomed in transcription view + return Destination(BackStackView, skip_current_view=True) + + + +class SeedTranscribeSeedQRConfirmSuccessView(View): + """ + The SeedQR we just scanned matched the one we just transcribed. + """ + def __init__(self, seed_num: int): + super().__init__() + self.seed_num = seed_num + + + def run(self): + from seedsigner.gui.screens.screen import LargeIconStatusScreen + self.run_screen( + LargeIconStatusScreen, + title=_("Confirm SeedQR"), + status_headline=_("Success!"), + text=_("Your transcribed SeedQR successfully scanned and yielded the same seed."), + show_back_button=False, + button_data=[ButtonOption("OK")], + ) + + return Destination(SeedOptionsView, view_args={"seed_num": self.seed_num}) diff --git a/tests/base.py b/tests/base.py index 100ac741..74370dc7 100644 --- a/tests/base.py +++ b/tests/base.py @@ -13,7 +13,7 @@ sys.modules['seedsigner.hardware.buttons'] = MagicMock() sys.modules['seedsigner.hardware.camera'] = MagicMock() from seedsigner.controller import Controller, FlowBasedTestException, StopFlowBasedTest -from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON, RET_CODE__POWER_BUTTON +from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON, RET_CODE__POWER_BUTTON, ButtonOption from seedsigner.hardware.microsd import MicroSD from seedsigner.models.settings import Settings from seedsigner.views.view import Destination, MainMenuView, UnhandledExceptionView, View @@ -140,6 +140,12 @@ class FlowStep: +class FlowTestInvalidButtonDataInstanceTypeException(FlowBasedTestException): + """ The button_data contained an item that was not a ButtonOption instance """ + pass + + + class FlowTestInvalidButtonDataSelectionException(FlowBasedTestException): """ The FlowStep's button_data_selection value was not found in the View's button_data """ pass @@ -250,6 +256,12 @@ class FlowTest(BaseTest): """ cur_flow_step = sequence[0] + if "button_data" in kwargs: + # Verify that they are all proper ButtonOption instances + for button_option in kwargs.get("button_data"): + if not isinstance(button_option, ButtonOption): + raise FlowTestInvalidButtonDataInstanceTypeException(f"button_data must be a list of ButtonOption instances, not {type(button_option)}: {button_option}") + if cur_flow_step.button_data_selection: # We're mocking out the View.run_screen() method, so we'll get all of the # input args that are normally passed into the Screen.run() method, diff --git a/tests/screenshot_generator/generator.py b/tests/screenshot_generator/generator.py index ca697eb9..b773a4af 100644 --- a/tests/screenshot_generator/generator.py +++ b/tests/screenshot_generator/generator.py @@ -290,6 +290,9 @@ def generate_screenshots(locale): ScreenshotConfig(seed_views.SeedTranscribeSeedQRZoomedInView, dict(seed_num=0, seedqr_format=QRType.SEED__SEEDQR, initial_block_x=2, initial_block_y=2), screenshot_name="SeedTranscribeSeedQRZoomedInView_12_Standard"), ScreenshotConfig(seed_views.SeedTranscribeSeedQRConfirmQRPromptView, dict(seed_num=0)), + ScreenshotConfig(seed_views.SeedTranscribeSeedQRConfirmWrongSeedView), + ScreenshotConfig(seed_views.SeedTranscribeSeedQRConfirmInvalidQRView), + ScreenshotConfig(seed_views.SeedTranscribeSeedQRConfirmSuccessView, dict(seed_num=0)), # Screenshot can't render live preview screens # ScreenshotConfig(seed_views.SeedTranscribeSeedQRConfirmScanView, dict(seed_num=0)), diff --git a/tests/test_flows_seed.py b/tests/test_flows_seed.py index 2e63e702..3ca18fea 100644 --- a/tests/test_flows_seed.py +++ b/tests/test_flows_seed.py @@ -1,4 +1,5 @@ from typing import Callable +from unittest.mock import patch import pytest # Must import test base before the Controller @@ -8,7 +9,7 @@ from base import FlowTestInvalidButtonDataSelectionException from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON, ButtonOption from seedsigner.models.settings import Settings, SettingsConstants from seedsigner.models.seed import ElectrumSeed, Seed -from seedsigner.views.view import ErrorView, MainMenuView, OptionDisabledView, View, NetworkMismatchErrorView +from seedsigner.views.view import MainMenuView, OptionDisabledView, View, NetworkMismatchErrorView from seedsigner.views import seed_views, scan_views, settings_views @@ -419,6 +420,45 @@ class TestSeedFlows(FlowTest): ) + @patch("seedsigner.gui.screens.seed_screens.SeedTranscribeSeedQRZoomedInScreen", autospec=True) + def test_transcribe_seedqr_and_verify(self, mock_zoomed_in_screen: Callable): + """ + """ + # Load a finalized Seed into the Controller + mnemonic = ["abandon"] * 11 + ["about"] + self.controller.storage.set_pending_seed(Seed(mnemonic=mnemonic)) + self.controller.storage.finalize_pending_seed() + + def load_wrong_seed_into_decoder(view: View): + view.decoder.add_data("0138" * 24) + + def load_right_seed_into_decoder(view: View): + view.decoder.add_data("0000" * 11 + "0003") + + self.run_sequence([ + FlowStep(MainMenuView, button_data_selection=MainMenuView.SEEDS), + FlowStep(seed_views.SeedsMenuView, screen_return_value=0), + FlowStep(seed_views.SeedOptionsView, button_data_selection=seed_views.SeedOptionsView.BACKUP), + FlowStep(seed_views.SeedBackupView, button_data_selection=seed_views.SeedBackupView.EXPORT_SEEDQR), + FlowStep(seed_views.SeedTranscribeSeedQRFormatView, button_data_selection=seed_views.SeedTranscribeSeedQRFormatView.STANDARD_12), + FlowStep(seed_views.SeedTranscribeSeedQRWarningView), + FlowStep(seed_views.SeedTranscribeSeedQRWholeQRView), + FlowStep(seed_views.SeedTranscribeSeedQRZoomedInView, is_redirect=True), # Live interactive screens are a bit weird; not sure why `is_redirect` is necessary here + FlowStep(seed_views.SeedTranscribeSeedQRConfirmQRPromptView, button_data_selection=seed_views.SeedTranscribeSeedQRConfirmQRPromptView.SCAN), + + # Intentionally "scan" the wrong SeedQR + FlowStep(seed_views.SeedTranscribeSeedQRConfirmScanView, before_run=load_wrong_seed_into_decoder), + FlowStep(seed_views.SeedTranscribeSeedQRConfirmWrongSeedView), + FlowStep(seed_views.SeedTranscribeSeedQRZoomedInView, is_redirect=True), # Live interactive screens are still weird + + # Now scan the correct SeedQR + FlowStep(seed_views.SeedTranscribeSeedQRConfirmQRPromptView, button_data_selection=seed_views.SeedTranscribeSeedQRConfirmQRPromptView.SCAN), + FlowStep(seed_views.SeedTranscribeSeedQRConfirmScanView, before_run=load_right_seed_into_decoder), + FlowStep(seed_views.SeedTranscribeSeedQRConfirmSuccessView), + FlowStep(seed_views.SeedOptionsView), + ]) + + class TestMessageSigningFlows(FlowTest): MAINNET_DERIVATION_PATH = "m/84h/0h/0h/0/0" @@ -675,3 +715,4 @@ class TestMessageSigningFlows(FlowTest): self.settings.set_value(SettingsConstants.SETTING__NETWORK, SettingsConstants.MAINNET) expect_unsupported_derivation(self.load_custom_derivation_into_decoder) + From c0d8f39f63d8823498a0c1b67cc49f89113bef7e Mon Sep 17 00:00:00 2001 From: kdmukai Date: Sat, 1 Feb 2025 17:07:07 -0600 Subject: [PATCH 2/4] Sync submodule to new screenshots --- seedsigner-screenshots | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/seedsigner-screenshots b/seedsigner-screenshots index 8e97151b..c157f6e0 160000 --- a/seedsigner-screenshots +++ b/seedsigner-screenshots @@ -1 +1 @@ -Subproject commit 8e97151b7e0658dcffc02e7551e1606abcc572bf +Subproject commit c157f6e0dcbe71026280fabb0431e7b9742d296b From 9c8a294268e6a8054e43a8ab5ac4fd6a8e75443d Mon Sep 17 00:00:00 2001 From: kdmukai Date: Sat, 1 Feb 2025 17:18:58 -0600 Subject: [PATCH 3/4] Add missing failure path to FlowTest --- tests/test_flows_seed.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_flows_seed.py b/tests/test_flows_seed.py index 3ca18fea..c1bcc0bb 100644 --- a/tests/test_flows_seed.py +++ b/tests/test_flows_seed.py @@ -432,6 +432,9 @@ class TestSeedFlows(FlowTest): def load_wrong_seed_into_decoder(view: View): view.decoder.add_data("0138" * 24) + def load_completely_wrong_qr_type_into_decoder(view: View): + view.decoder.add_data("I like cheese") + def load_right_seed_into_decoder(view: View): view.decoder.add_data("0000" * 11 + "0003") @@ -451,6 +454,12 @@ class TestSeedFlows(FlowTest): FlowStep(seed_views.SeedTranscribeSeedQRConfirmWrongSeedView), FlowStep(seed_views.SeedTranscribeSeedQRZoomedInView, is_redirect=True), # Live interactive screens are still weird + # Intentionally scan QR data that makes no sense for this flow + FlowStep(seed_views.SeedTranscribeSeedQRConfirmQRPromptView, button_data_selection=seed_views.SeedTranscribeSeedQRConfirmQRPromptView.SCAN), + FlowStep(seed_views.SeedTranscribeSeedQRConfirmScanView, before_run=load_completely_wrong_qr_type_into_decoder), + FlowStep(seed_views.SeedTranscribeSeedQRConfirmInvalidQRView), + FlowStep(seed_views.SeedTranscribeSeedQRZoomedInView, is_redirect=True), # Live interactive screens are still weird + # Now scan the correct SeedQR FlowStep(seed_views.SeedTranscribeSeedQRConfirmQRPromptView, button_data_selection=seed_views.SeedTranscribeSeedQRConfirmQRPromptView.SCAN), FlowStep(seed_views.SeedTranscribeSeedQRConfirmScanView, before_run=load_right_seed_into_decoder), From e028b187a06d80fcc928db4376275a17608e01e7 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Sat, 1 Feb 2025 17:39:28 -0600 Subject: [PATCH 4/4] Additional test for check on `button_data` types --- tests/test_flows.py | 41 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/tests/test_flows.py b/tests/test_flows.py index acb8e8a3..249f25be 100644 --- a/tests/test_flows.py +++ b/tests/test_flows.py @@ -1,15 +1,15 @@ import pytest # Must import test base before the Controller -from base import FlowTest, FlowStep, FlowTestMissingRedirectException, FlowTestUnexpectedRedirectException, FlowTestUnexpectedViewException, FlowTestInvalidButtonDataSelectionException +from base import FlowTest, FlowStep, FlowTestMissingRedirectException, FlowTestUnexpectedRedirectException, FlowTestUnexpectedViewException, FlowTestInvalidButtonDataSelectionException, FlowTestInvalidButtonDataInstanceTypeException from seedsigner.controller import Controller -from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON, RET_CODE__POWER_BUTTON +from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON, RET_CODE__POWER_BUTTON, ButtonListScreen, ButtonOption from seedsigner.models.seed import Seed from seedsigner.views import scan_views from seedsigner.views.psbt_views import PSBTSelectSeedView from seedsigner.views.seed_views import SeedBackupView, SeedMnemonicEntryView, SeedOptionsView, SeedsMenuView -from seedsigner.views.view import MainMenuView, PowerOptionsView, UnhandledExceptionView +from seedsigner.views.view import Destination, MainMenuView, PowerOptionsView, UnhandledExceptionView, View from seedsigner.views.tools_views import ToolsMenuView, ToolsCalcFinalWordNumWordsView @@ -156,4 +156,39 @@ class TestFlowTest(FlowTest): FlowStep(MainMenuView, screen_return_value=Exception("Test exception")), FlowStep(UnhandledExceptionView), ]) + + + def test_raise_exception_on_bad_button_data_type(self): + """ + Ensure that the FlowTest raises an exception if a Screen's button_data has + non-ButtonOption entries. + """ + class MyBadButtonDataTestView(View): + def run(self): + self.run_screen( + ButtonListScreen, + button_data=[ButtonOption("this is fine"), "this is not"] + ) + + class MyGoodButtonDataTestView(View): + def run(self): + self.run_screen( + ButtonListScreen, + button_data=[ButtonOption("this is fine"), ButtonOption("this is also fine")] + ) + return Destination(MainMenuView) + + + # Should catch the bad button_data + with pytest.raises(FlowTestInvalidButtonDataInstanceTypeException): + self.run_sequence([ + FlowStep(MyBadButtonDataTestView), + FlowStep(MainMenuView), # Need a next Destination to force the first step to run + ]) + + # But if it's all ButtonOption instances, it should be fine + self.run_sequence([ + FlowStep(MyGoodButtonDataTestView), + FlowStep(MainMenuView), # Need a next Destination to force the first step to run + ])