From 799bffe916b395a1c8aef6ae3204c10f24ac0e91 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 09:08:46 +0000 Subject: [PATCH 1/3] cli: Name the real Bitcoin Core requirement when it is missing Without bitcoin-cli every ms32 command said "Install a reviewed bitcoin-cli before creating a backup", even commands that create nothing, and the codex32 hint didn't say what it leaves out. Say that Bitcoin Core 32 or newer must run with RPC enabled and that an unsynced regtest or signet node is enough for practice. Then say what the command uses Core for: the master fingerprint and correction ranking for secret, share and correct (with the codex32 fallback and what it omits), or giving Core the master key for create and wallet. ms32 correct now connects before its search instead of after it, so a missing Core no longer costs up to ten seconds of discarded work. Closes #84 Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa --- src/codex32/_bitcoin_core.py | 7 ++++-- src/codex32/cli.py | 16 ++++++------- tests/test_bitcoin_core.py | 11 +++++++-- tests/test_cli.py | 35 ++++++++++++++++++++++++++++- tests/test_correction_disclosure.py | 11 +++++++-- 5 files changed, 65 insertions(+), 15 deletions(-) diff --git a/src/codex32/_bitcoin_core.py b/src/codex32/_bitcoin_core.py index 5b78a74..67052fb 100644 --- a/src/codex32/_bitcoin_core.py +++ b/src/codex32/_bitcoin_core.py @@ -102,7 +102,10 @@ def connect( ) -> BitcoinCore: executable = shutil.which("bitcoin-cli") if executable is None: - raise BitcoinCoreError("Install a reviewed bitcoin-cli before creating a backup.") + raise BitcoinCoreError( + "bitcoin-cli was not found. Install a reviewed Bitcoin Core 32 or newer and run it with RPC " + "enabled. An unsynced regtest or signet node is enough for practice." + ) choices: list[BitcoinCore] = [] for chain, _label in _CHAINS: client = cls(executable, chain, 0) @@ -119,7 +122,7 @@ def connect( choices.append(cls(executable, chain, version)) if not choices: raise BitcoinCoreError( - "No local Bitcoin Core RPC server found.\nStart Bitcoin Core " + "No local Bitcoin Core 32 or newer RPC server found.\nStart Bitcoin Core " "with local RPC enabled.\nFor signet practice: bitcoin-qt -signet -server" ) if len(choices) > 1: diff --git a/src/codex32/cli.py b/src/codex32/cli.py index 6e81e43..d2603b1 100644 --- a/src/codex32/cli.py +++ b/src/codex32/cli.py @@ -452,12 +452,13 @@ def _connected_core(fallback: str | None = None) -> BitcoinCore: except KeyboardInterrupt as error: raise _CoreSelectionInterrupted from error except BitcoinCoreError as error: - suggestion = ( - f" Run 'codex32 {fallback}' instead for a Core-independent operation." + use = ( + "uses Bitcoin Core to show the master fingerprint and rank corrections. " + f"'codex32 {fallback}' works without Core but doesn't show the fingerprint." if fallback is not None - else "" + else "gives Bitcoin Core the master key." ) - raise _CommandError(str(error) + suggestion) from error + raise _CommandError(f"{error}\nThis command {use}") from error def _create( @@ -611,6 +612,8 @@ def _correct( raise _UsageError("--bytes does not match the valid master-seed backup length.") _print("The codex32 string is already valid.") return 0 + if context.master_seed: + core = core or _connected_core("correct") search_value, erased, immutable = normalized, normalized, normalized[: separator + 1] interpreted = _case_interpretation(normalized, immutable, context.profiles, None) if interpreted is not None: @@ -632,15 +635,12 @@ def _correct( raise _CommandError("The correction search did not complete within ten seconds.") if not candidates: raise _CommandError("No valid correction found. Check the original backup.") - if context.master_seed and len(candidates) > 1: - core = core or _connected_core("correct") + if core is not None and len(candidates) > 1: candidates = _best(candidates, fingerprint_match=_fingerprint_matcher(core.fingerprint)) if len(candidates) != 1: raise _CommandError("Several corrections are possible. Check the original backup.") fixed = candidates[0] _require_correction_confirmation(fixed.low_checksum_discrimination) - if context.master_seed: - core = core or _connected_core("correct") warning = ( "Warning: This is only a correction suggestion. Compare it with the original backup before using it." ) diff --git a/tests/test_bitcoin_core.py b/tests/test_bitcoin_core.py index f6323c9..7c97877 100644 --- a/tests/test_bitcoin_core.py +++ b/tests/test_bitcoin_core.py @@ -176,7 +176,7 @@ def test_preflight_rejection_is_helpful_without_echoing_core_output( message = str(failure.value) assert message == ( - "No local Bitcoin Core RPC server found.\n" + "No local Bitcoin Core 32 or newer RPC server found.\n" "Start Bitcoin Core with local RPC enabled.\n" "For signet practice: bitcoin-qt -signet -server" ) @@ -199,7 +199,14 @@ def run(command: list[str], **_options: object) -> subprocess.CompletedProcess[s monkeypatch.setattr("codex32._bitcoin_core.shutil.which", lambda _name: "/reviewed/bitcoin-cli") monkeypatch.setattr(subprocess, "run", run) - with pytest.raises(BitcoinCoreError, match="No local Bitcoin Core RPC server"): + with pytest.raises(BitcoinCoreError, match="No local Bitcoin Core 32 or newer RPC server"): + BitcoinCore.connect() + + +def test_missing_bitcoin_cli_names_the_core_requirement(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("codex32._bitcoin_core.shutil.which", lambda _name: None) + + with pytest.raises(BitcoinCoreError, match="Bitcoin Core 32 or newer and run it with RPC enabled"): BitcoinCore.connect() diff --git a/tests/test_cli.py b/tests/test_cli.py index 889b175..8125545 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -32,7 +32,7 @@ parse_codex32, recover_secret, ) -from codex32._bitcoin_core import BitcoinCore +from codex32._bitcoin_core import BitcoinCore, BitcoinCoreError from codex32.bech32 import _chars_to_u5, bech32_encode from codex32.checksums import _CODEX32, _CODEX32_LONG from codex32.cli import main, ms_main @@ -287,6 +287,39 @@ def test_check_accepts_shared_core_lightning_artifacts(value: str) -> None: assert result.stderr == "" +def _missing_core(*_args: object, **_kwargs: object) -> BitcoinCore: + raise BitcoinCoreError("bitcoin-cli was not found.") + + +@pytest.mark.parametrize("command", ("secret", "share", "correct")) +def test_missing_core_names_the_codex32_fallback( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], command: str +) -> None: + def search(*_args: object, **_kwargs: object) -> None: + raise AssertionError("searched before connecting to Bitcoin Core") + + monkeypatch.setattr("codex32.cli.BitcoinCore.connect", _missing_core) + monkeypatch.setattr("codex32.cli._scheduled_candidates", search) + monkeypatch.setattr(sys, "stdin", io.StringIO(VECTOR_1["secret_s"].replace("x", "q", 1) + "\n")) + + assert ms_main([command, "d"] if command == "share" else [command]) == (3 if command == "correct" else 1) + assert capsys.readouterr().err == ( + f"ms32 {command}: bitcoin-cli was not found.\nThis command uses Bitcoin Core to show the master " + f"fingerprint and rank corrections. 'codex32 {command}' works without Core but doesn't show the " + "fingerprint.\n" + ) + + +def test_missing_core_for_wallet_setup_offers_no_fallback(monkeypatch: pytest.MonkeyPatch) -> None: + cli = importlib.import_module("codex32.cli") + monkeypatch.setattr("codex32.cli.BitcoinCore.connect", _missing_core) + + with pytest.raises(cli._CommandError) as failure: + cli._connected_core() + + assert str(failure.value) == "bitcoin-cli was not found.\nThis command gives Bitcoin Core the master key." + + def test_check_does_not_derive_wallet_keys(monkeypatch: pytest.MonkeyPatch) -> None: cli_module = importlib.import_module("codex32.cli") diff --git a/tests/test_correction_disclosure.py b/tests/test_correction_disclosure.py index 08592ee..48c04e3 100644 --- a/tests/test_correction_disclosure.py +++ b/tests/test_correction_disclosure.py @@ -97,17 +97,24 @@ def test_public_api_full_checksum_erasure_completion_carries_risk(source, degree @pytest.mark.parametrize("entrypoint", (cli.main, cli.ms_main)) @pytest.mark.parametrize("plain", (False, True)) def test_noninteractive_gate_emits_only_operational_error(entrypoint, plain): - stdout, stderr = io.StringIO(), io.StringIO() + stdout, stderr, connected = io.StringIO(), io.StringIO(), [] + + def connect(*args): + connected.append(_FakeBitcoinCore()) + return connected[-1] + with ( patch.object(sys, "stdin", io.StringIO(VECTOR_1["secret_s"][:-1] + "?")), contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr), patch.object(_cli_input, "_correction_candidates", return_value=((_candidate(),), True, None)), + patch.object(cli.BitcoinCore, "connect", connect), ): status = entrypoint(["correct", *(["--plain"] if plain else [])]) prog = "codex32" if entrypoint is cli.main else "ms32" assert status == 3 and stdout.getvalue() == "" - assert stderr.getvalue() == f"{prog}: interactive confirmation required\n" + # ms32 connects to Core before searching; that prints only a blank line. + assert stderr.getvalue() == "\n" * len(connected) + f"{prog}: interactive confirmation required\n" @pytest.mark.parametrize("answer", ("y", "Y", "yes", "Yes", "n", "", "other", None)) From c583b2479359f1ced43420422e02cfa8c3d189e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 14:06:55 +0000 Subject: [PATCH 2/3] cli: Keep Core selection off pipes and redirected stderr Connecting before the search made two problems easier to hit. With damaged input piped to `ms32 correct` and two Core networks running, the network prompt read the exhausted pipe forever. With stderr redirected, "Using Bitcoin Core on ..." and a blank line came before `interactive confirmation required`, which the security model says must be the only message. Ask for a network only when stdin is a terminal; a pipe now gets "More than one local Bitcoin Core network is running." Print Core's messages only when stderr is a terminal. The gate test's fake now reports like the real one, so it catches the extra output. Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa --- src/codex32/cli.py | 8 +++++--- tests/test_cli.py | 28 ++++++++++++++++++++++++++++ tests/test_correction_disclosure.py | 12 ++++++------ 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/codex32/cli.py b/src/codex32/cli.py index d2603b1..758fd8d 100644 --- a/src/codex32/cli.py +++ b/src/codex32/cli.py @@ -442,12 +442,14 @@ def _initialize_wallet( def _connected_core(fallback: str | None = None) -> BitcoinCore: + # A pipe can't choose a network, and redirected stderr keeps only the gate's error. try: core = BitcoinCore.connect( - lambda prompt: _text(prompt, optional=True), - lambda message: _print(message, err=True), + (lambda prompt: _text(prompt, optional=True)) if sys.stdin.isatty() else None, + (lambda message: _print(message, err=True)) if sys.stderr.isatty() else None, ) - _print("", err=True) + if sys.stderr.isatty(): + _print("", err=True) return core except KeyboardInterrupt as error: raise _CoreSelectionInterrupted from error diff --git a/tests/test_cli.py b/tests/test_cli.py index 8125545..a734aeb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4,6 +4,7 @@ import contextlib import importlib import io +import json import re import subprocess import sys @@ -123,6 +124,9 @@ def initialize( return "test-wallet" +_REAL_CONNECT = BitcoinCore.connect + + @pytest.fixture(autouse=True) def _offline_core(monkeypatch): monkeypatch.setattr("codex32.cli.BitcoinCore.connect", lambda *args, **kwargs: _FakeBitcoinCore()) @@ -320,6 +324,30 @@ def test_missing_core_for_wallet_setup_offers_no_fallback(monkeypatch: pytest.Mo assert str(failure.value) == "bitcoin-cli was not found.\nThis command gives Bitcoin Core the master key." +def test_piped_input_never_answers_the_core_network_choice( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + def run(command: list[str], **_options: object) -> subprocess.CompletedProcess[str]: + chain = command[1].removeprefix("-chain=") + if chain not in ("main", "signet"): + return subprocess.CompletedProcess(command, 1, "", "") + response = {"version": 320000} if command[-1] == "getnetworkinfo" else {"chain": chain} + return subprocess.CompletedProcess(command, 0, json.dumps(response), "") + + damaged = VECTOR_1["secret_s"].replace("x", "q", 1) + reads = iter((damaged,)) # A second read would be the pipe's EOF answering the network prompt. + monkeypatch.setattr("codex32._bitcoin_core.shutil.which", lambda _name: "/reviewed/bitcoin-cli") + monkeypatch.setattr(subprocess, "run", run) + monkeypatch.setattr(BitcoinCore, "connect", _REAL_CONNECT) + monkeypatch.setattr(sys, "stdin", io.StringIO(damaged)) + monkeypatch.setattr("codex32._cli_input._stdin", lambda: next(reads)) + + assert ms_main(["correct"]) == 3 + assert capsys.readouterr().err.startswith( + "ms32 correct: More than one local Bitcoin Core network is running.\nThis command uses Bitcoin Core" + ) + + def test_check_does_not_derive_wallet_keys(monkeypatch: pytest.MonkeyPatch) -> None: cli_module = importlib.import_module("codex32.cli") diff --git a/tests/test_correction_disclosure.py b/tests/test_correction_disclosure.py index 48c04e3..dbfb06f 100644 --- a/tests/test_correction_disclosure.py +++ b/tests/test_correction_disclosure.py @@ -97,11 +97,12 @@ def test_public_api_full_checksum_erasure_completion_carries_risk(source, degree @pytest.mark.parametrize("entrypoint", (cli.main, cli.ms_main)) @pytest.mark.parametrize("plain", (False, True)) def test_noninteractive_gate_emits_only_operational_error(entrypoint, plain): - stdout, stderr, connected = io.StringIO(), io.StringIO(), [] + stdout, stderr = io.StringIO(), io.StringIO() - def connect(*args): - connected.append(_FakeBitcoinCore()) - return connected[-1] + def connect(ask=None, tell=None): + if tell is not None: + tell("Using Bitcoin Core on signet.") + return _FakeBitcoinCore() with ( patch.object(sys, "stdin", io.StringIO(VECTOR_1["secret_s"][:-1] + "?")), @@ -113,8 +114,7 @@ def connect(*args): status = entrypoint(["correct", *(["--plain"] if plain else [])]) prog = "codex32" if entrypoint is cli.main else "ms32" assert status == 3 and stdout.getvalue() == "" - # ms32 connects to Core before searching; that prints only a blank line. - assert stderr.getvalue() == "\n" * len(connected) + f"{prog}: interactive confirmation required\n" + assert stderr.getvalue() == f"{prog}: interactive confirmation required\n" @pytest.mark.parametrize("answer", ("y", "Y", "yes", "Yes", "n", "", "other", None)) From 256998066e5ba5c46274c1bd9dd548ed3a3b63c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 09:34:05 +0000 Subject: [PATCH 3/3] test: Raise the size budget to 5,250 lines Ben authorized raising the budget so #91 fits. The stack tip with the open fix PRs was at 5,197 of 5,200, and #91 adds 26 lines. Update the enforcing test and both places that document the number. Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa --- AGENTS.md | 2 +- docs/developer/api.md | 2 +- tests/test_cli.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fd47d95..cf7df8c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,7 +61,7 @@ avoid comments or tests that restate the implementation. Add or update concise docstrings when changing public behavior. Write codex32 in lowercase except when referring to the Codex32 Book. -Keep the installed package below 5,200 logical review lines, as enforced by the +Keep the installed package below 5,250 logical review lines, as enforced by the existing test. New dependencies, public API signature or return-shape changes, and lint suppressions require user authorization; an explicit request can already provide that authorization. diff --git a/docs/developer/api.md b/docs/developer/api.md index 4e6cd06..dadf99f 100644 --- a/docs/developer/api.md +++ b/docs/developer/api.md @@ -105,7 +105,7 @@ unsupported but remains in the review scope. ### Size budget -V1 keeps the installed package below 5,200 logical review lines, excluding +V1 keeps the installed package below 5,250 logical review lines, excluding blank and comment-only lines while counting subpackages recursively. Changing the budget requires explicit review and authorization together with the matching documentation and enforcement update. diff --git a/tests/test_cli.py b/tests/test_cli.py index a734aeb..dac9620 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2254,7 +2254,7 @@ def test_production_size_budgets_are_enforced() -> None: for path in package.rglob("*.py") } - assert sum(counts.values()) < 5200, counts + assert sum(counts.values()) < 5250, counts @pytest.mark.parametrize(