Skip to content

fix: allow clearing wrong backup word - #1289

Open
jvsena42 wants to merge 7 commits into
masterfrom
fix/634-clear-wrong-backup-word
Open

jvsena42 wants to merge 7 commits into
masterfrom
fix/634-clear-wrong-backup-word

Conversation

@jvsena42

@jvsena42 jvsena42 commented Sep 16, 2026

Copy link
Copy Markdown
Member

Fixes #634
Related to #1275

This PR lets the user clear a wrong word on the Confirm Recovery Phrase screen, including a wrong last word.

Description

  • Adds a tap on the red (wrong) word so it can be removed, as the design note on the Incorrect frame asks, in addition to tapping its chip again.
  • Fixes the wrong last word being impossible to clear, because the old code returned early once every slot was filled.
  • Tracks selected chips by position instead of by word, so a phrase with repeated words selects and clears the right chip.
  • Keeps the chip order stable while the phrase is unchanged and saves only chip positions, so recovery words are no longer written to saved state.
  • Keeps correct words locked: they cannot be tapped away, and Continue still needs every word in the right place.

Out of Scope

  • Overwrite on tap: the issue asks for tapping another word to replace the wrong one. That is deliberately not implemented. The Figma note on 'Backup Recovery Phrase Incorrect' says to "tap either the word in red or the button... No overwrite", so tapping another chip while a wrong word is shown does nothing. Issue author or design to confirm before merge.
  • Activity recreation (font scale, dark mode, locale or window change) inside the backup sheet resets the confirmation to a fresh shuffle with empty slots. The sheet resets its view model on dispose and reloads the phrase, which clears the saved order and selection. On master the selected words survived while the chips reshuffled, so chip states could point at the wrong words. Keeping progress across recreation needs changes to the sheet lifecycle.
  • ConfirmMnemonicScreenTest.kt chipOrderAndSelection_surviveRecreation hosts the screen outside the sheet with a fixed phrase, so it covers restoration only when the phrase is unchanged, not the in-sheet path above.
  • Disabled Continue is a filled button in the app and text-only in Figma; unchanged from master.
  • iOS parity: BackupConfirmMnemonic.swift removes the last word by tapping its chip but has no red-word tap. Needs a follow-up in bitkit-ios, together with the journey below.

Design

Preview

Captured on the dev emulator with the public BIP39 test phrase in a temporary local build (not committed). No before screenshots: the master build was not installed on the shared emulator during this run.

Figma: Confirm Figma: Incorrect
Figma Confirm Figma Incorrect
Initial Wrong word in red Other chip ignored
Initial Wrong word in red Other chip ignored
Red word tapped: cleared Chip re-tapped: cleared Wrong word in slot 7
Red word tapped Chip re-tapped Wrong word slot 7
11 correct, Continue disabled All correct, Continue enabled After activity recreation
11 correct All correct After recreation

Recording of selecting a wrong word, clearing it by tapping the red word and its chip, and filling to 11 words:

634-clear-wrong-word.mp4

QA Notes

Manual Tests

Use a throwaway wallet; the route reveals the recovery phrase.

  • 1. Settings → Security → Back up wallet → Show Mnemonic → Continue → Confirm Mnemonic: 12 unselected chips, empty slots, Continue disabled.
  • 2a. Confirm Mnemonic → tap chip for word 1, then chip for word 3: slot 1 green, slot 2 red.
    • 2b. Tap chip for word 2: nothing changes, slot 2 stays red.
    • 2c. Tap the red word in slot 2: slot 2 clears and the chip for word 3 is unselected.
    • 2d. Tap chip for word 3 twice: red word appears, then clears.
  • 3. Tap chip for word 1 or its green word in slot 1: nothing changes.
  • 4. Fill words 2-6, then tap chip for word 8: slot 7 (right column) red; tap it: cleared.
  • 5. Fill all 12 words in order: all green, Continue enabled → Warning opens.
  • 6. With some words selected → change font scale or dark mode and return: fresh shuffle with empty slots, no crash, flow can still be completed.

Steps 1-6 were run on the dev emulator. Eleven correct words plus a wrong 12th cannot be reached on device, since the only chip left is the correct one; that path is covered by a unit test.

Journey covering the above, committed at journeys/backup/confirm-mnemonic-clear-wrong-word.xml
with its row in journeys/README.md (suites and cross-platform). Applied to master it fails at
the red-word tap, since SelectedWord-N is not clickable there.

journeys/backup/confirm-mnemonic-clear-wrong-word.xml
diff --git a/journeys/backup/confirm-mnemonic-clear-wrong-word.xml b/journeys/backup/confirm-mnemonic-clear-wrong-word.xml
new file mode 100644
index 000000000..8e2b77c6f
--- /dev/null
+++ b/journeys/backup/confirm-mnemonic-clear-wrong-word.xml
@@ -0,0 +1,45 @@
+<journey name="confirm mnemonic clear wrong word">
+  <description>
+    Proves that a wrong word on Confirm Recovery Phrase can be cleared by tapping either the red word
+    or its own chip, that nothing else can be selected while a wrong word is shown, and that correct
+    words stay locked in. Follows the Figma note on 'Backup Recovery Phrase Incorrect'.
+    Precondition: onboarded dev wallet with PIN disabled (or a PIN you know), no BIP39 passphrase.
+    Stopping before Continue leaves the backupVerified setting unchanged.
+    The route reveals the recovery phrase. Run it only on a throwaway wallet, and never copy the words
+    into logs, reports or files; refer to them by position. "Word N" below means the Nth word shown on
+    the Your Recovery Phrase screen, and "chip for word N" means the shuffled button tagged
+    "Word-(that word)". Every slot carries the testTag "SelectedWord-N" whether filled or not, but the
+    row is only clickable while it shows a wrong word, so assert on the word's colour and on whether
+    the row reacts to a tap, not on the tag being present.
+  </description>
+  <actions>
+    <action>Tap the menu icon (testTag "HeaderMenu")</action>
+    <action>Tap "Settings" (testTag "DrawerSettings")</action>
+    <action>Tap the Security tab (testTag "Tab-security")</action>
+    <action>Tap "Back up your wallet" (testTag "BackupWallet")</action>
+    <action>Tap "Tap To Reveal" (testTag "TapToReveal") and note the 12 words by position only</action>
+    <action>Tap "Continue" (testTag "ContinueShowMnemonic")</action>
+    <action>Verify that "Confirm Recovery Phrase" (testTag "backup_confirm_mnemonic_screen") is visible with 12 word chips and empty slots 1 to 12</action>
+    <action>Tap the chip for word 1</action>
+    <action>Verify that slot 1 (testTag "SelectedWord-1") shows word 1 in green</action>
+    <action>Tap the chip for word 3</action>
+    <action>Verify that slot 2 (testTag "SelectedWord-2") shows word 3 in red</action>
+    <action>Tap the chip for word 2</action>
+    <action>Verify that slot 2 still shows word 3 in red and the chip for word 2 is not selected</action>
+    <action>Tap the red word in slot 2 (testTag "SelectedWord-2")</action>
+    <action>Verify that slot 2 is empty and the chip for word 3 is back in its unselected style</action>
+    <action>Tap the chip for word 3</action>
+    <action>Verify that slot 2 shows word 3 in red</action>
+    <action>Tap the chip for word 3 again</action>
+    <action>Verify that slot 2 is empty and the chip for word 3 is back in its unselected style</action>
+    <action>Tap the green word in slot 1 (testTag "SelectedWord-1")</action>
+    <action>Verify that slot 1 still shows word 1 in green</action>
+    <action>Tap the chip for word 1</action>
+    <action>Verify that slot 1 still shows word 1 in green and slot 2 is still empty</action>
+    <action>Tap the chips for words 2 to 11 in order</action>
+    <action>Verify that slots 1 to 11 show green words and "Continue" (testTag "ContinueConfirmMnemonic") is disabled</action>
+    <action>Tap the chip for word 12</action>
+    <action>Verify that all 12 slots are green and "Continue" (testTag "ContinueConfirmMnemonic") is enabled</action>
+    <action>Press the system back button until the home screen (testTag "HomeScrollView") is visible, without tapping Continue</action>
+  </actions>
+</journey>

Automated Checks

  • Unit tests added: ConfirmMnemonicSelectionTest.kt covers the selection logic: words added in order and completion, clearing a wrong word by its chip or by the red word, other chips ignored after a wrong word, a wrong last word cleared when every slot is filled (regression), correct words locked, a full correct selection ignoring taps, repeated words, and out-of-range taps.
  • Instrumented tests added: ConfirmMnemonicScreenTest.kt covers red-word clearing, locked correct words, ignored and re-tapped chips, and state restoration with an unchanged phrase. Compiled locally but not run, because connected tests uninstall the dev app on the shared emulator.
  • Local verification: just compile, just test, just lint.
  • CI: standard compile, unit test, and detekt checks run by the PR bot.

jvsena42 and others added 2 commits September 16, 2026 18:27
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jvsena42 jvsena42 self-assigned this Sep 16, 2026
@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with no actionable correctness, security, or repository-rule violations identified.

Summary

The PR changes recovery-phrase confirmation to track shuffled chip positions rather than word strings, allowing the current incorrect word—including the final word—to be removed from either the selected row or its chip.

  • Preserves shuffled chip order and selected positions through saveable-state restoration when the phrase is unchanged.
  • Keeps confirmed correct words locked and blocks further selection while the latest word is incorrect.
  • Adds focused reducer unit tests, Compose interaction/restoration tests, and a changelog entry.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Tap shuffled chip] --> B{Current last word incorrect?}
    B -- Yes --> C{Tapped its chip?}
    C -- Yes --> D[Remove last selected index]
    C -- No --> E[Keep selection unchanged]
    B -- No --> F{Chip already selected or phrase full?}
    F -- Yes --> E
    F -- No --> G[Append shuffled chip index]
    H[Tap red selected word] --> D
    G --> I{All positions match original phrase?}
    D --> I
    I -- Yes --> J[Enable Continue]
    I -- No --> K[Keep Continue disabled]
Loading

Reviews (1) · Last reviewed commit: "test: cover backup word clearing and sta..."

@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 4c96022 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — no HIGH/MEDIUM findings at head 72560bfaa. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Traced reduceMnemonicSelection and isMnemonicSelectionComplete at the PR head (ConfirmMnemonicScreen.kt:89-121) against the only consumer of the result: Continue is enabled solely by isComplete (line 239), which requires stack.size == original.size and every position's chip word to equal original[position], so the backupVerified write in BackupNavSheetViewModel.onSuccessContinue (line 141) is unreachable from this screen unless all words are in the right slot. A wrong word can only ever be the last entry, because the reducer refuses to append while the last position is incorrect, and only that entry can be dropped (line 96-97), so correct words are never removed by the chip or by the red-word row; onSelectedWordPress feeds the last stack index back through the same reducer, and SelectedWordItem enables the click only when the row shows a wrong word. Duplicate BIP39 words are handled per chip index, not per string, and a second chip with the same word is still selectable since the duplicate check is on indices (line 99). The wrong-last-word case is fixed: the old firstNullIndex == -1 early return is gone and a full stack whose last entry is wrong still allows dropLast. Back (popBackStack), sheet dismiss (resetState + NavHost disposal) and activity recreation all yield a fresh shuffle with an empty stack because both rememberSaveable calls are keyed on originalSeed and the view model resets to the 24-word placeholder on dispose; the placeholder-keyed restore of a 12-index order cannot index out of range because the placeholder has 24 words. Saved state now holds only Int lists (ArrayList/EmptyList/SingletonList are all Serializable), no mnemonic words. Behaviour matches the issue #1275 row for #634 (tap red word or its chip clears; no overwrite). No iOS twin PR found. Pre-existing and untouched by this PR: if keychain.loadString(BIP39_MNEMONIC) returns null, the placeholder 'secret' x24 mnemonic flows into ShowMnemonic and ConfirmMnemonic and could be 'confirmed'; that is a broken-wallet state and not a regression here.

@jvsena42
jvsena42 requested a review from ovitrif September 18, 2026 13:26
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ovi-reviewer[bot]

This comment was marked as resolved.

jvsena42 and others added 2 commits September 18, 2026 11:17
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-backup-word

# Conflicts:
#	journeys/README.md

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — no blocking HIGH/MEDIUM findings at head 50f871aba. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Follow-up on 6095019 and 55ada41 (new journey): Reviewed the two journey commits (6095019, 55ada41) against ConfirmMnemonicScreen.kt at head 50f871a. Every tag the journey names exists at head: HeaderMenu, DrawerSettings, Tab-security (CustomTabRowWithSpacing), BackupWallet, TapToReveal, ContinueShowMnemonic, backup_confirm_mnemonic_screen, Word-, SelectedWord-, ContinueConfirmMnemonic and HomeScrollView; Settings > Security > Back up wallet opens Sheet.Backup() at ShowMnemonic, so the screen order matches. Each step follows reduceMnemonicSelection (lines 89-102): chip for word 2 is ignored while slot 2 is red, the red row and the re-tapped chip both drop the last index, the green row is not clickable (clickableAlpha adds no modifier when disabled) and the chip for word 1 is ignored because its index is already in the stack; Continue is enabled only by isComplete. With the fix reverted the red-word tap has no click handler, so the journey fails at the 'slot 2 is empty' check. Seed handling is safe: the precondition is a throwaway dev (regtest) wallet, words are referred to by position and must not be copied, BlockScreenshots is a no-op on debug so colour checks can use a screenshot, and the runner stops before Continue so backupVerified is untouched. The identifier row for SelectedWord- and the not-ported cross-platform row answer the earlier ovi-reviewer thread.

ovi-reviewer[bot]

This comment was marked as resolved.

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck

@ovitrif

ovitrif commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

@jvsena42 e2e ci red, should sync with master to retrigger

ovitrif

This comment was marked as resolved.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ✅ Approve


Reaudit: diff 1 file.

Findings:
N/A

Audit:
Audited - no findings.

QA: emu-1 redroid emulator on Android 15 (API 35), dev regtest debug build

  1. passed: Confirm Recovery Phrase opened with 12 unselected chips, 12 empty slots, and Continue disabled.

    Test 1:
    Settings → Security → Back up wallet → Show Mnemonic → Continue → Confirm Mnemonic: 12 unselected chips,…
    1.mp4
  2. passed: All 2a–2d wrong-word selection, blocking, red-word clearing, and chip re-tap behaviors passed.

    Test 2:
    **2a.** Confirm Mnemonic → tap chip for word 1, then chip for word 3: slot 1 green, slot 2 red.
    2.mp4
  3. passed: The correct first word stayed locked when tapping either its selected row or chip.

    Test 3:
    Tap chip for word 1 or its green word in slot 1: nothing changes.
    3.mp4
  4. passed: A wrong seventh word appeared in the right column and cleared when its row was tapped.

    Test 4:
    Fill words 2-6, then tap chip for word 8: slot 7 (right column) red; tap it: cleared.
    4.mp4
  5. passed: All 12 correct words enabled Continue and advanced to the recovery-phrase warning screen.

    Test 5:
    Fill all 12 words in order: all green, Continue enabled → Warning opens.
    5.mp4
  6. passed: Font-scale recreation produced a fresh empty shuffle, then the completed flow reached the warning screen.

    Test 6:
    With some words selected → change font scale or dark mode and return: fresh shuffle with empty slots, no…
    6.mp4

Ticked Manual Tests 1–6, including 2a–2d.

Tip

Test 1 worth a journey:

  • Open the menu and tap Settings
  • Open Security and tap Back up your wallet
  • Tap Tap To Reveal and note the 12 words by position
  • Tap Continue
  • Verify 12 chips are unselected, all slots are empty, and Continue is disabled

Test 2 worth a journey:

  • Tap the chip for word 1, then the chip for word 3
  • Verify slot 1 is correct and slot 2 is wrong
  • Tap the chip for word 2 and verify slot 2 does not change
  • Tap the wrong word in slot 2 and verify it clears
  • Tap the chip for word 3 twice and verify the wrong word appears, then clears

Test 3 worth a journey:

  • Start with word 1 correct in slot 1
  • Tap the correct word in slot 1 and verify it stays
  • Tap the chip for word 1 and verify slot 1 stays correct and slot 2 stays empty

Test 4 worth a journey:

  • Fill words 1 through 6 in order
  • Tap the chip for word 8
  • Verify slot 7 in the right column is wrong
  • Tap the wrong word in slot 7 and verify it clears

Test 5 worth a journey:

  • Open Confirm Recovery Phrase and select all 12 words in order
  • Verify every slot is correct and Continue is enabled
  • Tap Continue
  • Verify the recovery-phrase warning screen appears

Test 6 worth a journey:

  • Select the first three recovery words
  • Change the device font scale
  • Verify a fresh shuffle, 12 empty slots, and disabled Continue
  • Select all 12 words in the new order
  • Tap Continue and verify the warning screen
  • Restore the original device font scale and verify the app remains open

Coverage:
QA: 6 of 6 manual tests passed


Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)

@jvsena42
jvsena42 requested a review from ovitrif September 18, 2026 20:19

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ✅ Approve


Reaudit: diff 1 file.

Findings:
N/A

Audit:
Awaits QA.

Coverage:
QA: journeys and manual tests queued


Reviewed by claude-opus-5-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)

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.

[Bug]: GUI: Seed phrase confirmation behaviour

2 participants