Skip to content

Commit 83cadeb

Browse files
claudeBenWestgate
authored andcommitted
correct: Say why the entry was invalid when suggesting a repair
`correct` and the "Possible correction" prompt in secret, share, wallet and create --existing showed only the repair. Print one line to stderr first, "Invalid: <reason>", using the reason `check` already gives. Callers pass the parse error they already caught, so nothing is parsed twice. A mixed-case string now says codex32 strings are all uppercase or all lowercase and that either case recovers the same wallet. Closes #85 Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
1 parent 115f2c2 commit 83cadeb

4 files changed

Lines changed: 52 additions & 15 deletions

File tree

‎src/codex32/_cli_input.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
CodexError,
2828
DuplicateShareIndex,
2929
ExistingTargetIndex,
30+
InvalidCase,
3031
InvalidChecksum,
3132
InvalidLength,
3233
InvalidThreshold,
@@ -204,6 +205,7 @@ def _confirm_correction(
204205
candidate: CorrectionCandidate,
205206
accepted: list[Artifact],
206207
basis: bool,
208+
reason: str,
207209
fingerprint: Callable[[MasterSeed], bytes] | None = None,
208210
) -> bool | None:
209211
_require_correction_confirmation(candidate.low_checksum_discrimination)
@@ -226,6 +228,7 @@ def _confirm_correction(
226228
except CodexError:
227229
_stderr("Rejected: Could not recover a valid Bitcoin master seed using this correction.")
228230
return None
231+
_stderr(f"Invalid: {reason}")
229232
_stderr(f"Possible correction:\n\n{fingerprint_text}{_card_text(artifact.text, sys.stderr.isatty())}\n")
230233
return _confirmation_input(
231234
"Does this entire string exactly match your recovery card? [y/N]: "
@@ -644,7 +647,7 @@ def _redirected(
644647
for token in tokens:
645648
try:
646649
artifact = _parse(token, profiles)
647-
except InputError:
650+
except InputError as error:
648651
if one:
649652
raise
650653

@@ -671,7 +674,7 @@ def allowed(candidate: CorrectionCandidate) -> bool:
671674
)
672675
if len(candidates) != 1:
673676
raise
674-
if not _confirm_correction(candidates[0], accepted, basis, fingerprint):
677+
if not _confirm_correction(candidates[0], accepted, basis, str(error), fingerprint):
675678
raise CorrectionDeclined
676679
artifact = candidates[0].artifact
677680
_validate_operational_artifact(
@@ -686,6 +689,7 @@ def allowed(candidate: CorrectionCandidate) -> bool:
686689

687690

688691
_FRIENDLY_SET_ERRORS: dict[type[Exception], str] = {
692+
InvalidCase: "A codex32 string is all uppercase or all lowercase; either case recovers the same wallet.",
689693
InvalidChecksum: "The checksum does not match.",
690694
MismatchedProfile: "These strings are for different applications.",
691695
MismatchedThreshold: "These strings require different numbers of shares.",
@@ -786,7 +790,7 @@ def allowed(candidate: CorrectionCandidate) -> bool:
786790
)
787791
)
788792
confirmation = (
789-
_confirm_correction(candidates[0], accepted, basis, fingerprint)
793+
_confirm_correction(candidates[0], accepted, basis, str(error), fingerprint)
790794
if len(candidates) == 1
791795
else None
792796
)

‎src/codex32/cli.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
_confirm_correction,
2626
_entered_groups,
2727
_fingerprint_matcher,
28+
_parse,
2829
_render_groups,
2930
_require_correction_confirmation,
3031
_scheduled_candidates,
@@ -264,7 +265,7 @@ def _creation_source(
264265
and isinstance(candidate := candidates[0].artifact, Secret)
265266
and candidate.profile is profile
266267
):
267-
confirmation = _confirm_correction(candidates[0], [], False, fingerprint)
268+
confirmation = _confirm_correction(candidates[0], [], False, str(error), fingerprint)
268269
if confirmation is True:
269270
return candidate
270271
if confirmation is False:
@@ -603,9 +604,9 @@ def _correct(
603604
if context.master_seed and hrp != Profile.MS.value:
604605
raise _UsageError("This command accepts only Bitcoin master-seed input beginning with ms1.")
605606
try:
606-
parse_codex32(normalized)
607-
except CodexError:
608-
pass
607+
_parse(normalized, None)
608+
except _UsageError as error:
609+
reason = str(error)
609610
else:
610611
if isinstance(byte_length, int) and len(normalized) != _ms_text_length(byte_length):
611612
raise _UsageError("--bytes does not match the valid master-seed backup length.")
@@ -639,6 +640,7 @@ def _correct(
639640
raise _CommandError("Several corrections are possible. Check the original backup.")
640641
fixed = candidates[0]
641642
_require_correction_confirmation(fixed.low_checksum_discrimination)
643+
_print(f"Invalid: {reason}", err=True)
642644
if context.master_seed:
643645
core = core or _connected_core("correct")
644646
warning = (

‎tests/test_cli.py‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,9 @@ def answer(prompt: str) -> str:
376376
assert sys.stdout is not sys.stderr
377377
assert prompts == ["Enter a codex32 string:\n> "] * 2
378378
assert rejected not in captured.out
379-
assert "Rejected: Use either all uppercase or all lowercase letters." in captured.err
380-
assert "Rejected: Use either all uppercase or all lowercase letters.\n\n" in captured.err
379+
assert (
380+
"Rejected: A codex32 string is all uppercase or all lowercase; either case recovers the same wallet.\n\n"
381+
) in captured.err
381382
assert captured.err.endswith("\n\n")
382383

383384

@@ -1760,6 +1761,33 @@ def test_cli_rejects_sixteen_consecutive_erasures_as_outside_regular_bound() ->
17601761
assert "No valid correction found" in result.stderr
17611762

17621763

1764+
_UPPER_SECRET = VECTOR_1["secret_s"].upper()
1765+
1766+
1767+
@pytest.mark.parametrize(
1768+
("damaged", "reason"),
1769+
(
1770+
(_UPPER_SECRET[:-1] + "w", "A codex32 string is all uppercase or all lowercase;"),
1771+
(
1772+
_UPPER_SECRET[:3] + "A" + _UPPER_SECRET[4:],
1773+
"The threshold must be 0 or a number from 2 through 9;",
1774+
),
1775+
(
1776+
_UPPER_SECRET[:8] + "A" + _UPPER_SECRET[9:],
1777+
"An unshared secret (threshold 0) must use S as its index.",
1778+
),
1779+
(_UPPER_SECRET[:12] + "B" + _UPPER_SECRET[13:], "The character 'b' is not allowed"),
1780+
(_UPPER_SECRET[:20] + "Q" + _UPPER_SECRET[20:], "This input has 49 characters."),
1781+
),
1782+
)
1783+
def test_correct_says_why_the_input_was_invalid(damaged: str, reason: str) -> None:
1784+
result = _invoke(["correct"], damaged)
1785+
1786+
assert result.exit_code == 1
1787+
assert result.stderr.startswith(f"Invalid: {reason}")
1788+
assert result.stderr.endswith(f"{_UPPER_SECRET}\n")
1789+
1790+
17631791
def test_correct_rejects_malformed_immutable_hrp_as_usage() -> None:
17641792
damaged = "é" + VECTOR_1["secret_s"][1:]
17651793

@@ -2537,8 +2565,11 @@ def test_operational_candidate_whole_card_confirmation(monkeypatch, capsys, resp
25372565
monkeypatch.setattr(module, "_editable_input", lambda prompt: prompts.append(prompt) or response)
25382566
monkeypatch.setattr(module.sys.stderr, "isatty", lambda: True)
25392567
candidate = CorrectionCandidate(artifact, (), 1, 0, 0, None, capture_space_bits=65)
2540-
assert module._confirm_correction(candidate, [], False, _FakeBitcoinCore().fingerprint) is accepted
2568+
reason = "The checksum does not match."
2569+
fingerprint = _FakeBitcoinCore().fingerprint
2570+
assert module._confirm_correction(candidate, [], False, reason, fingerprint) is accepted
25412571
output = capsys.readouterr().err
2572+
assert output.startswith(f"Invalid: {reason}\nPossible correction:\n\n")
25422573
assert "Master fingerprint: 3F3521A6\n\n" in output
25432574
assert module._card_text(artifact.text) in output
25442575
assert "> " not in output
@@ -2556,12 +2587,12 @@ def test_final_share_preview_is_isolated_and_basis_has_no_preview(monkeypatch, c
25562587
accepted = [first]
25572588
monkeypatch.setattr(module.sys, "stdin", _TTYInput())
25582589
monkeypatch.setattr(module, "_editable_input", lambda prompt: "n")
2559-
assert not module._confirm_correction(candidate, accepted, False, _FakeBitcoinCore().fingerprint)
2590+
assert not module._confirm_correction(candidate, accepted, False, "", _FakeBitcoinCore().fingerprint)
25602591
assert accepted == [first]
25612592
assert "Master fingerprint: FAB6868A\n\n" in capsys.readouterr().err
2562-
assert not module._confirm_correction(candidate, accepted, True, _FakeBitcoinCore().fingerprint)
2593+
assert not module._confirm_correction(candidate, accepted, True, "", _FakeBitcoinCore().fingerprint)
25632594
assert "Master fingerprint" not in capsys.readouterr().err
2564-
assert not module._confirm_correction(candidate, [], False, _FakeBitcoinCore().fingerprint)
2595+
assert not module._confirm_correction(candidate, [], False, "", _FakeBitcoinCore().fingerprint)
25652596
assert "Master fingerprint" not in capsys.readouterr().err
25662597

25672598

@@ -2576,7 +2607,7 @@ def fail(seed):
25762607
candidate = CorrectionCandidate(
25772608
parse_codex32(VECTOR_1["secret_s"]), (), 1, 0, 0, None, capture_space_bits=65
25782609
)
2579-
assert not module._confirm_correction(candidate, [], False, fail)
2610+
assert not module._confirm_correction(candidate, [], False, "", fail)
25802611
assert "Could not recover a valid Bitcoin master seed using this correction." in capsys.readouterr().err
25812612

25822613

‎tests/test_correction_disclosure.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ def test_yes_still_requires_independent_whole_card_acceptance(monkeypatch):
203203
answers = iter(("YES", "n"))
204204
monkeypatch.setattr(_cli_input, "_editable_input", lambda prompt: prompts.append(prompt) or next(answers))
205205
with patch.object(sys, "stdin", _TTYInput()), contextlib.redirect_stderr(_TTYOutput()):
206-
assert _cli_input._confirm_correction(candidate, [], False) is False
206+
assert _cli_input._confirm_correction(candidate, [], False, "") is False
207207
assert prompts == [
208208
"If you understand this, type YES to attempt to correct the data: ",
209209
"Does this entire string exactly match your recovery card? [y/N]: ",

0 commit comments

Comments
 (0)