api: Remove unused public entry points - #130
BenWestgate wants to merge 2 commits into
Conversation
core_descriptors(private=False) asked a named Bitcoin Core wallet for its HD key and built public descriptors from it, but only type-checked the seed it was given, so a wallet holding a different seed returned that wallet's descriptors. Nothing calls it: ms32 wallet hands Core the root xprv with addhdkey and Core builds the descriptors itself with createwalletdescriptor, and Core's listdescriptors and exportwatchonlywallet already provide public descriptors. We are pre-1.0, so delete public API that no command or tool calls instead of maintaining it: - core_descriptors, the WalletPublicDeriver protocol, the descriptor checksum (DESCSUM) and record helpers, and the Core adapter's public_descriptors, gethdkeys and derivehdkey calls; - generate_core_lightning_secret and CreationCeremony.core_lightning, which created fresh Core Lightning secrets that no command creates. Parsing, recovering, deriving and re-sharing an existing cl secret are unchanged. The real-Core fixture and installed-wheel smoke test now check xprv and tprv serialization through master_xprv. The package exports 22 names. The installed package drops from 5,092 to 4,868 logical lines. Fixes #128 Refs #63 Claude-Session: https://claude.ai/code/session_01T233rKgZqE5wzDm3EVTHL1
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head security review: ACK 3133c8e96ee241ec1b3427e214a5fada5babc84f.
I reviewed both fixes and their surrounding API removals. Removing core_descriptors eliminates the mismatched-seed public path from #128 rather than trying to authenticate an API with no repository caller; the live Core initialization path still uses the root xprv and Core-native descriptor creation. Removing fresh Core Lightning generation leaves parsing, recovery, and CreationCeremony.from_secret() re-sharing of existing cl secrets covered. The CorrectionEdit change keeps observed and replacement accessible as fields while excluding both from dataclass repr, and the regression verifies restored characters are not rendered by a correction candidate.
The real Bitcoin Core fixture is green on this head. The full Python-package run is currently queued, so this review does not claim that matrix has completed. No correctness or security blocker found in the current diff. Human review/authorship and integration remain separate.
3133c8e to
7171b1b
Compare
BenWestgate
changed the title
BenWestgate
added
area: api
ChatGPT Codex Connector
|
@codex review |
![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: 7171b1b705
ℹ️ 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".
Fresh Core Lightning generation was removed from the public API, but the architecture, capability table, identifier policy, and security model still described it as supported. Align those contracts with the remaining behavior: existing CL secrets may still be parsed, recovered, corrected, derived, and re-shared.\n\nRefs #128
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 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 2cbd279. #128 is fixed by removing the unauthenticated public-descriptor path entirely; the stale CL-generation docs are reconciled. Exact-head package and Core-fixture CI are green.
This branch has not been deployed
2 participants
core_descriptors(secret, private=False, integration=..., wallet=...)derived public descriptors from whichever seed the named Bitcoin Core wallet held while only type-checking the suppliedsecret. Nothing in the repository calls that path:ms32 walletgives Core the root xprv and Core creates the account-0 descriptors itself.Remove that unused pre-1.0 API rather than maintaining an unauthenticated seed/wallet pairing:
core_descriptors,WalletPublicDeriver, descriptor checksum/record helpers, and the Core adapter's public-derivation calls;Parsing, recovery, derivation, and re-sharing of existing
clsecrets remain supported.master_xprvremains the wallet API. The real-Core fixture and installed-wheel smoke test usemaster_xprvfor xprv/tprv serialization.The correction-rendering fix previously stacked here is split into focused #132, so this PR now fixes only #128 and supersedes the narrower API cleanup in #64. Current head
2cbd279also removes the stale CL-generation claims identified by Codex; that review thread is resolved, the Python matrix and real-Core fixture are green, and Codex reports no major issue on the exact head.Fixes #63
Fixes #128
Supersedes #64
Generated with Claude Code; human review/authorship and integration remain required.