Repository navigation
generation: Validate roots before re-sharing - #126
BenWestgate wants to merge 2 commits into
Conversation
BenWestgate
added
bug
![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: f47cb9721a
ℹ️ 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".
Clarify that a format-valid parsed MasterSeed does not by itself establish the BIP32-root precondition for creation. Document the from_secret check before identifier selection or entropy and distinguish Core Lightning. Matches the already-tested behavior of f47cb97; no runtime change. AI-assisted response to the PR #126 review; refs #125.
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head follow-up review: ACK 8cd584e02c33aad0043762215def5452a9c00f1d.
The runtime check is at the correct public creation boundary: an existing MasterSeed must form a valid BIP32 root before threshold/header processing can select a random identifier or emit shares, while Core Lightning remains unaffected. The regression proves the invalid Bitcoin path stops before entropy selection. The follow-up documentation accurately distinguishes format-valid parsing from the creation precondition.
GitHub's current-head Python-package run completed successfully. No correctness or security blocker found. Human review and signed integration remain.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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.
Codex current-head review at 8cd584e: no findings. Existing Bitcoin MasterSeed inputs now fail BIP32-root validation before identifier selection or entropy; Core Lightning remains intentionally exempt.
|
@codex review Please review current head |
This comment has been minimized.
This comment has been minimized.
8cd584e to
b4c9e5d
Compare
Clarify that a format-valid parsed MasterSeed does not by itself establish the BIP32-root precondition for creation. Document the from_secret check before identifier selection or entropy and distinguish Core Lightning. Matches the already-tested behavior of f47cb97; no runtime change. AI-assisted response to the PR #126 review; refs #125.
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. |
|
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. 🎉 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". |
|
cACK validating roots before proceeding with |
b4c9e5d to
90910ce
Compare
Clarify that a format-valid parsed MasterSeed does not by itself establish the BIP32-root precondition for creation. Document the from_secret check before identifier selection or entropy and distinguish Core Lightning. Matches the already-tested behavior of f47cb97; no runtime change. AI-assisted response to the PR #126 review; refs #125.
Clarify that a format-valid parsed MasterSeed does not by itself establish the BIP32-root precondition for creation. Document the from_secret check before identifier selection or entropy and distinguish Core Lightning. Matches the already-tested behavior of f47cb97; no runtime change. AI-assisted response to the PR #126 review; refs #125.
919089b to
29b2a5c
Compare
Clarify that a format-valid parsed MasterSeed does not by itself establish the BIP32-root precondition for creation. Document the from_secret check before identifier selection or entropy and distinguish Core Lightning. Matches the already-tested behavior of f47cb97; no runtime change. AI-assisted response to the PR #126 review; refs #125.
A parsed Bitcoin secret validates the codex32 format but need not form a valid BIP32 root. Reject that state at the public from_secret ceremony boundary before identifier randomness or recovery-card output, using the existing stdlib-only validator. The supplied-byte path was already covered by PR #16. Retain format parsing and Core Lightning behavior. The header-collision comparison is kept on one formatted line in the same function; its behavior is unchanged and the package remains at 5,198 logical review lines. The 69 focused creation/API tests pass normally and with optimization; Ruff, strict mypy, build/twine, and whitespace checks pass. The full normal suite passes 953 tests; the optimized full suite is running. AI-assisted fix requested by the maintainer; fixes #125.
Clarify that a format-valid parsed MasterSeed does not by itself establish the BIP32-root precondition for creation. Document the from_secret check before identifier selection or entropy and distinguish Core Lightning. Matches the already-tested behavior of f47cb97; no runtime change. AI-assisted response to the PR #126 review; refs #125.
29b2a5c to
0948e52
Compare
CreationCeremony.from_secret()could emit Bitcoin recovery cards withoutchecking whether the encoded seed formed a valid BIP32 root. #16 fixed supplied
raw bytes but left this existing-secret path from Kimi F7 / consolidated M8.
Check the root before identifier randomness or share generation. Keep codex32
parsing as format validation and preserve Core Lightning behavior. The regression
forces the rare invalid-root outcome and proves it stops before any entropy call.
The same-function header comparison is compacted without changing its behavior;
the installed package remains below
<5200at 5,198 logical lines.Validation: 69 focused tests and all 953 tests pass both normally and under
python -O. Ruff, strict mypy,build/twine, and
git diff --checkpass; the BSD notice is present in both builtartifacts. Review after #53, whose API changes are the base of this focused diff.
Fixes #125. Refs #20, #16. AI assistance was used for this maintainer-requested
change. Human review and signed integration remain with the maintainer.