wallet: Validate Bitcoin Core state types - #12
BenWestgate wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f60a4600e
ℹ️ 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".
3f60a46 to
d574d1e
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
d574d1e to
5d128b8
Compare
BenWestgate
left a comment
There was a problem hiding this comment.
Please justify why this function is actually necessary to the function of this project or remove it.
|
Resolution of the 2026-09-28 review question: the legacy public descriptor-record helper is not necessary as a supported public API. That concern is deliberately separated into #63 / PR #64, which removes |
BenWestgate
left a comment
There was a problem hiding this comment.
Release-gate ACK 5d128b8.
The current one-commit head keeps this PR scoped to fail-closed Bitcoin Core state typing. The explicit-null unlock-state bug is fixed; boolean/string numeric lookalikes are rejected; relock verification requires exact integer zero. The separate descriptor-API question is correctly isolated in #64. All inline threads are resolved and exact-head Python-package run 333 is green.
No remaining code blocker from this review.
|
Agent release-gate review at exact head |
5d128b8 to
afee360
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Require exact nonnegative integer types for wallet counts and unlock state, including final relock verification. This prevents booleans and malformed RPC values from passing numeric equality checks. Security: fail closed on untrusted Bitcoin Core state while preserving valid encrypted and unencrypted wallet flows. Validation: python -m pytest -q; python -O -m pytest -q; Ruff check and format; strict mypy; differential_wallet.py --verify.
afee360 to
a860035
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
What
walletlock;Why
Python treats booleans as integers, so values such as
Falsepreviously passed numeric zero checks. Core RPC responses are untrusted and wallet eligibility and relocking must fail closed. This adapts Rob1Ham#10 and closes the same type-confusion path in final relock verification.Current head
5d128b8is one focused commit and remains mergeable intoreviewability-v1; the prior inline review threads are resolved. The separate public descriptor API question raised during review is handled by focused #64 rather than widening this state-validation fix.Reviewed-head validation
python -O;The differential-wallet verifier cited above was subsequently removed by merged #7 together with the
bip32/Coincurve test dependency. On the final integration tip, use the current real-Bitcoin-Core fixture path from merged #51 plus the focused malformed-state/relocking tests and normal CI; do not reintroduce the removed Python wallet oracle merely to reproduce the old validation line. #12 can be integrated after the #42/#57/#46/#80/#81 runtime stack settles.