wallet: Check existing seed before sharing - #81
BenWestgate wants to merge 5 commits into
Conversation
BenWestgate
added
bug
![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: 1af1e2c951
ℹ️ 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
area: security
ChatGPT Codex Connector
|
Current-head agent review ACK for |
1899011 to
0793590
Compare
ff0f3b2 to
7d5ebfd
Compare
Reassign the expected_fingerprint argument instead of copying it into a local, and give existing_secret its None default before the source checks instead of in an else branch. Behavior is unchanged. The installed package drops from 5161 to 5159 logical review lines, which keeps the integrated #7/#42/#57/#46/#80/#81 tip under the <5200 budget. Security: the record gate still runs before any card is generated or shown, and interrupts at that gate still raise _WalletSetupInterrupted. Validation: ruff check, ruff format --check, mypy src/codex32, and pytest (918 passed, with and without -O). Refs #81, #38. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018az69UX4773mYohXAtE8kD
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d5ebfd518
ℹ️ 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".
|
AI-assisted release-gate recheck at current head |
0793590 to
0867a1b
Compare
37f6a25 to
9c6259b
Compare
Reassign the expected_fingerprint argument instead of copying it into a local, and give existing_secret its None default before the source checks instead of in an else branch. Behavior is unchanged. The installed package drops from 5161 to 5159 logical review lines, which keeps the integrated #7/#42/#57/#46/#80/#81 tip under the <5200 budget. Security: the record gate still runs before any card is generated or shown, and interrupts at that gate still raise _WalletSetupInterrupted. Validation: ruff check, ruff format --check, mypy src/codex32, and pytest (918 passed, with and without -O). Refs #81, #38. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018az69UX4773mYohXAtE8kD
|
Agent review of refreshed head 9c6259b: the existing seed reaches the typed fingerprint or explicit no-record decision before CreationCeremony or card output; the checked fingerprint is passed into BitcoinCore.initialize, whose identity check runs before wallet selection or mutation. Ctrl-C/EOF at the early gate preserves the existing-backup message. The two resolved inline fixes replay as 99a9119 (interruption) and a8cc840 (stable raw-seed identifier). Focused regressions and the published 21-check matrix pass. No remaining code blocker found; the agent-authored commits still need responsible-human rewrite/squash under repository policy. |
fb8d671 to
f579184
Compare
Reassign the expected_fingerprint argument instead of copying it into a local, and give existing_secret its None default before the source checks instead of in an else branch. Behavior is unchanged. The installed package drops from 5161 to 5159 logical review lines, which keeps the integrated #7/#42/#57/#46/#80/#81 tip under the <5200 budget. Security: the record gate still runs before any card is generated or shown, and interrupts at that gate still raise _WalletSetupInterrupted. Validation: ruff check, ruff format --check, mypy src/codex32, and pytest (918 passed, with and without -O). Refs #81, #38. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018az69UX4773mYohXAtE8kD
9c6259b to
7686cb0
Compare
b24d72c to
d886238
Compare
An existing hex seed or codex32 master secret previously reached the wallet-record fingerprint check only after new recovery cards had been generated and confirmed. Check the typed record immediately after parsing the source, before any card output or ceremony. Preserve the explicit recordless path at the same early decision point, and pass the checked result through to wallet initialization so it is not prompted twice. Cover matching, mismatching, and recordless flows for both source encodings. Refs #30.
Translate Ctrl-C or EOF at the early wallet-record gate for ms32 create --existing into the existing wallet-setup interruption path. This keeps an operator from being told to invalidate a pre-existing recovery card before any new share ceremony has started. Add a focused regression proving the interruption occurs before share creation or output and preserves the valid-backup message.
Reassign the expected_fingerprint argument instead of copying it into a local, and give existing_secret its None default before the source checks instead of in an else branch. Behavior is unchanged. The installed package drops from 5161 to 5159 logical review lines, which keeps the integrated #7/#42/#57/#46/#80/#81 tip under the <5200 budget. Security: the record gate still runs before any card is generated or shown, and interrupts at that gate still raise _WalletSetupInterrupted. Validation: ruff check, ruff format --check, mypy src/codex32, and pytest (918 passed, with and without -O). Refs #81, #38. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018az69UX4773mYohXAtE8kD
A raw seed imported with create --existing was assigned a temporary random identifier for the no-record safety screen, then assigned a different random identifier when the new share set was created. Reuse the first identifier as the share-set identifier so the safety screen describes the backup that will actually be produced.\n\nExtend the recordless-creation regression to require the displayed, emitted, and imported identifiers to agree.\n\nRefs #30
7686cb0 to
aa2d333
Compare
This branch has not been deployed
2 participants
Refs #30. Focused follow-up to #57 after the behavior-preserving #105 cleanup and #80.
What and why
ms32 create --existingnow checks a supplied hex seed or codex32 master secret against the fingerprint on the separate wallet record before generating or showing a new recovery card. A mismatch can be corrected before a share ceremony begins. The explicit no-record path presents the recovered fingerprint and backup identifier for visual confirmation at the same early point. Wallet initialization receives the already checked result and does not prompt twice.Ctrl-C or EOF during this early check reports that the existing backup remains valid. For a raw hex seed, the identifier shown during the no-record decision is the identifier used for the new shares and imported secret. Fresh
ms32 createstill asks the operator to record its newly generated fingerprint.Review shape
Head
aa2d333is stacked directly on #80 (a29753e). Review only the five commits in this PR. Every stable patch-id matches the previously reviewed #81 head; the only change is the moved base. With #105 before #80, the integrated installed source is 5,188 logical lines, within the maintainer-approved<5200cap. The existing inline review threads are resolved.Verification
python -O.git diff --checkpass on the published tree.Human review order: #42 → #57 → #105 → #80 → #81 → #95. The agent-authored commits need responsible-human rewrite or squash under repository policy before integration.