Wallet: use Core for setup and remove test crypto deps by BenWestgate · Pull Request #7 · BenWestgate/python-codex32

BenWestgate · 2026-09-17T11:45:24Z

Summary

remove the remaining test-only bip32 / Coincurve dependency chain and CI install step;
remove the Python wallet oracle and differential-wallet tool;
keep Core-derived seed→fingerprint fixtures as frozen data under tests/data/wallet_fingerprints.json, with lookup/stub behavior isolated in tools/_wallet_test_vectors.py;
hand the validated master xprv to Bitcoin Core over child stdin and let Core create the standard account-0 BIP44/49/84/86 descriptors with addhdkey + createwalletdescriptor;
honor numeric recovery timestamps by re-importing one active private descriptor over stdin and letting Core perform its native timestamp-based wallet rescan with the two-hour safety window;
keep wallet interoperability coverage through unit fakes plus isolated real Bitcoin Core v32 regtest/main-chain smoke checks;
carry the already-reviewed Support Python 3.10 through 3.15 #36 Python 3.10–3.15 compatibility work into reviewability-v1.

The Core-native setup deliberately restricts ms32 wallet --account to 0 until Bitcoin Core exposes an account selector; #68 tracks restoring nonzero-account support. Python still performs no secp256k1 public-key work.

Review status

Current remote head f74e5a0 contains the previously reviewed Core-native work, the transparently agent-authored timestamp follow-up, and one documentation-only correction replacing the obsolete rescanblockchain entry in the documented RPC list with the importdescriptors call the implementation actually uses. The timestamp follow-up should be human-reviewed and rewritten/squashed under the repository authorship policy before merge.

All inline review findings are resolved. The release-gate review ACK for the Core-native setup remains applicable, with the timestamp delta separately verified against Core 32.0rc2.

#51 is the independent real-Core CI follow-up and is restacked directly on this current head as 664a668; both exact-head workflows are green.

Validation

focused Bitcoin Core / CLI-help tests: 39 passed;
Ruff check/format and git diff --check: clean;
isolated Bitcoin Core 32.0rc2 regtest passed with two block-time eras, proving a nonzero timestamp recovers the recent output while excluding the older output outside Core's scan window;
the same regtest confirms 0 still recovers full history, the restored receiving descriptor remains usable, and encrypted-wallet relocking still succeeds;
exact-head GitHub Python-package run 36760581305 on f74e5a0: success;
stacked ci: Verify wallet fixtures against Bitcoin Core #51 head 664a668: Python package run 36761397071 and Bitcoin Core wallet fixtures run 36761397161 both succeeded.

Closes #3
Refs #6
Closes #18
Refs #68

chatgpt-codex-connector · 2026-09-17T11:45:29Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

BenWestgate · 2026-09-17T11:51:08Z

Can we support Python 14 now that coincurve has been removed?

BenWestgate · 2026-09-21T07:57:41Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c58b5ffb21

ℹ️ 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".

chatgpt-codex-connector · 2026-09-21T09:37:49Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

BenWestgate · 2026-09-21T09:55:41Z

Yes, removing Coincurve clears one blocker, but Python 3.14 support is broader than this dependency-removal PR because the full package and CI matrix still need validation on 3.14. That follow-up is tracked in #8 so this PR stays focused.

BenWestgate · 2026-09-21T23:08:19Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 341ccc73db

ℹ️ 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 · 2026-09-25T13:37:28Z

Follow-up to the earlier Python-version question: yes. PR #36 now carries the interpreter-range work separately, stacked on this dependency-removal PR. It supports Python 3.10 through 3.15 and runs the remaining 3.10/3.11/3.14/3.15 compatibility legs on Ubuntu while the 3.12/3.13 matrix keeps cross-platform coverage.

Refresh the Python 3.10-3.15 compatibility branch onto the final #7 wallet-test dependency removal so it can be integrated without a stale-stack conflict.\n\nValidation: fresh .[dev] install; 864 pytest tests; Ruff check and format; strict mypy; git diff --check.

BenWestgate · 2026-09-25T17:28:59Z

@codex review

chatgpt-codex-connector · 2026-09-25T17:31:17Z

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 90e546048e

ℹ️ 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

AI-generated review (Claude), posted at the maintainer's request.

ACK 90e5460

I recomputed all nine frozen fingerprints in wallet_fingerprints.json with an independent pure-Python secp256k1/HASH160; all match.
With the Python oracle gone, nothing on the PR matrix checks these values against Core. #51 is meant to, but see my note there: as written it won't run.

BenWestgate · 2026-09-27T14:16:14Z

Release-gate state changed after the prior ACK: #36 has now merged into this PR's head branch, so current head is 8338c72 rather than reviewed 90e5460. GitHub currently reports the PR non-mergeable against reviewability-v1, and there is no fresh PR workflow run associated with 8338c72. Do not treat the earlier ACK/green matrix as covering the current head. Before human review/merge, refresh this branch onto current reviewability-v1, preserve both #15's package-license metadata and #36's Python 3.10–3.15 metadata when resolving the packaging conflict, then rerun the full matrix.

Core's createwalletdescriptor has no timestamp parameter. Reimport one newly created active descriptor with its existing range and next index through bitcoin-cli stdin, letting Core apply its time window to a wallet-wide scan without guessing a block height. Keep private material out of arguments and relock on failure. Cover genesis and nonzero timestamps with unit tests and a two-era Core v32 regtest that skips older outputs while recovering recent ones.

BenWestgate · 2026-09-30T18:29:05Z

Response to the timestamp review note on docs/developer/api.md: yes, a nonzero Unix timestamp should shorten rescans. Commit 73b1538 now reimports one newly created active descriptor with that timestamp (preserving range and next index) via bitcoin-cli stdin, letting Core apply its two-hour safety window to a wallet-wide scan. No block height is guessed. A two-era official Core 32.0rc2 regtest recovers a recent output and skips an older one; 0 still scans genesis and now skips history. The follow-up commit is agent-authored and needs human review/rewrite or squash before merge. I left the existing pending human review draft untouched; GitHub would not let me post a threaded reply while it remains pending.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 73b15380e5

ℹ️ 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".

chatgpt-codex-connector · 2026-09-30T18:43:12Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

BenWestgate · 2026-09-30T20:14:54Z

Agent release-gate re-review at current head f74e5a0: the substantive timestamp change at 73b1538 already received Codex review; the only later delta is the one-line API-map correction replacing obsolete rescanblockchain with the actual importdescriptors RPC. I inspected that exact delta, and exact-head Python-package run 36760581305 is green. No additional code-review blocker remains on this head. The agent-authored timestamp/docs commits still require the planned human review/rewrite or squash before merge.

BenWestgate · 2026-09-30T21:40:32Z

@codex review

chatgpt-codex-connector · 2026-09-30T21:44:00Z

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: f74e5a0af9

ℹ️ 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

LGTM

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

Make the secret-output invariant literal about intentional recovery/export output, define the trusted-computer boundary consistently, and warn that shell command text can be retained even when stdin is safe from argv exposure. Update the shared CLI safety footer and its exact-help regression. Validation: focused help regression; Ruff check/format; strict mypy for the parser; git diff --check. The full generic-HRP module still reaches the pre-existing bip32 test dependency tracked by #3/#6 and fixed by #7. fixes #4

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 commented Sep 17, 2026

View reviewed changes

Comment thread tests/test_cli.py

Comment thread tests/test_cli.py Outdated

Comment thread tests/test_correction_indel.py Outdated

Comment thread tests/test_generation.py

Comment thread tests/test_generation.py Outdated

Comment thread tests/test_generic_hrp.py

This was referenced Sep 17, 2026

Packaging: support Python 3.14 #8

Closed

Verify frozen wallet fingerprint fixtures against real Bitcoin Core in CI #9

Closed

Support Python 14 now that coincurve has been removed #17

Closed

chatgpt-codex-connector Bot reviewed Sep 21, 2026

View reviewed changes

Comment thread tools/_wallet_test_vectors.py Outdated

Comment thread .github/workflows/python-package.yml

BenWestgate force-pushed the 3-remove-python-bip32-dependency branch from c58b5ff to 341ccc7 Compare September 21, 2026 09:37

chatgpt-codex-connector Bot reviewed Sep 21, 2026

View reviewed changes

Comment thread tools/_wallet_test_vectors.py Outdated

BenWestgate mentioned this pull request Sep 23, 2026

Support Python 3.10 through 3.15 #36

Merged

BenWestgate added the gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review. label Sep 24, 2026

BenWestgate mentioned this pull request Sep 24, 2026

ci: Verify wallet fixtures against Bitcoin Core #51

Merged

BenWestgate added area: ci Continuous integration and workflow configuration. area: wallet/core Wallet integration and Bitcoin Core boundaries. labels Sep 24, 2026

This was referenced Sep 25, 2026

Remove internal hardened BIP32 derivation via Bitcoin Core HD-key RPCs #6

Closed

docs: Prepare the v1 review handoff #38

Open

BenWestgate mentioned this pull request Sep 25, 2026

docs: Define trusted-computer boundary #59

Draft

BenWestgate commented Sep 25, 2026

View reviewed changes

BenWestgate mentioned this pull request Sep 25, 2026

release: Qualify exact artifacts before publish #52

Open

This was referenced Sep 27, 2026

Reduce secp256k1 supply-chain trust in backup/recovery path #3

Closed

Support Python 3.10 through 3.15 #18

Closed

BenWestgate mentioned this pull request Sep 30, 2026

wallet: Distinguish unavailable Bails checks #80

Open

BenWestgate commented Sep 30, 2026

View reviewed changes

Comment thread docs/developer/api.md Outdated

chatgpt-codex-connector Bot reviewed Sep 30, 2026

View reviewed changes

Comment thread docs/developer/api.md Outdated

docs: Correct Core rescan RPC list

f74e5a0

BenWestgate commented Sep 30, 2026

View reviewed changes

BenWestgate merged commit 55d06b0 into reviewability-v1 Sep 30, 2026
20 checks passed

BenWestgate deleted the 3-remove-python-bip32-dependency branch September 30, 2026 21:59

Conversation

BenWestgate commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Review status

Validation

Uh oh!

chatgpt-codex-connector Bot commented Sep 17, 2026

Uh oh!

BenWestgate commented Sep 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BenWestgate commented Sep 21, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Sep 21, 2026

Uh oh!

BenWestgate commented Sep 21, 2026

Uh oh!

BenWestgate commented Sep 21, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

BenWestgate commented Sep 25, 2026

Uh oh!

BenWestgate commented Sep 25, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 25, 2026

Uh oh!

BenWestgate left a comment

Choose a reason for hiding this comment

Uh oh!

BenWestgate commented Sep 27, 2026

Uh oh!

Uh oh!

BenWestgate commented Sep 30, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Sep 30, 2026

Uh oh!

BenWestgate commented Sep 30, 2026

Uh oh!

BenWestgate commented Sep 30, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 30, 2026

Uh oh!

BenWestgate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

BenWestgate commented Sep 17, 2026 •

edited

Loading