diff --git a/docs/developer/api.md b/docs/developer/api.md index 4e6cd06..fdceb81 100644 --- a/docs/developer/api.md +++ b/docs/developer/api.md @@ -259,11 +259,13 @@ The already confirmed `k-1` masks remain fixed. Neither padding rule is BIP93 validity: parsed S strings may use any application-valid discarded bits, which re-sharing preserves exactly. CRC never applies to shares. -Explicit output indices preserve caller order. A share count uses +Explicit output indices preserve first-occurrence order. Repeated indices mean +physical copies and are moved into later rounds, so every first occurrence is +produced before its copies (`aacd` therefore runs as `acda`). A share count uses `SystemRandom.sample` over the 31 ordinary indices and preserves sample order. -There is no entropy injection, sorting, caller-supplied partial-basis -completion, or BIP39 generation. Ceremonies reject copying and serialization; -the CLI does not resume an interrupted ceremony. +There is no entropy injection, caller-supplied partial-basis completion, or +BIP39 generation. Ceremonies reject object copying and serialization; the CLI +does not resume an interrupted ceremony. ## Recovery and additional-share derivation diff --git a/src/codex32/_cli_input.py b/src/codex32/_cli_input.py index cf5d25d..b0e7870 100644 --- a/src/codex32/_cli_input.py +++ b/src/codex32/_cli_input.py @@ -26,7 +26,6 @@ from codex32.errors import ( CodexError, DuplicateShareIndex, - ExistingTargetIndex, InvalidChecksum, InvalidLength, InvalidThreshold, @@ -617,10 +616,7 @@ def _validate_operational_artifact( *, basis: bool, one: bool, - excluded_index: str | None, ) -> None: - if basis and artifact.header.index == excluded_index: - raise ExistingTargetIndex("That index was requested for the additional share.") if not one and (accepted or isinstance(artifact, Share) or basis): recovering = not basis and isinstance(artifact, Share) validator = _validate_recovery_prefix if recovering else _validate_basis_prefix @@ -632,7 +628,6 @@ def _redirected( *, basis: bool, one: bool, - excluded_index: str | None, fingerprint: Callable[[MasterSeed], bytes] | None, ) -> list[Artifact]: tokens = _stdin().split() @@ -655,7 +650,6 @@ def allowed(candidate: CorrectionCandidate) -> bool: accepted, basis=basis, one=one, - excluded_index=excluded_index, ) except CodexError: return False @@ -679,7 +673,6 @@ def allowed(candidate: CorrectionCandidate) -> bool: accepted, basis=basis, one=one, - excluded_index=excluded_index, ) accepted.append(artifact) return accepted @@ -735,7 +728,7 @@ def _interactive( *, basis: bool, one: bool, - excluded_index: str | None, + requested: str, profiles: tuple[Profile, ...] | None, initial_prefix: str, fingerprint: Callable[[MasterSeed], bytes] | None, @@ -750,7 +743,6 @@ def validate(artifact: Artifact) -> None: accepted, basis=basis, one=one, - excluded_index=excluded_index, ) def allowed(candidate: CorrectionCandidate) -> bool: @@ -811,7 +803,7 @@ def allowed(candidate: CorrectionCandidate) -> bool: validate(artifact) except CodexError as error: _stderr(f"Rejected: {_FRIENDLY_SET_ERRORS.get(type(error), str(error))}\n") - duplicate = isinstance(error, (DuplicateShareIndex, ExistingTargetIndex)) + duplicate = isinstance(error, DuplicateShareIndex) prefill = "" if duplicate else _retry_text(entered, prefix) continue prefill = "" @@ -822,6 +814,8 @@ def allowed(candidate: CorrectionCandidate) -> bool: if not accepted: required = artifact.header.threshold accepted.append(artifact) + if requested and set(requested) <= {item.header.index for item in accepted}: + break # Copying entered cards needs no threshold. prefix = _accepted_prefix(accepted) grouped_prefix = len(entered.split()) > 1 if len(accepted) < required: @@ -833,7 +827,7 @@ def read_artifacts( *, basis: bool = False, one: bool = False, - excluded_index: str | None = None, + requested: str = "", profiles: tuple[Profile, ...] | None = None, initial_prefix: str = "", fingerprint: Callable[[MasterSeed], bytes] | None = None, @@ -843,13 +837,12 @@ def read_artifacts( profiles, basis=basis, one=one, - excluded_index=excluded_index, fingerprint=fingerprint, ) result = _interactive( basis=basis, one=one, - excluded_index=excluded_index, + requested=requested, profiles=profiles, initial_prefix=initial_prefix, fingerprint=fingerprint, diff --git a/src/codex32/_cli_parser.py b/src/codex32/_cli_parser.py index c247210..e32a8f2 100644 --- a/src/codex32/_cli_parser.py +++ b/src/codex32/_cli_parser.py @@ -15,9 +15,7 @@ class _Parser(argparse.ArgumentParser): def error(self, message: str) -> NoReturn: if message.startswith("the following arguments are required: "): message = ( - "Choose an index for the additional share." - if message.endswith("INDEX") - else "Choose a command." + "Choose indices for the new shares." if message.endswith("INDICES") else "Choose a command." ) elif message.startswith("unrecognized arguments: "): message = "Remove or correct these arguments: " + message.removeprefix("unrecognized arguments: ") @@ -113,11 +111,11 @@ def parser(prog: str = "codex32", *, master_seed: bool = False) -> argparse.Argu "derive a share from codex32 strings", ) share.description = ( - "Derive a share at INDEX using exactly the threshold number of codex32 strings from the same set. " - "Use different input indices; one input may be the secret. " - "INDEX must differ from S and the input indices." + "Derive a share at each of INDICES using the threshold number of codex32 strings from the same set. " + "Use different input indices; one input may be the secret. An entered or repeated index makes a copy; " + "copies need only the entered cards. INDICES cannot include S." ) - share.add_argument("index", metavar="INDEX", help="index for the derived share") + share.add_argument("index", metavar="INDICES", help="indices for the new shares, such as d or cdf") share.add_argument("--plain", action="store_true", help="print without formatting or card confirmation") correct = _command(commands, "correct", "suggest repairs for a damaged codex32 string") @@ -176,7 +174,9 @@ def parser(prog: str = "codex32", *, master_seed: bool = False) -> argparse.Argu metavar="COUNT", help="number of shares to output (defaults: 3 for threshold 2; 5 for threshold 3)", ) - create.add_argument("--indices", metavar="INDICES", help="exact share indices, in output order") + create.add_argument( + "--indices", metavar="INDICES", help="exact share indices; repeats make copies, output last" + ) create.add_argument( "--existing", action="store_true", diff --git a/src/codex32/bip93.py b/src/codex32/bip93.py index 9e3757c..3f5ce19 100644 --- a/src/codex32/bip93.py +++ b/src/codex32/bip93.py @@ -280,7 +280,7 @@ def _validate_share_set(artifacts: Sequence[Share | Secret], *, require_exact: b if threshold not in range(2, 10): raise MismatchedThreshold("linear sharing requires threshold 2 through 9") if len(copied) > threshold or (require_exact and len(copied) != threshold): - raise WrongShareCount(f"threshold is {threshold}, but {len(copied)} artifacts were supplied") + raise WrongShareCount(f"threshold is {threshold}, but {len(copied)} string(s) were supplied") first_tail, checksum_length, encoded_length = _artifact_tail(first) tails = [first_tail] indices = [first.header.index] diff --git a/src/codex32/cli.py b/src/codex32/cli.py index 6e81e43..15f8bb0 100644 --- a/src/codex32/cli.py +++ b/src/codex32/cli.py @@ -39,7 +39,6 @@ Header, Secret, Share, - _normalize_target, derive_share, parse_codex32, recover_secret, @@ -54,6 +53,7 @@ from codex32.generation import ( ConfirmationResult, CreationCeremony, + _indices, generate_master_seed, ) from codex32.profiles import Profile, _profile_rules @@ -173,30 +173,36 @@ def _emit( ) -def _share_command(index: str, plain: bool, context: _CliContext, core: BitcoinCore | None = None) -> int: +def _share_command(indices: str, plain: bool, context: _CliContext, core: BitcoinCore | None = None) -> int: try: - index = _normalize_target(index, label="share index") - except CodexError as error: - raise _UsageError(f"Choose one share index from {IDX_SORT[1:].upper()}.") from error + requested = _indices(indices) + except CodexError: + requested = () + if not requested: + raise _UsageError(f"Choose share indices from {IDX_SORT[1:].upper()}.") artifacts = _artifacts( basis=True, - excluded_index=index, + requested="".join(requested), profiles=context.profiles, initial_prefix=context.initial_prefix, fingerprint=core.fingerprint if core is not None else None, ) - try: - derived = derive_share(artifacts, index) + entered = {artifact.header.index: artifact for artifact in artifacts} + try: # Resolve every card before printing any, so a failure prints none. + shares = [ + entered[index] if index in entered else derive_share(artifacts, index) for index in requested + ] except CodexError as error: raise _CommandError(str(error)) from error - _emit(derived, plain) - if sys.stdin.isatty() and sys.stdout.isatty() and not plain: - try: - _confirm_card(derived) - except (EOFError, KeyboardInterrupt) as error: - _print("Recovery card not confirmed.", err=True) - return 130 if isinstance(error, KeyboardInterrupt) else 2 - _print("Recovery card confirmed.", err=True) + for share in shares: + _emit(share, plain) + if sys.stdin.isatty() and sys.stdout.isatty() and not plain: + try: + _confirm_card(share) + except (EOFError, KeyboardInterrupt) as error: + _print("Recovery card not confirmed.", err=True) + return 130 if isinstance(error, KeyboardInterrupt) else 2 + _print("Recovery card confirmed.", err=True) return 0 diff --git a/src/codex32/generation.py b/src/codex32/generation.py index f135d19..277e693 100644 --- a/src/codex32/generation.py +++ b/src/codex32/generation.py @@ -91,9 +91,12 @@ def _indices(values: Sequence[str] | str) -> tuple[str, ...]: raise InvalidShareSelection("at most 31 shares may be requested") copied: tuple[object, ...] = tuple(values[position] for position in range(len(values))) normalized = tuple(_index(value) for value in copied) - if len(set(normalized)) != len(normalized): - raise InvalidShareSelection("output indices must be distinct") - return normalized + # Repeats are copies, output in rounds after every original: aacd runs a, c, d, a. + rounds = sorted( + enumerate(normalized), + key=lambda item: (normalized[: item[0]].count(item[1]), normalized.index(item[1])), + ) + return tuple(index for _position, index in rounds) def _selection(threshold: int, share_count: object, indices: Sequence[str] | str | None) -> tuple[str, ...]: @@ -107,8 +110,8 @@ def _selection(threshold: int, share_count: object, indices: Sequence[str] | str return tuple(secrets.SystemRandom().sample(ORDINARY_INDICES, share_count)) assert indices is not None selected = _indices(indices) - if len(selected) < threshold: - raise InvalidShareSelection(f"at least {threshold} indices are required") + if len(set(selected)) < threshold: + raise InvalidShareSelection(f"at least {threshold} distinct indices are required") return selected @@ -242,7 +245,7 @@ def master_seed( indices: Sequence[str] | str | None = None, identifier: str | None = None, ) -> CreationCeremony: - """Start a ceremony for a fresh shared Bitcoin master seed.""" + """Start a ceremony for a fresh shared Bitcoin master seed; repeated indices are later copies.""" threshold = _threshold(threshold, allow_zero=False) _supplied, byte_length = _seed_input(None, byte_length) identifier = _random_identifier() if identifier is None else _identifier(identifier) @@ -265,7 +268,7 @@ def core_lightning( indices: Sequence[str] | str | None = None, identifier: str | None = None, ) -> CreationCeremony: - """Start a ceremony for a fresh shared Core Lightning secret.""" + """Start a ceremony for a fresh shared Core Lightning secret; repeated indices are later copies.""" threshold = _threshold(threshold, allow_zero=False) identifier = _random_identifier() if identifier is None else _identifier(identifier) return cls._start( @@ -288,7 +291,7 @@ def from_secret( indices: Sequence[str] | str | None = None, identifier: str | None = None, ) -> CreationCeremony: - """Start a ceremony that shares an existing validated secret.""" + """Start a ceremony that shares an existing validated secret; repeated indices are later copies.""" if not isinstance(secret, (MasterSeed, CoreLightningSecret)): raise TypeError("from_secret accepts only MasterSeed or CoreLightningSecret") threshold = _threshold(threshold, allow_zero=False) @@ -343,7 +346,8 @@ def next_share(self) -> Share: self._secret = candidate break else: - pending = derive_share(tuple(self._basis), index) + copies = [share for share in self._basis if share.header.index == index] + pending = cast(Share, copies[0]) if copies else derive_share(tuple(self._basis), index) self._pending = pending return pending diff --git a/tests/test_cli.py b/tests/test_cli.py index 889b175..b15cd32 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1194,7 +1194,7 @@ def test_share_supports_ms_cl_and_bip39() -> None: assert bip39.stdout.strip() == SHARING_VECTORS["bip39_12w"]["D"] -@pytest.mark.parametrize("index", ("b", "i", "1", "s", "aa")) +@pytest.mark.parametrize("index", ("b", "i", "1", "s", "ds", "")) def test_share_rejects_invalid_target_before_prompting( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], @@ -1210,7 +1210,7 @@ def forbidden(_prompt: str) -> str: assert main(["share", index]) == 2 assert capsys.readouterr().err == ( - "codex32 share: Choose one share index from ACDEFGHJKLMNPQRTUVWXYZ023456789.\n" + "codex32 share: Choose share indices from ACDEFGHJKLMNPQRTUVWXYZ023456789.\n" ) @@ -1219,17 +1219,16 @@ def test_share_argument_errors_are_actionable() -> None: assert missing.exit_code == 2 assert missing.stderr == ( - "usage: codex32 share [-h] [--plain] INDEX\n" - "codex32 share: Choose an index for the additional share.\n" + "usage: codex32 share [-h] [--plain] INDICES\ncodex32 share: Choose indices for the new shares.\n" ) -def test_tty_share_rejects_target_index_as_soon_as_entered( +def test_tty_share_copies_an_entered_card_without_the_threshold( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: input_module = importlib.import_module("codex32._cli_input") - answers = iter((VECTOR_2["derived_D"], VECTOR_2["share_A"], VECTOR_2["share_C"])) + answers = iter((VECTOR_2["derived_D"],)) editor = _FakeLineEditor() def answer(_prompt: str) -> str: @@ -1240,13 +1239,28 @@ def answer(_prompt: str) -> str: monkeypatch.setattr(input_module, "_line_editor", editor) monkeypatch.setattr(builtins, "input", answer) - assert main(["share", "d"]) == 0 + assert main(["share", "dd"]) == 0 captured = capsys.readouterr() - assert captured.out.strip() == VECTOR_2["derived_D"] - assert "Rejected: That index was requested for the additional share." in captured.err + assert captured.out.split() == [VECTOR_2["derived_D"]] * 2 + assert "Rejected" not in captured.err assert editor.inserted == [] +def test_share_derives_several_indices_and_copies_entered_ones() -> None: + result = _invoke(["share", "dc"], VECTOR_2["share_A"], VECTOR_2["share_C"]) + + assert result.exit_code == 0 + assert result.stdout.split() == [VECTOR_2["derived_D"], VECTOR_2["share_C"]] + + +def test_share_prints_nothing_when_any_requested_card_fails() -> None: + result = _invoke(["share", "ac"], VECTOR_2["share_A"]) + + assert result.exit_code == 1 + assert result.stdout == "" + assert result.stderr == "codex32 share: threshold is 2, but 1 string(s) were supplied\n" + + def test_create_defaults_to_an_unshared_128_bit_master_seed() -> None: result = _invoke_confirmed_create(["create"]) artifacts = _output_artifacts(result) diff --git a/tests/test_generation.py b/tests/test_generation.py index 878e795..2e15754 100644 --- a/tests/test_generation.py +++ b/tests/test_generation.py @@ -187,6 +187,27 @@ def test_invalid_share_selections(arguments: dict[str, object]) -> None: CreationCeremony.master_seed(identifier="test", **arguments) # type: ignore[arg-type] +@pytest.mark.parametrize( + ("indices", "order", "copies"), + (("aacd", "acda", ((0, 3),)), ("acdd", "acdd", ((2, 3),)), ("caac", "caca", ((0, 2), (1, 3)))), +) +def test_repeated_indices_are_copies_after_every_original( + indices: str, order: str, copies: tuple[tuple[int, int], ...] +) -> None: + source = generate_master_seed(bytes(range(16)), identifier="test") + for ceremony in ( + CreationCeremony.master_seed(threshold=2, indices=indices, identifier="test"), + CreationCeremony.from_secret(source, threshold=2, indices=indices, identifier="name"), + ): + shares = [] + for _ in indices: + shares.append(ceremony.next_share()) + assert ceremony.confirm(shares[-1].text).accepted + assert "".join(share.header.index for share in shares) == order + assert all(shares[first].text == shares[copy].text for first, copy in copies) + ceremony.finish() + + def test_oversized_index_strings_are_bounded_before_normalization( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/tests/test_generic_hrp.py b/tests/test_generic_hrp.py index 9111a7d..0d6c011 100644 --- a/tests/test_generic_hrp.py +++ b/tests/test_generic_hrp.py @@ -148,9 +148,10 @@ def test_cli_split_and_unknown_neutral_summary() -> None: ) in secret_help share_help = _invoke(ms_main, ["share", "--help"])[1] assert ( - "Derive a share at INDEX using exactly the threshold number of codex32 strings\n" - "from the same set. Use different input indices; one input may be the secret.\n" - "INDEX must differ from S and the input indices." + "Derive a share at each of INDICES using the threshold number of codex32\n" + "strings from the same set. Use different input indices; one input may be the\n" + "secret. An entered or repeated index makes a copy; copies need only the\n" + "entered cards. INDICES cannot include S." ) in share_help wallet_help = _invoke(ms_main, ["wallet", "--help"])[1] assert wallet_help == (