Merge pull request #677 from kdmukai/transcribe_seedqr_verify_bugfix

newtonick · web-flow · commit c8a8b9f5a268 · 2025-02-01T22:41:39.000-05:00
[Bugfix] Verification Views after transcribing SeedQR fixes, refactors, test, screenshots
diff --git a/seedsigner-screenshots b/seedsigner-screenshots
@@ -1 +1 @@
-Subproject commit 8e97151b7e0658dcffc02e7551e1606abcc572bf
+Subproject commit c157f6e0dcbe71026280fabb0431e7b9742d296b
diff --git a/src/seedsigner/views/seed_views.py b/src/seedsigner/views/seed_views.py
@@ -850,7 +850,6 @@ def run(self):
 
 
 
-
 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 @@ def run(self):
         )
 
 
+
 class SeedBIP85InvalidChildIndexView(View):
     def __init__(self, seed_num: int, num_words: int):
         super().__init__()
@@ -1419,19 +1419,21 @@ def run(self):
     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
 
 
     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 @@ def run(self):
                 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 @@ def run(self):
             # 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 @@ def run(self):
 
         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 @@ def __init__(self, seed_num: int):
     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 @@ def run(self):
 
 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
@@ -13,7 +13,7 @@
 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 @@ def __post_init__(self):
 
 
 
+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 @@ def run_screen(view: View, *args, **kwargs):
                     """
                     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
@@ -290,6 +290,9 @@ def PSBTOpReturnView_raw_hex_data_cb_before():
                 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.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 @@ def test_raise_exception_via_screen_return_value(self):
             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
+        ])
 
diff --git a/tests/test_flows_seed.py b/tests/test_flows_seed.py
