[Bugfix] Fix back button trap in seed backup verification review

[none34829]

Description

Problem or Issue being addressed

Fixes #854.

During seed backup verification, if you pick the wrong word you get a "Wrong Word!" screen with two options: "Review seed words" and "Try again." If you choose "Review seed words," it shows the seed words with a back button. Pressing back returns you to the "Wrong Word!" screen, creating an infinite loop:

Wrong Word → Review seed words → Seed words display → Back → Wrong Word → (repeat)

The only escape is remembering the correct word and using "Try again."

Solution

Added skip_current_view=True to the Destination returned when the user selects "Review seed words" in SeedWordsBackupTestMistakeView. This prevents the mistake view from being pushed onto the back stack, so pressing BACK from the word review returns to a fresh verification attempt instead of looping back to the error screen.

One-line fix in seed_views.py. Same pattern used by SeedWordsWarningView (line 1019) to prevent going BACK to warning screens.

Additional Information

Includes a FlowTest (test_review_words_after_wrong_answer_no_back_trap) that:

• Walks the full backup verification flow
• Forces a wrong word selection using deterministic random seeding (rand_seed=3, cur_index=2)
• Selects "Review seed words" from the mistake screen
• Presses BACK from the word review
• Asserts we land on SeedWordsBackupTestView (fresh verification), NOT SeedWordsBackupTestMistakeView (the loop)

Verified the test fails without the fix (lands on MistakeView) and passes with it.

---

This pull request is categorized as a:

• New feature
• Bug fix
• Code refactor
• Documentation
• Other

---

Checklist

I ran pytest locally

• All tests passed before submitting the PR
• I couldn't run the tests
• N/A

---

I included screenshots of any new or modified screens

Should be part of the PR description above.

• Yes
• No
• N/A

---

I added or updated tests

Any new or altered functionality should be covered in a unit test. Any new or updated sequences require FlowTests.

• Yes
• No, I'm a fool
• N/A

---

I tested this PR hands-on on the following platform(s):

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

---

I have reviewed these notes:

• Keep your changes limited in scope.
• If you uncover other issues or improvements along the way, ideally submit those as a separate PR.
• The more complicated the PR, the harder it is to review, test, and merge.
• We appreciate your efforts, but we're a small team of volunteers so PR review can be a very slow process.
• Please only "@" mention a contributor if their input is truly needed to enable further progress.

• I understand

---

Thank you! Please join our Devs' Telegram group to get more involved.

[PROWLERx15]

Tested this locally and the fix works as intended.

After selecting the wrong word and then choosing "Review seed words", pressing BACK no longer returns to the "Wrong Word!" screen. It takes you back into the verification flow, which matches the earlier discussion around this issue. So this appears to fix the actual back-button loop without turning review into an escape hatch out of verification.

Also, just to be clear, this does not look like the same problem as the earlier NACK in #855. That PR was NACKed because it changed the flow in a way that let users back out of verification entirely. This change looks narrower and seems to address the specific loop Keith called out without weakening the verification flow.

One thing still missing is test coverage for the BIP-85 child seed flow. Since #854 calls out both normal seed backup verification and BIP-85 child seed backup verification, can you add the test for the BIP-85 path as well?

[Chaitanya-Keyal]

I'm currently NACK on this PR, but Concept ACK.

PR #855 attempted to address the same issue and was closed. The discussion there is meaningful, and I agree with most of the points raised in that thread.

I do think the current flow is intentional, as Alvaro noted here: #855 (comment)

At the same time, I agree that sending BACK to the error screen is confusing, and the proposed change to skip that view does keep the user in verification context. However, this does not actually result in a fresh verification attempt, despite the PR description saying that it does.

From hands-on testing and viewing the back_stack in logs, after hitting a mistake and selecting Review seed words, pressing BACK returns to verification with prior state still preserved. The flow returns to SeedWordsBackupTestView with confirmed_list already populated, instead of resetting the attempt.

Concrete example:

• Suppose user correctly verifies words 1, 9, and 4.
• Then user gets word #5 wrong.
• On the Wrong Word screen, they choose Review seed words.
• After reviewing and pressing BACK, they return to SeedWordsBackupTestView carrying confirmed_list with previously validated indexes ([0,8,3]), which is not a fresh run.

That can be even more confusing, because the verification resumes as if earlier answers are still trusted after the user intentionally stepped out to review. That is, the words that were in the said list will not be re-verified, after the user potentially corrected their backup or created a new one.

My preference:

• BACK from seed word review should return to verification flow, as this PR does.
• But on this review-back path, verification state should be reset so attempt restarts from the beginning.

[none34829]

Good catches, both of you.

@Chaitanya-Keyal — you're right. With skip_current_view=True alone, BACK from the word review pops the old SeedWordsBackupTestView off the stack with its stale confirmed_list. That's not a fresh attempt.

I've pushed a follow-up: instead of navigating to SeedWordsView directly, the "Review seed words" button now navigates to SeedWordsBackupTestPromptView (the "Verify / Skip" screen) with skip_current_view=True. From there the user can re-enter verification from scratch with an empty confirmed_list, or skip out. The back stack is clean — no stale test state.

The trade-off is the user doesn't jump straight into the word pages anymore — they go through the prompt first. But it means no stale state and a clear exit path, which I think is the right call.

@PROWLERx15 — added a BIP-85 flow test covering the same review-back path for child seed verification.

[none34829]

Pushed a follow-up- "Review seed words" now routes through SeedWordsWarningView so the back stack is clean and confirmed_list resets. Added a BIP-85 child seed test too @Chaitanya-Keyal @PROWLERx15

[PROWLERx15]

Just a small note, I know you're eager to contribute, which is great to see.

But I think you might have missed something Keith mentioned during the Summer of Bitcoin onboarding call.
Generally, we try to avoid tagging people for reviews unless it’s absolutely necessary. Luckily, it was just us here and not the maintainers, but their time is quite limited, so they prefer not to be pinged directly.

You can take a look at this gist for the summary of the call: https://gist.github.com/Chaitanya-Keyal/69225236ec8404a595742daa3c8522c1

Thank you for making the changes. I'll review them when possible :)

[none34829]

Noted- will avoid the tags going forward. Thanks for the heads-up.

[Chaitanya-Keyal]

Retested on latest PR updates: still NACK from my side.

Routing through SeedWordsWarningView removes the direct Wrong Word -> Review -> Wrong Word loop, but it does not fix the underlying UX issue. It adds an extra warning hop, and BACK still returns into stale verification context instead of a clean restart. I can reproduce this both when proceeding through the warning and when skipping it (BACK on the first seed-words page still goes back into stale verification flow).

So this reshapes the loop rather than resolving it. I still think the review-back path should produce a clean, unambiguous verification restart without extra warning hops.

Reading the thread in #855, Keith mentioned a couple of possible fixes, maybe you could try looking into those. IMO, it might be messy, I'm not sure, but the most ideal fix would be to manipulate the back_stack and clear out that confirmed_list.

---

Also, I'm normally completely okay with being tagged on PRs like this. But it is kinda frustrating when, after calling for a re-review, behavior claimed as "fixed/tested" in comments does not match hands-on results. I'd strongly recommend checking stdout logs and the back_stack state while testing this full flow.

[none34829]

Fair point on the stale confirmed_list; shoulda caught that in the back_stack logs. pushed a different approach.

Instead of routing through SeedWordsWarningView, I added a review_mode parameter to SeedWordsView (default False, no behavior change elsewhere). When review_mode=True and the user presses BACK on page 1, it navigates directly to a fresh SeedWordsBackupTestPromptView instead of popping the back stack. No stale state, no extra hops.

The flow is now:

Wrong Word → "Review seed words" → SeedWordsView(review_mode=True) → BACK on page 1 → fresh SeedWordsBackupTestPromptView

Verified in the back_stack logs- the SeedWordsBackupTestPromptView at the end has no confirmed_list, and the SeedWordsBackupTestMistakeView is skipped from the stack.