Remove Python wallet test dependencies - #7
Open
BenWestgate wants to merge 3 commits into
Open
BenWestgate wants to merge 3 commits into
BenWestgate wants to merge 3 commits into
Conversation
Drop the bip32/Coincurve test-only oracle and its CI install path now that wallet interoperability is verified with frozen fixtures and real Bitcoin Core v32 integration. Keep fingerprint callback tests deterministic without adding another EC implementation.\n\nSecurity: no Python secp256k1 dependency remains in runtime, CI, or test tooling.\n\nValidation: 857 pytest tests and 857 optimized tests pass; mypy, Ruff, formatting, diff check, 57-case correction differential, build/Twine, clean no-deps wheel smoke, Core v32 regtest, and isolated Core v32 main-chain account-7 smoke all pass.\n\nFixes #3. Fixes #6.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
|
Can we support Python 14 now that |
BenWestgate
commented
Sep 17, 2026
Consolidate wallet fingerprint fixtures so tests that assert fingerprint semantics use values independently verified by Bitcoin Core. Keep arbitrary-seed CLI orchestration on an explicit stable stub while preserving real fixture values for known public seeds. Restore the unshared identifier expectations in generation coverage without treating them as BIP93 fingerprint vectors, and make the real Core v32 regtest verify every frozen fingerprint.\n\nrefs #3\n\nValidation: pytest -q (860 passed); python -O -m pytest -q (860 passed); mypy src/codex32; ruff check .; ruff format --check .; Bitcoin Core v32.0rc1 regtest.
A BIP32 master fingerprint is a wallet property, not a codex32 one. Asserting that a BIP93 vector "has identifier 8u6j" mixed the two specs: BIP93 already assigns those vectors identifiers of its own (test, name, cash), and nothing in BIP93 produces a fingerprint-derived value. Split the fixture table so the distinction is structural. Keep the BIP93 vector seeds as wallet fixtures, since the CLI display tests need a realistic fingerprint for ms10test material, and add arbitrary fixtures covering every BIP93 seed length. Derive FIXTURE_SEED from the arbitrary half alone so a length lookup cannot resolve to vector material; four of the nine seeds are 16 bytes, so a naive mapping would have picked one by insertion order. Move the identifier test onto those arbitrary fixtures, which widens it from three seeds to all six seed lengths and leaves VECTOR_1 and VECTOR_3 unused again. Name the test-double value STUB_FINGERPRINT and use it in test_generic_hrp.py, which still re-implemented the stub locally. Every fingerprint in the table was checked against a real Bitcoin Core node. Note that tools/bitcoin_core_regtest.py, which makes the same assertion, is not run by any workflow, so these fixtures have no automated verification. refs #3 Validation: pytest -q (863 passed); python -O -m pytest -q (863 passed); mypy src/codex32; ruff check .; ruff format --check .; differential_correction.py --verify (57 cases); all 9 CORE_FINGERPRINTS entries derived against Bitcoin Core v31.1.0 regtest (0 mismatches) -- not v32, which BitcoinCore.connect requires but which was unavailable here; deriveaddresses/validateaddress are version-stable and the 3 unchanged vector entries reproduced exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bip32/ Coincurve dependency chain and CI install stepreviewability-v1already contains the Core v32 wallet-integration work throughc118a83; this change removes the last Python secp256k1 dependency from CI and test tooling.Validation
pytest -q: 857 passedpython -O -m pytest -q: 857 passed--no-depswheel smoke testCloses #3
Closes #6