Merge pull request #677 from kdmukai/transcribe_seedqr_verify_bugfix

[Bugfix] Verification Views after transcribing SeedQR fixes, refactors, test, screenshots

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.run_screen(
-        self.decoder = DecodeQR(wordlist_language_code=wordlist_language_code)
+            ScanScreen,
-        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(
+                    return Destination(SeedTranscribeSeedQRConfirmWrongSeedView, skip_current_view=True)
-                        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)
-                
                 else:
-                    from seedsigner.gui.screens.screen import LargeIconStatusScreen
+                    return Destination(SeedTranscribeSeedQRConfirmSuccessView, view_args={"seed_num": self.seed_num})
-                    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(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})
 
 
 

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,

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)),

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
+        ])
 

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,54 @@ 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_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")
+
+        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
+
+            # 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),
+            FlowStep(seed_views.SeedTranscribeSeedQRConfirmSuccessView),
+            FlowStep(seed_views.SeedOptionsView),
+        ])
+
+
 
 class TestMessageSigningFlows(FlowTest):
     MAINNET_DERIVATION_PATH = "m/84h/0h/0h/0/0"
@@ -675,3 +724,4 @@ class TestMessageSigningFlows(FlowTest):
         self.settings.set_value(SettingsConstants.SETTING__NETWORK, SettingsConstants.MAINNET)
         expect_unsupported_derivation(self.load_custom_derivation_into_decoder)
 
+