Repository navigation
Add comprehensive codex32 CLI, correction, and wallet integration - #94
BenWestgate wants to merge 82 commits into
Conversation
Preserve the already reviewed immutable artifact, profile, sharing, generation, and fixed-correction work before reducing the project for v1 reviewability. This intentionally records the accepted prototype as one checkpoint rather than reconstructing its development history.\n\nThe next commits remove speculative features and simplify boundaries while keeping each intermediate tree green. The unrelated root files named json and os remain untracked and are not part of this checkpoint.
Move codex32 checksum selection into the common format layer so application profiles own only their payload rules. Select regular and Long checksums from expanded HRP plus data length as proposed by BIP93 PR #2258, including the invalid 94/95 gap and the 1023-symbol upper bound.\n\nUpdate the official boundary fixtures for 43 through 47 byte master seeds and reject legacy short-checksum encodings for 44 through 46 bytes. Sharing and fixed correction now consume the same format selector instead of duplicating profile-specific checksum policy.
Keep one exact-threshold interpolation path for the four fixed applications while removing profile opt-in machinery and the unused target-exclusion API. A derived share now has one simple rule: its ordinary index must not already occur in the basis.\n\nReuse the validated format decode result when extracting checksum-bearing interpolation tails. This avoids repeating checksum selection in the sharing layer while preserving payload-plus-checksum interpolation and mandatory reparse of every result.
Reduce generation.py from 600 to 265 lines by removing seed-derived shared-set tags, exclusion lists, metadata fallback, and Core Lightning generation. Fresh shared identifiers are now independent random u5 metadata; raw seeds and re-sharing require an explicit identifier, while the reviewed fingerprint default remains only for fresh unshared master seeds.\n\nCurrent Core Lightning documents codex32 as a legacy pre-v25.12 recovery format and use mnemonics for new nodes, so v1 retains CL parsing and sharing but does not invent a fresh-node generation workflow. Preserve the security-critical mask path: one OS-random byte batch per basis, unbiased u5 mapping, CRC rejection for S, and ordered random share-index sampling.
Delete the insertion/deletion search, ranking model, timeouts, worker pool, provisional result records, and 307-line structural test suite. No published indel ECWs exist, and this speculative surface more than doubled the code a reviewer had to trust.\n\nRetain the 645-line PR #70-derived fixed BCH core and application-agnostic worksheet residue adapter. The transitional CLI now handles only substitutions and explicit erasures, never edits the HRP or separator, writes correction suggestions to stderr, and returns nonzero so suggestions cannot be mistaken for authenticated recovery output.
Rewrite the transitional 885-line command module as a 341-line adapter over the reviewed API. Keep verify, exact-threshold secret recovery, fresh-share derivation, ms generation, strict worksheet checksum completion, and fixed BCH/residue correction. Add the installed codex32 entry point.\n\nUse one bounded stdin boundary and one plain formatter. Positional set headers make creation ceremonies explicit; redirected recovery accepts at most nine artifacts and terminal recovery requests one share at a time. Pretty master secrets may display the public BIP32 fingerprint, while shares never do. Remove memory locking claims, global formatting state, hidden account files, descriptor routing, and all CLI-owned domain algorithms.
Replace the generic descriptor-policy parser and hidden extension points with a stateless MasterSeed-only adapter. The adapter exposes only the three v1 operations needed for Bitcoin interoperability: a master xprv, a BIP48 coordinator xpub with origin information, and four fixed Bitcoin Core descriptor templates.\n\nKeep account, network, timestamp, and private/public selection explicit. Public descriptors contain account xpubs; private descriptors deliberately retain Bitcoin Core's root-xprv-plus-path form and the CLI warns that this grants root authority. No parser, policy language, account database, RPC, or network behavior remains.\n\nAdd official BIP93 xprv coverage, frozen BIP48 and descriptor fixtures, descriptor checksum coverage, strict non-MasterSeed boundary tests, and thin CLI tests. The replacement removes substantially more production code than it adds.
Remove superseded gate plans, generated review portfolios, generic scaffolding, and redundant release workflows from the deliverable. Replace them with concise final-state architecture, capability, security, source, divergence, CLI, and traceability documents that map each supported behavior directly to one code owner and its tests.\n\nShrink the package root to nineteen intentional names and remove obsolete descriptor-era error aliases. Add an enforced public-surface test and a TTY recovery regression; the latter also fixes uppercase prefix prefilling for subsequent shares.\n\nKeep one CI workflow for Python 3.12 through 3.14, remove the unused setuptools-scm build dependency, and define the source-distribution manifest explicitly. The resulting production package is 2,438 physical Python lines across twelve modules, with no module above 650 lines.
Apply Ruff's formatter to the source and test trees as a standalone mechanical change. No behavior, interface, fixture value, or assertion is changed. Keeping formatting separate leaves the preceding functional and scope-reduction commits independently reviewable.
Describe the independent source of each data-only fixture family and point reviewers to the frozen revisions and digests in the source manifest. Make explicit that tests do not derive expected values from the production implementation.
Replace Click with an explicit argparse entry point and reject abbreviated long options so command-line mistakes fail closed. Preserve command behavior and add direct and installed-entry-point coverage. Enable strict mypy checking, confine the untyped bip32 dependency to a narrow adapter, and document the reviewed runtime dependency tree. Keep Ruff as the sole linter and remove stale Flake8 configuration. Rewrite the README introduction for Bitcoin users, clarify security limitations, and ignore observed generated files and caches.
Replace the module-wide mypy override with an exact import-untyped suppression at the sole bip32 import. This keeps the strict configuration straightforward and makes the exception visible at the dependency boundary.\n\nStrict mypy will report the suppression as unused if bip32 later publishes type information.
Replace the ambiguous top-level xpub and descriptors commands with goal-oriented wallet subcommands for multisig coordination, watch-only Bitcoin Core imports, and private restoration. Require users to choose between restore and watch-only instead of defaulting across the private-key boundary. Move bounded stdin and interactive entry into a small private adapter. Collect shares sequentially, display the known prefix without making it editable, validate compatibility after every entry, and retry rejected input without retaining it. Reuse BIP93 share-set validation rather than duplicating domain rules in the CLI. Emit compact single-line importdescriptors JSON while keeping prompts, status, and private-key warnings on stderr. Document the watch-only and encrypted restoration workflows, remove the alpha-era migration document, and extend tests for the command hierarchy, input flow, stream separation, interrupt handling, and production size budgets.
Rename the verify command to check and replace internal object representations with labeled, human-readable validation results. Show help when codex32 is run without a command and clarify command descriptions. Allow rejected terminal input to be edited on the next attempt using the optional standard-library Readline interface. Disable automatic history, retain only the latest rejected entry, preserve immutable known prefixes, and keep prompts and status on stderr so stdout remains safe for pipelines. Document the temporary-memory and terminal-scrollback tradeoffs, update the CLI guidance, and add regression coverage for retries, stream separation, interrupt handling, application labels, and removed commands.
Replace implementation-oriented CLI wording with language centered on backups, secrets, shares, and recovery. Present validation results without echoing protected input or exposing Python object representations, and add clear application names and recovery-threshold descriptions. Improve interactive entry with immutable prompts, immediate share-set validation, and transient editable retries that do not use persistent Readline history. Preserve stderr for prompts and warnings and stdout for requested machine-readable results. Simplify correction by inferring supported profiles from intact prefixes, removing the unused prefix option, and limiting explicit erasure positions to worksheet residues. Clarify correction suggestions and sensitive wallet output without changing the underlying public correction API. Generate threshold-plus-two shares by default, while preserving explicit share counts and indices. Separate the argparse grammar from command execution and retain only the 3,000-line installed-package size limit. Clarify public API errors, comments, docstrings, and supporting documentation. Expand exact CLI coverage for prompts, failures, output streams, correction behavior, generation defaults, and protected-input handling.
Report impossible profile lengths and master-seed byte alignment before checksum failures. Separate lexical, shape, and checksum validation so diagnostics can use the literal application prefix without allowing an unverified artifact across the parsing boundary. Use clearer messages for invalid characters, positions, prefixes, headers, case, and checksums. Distinguish accepted secrets, recovery shares, and mixed derivation-basis strings during interactive entry. Default artifact output to human-readable formatting when its destination is a terminal while preserving canonical output for pipelines. Add --no-pretty as an explicit terminal override and update documentation and regression coverage.
Make creation explicit at the command boundary. Fresh generation no longer prompts for input, while --existing reads one codex32 secret or hexadecimal seed. Support threshold-only random identifiers and retain complete headers as an explicit override. Extend generation to Core Lightning secrets and share sets. Use fingerprint identifiers only for fresh unshared Bitcoin master seeds; use random defaults for shared sets, supplied seeds, Core Lightning secrets, and re-sharing. Produce two more shares than the recovery threshold by default. Improve CLI help, validation errors, protected-input retries, transcription formatting, worksheet guidance, and correction options. Keep protected input and status on stderr, preserve canonical pipeline output, and remind users to test recovery from what they wrote down. Expand tests and documentation for generation, identifier tradeoffs, air-gap transfers, inheritance recovery, and contributor practices. Keep the installed package within its 3,000-line review budget.
Allow a compatible complete secret to finish interactive recovery after one or more shares have already been entered. Keep the public recovery API restricted to exactly the threshold number of ordinary shares. Use share-numbered prompts for recovery-oriented commands and retain string-numbered prompts when collecting a derivation basis. Replace internal candidate-reparse details with a concise message when no valid correction is available. Document the deliberately bounded future indel search. Rank structural candidates using estimated ambiguity and correction-addend Hamming weight, with CRC padding only as a secondary hint that cannot validate, prune, or remove candidates. Add CLI coverage for completing secret, xprv, multisig-xpub, and Bitcoin Core exports with a secret entered after compatible shares.
…dule to handle user inputs more effectively. Updated documentation to reflect changes in the CLI and architecture. Removed outdated API migration guide.
Configure Ruff at 110 columns and mechanically format the Python tree. This is the smallest tested policy that keeps the installed package below its 3,000-line review budget without hand-minifying source. The formatted baseline has no behavior changes and passes ordinary and optimized tests, strict typing, lint, formatting, and the frozen correction corpus.
Close Gate 0 by restricting only fresh CLI master-seed generation to 16- and 32-byte sizes while preserving the full BIP93 API and import range. Bound explicit share-index selectors before copying or normalization to resolve the delegated scan's low-severity availability finding. Reserve 1.0.0rc1, pin the formatter baseline, and align security, capability, provenance, dependency, traceability, and accepted-risk records with the implemented behavior. Add direct regression coverage for every changed boundary.
Add frozen, slotted correction context, edit, and candidate records plus a deterministic public API over the existing fixed BCH decoder. Keep the HRP and separator immutable, reparse every candidate, constrain wallet context before return, and keep BIP39 correction API-only. Collapse private decoder diagnostics into the public no-candidate result, route the CLI through the public boundary, and add all-profile, malformed-corpus, differential, and bounded fuzz evidence while preserving the 3,000-line package limit.
Carry the owner-authored bip32 Coincurve 21 range patch by exact source hash and pin every supported 3.12/3.13 Coincurve wheel. Keep Python 3.14 as a non-blocking probe because no selected native wheel exists yet.
Accept Python 3.12's argparse spelling for dual option names and resolve the installed .exe launcher on Windows. These changes preserve the tested CLI contract while allowing the required Gate 2 platform matrix to run.
Check the runtime threshold type before range membership so numerically equal floats cannot enter immutable BIP93 headers and fail later during symbol encoding. Security: strengthens the validated-artifact construction boundary without changing valid integer thresholds. Validation: focused public API and BIP93 tests; Ruff check and format; strict mypy.
Apply the existing stdlib BIP32 root validity check to caller-supplied seed bytes before constructing a MasterSeed. Fresh generation already retries the same invalid roots. Security: prevents an unusable supplied root from crossing the master-seed generation boundary. Validation: python -m pytest -q; python -O -m pytest -q; Ruff check and format; strict mypy; differential_wallet.py --verify.
Remove the remaining bip32/Coincurve test-only oracle and CI install path, keep Bitcoin Core-derived fingerprint fixtures as frozen test data, and retain real-Core integration coverage without loading a Python secp256k1 implementation. Carry the reviewed Python 3.10 through 3.15 compatibility work in the same integration patch. Closes #3 Closes #18 Refs #6
Pass the validated master xprv to addhdkey over stdin and let Core create the four standard account-0 descriptor types. This removes the redundant exact public-descriptor comparison while preserving destination revalidation, historical recovery, and encrypted-wallet relocking. Restrict CLI account selection to zero until Core exposes a native selector (refs #68). Validated with 874 normal and 874 optimized tests, Ruff, mypy, and isolated Bitcoin Core 32.0rc2 regtest and main-chain fixtures.
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.
Run the pinned Bitcoin Core v32 fixture verifier on relevant pull requests, weekly, and on demand so frozen seed-to-fingerprint data cannot silently drift from Core. Fixes #9
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BenWestgate
added
gate: adversarial review
Apply the established majority-case interpretation to standalone correction while preserving immutable context, entered edit semantics, and disclosure accounting. Account the normalized retry frontier even when the first optional search reaches its deadline after finding a candidate, so cumulative capture mass remains fail-closed. Normalize ordinary grouping spaces before locating an immutable prefix in the public API, so grouped input cannot shift the mixed-case boundary into a locked header. Report truncation from the shared mixed-case schedule: combine both full passes' completeness, and mark returned candidates search_complete=False when either pass truncated. The deadline regressions cover a string only the erasure reading corrects (five minority-case P) and one only case normalization corrects (fifteen minority-case X, one mistyped); both recover within ten seconds while the normalized exhaustive optional search may truncate. Fixes #37.
Give standalone correction a stable status contract: 0 for already-valid input, 1 when a suggestion is emitted, 2 for command or input syntax errors, and 3 when no usable suggestion is emitted. Keep incomplete best-effort suggestions at status 1 and document status 3 only for incomplete searches without a usable suggestion. Fixes #39.
Remove unreachable creation guards and the permanently false correction ambiguity field, align the CLI test Core stub with production, and move reference-only correction helpers out of the installed package. Security: fail-closed correction and wallet behavior are unchanged. Refs #38.
After both mixed-case interpretations complete their required preflight, the CLI competitor scheduler must not rerun that same required work under the already-consumed shared deadline. Restrict only the executed follow-up work to optional character classes while retaining the full admitted frontier for cumulative disclosure accounting. This preserves a required-pass candidate if optional work reaches the deadline and keeps the public capture-mass calculation unchanged. The regression asserts that both full mixed-case follow-up searches enter optional-only mode. Refs #37.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| raise RuntimeError("bitcoin-cli was not found") | ||
| daemon = subprocess.Popen( | ||
| [ | ||
| arguments.bitcoind, |
There was a problem hiding this comment.
AI-assisted response: false positive under this repository’s documented trust boundary. --bitcoind is an operator-selected local verification executable, not untrusted recovery input, and subprocess.Popen receives an argv list with no shell. No attacker-controlled string is evaluated as shell syntax.
| raise RuntimeError("bitcoin-cli was not found") | ||
| daemon = subprocess.Popen( | ||
| [ | ||
| arguments.bitcoind, |
There was a problem hiding this comment.
AI-assisted response: false positive for the same reason. --bitcoind/--bitcoin-cli select trusted local verification executables; subprocess calls use argv sequences with shell=False, and the generated wrapper shell-quotes the resolved CLI/datadir and forwards arguments via "$@".
(cherry picked from commit 4d7cf29)
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head review at 5325eb7: three findings.
[P1] Bind public descriptors to the supplied master seed — src/codex32/_bitcoin_core.py:199-212. public_descriptors(secret, wallet=...) ignores secret and derives entirely from the selected wallet; seed A + wallet B therefore returns B’s receive descriptors under an API framed around A. Verify the wallet root/fingerprint against secret before returning descriptors.
[P2] Check the deadline inside non-yielding indel traversal — src/codex32/indel.py:190-222,478-496. _search_target checks time before a layer and after yielded views, but _views.walk() can perform combinatorial insertion/transposition work while rejecting every candidate before yield (notably repeated-symbol inputs). Poll the deadline inside the traversal/inner loops so the correction time bound is real.
[P2] Redact correction edits from default repr — src/codex32/correction.py:84-100,618-630. The generated dataclass repr includes each CorrectionEdit.replacement, which is copied from corrected recovery text, so logging/assertion diagnostics can disclose reconstructed secret/share symbols even though artifact reprs are redacted. Give correction candidates/edits a redacted repr.
Focused Core/wallet tests: 52 passed. The two CodeQL command-line alerts are false positives under the documented trusted-local-executable boundary; replied inline.
* wallet: Require the recorded fingerprint before import Gate restore and existing-seed wallet initialization on the independently recorded BIP32 master fingerprint before any Bitcoin Core wallet mutation. Keep the correction path from disclosing or reusing a fingerprint derived from the candidate being authenticated. Fixes #30. * docs: Tighten restore guidance Clarify the wallet-record fingerprint wording, remove the redundant pre-check, require chain history before offline rescans, and follow the maintained Core offline-signing tutorial. Refs #57. * wallet: Clarify recordless restore guidance Make the recovery guide explicitly identify the trusted offline signer when loading the blank descriptor wallet. Replace the hardware-wallet example in the no-record warning with a watch-only wallet, which is the intended independent fingerprint reference.
CorrectionEdit was a plain dataclass, so repr() of a correction candidate disclosed each edit's observed and replacement characters. The replacement can be true recovery text. Exclude those two fields from dataclass repr while preserving explicit field access, equality, hashing, and correction behavior. Add a regression that repairs eight erasures and verifies the candidate representation does not reveal the restored characters. Fixes #129
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI current-head review of 6dd7828: process ACK only. This PR explicitly identifies itself as a moving release-gate preview, not the frozen v1 integration candidate, so I am not treating the 82-commit/94-file comparison as merge-ready or issuing a code ACK. The stated boundary is correct: keep this draft unmergeable until #38 freezes the exact candidate and the final human-authored master PR carries the reviewer handoff.
![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: 6dd782800c
ℹ️ 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".
| if expected_fingerprint is None: | ||
| expected_fingerprint = fingerprint | ||
| elif fingerprint != expected_fingerprint: | ||
| raise BitcoinCoreError("Bitcoin Core returned inconsistent master fingerprints.") |
There was a problem hiding this comment.
When wallet contains root B but the caller supplies master seed A, _root_xpub() and _derived_key() derive everything from B, while this check only verifies that B's fingerprint is consistent across purposes. Because secret is otherwise unused after its type check, core_descriptors(A, integration=client, wallet=...) silently returns descriptors for B; exporting them as A's watch-only policy can direct deposits to addresses that A cannot recover. Compare the returned origin fingerprint with the fingerprint derived from secret before returning these records.
Useful? React with 👍 / 👎.
|
|
||
| class MissingEncoding(CodexError): ... | ||
|
|
||
| """Bech32 character, container, and bit-conversion helpers.""" |
There was a problem hiding this comment.
This rewrite removes Pieter Wuille's copyright and MIT notice from bech32.py (and likewise from checksums.py) even though convertbits still explicitly identifies itself as derived from segwit_addr.py and the checksum implementation remains derived code. The packaged root LICENSE contains only Ben Westgate's copyright, so wheels and sdists produced by the new release workflow omit the upstream notice that the MIT license requires distributions of substantial copies to retain.
Useful? React with 👍 / 👎.
| One-shot functions create only unshared secrets: | ||
|
|
||
| ```python | ||
| generate_master_seed(seed_bytes=None, *, byte_length=None, identifier=None) |
There was a problem hiding this comment.
The published one-shot API signature omits the fingerprint parameter even though a fresh call with the documented default identifier=None raises ValueError unless that callback is supplied. As written, users following this API section have no documented way to obtain the stated fingerprint-derived default identifier, and generate_master_seed() appears callable with all defaults but cannot generate a seed. Include the parameter and its required callback contract in this signature and surrounding explanation.
Useful? React with 👍 / 👎.
| -C normalized-sdist -cf - "codex32-${GITHUB_REF_NAME#v}" | gzip -n > "$archive.new" | ||
| mv "$archive.new" "$archive" | ||
| - name: Attach distributions to the GitHub release | ||
| run: gh release upload "$GITHUB_REF_NAME" dist/* |
There was a problem hiding this comment.
If this build job is rerun after gh release upload succeeded—for example because the following artifact-upload step failed—the same wheel and sdist names already exist on the release, so this command exits instead of allowing the release pipeline to recover. The GitHub CLI documentation exposes --clobber specifically for replacing same-named assets; otherwise the workflow should detect and verify existing assets before uploading them.
Useful? React with 👍 / 👎.
This branch has not been deployed
2 participants
Summary
This is a major feature release that adds a complete command-line interface, error correction capabilities, and Bitcoin wallet integration to the codex32 reference implementation. The changes transform the library from a basic encoding/decoding tool into a production-ready system for managing BIP32 seed backups.
Key Changes
Command-Line Interface
cli.py: Main CLI entry point with subcommands for create, recover, correct, and derive operations_cli_parser.py: Complete argument parser with non-abbreviating grammar_cli_input.py: Interactive input handling with bounded stdin, correction suggestions, and user confirmation flowsError Correction System
correction.py: Fixed BCH error correction with reverse-indexed coordinates, supporting both erasures and errorsindel.py: Structural alignment enumeration for bounded error correction_alignment.py: Incremental syndrome computation with caching for efficient polynomial evaluation_competitors.py: CLI scheduling and conservative proof generation that preserves candidate rankingWallet Integration
wallet.py: Bitcoin wallet interoperability for validated master seeds_bitcoin_core.py: Bitcoin Core subprocess adapter for descriptor-based wallet operations_bip32.py: Minimal stdlib-only BIP32 root key handling and xprv serializationProfile System
profiles/__init__.py: Fixed selection of supported application profilesprofiles/ms32.py: Bitcoin master-seed profile with validation rulesprofiles/cl32.py: Core Lightning HSM-secret profileprofiles/bip39.py: Migration-only BIP39 profile for existing backupsGeneration Module
generation.py: Electronic generation for ms and Core Lightning share sets with security invariantsCore Library Updates
bip93.py: Enhanced with additional validation and helper functionsbech32.py: Refactored for clarity and maintainabilitychecksums.py: Reorganized checksum constantserrors.py: Expanded exception hierarchy for better error handlinggf32.py: Canonical GF(32) arithmetic operations__init__.py: Updated public API exportsTesting & Documentation
Build & Distribution
pyproject.tomlwith pinned setuptools versionNotable Implementation Details
https://claude.ai/code/session_01T233rKgZqE5wzDm3EVTHL1