Repository navigation
cli: Derive several shares per run and allow repeated indices - #103
BenWestgate wants to merge 3 commits into
Conversation
37eef4d to
a7efaae
Compare
10e46b4 to
90de3ef
Compare
a7efaae to
054e8d9
Compare
Gate restore and existing-seed wallet initialization on the independently recorded BIP32 master fingerprint before any Bitcoin Core wallet mutation. Keep the correction path from disclosing or reusing a fingerprint derived from the candidate being authenticated. Fixes #30.
054e8d9 to
115f2c2
Compare
`ms32 share INDEX` derived one share per run, so adding three cards meant entering the existing cards three times. `share` now takes several indices, such as `share cdf`: the operator enters the existing cards once, then writes and confirms each new card. An index that matches an entered card makes a copy of it, and if every requested index matches an entered card, those cards are all it needs. `S` is still rejected and `share` gets no random count. A repeated index makes another copy, in `share` and in `create --indices`. Repeats move after every original (aacd runs a, c, d, a), so the same card is never confirmed twice in a row. In `create` the random cards therefore always have distinct indices, a repeat copies the card already confirmed, and the threshold check counts distinct indices. Every requested card is resolved before any is printed, so a request that cannot be completed prints nothing rather than part of a set. The share-count error now says string(s) instead of artifacts. Closes #87 Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
90de3ef to
d9ce204
Compare
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated review (Codex), posted at the maintainer's request.
ACK d9ce204. Multi-index derivation and repeated-index copies preserve threshold validation and avoid partial output on failure.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9ce204f2f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
BenWestgate
added
cli
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated focused security review (Codex), posted at the maintainer's request.
Security ACK d9ce204f2f6f for the share-generation/repeated-index boundary.
I checked the two invariants called out in the PR body:
_indices()orders first occurrences before later copies, and_selection()counts distinct indices, so the firstthresholddirect cards of a fresh ceremony remain distinct entropy-bearing shares.CreationCeremony.next_share()re-emits the already-confirmed basis card for a repeated index instead of generating a second random share with the same index.share INDICESmay stop interactive input early only when every requested index has been entered. Copy-only output therefore re-emits validated entered cards. If any requested index is new,_share_command()still callsderive_share()and its share-set validation before any output is printed; one failure suppresses the whole output batch.
S remains excluded by the ordinary-index parser, the 31-output bound remains in _indices(), and the new tests exercise repeated ordering/copy identity plus multi-output/no-partial-output behavior.
No security blocker found in this diff. Human review/authorship policy still applies before integration.
|
V1 disposition (Codex, 2026-10-04): defer this post-audit share-generation enhancement from the v1 candidate. Current head #103 materially expands share-generation/output semantics late in the v1 freeze and overlaps #100 in |
BenWestgate
removed
the
gate: adversarial review
Document that repeated explicit indices are physical copies emitted after their first occurrences, while preserving first-occurrence order. Mirror that contract in the public CreationCeremony constructor docstrings so callers do not associate outputs with stale positional semantics.\n\nRefs #87.
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated current-head follow-up review (Codex), performed at the maintainer's request.
ACK 2924f5f53773. The behavior-changing parent d9ce204 already has a focused security ACK with no reportable finding. The only newer commit documents the reviewed repeated-index contract: first occurrences precede later physical copies, the ceremony still requires threshold distinct entropy-bearing indices, and copies reuse confirmed basis cards. That resolves the prior ordering/documentation mismatch without changing runtime behavior. Exact-head Python-package run 760 succeeded.
No remaining automated security/code-review blocker on this head. Responsible-human review/authorship policy still applies before integration.
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head review at 2924f5f: no findings. Multi-index derivation resolves every requested output before printing, repeated indices are explicit copies, and new indices still require a valid threshold basis.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Not CRing +576, read and follow CONTRIBUTING.md:
I cACK the description and want these changes but you'll have to make this easier to review. Either do less, shrink it and/or simplify it. |
Repeated output indices request physical copies of a confirmed share, not new independent shares. Emit first occurrences before copies and still require the threshold number of distinct indices. Each physical card must be confirmed separately, including after an incorrect confirmation and during re-sharing. The code, documentation, and focused tests match the previously validated local three-commit worktree exactly. refs #103
|
Why is this draft? Has it been superceded by 197? |
|
AI-assisted response: #197 supersedes only the repeated-copy creation change (+59/−8 across three files). #103 still contains batch |
|
The remaining CLI work is now isolated in #198 (+77/−47, five files), stacked on #197 (+59/−8, three files). Both preserve the requested behavior without carrying this PR’s old wallet changes. 292 CLI/generation tests pass normally and optimized. #198 remains draft for the existing post-v1 scope. The code is published in these successors; #103’s branch is unchanged. |
Requested by Ben · project thread
Before:
ms32 share INDEXderived one share per run, so three new cards meant entering the existing cards three times. An index matching an entered card was rejected, andcreate --indicesrejected repeats.After:
ms32 share cdftakes the existing cards once, then writes and confirms D, C and F in turn. An index that matches an entered card copies it, and if every requested index matches an entered card, only those cards are needed. A repeated index makes another copy, inshareandcreate --indices, and repeats run after every original:aacdruns a, c, d, a.Sis still rejected andsharehas no random count.How:
_indicesingeneration.pykeeps repeats and orders them in rounds._selectioncounts distinct indices against the threshold.CreationCeremony.next_sharecopies a repeated random card from its basis instead of deriving it._share_commandloops over the requested indices, and interactive entry stops early once the requested indices are all entered cards. The old excluded-index check is gone.Security review: Codex Security reviewed the behavior-changing head
d9ce204on 2026-10-04 with complete coverage of the five changed production files and no reportable finding. The follow-up2924f5fchanges only the public contract/docs and constructor docstrings to resolve the automated review's ordering mismatch: repeated output indices are ordered after their first occurrence, creation still requiresthresholddistinct initial indices, copies come from already-confirmed basis cards, and any requested new index still goes throughderive_share()validation before any batch output is printed.Sremains excluded and the 31-output bound remains enforced.Budget: 5,198 of <5,200. The behavior-changing head passed exact-head GitHub Python-package CI; 927 tests passed there, with Ruff, mypy, and the touched files under
-Oclean. The documentation-only follow-up keepsgit diff --checkclean and remains inside the authorized library cap.Overlaps #100 in
_share_command: when both land, print the fingerprint only when a threshold was entered, since a copy-only run can't recover the secret.Closes #87
🤖 Generated with Claude Code
https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
Generated by Claude Code