wallet: Require the recorded fingerprint before import - #57
BenWestgate wants to merge 1 commit into
Conversation
BenWestgate
added
area: cli
![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: 5eaf535bb7
ℹ️ 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".
|
Contract clarification applied in b50321a: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6da1f2a416
ℹ️ 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
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated review (Claude), posted at the maintainer's request. I wrote 5eaf535, so this is partly self-review.
Not ACKing 6da1f2a.
- 6da1f2a removes
identifier_origin/identifier_note, which the no-record path was built to show. If that's intended,invariants.md:16-18andmodel.md:232still promise it (agree with the Codex P2). If not, revert it. Either way #57 now differs from #28, which keeps them. - Policy: 6da1f2a is authored by
Codex Preflight <codex-preflight@localhost>, and AI_POLICY.md forbids agent authors. Its message also contains a literal\n\nand has no area prefix. 5eaf535 has aCo-Authored-By: Claudetrailer, which the same policy forbids. Squash-merge or reword. - Q:
create --existingimports an existing seed (timestamp 0) withrestore=False, so it skips the fingerprint gate. Should it passrestore=True? The Enter-if-none path keeps it usable.
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated review, posted at the maintainer's request.
Concept ACK 70a188a. The no-record identifier evidence is restored and matches #28 again.
One correctness item remains: ms32 create --existing supplies an existing seed but still reaches _initialize_wallet(..., restore=False), so it can import without the wallet-record gate. Treat --existing as a restore for wallet initialization.
|
Release-gate verification at current head |
|
One non-code release-gate item still remains despite the code ACK: the current PR history still contains |
|
Release-gate history check: the functional fix is ACKed at |
795ccdd to
53cd58b
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Check the recorded master fingerprint before any wallet selection, unlock, creation, or descriptor import. Apply the same restore gate to `ms32 wallet` and `ms32 create --existing`, including re-sharing; fresh creation only records the newly created identity. Without a wallet record, show the recovered fingerprint, backup identifier, supported codex32/Bails identifier evidence, and the explicit no-record warning before mutation. Security: this is the release-gate accident-safety boundary for wrong or mixed recovery material. Malicious replacement resistance remains the separate authenticated-descriptor work in #55. Validation on the identical recovery-gate files from the functionally ACKed head: full Python package matrix green; focused mismatch tests prove no Core RPC mutation occurs before identity verification. Rebased onto the current license-notice base without changing those recovery-gate files. Fixes #30. Refs #26.
53cd58b to
dcc0d41
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This branch has not been deployed
1 participant
Fixes #30. Library/CLI half of #26; GUI half is #28.
This adds the restore-time wallet identity gate before Bitcoin Core mutation.
BitcoinCore.initialize()accepts an expected fingerprint and checks it before wallet selection, unlock, creation or import.ms32 walletrequires the fingerprint from the wallet record; mismatch retries without changing Core.ms32 create --existinguses the same restore gate, including after re-sharing.ms32 createonly records the new fingerprint; it has no pre-existing wallet identity to authenticate.parse_fingerprint()accepts 8 hex digits in any case or spacing.#28 carries the same library/Core boundary plus the GUI flow. #43 tracks checksummed wallet-record fields; #55 tracks the seed-keyed encrypted descriptor backup; #56 tracks identifier-assisted correction ranking.
The branch is now one human-authored commit (
dcc0d41) directly on currentreviewability-v1headcf1a599: one commit ahead, zero behind. It preserves the exact functionally ACKed recovery-gate file contents from795ccddwhile retaining #15's license-notice base changes and removing the AI-authored/co-author-trailer history flagged by review. The rewritten head must complete its fresh GitHub Actions run before merge.The existing-seed regression test covers both an unchanged secret and re-sharing before import.