wallet: Distinguish unavailable Bails checks - #80
BenWestgate wants to merge 2 commits 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: 9ea2ed7093
ℹ️ 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 release-gate review, posted at the maintainer request. Code ACK 9ea2ed7: the unavailable RIPEMD-160 path now reports an inconclusive standard-Bails check instead of asserting a mismatch; a matching Bails-alpha SHA-256 identifier still wins; and the no-record fingerprint display plus explicit operator confirmation are unchanged. The new frozen-vector regression passes, both current GitHub package matrices are green (12/12 jobs), and the isolated #42/#45/#7/#51/#57/#46 composition plus this commit passes 918 tests at 5,142/5,200 source review lines. No code blocker found. This is not an authorship ACK: the agent-authored commit needs responsible human review and rewrite/squash before merge, and the stack must be refreshed after #7/#57/#46 integration.
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Exact-head release-gate recheck: ACK da14e0d. The prior P1 is resolved: the runtime now distinguishes an unavailable RIPEMD-160 check from a mismatch, still allows the Bails-alpha SHA-256 match to win, and both docs/security/model.md and invariant 4 describe that degraded outcome. git diff --check is clean; the focused identity/identifier tests pass in normal and python -O modes (9/9 each). No additional code blocker found. This remains stacked after #46 and needs the responsible human to review/rewrite or squash the agent-authored commits when the final #57/#46 integration is settled.
|
Agent release-gate re-review at exact head |
1899011 to
0793590
Compare
da14e0d to
0f5f4a8
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
|
AI-assisted release-gate recheck at exact head |
0793590 to
0867a1b
Compare
0f5f4a8 to
fb8d671
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Agent release-gate re-review: ACK fb8d671.
The refreshed runtime patch is unchanged from the reviewed Bails-availability fix, and the documentation conflict resolution preserves the current Core-native seed-source contract while adding the required “RIPEMD-160 unavailable = inconclusive, not mismatch” wording. The sole inline thread is resolved. Exact-head Python-package run 546 and Bitcoin Core wallet-fixture run 11 both succeeded; focused no-record/identity tests also passed normally and under python -O.
No remaining code-review blocker found. Integration/authorship policy remains: #80 follows #57 and its agent-authored commits should be human-reviewed/re-written or squashed before merge.
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
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
f579184 to
a29753e
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
d886238 to
52c23b3
Compare
a29753e to
23a2c05
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Exact-head release-gate re-review: ACK 23a2c05. The refreshed two-commit change distinguishes unavailable RIPEMD-160 from a Bails identifier mismatch, preserves the independently checkable Bails-alpha SHA-256 result, and aligns the security docs with that degraded outcome. No review threads remain. The combined exact stack through #95 passes 935 tests normally and optimized plus focused identity/identifier checks, Ruff/mypy/constants/differential verification. No code blocker found. These agent-authored commits require responsible-human rewrite/squash before integration.
52c23b3 to
361feb7
Compare
23a2c05 to
a0ab769
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head release-gate re-review: ACK a0ab769.
The current runtime change correctly separates “standard Bails check unavailable” from an identifier mismatch, while still allowing a Bails-alpha SHA-256 match to win. The security model and invariant describe that degraded outcome. The one inline finding is resolved. Exact-head Python-package run 662 and Bitcoin Core wallet-fixture run 33 both succeeded.
No code blocker found. These agent-authored commits still require responsible-human rewrite/squash before integration.
When RIPEMD-160 is unavailable, a valid standard Bails identifier cannot be checked. Preserve the Bails-alpha SHA-256 result and distinguish that inconclusive state from a completed identifier mismatch, so the no-record restore prompt does not claim the cards are wrong. Keep the independent fingerprint and explicit operator-confirmation boundary unchanged. Refs #79
The no-record restore flow can no longer claim a standard Bails identifier mismatch when RIPEMD-160 is unavailable. Record that platform-dependent inconclusive outcome in both the security model and invariant so reviewers can distinguish it from a completed comparison. Refs #79
a0ab769 to
3a35463
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! 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". |
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-assisted current-head review: ACK 3a35463. RIPEMD-160 absence is now explicitly inconclusive, SHA-256 alpha checking remains available, and the security docs match the behavior. Exact-head package and Core-fixture CI are green.
This branch has not been deployed
1 participant
Why
On a Python/OpenSSL build without RIPEMD-160, a valid standard Bails identifier is not checked, but the no-record restore prompt previously said the cards were wrong or mixed up. The frozen
d9k8vector reproduces this whenhashlib.new("ripemd160", ...)raisesValueError.Change
Report the standard Bails check as unavailable/inconclusive. Preserve the independent fingerprint and explicit no-record confirmation, and continue checking the Bails-alpha SHA-256 rule. Update the security model and invariant 4 to document that outcome.
Review shape
Current head
3a35463is the two focused #80 commits mechanically replayed directly onto current #99 (70c5da6). GitHub shows only this four-file Bails-availability/security-documentation delta. The prior inline finding remains resolved.Validation
All 51
test_bitcoin_core.pytests pass normally and underpython -Oon the refreshed stack. The exact-head GitHub matrices and Bitcoin Core fixture are green,git diff --checkis clean, and Codex found no major issue on reviewed commit3a35463e1b. The composed refreshed stack through #95 also passes all 941 tests normally and all 941 underpython -O, plus Ruff, format, strict mypy, and correction-constant verification.Human integration order is #57 → #105 → #99 → #80 → #81 → #95. These commits are agent-authored and require the repository's responsible-human review/rewrite or squash policy before merge.
Fixes #79
Refs #30, #46, #57, #81