Skip to content

wallet: Privatize Core descriptor records - #64

Open
BenWestgate wants to merge 1 commit into
codex/recovery-secret-switch-noticefrom
codex/63-private-core-descriptors
Open

BenWestgate wants to merge 1 commit into
codex/recovery-secret-switch-noticefrom
codex/63-private-core-descriptors

Conversation

@BenWestgate

@BenWestgate BenWestgate commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

What

Remove the obsolete supported Python API for constructing Bitcoin Core descriptor records:

  • remove core_descriptors from package-level codex32.__all__ and supported API documentation;
  • remove the unused public-deriver protocol/branch from wallet.py;
  • keep the fixed private descriptor builder only as private verification tooling;
  • make public-API and installed-wheel checks assert that descriptor-record construction is not exported.

Why

Current reviewability-v1 exports core_descriptors even though #7 moved wallet initialization to Bitcoin Core native addhdkey / createwalletdescriptor setup. External callers no longer need an import-record construction API or the obsolete WalletPublicDeriver abstraction. master_xprv remains a separately reviewed supported primitive.

The base has 24 package-level exports while docs/developer/api.md still says 25. Removing core_descriptors makes package __all__ 23 names. #53 publishes reference-vector helpers at their owning modules and deliberately does not add them to package-level codex32.__all__.

Current head

eb29499 is the single original human-authored change replayed onto current #95 (4ea72bb) after the refreshed restore stack/. The refresh deliberately drops the obsolete pre-#7 _bitcoin_core.py import-descriptor/account-7 hunk and keeps #7-deleted legacy tools deleted. Production _bitcoin_core.py is unchanged by this PR; v1 keeps Core-native account-0 wallet setup. #68 tracks future nonzero accounts when Core exposes the needed selector.

Validation

  • focused public-API / wallet / Core suite: 64 passed;
  • the same focused suite under python -O: 64 passed;
  • production-size budget plus public-API regression: 14 passed;
  • compileall and git diff --check: pass;
  • package __all__: 23 names at this head;
  • current-head GitHub matrix is rerunning after the mechanical refresh and must be green before freeze.

A disposable full local run stopped only because that ambient Python environment lacks the dev-only hypothesis package; the focused tests above do not require it. The repository CI installs the declared development test dependencies and is the authoritative full-matrix check.

Fixes #63.

AI assistance was used to mechanically refresh and validate this user-authorized branch-to-branch contribution; the source change remains one human-authored commit.

@BenWestgate BenWestgate added area: api Public and supported Python API boundaries. area: wallet/core Wallet integration and Bitcoin Core boundaries. gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review. labels Sep 28, 2026 — with ChatGPT Codex Connector

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

ACK a0e3a15.

Copy link
Copy Markdown
Owner Author

Review sequencing note: #64 remains the focused fix for #63's public API boundary and preserves the currently supported arbitrary --account behavior. New issue #68 records a later Core-native direction (addhdkey + createwalletdescriptor) that would remove the remaining Python private-descriptor builder and, until Core exposes an account parameter there, intentionally limit native descriptor creation to account 0. That later rewrite is not present on this PR or current reviewability-v1, so it should be reviewed separately rather than silently folded into #64.

Copy link
Copy Markdown
Owner Author

Agent release-gate review at exact head a0e3a15: removing core_descriptors from the supported package API is the right boundary. The remaining private builder is used only by the Core adapter/tests/tools, the unused public-deriver abstraction is removed, master_xprv is unchanged, and public-API tests explicitly reject descriptor-record export. Exact-head run 36467502754 succeeded. No remaining code-review blocker found.

@BenWestgate
BenWestgate force-pushed the codex/63-private-core-descriptors branch 2 times, most recently from 808b6fd to f3b17be Compare October 1, 2026 00:27
@BenWestgate

BenWestgate commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner Author

AI-assisted release-gate recheck of current head f3b17be.

Code ACK. This refresh replays only the still-valid public-API cleanup onto current reviewability-v1: core_descriptors and the obsolete WalletPublicDeriver are no longer supported exports, while #7's Core-native addhdkey/createwalletdescriptor production path is preserved unchanged and the legacy tools deleted by #7 stay deleted. The private _core_descriptors helper remains only for internal test/reference verification, and its docstring reflects that boundary.

Focused wallet/public-API/Core tests pass 64/64 normally and 64/64 under python -O; API/budget checks pass 14/14; git diff --check and compile checks are clean. Exact-head GitHub CI is fully green: the real-Core fixture plus all Python-package OS/compatibility jobs succeeded. No unresolved review threads remain and GitHub reports the PR clean/mergeable.

No remaining code-review blocker found on this head; it is ready for human review.

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-assisted current-head review performed at the maintainer's request and disclosed per docs/developer/AI_POLICY.md.

ACK f3b17be. The refreshed one-commit diff cleanly removes the obsolete supported core_descriptors surface without touching the Core-native account-0 initialization introduced by #7. master_xprv remains public; the fixed descriptor builder is private verification tooling only; installed/public-API checks assert the removed symbol is no longer exported. The package __all__ count becomes 23 as intended. Exact-head Python-package and Bitcoin Core fixture workflows both pass. No correctness findings; ready for human review/integration after the runtime stack settles.

@BenWestgate
BenWestgate force-pushed the codex/63-private-core-descriptors branch from f3b17be to 43af20d Compare October 1, 2026 18:50
@BenWestgate
BenWestgate changed the base branch from reviewability-v1 to codex/recovery-secret-switch-notice October 1, 2026 18:50
@chatgpt-codex-connector

Copy link
Copy Markdown

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.

1 similar comment
@chatgpt-codex-connector

Copy link
Copy Markdown

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
BenWestgate force-pushed the codex/recovery-secret-switch-notice branch from cebecfc to 4ea72bb Compare October 1, 2026 22:16
The package-level core_descriptors adapter exposes import-record construction that runtime callers no longer need. Bitcoin Core already owns public derivation, while private descriptor construction is only an implementation detail of the Core adapter.

Remove the unused public-deriver protocol and branch, keep the record builder private, and make installed/public-API checks enforce that boundary. Internal tests and verification tools continue to exercise the same fixed descriptor templates and arbitrary account handling.

Fixes #63
@BenWestgate
BenWestgate force-pushed the codex/63-private-core-descriptors branch from 43af20d to eb29499 Compare October 2, 2026 00:42
@chatgpt-codex-connector

Copy link
Copy Markdown

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: api Public and supported Python API boundaries. area: wallet/core Wallet integration and Bitcoin Core boundaries. gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant