SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
699 stars 161 forks source link

[bugfix] Prevent SettingsQR from trying to enable Persistent Settings when SD card is not inserted #464

Closed kdmukai closed 1 year ago

kdmukai commented 1 year ago

fixes #457

The problem

If you remove the SD card and then scan in a SettingsQR that attempts to enable Persistent Settings, you get a System Error exception.

The fix

SettingsIngestSettingsQRView_not_persistent SettingsIngestSettingsQRView_persistent

Additional changes

jdlcdl commented 1 year ago

As of a48620f

ACK tested

It's nice having the separate confirmation screens that say exactly where settings are.

Tested on pi2 Raspi-os and pi0 seedsigner-os, all tests passing too.

kdmukai commented 1 year ago

All credit goes to you on this one, @jdlcdl! You found the bug AND figured out the cause that was totally eluding me.

jdlcdl commented 1 year ago

In case it helps:

I've resolved this conflict locally by keeping most changes for both pr_464 and latest dev, in two separate sections of ./tests/screenshot_generator/generator.py with all tests passing and 6 additional successful screenshots bringing total number to 100.

 diff --cc tests/screenshot_generator/generator.py
index 6592ed3,c50e719..0000000
--- a/tests/screenshot_generator/generator.py
+++ b/tests/screenshot_generator/generator.py
@@@ -124,12 -123,7 +124,13 @@@ def test_generate_screenshots(target_lo
              PowerOptionsView,
              RestartView,
              PowerOffView,
 +            NotYetImplementedView,
 +            (UnhandledExceptionView, dict(error=UnhandledExceptionViewFood)),
 +            NetworkMismatchErrorView,
 +            (OptionDisabledView, dict(settings_attr=SettingsConstants.SETTING__MESSAGE_SIGNING)),
 +
 +
+             (settings_views.SettingsIngestSettingsQRView, dict(data="settings::v1 name=Uncle_Jim's_noob_mode")),
          ],
          "Seed Views": [
              seed_views.SeedsMenuView,
@@@ -221,12 -215,19 +222,24 @@@
              tools_views.ToolsAddressExplorerAddressListView,
              #tools_views.ToolsAddressExplorerAddressView,
          ],
 -        "Settings Views": settings_views_list,
 +        "Settings Views": settings_views_list + [
 +            settings_views.IOTestView,
 +            settings_views.DonateView,
 +            (settings_views.SettingsIngestSettingsQRView, dict(data=settingsqr_data_persistent), "SettingsIngestSettingsQRView_persistent"),
 +            (settings_views.SettingsIngestSettingsQRView, dict(data=settingsqr_data_not_persistent), "SettingsIngestSettingsQRView_not_persistent"),
 +        ],
+         "Misc Error Views": [
+             NotYetImplementedView,
+             (UnhandledExceptionView, dict(error=UnhandledExceptionViewFood)),
+             NetworkMismatchErrorView,
+             (OptionDisabledView, dict(settings_attr=SettingsConstants.SETTING__MESSAGE_SIGNING)),
+             (ErrorView, dict(
+                 title="Error",
+                 status_headline="Unknown QR Type",
+                 text="QRCode is invalid or is a data format not yet supported.",
+                 button_text="Back",
+             )),
+         ],
      }
jdlcdl commented 1 year ago

as of rebased d810d12

ACK tested.

on pi0 w/ hash 2b76dd99f767efb22472cca7f8d29fb254dba7e6ea682843342822d5db7f0fb3

newtonick commented 1 year ago

ACK and tested