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

[kdmukai]

Description

Expected sequence:
Transcribe a SeedQR, choose to verify your results. After your SeedQR is scanned, see either success (the SeedQR that was just scanned matched the SeedQR you just transcribed) or error (did not match or the QR code you just scanned is a completely unexpected type).

BUG: An Exception is raised after scanning the SeedQR due to bad button_data.

---

Outdated implementation hiding an l10n bug

SeedTranscribeSeedQRConfirmScanView was an outdated implementation that violated our "1 View instantiates 1 Screen" rule; after scanning back the transcribed SeedQR it was calling its own collection of follow up sub-Screens.

Also these sub-Screens had outdated button_data that weren't ButtonOption lists. Which, by design in the l10n refactor, now throws an Exception.

But a proper FlowTest couldn't be written for each of these Screens because they were improperly nested inside that one big View (my fault; I wrote all that original code). And so none of our automated tests could alert of us of this Exception.

---

The fixes in this PR

• Pull those sub-Screen calls into their own individual Views to match the conventions used everywhere else.
• Properly instantiate the button_list items as ButtonOptions.
• Write a FlowTest to hit each failure condition when verifying the transcribed SeedQR with eventual success.
• Add screenshots for the now properly isolated new Views: SeedSigner/seedsigner-screenshots@c157f6e
• Additional sanity checking in FlowTest.run_screen (in tests/base.py) to check for any bad button_list inputs (i.e. they all must be ButtonOptions).
• Additional test case to verify that FlowTest will catch bad button_list input.

Note: The edits in SeedTranscribeSeedQRConfirmScanView to pull out the three new Views are almost entirely just copy-paste. As is often the case, the web diff here looks more dramatic than it actually is.

Misc:

• Light refactors throughout the Transcribe SeedQR flow for compatibility with FlowTest.

---

This pull request is categorized as a:

• Bug fix
• Code refactor

Checklist

• I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

• Yes

I have tested this PR on the following platforms/os:

• Raspberry Pi OS Manual Build
• SeedSigner OS on a Pi0/Pi0W board

[fedebuyito]

Hi @kdmukai , when I scan compact (with standard export) or standard (with compact export) results on success, too. I presume that is because of same seed case:

Pytested on 3.10 and 3.12, Kali Linux.

[newtonick]

I can only get this to trigger if I scan a valid QR that SeedSigner doesn't support decoding the contents of (for example an encoded web url). If I scan a valid QR that SeedSigner is able to decode (and isn't a SeedQR) than I get the "Not Yet Implemented" warning and get kicked to the Main Menu (even using the back button). I don't think this PR needs to address this outlier edge case. I just wanted to mention it since the question "Will this case ever happen?" was asked.

[kdmukai]

I was able to get to the InvalidQRView if I scanned an xpub from Sparrow or a generic text ("I like cheese") QR.

But an animated PSBT QR got me to NotYetImplementedView.

Oh, actually I get it now. The else on line 1664 should have been omitted. The final return condition should be a catch-all for ALL remaining possibilities. But as it's written it's missing the case where:

decoder.is_complete == True and decoder.is_seed == False

I'll log it as an Issue / TODO, but I don't think it's worth holding up this release.

[newtonick]

ACK, reviewed and tested. I was able to recreate the bug and see that this PR resolved it. Great catch in finding and fixing this bug. I appreciate you going the extra mile to create the test flow to prevent the same or similar bug from happening in the future.

[kdmukai]

@fedebuyito, yes, it's a bit of an unintended edge case that the verification step doesn't care if the SeedQR format matches (Standard vs Compact).

For the intended use case I don't think it matters though.

[fedebuyito]

@fedebuyito, yes, it's a bit of an unintended edge case that the verification step doesn't care if the SeedQR format matches (Standard vs Compact).

For the intended use case I don't think it matters though.

Right, Keith. It makes sense. Thank you for the reply. Great fixs and refactors!