Skip to content

[Bugfix] Fix back-navigation trap in Electrum Seeds entry flow - #960

Open
PROWLERx15 wants to merge 5 commits into
SeedSigner:devfrom
PROWLERx15:fix-electrum-seeds-flow
Open

[Bugfix] Fix back-navigation trap in Electrum Seeds entry flow#960
PROWLERx15 wants to merge 5 commits into
SeedSigner:devfrom
PROWLERx15:fix-electrum-seeds-flow

Conversation

@PROWLERx15

Copy link
Copy Markdown
Contributor

Description

This PR fixes a back-navigation trap in the Electrum Seed entry flow and adds new FlowTests which cover backing out of Electrum Seed entry from all possible entry points.

Problem or Issue being addressed

When entering an Electrum seed, the user is first shown a Warning screen (SeedElectrumMnemonicStartView) explaining that some features are disabled. This screen explicitly disables the back button.

However, when navigating forward to the keyboard entry, the SeedElectrumMnemonicStartView did not set skip_current_view=True. As a result, the Warning screen was preserved in the back stack. If a user started typing their seed but then decided to go BACK to cancel, they would be navigated back into the Warning screen. Since the Warning screen has no back button, the user is trapped and forced to reboot the device or complete the seed entry flow.

Solution

Modified the SeedElectrumMnemonicStartView and set skip_current_view=True.
This safely omits the Warning screen from the back stack, allowing the mnemonic keyboard's BACK button to correctly route the user back to the previous screen. I've also added new FlowTests to verify the changes.

Additional Information

This bug can be reproduced using these flows:

  • Load Seed Flow: Main Menu ➔ Seeds ➔ Enter Electrum seed ➔ [Warning Screen] ➔ BACK
  • Address Explorer Flow: Main Menu ➔ Tools ➔ Address Explorer ➔ Enter Electrum seed ➔ [Warning Screen] ➔ BACK
  • PSBT Signing Flow: Main Menu ➔ Scan (PSBT) ➔ Enter Electrum seed ➔ [Warning Screen] ➔ BACK
  • Sign Message Flow: Main Menu ➔ Scan (Message) ➔ Enter Electrum seed ➔ [Warning Screen] ➔ BACK

Screenshots


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


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant