Repository navigation
checksums: XOR CRC2-CRC4 padding with all ones - #216
BenWestgate wants to merge 6 commits into
Conversation
Match the spare-bit CRC of the codex32 key-expressions draft. Without the XOR, a 24-byte seed extended with zero bits to 64 bytes keeps valid CRC3 padding; with it, that extension never matches, since 320 bits is not a multiple of CRC3's period of 7. CRC1 keeps no XOR, because one would give every all-zero 28-byte seed valid padding. Correction tie-breaks prefer the new padding, then the padding written before this change, then neither. BIP93 vectors 3 and 4 now generate their alternate 3 and 7 strings. pytest: 913 passed. Ruff check and format, mypy: passed. Claude-Session: https://claude.ai/code/session_019DuBKzoVE7oBFRnzoGH2ps
Codex Review Summary: ✅ CompletedThis 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. |
|
@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". |
BenWestgate
left a comment
There was a problem hiding this comment.
Remove the legacy padding check. This is backwards compatibility we dont need for pre beta software.
|
Why are you keeping has_legacy_padding at all? We are pre Beta we dont need this kind of backwards compatibility. |
The legacy padding check is gone, so nothing should model the padding written before the all-ones XOR. test_cli_tie_breaks_follow_hamming_crc_then_fingerprint already covers the Hamming, CRC, fingerprint order. Drop the API doc line about historical padding for the same reason. pytest on a squash of this branch: 912 passed, also with -O. Ruff check and format: passed. Claude-Session: https://claude.ai/code/session_019DuBKzoVE7oBFRnzoGH2ps
|
It's gone. Your 024efb2 removed The function still shows up in the first commit's history. A squash merge leaves one clean commit. I tried force-pushing that squash, but my session isn't allowed to rewrite this branch. Generated by Claude Code |
Before: generated ms strings fill their spare bits with a CRC that starts from 1 and has no final XOR. A 24-byte seed extended with zero bits and read as a 64-byte seed keeps valid CRC3 padding, so the padding cannot tell the two apart.
After: CRC2 to CRC4 end with an all-ones XOR, matching the spare-bit CRC in the codex32 key-expressions draft. That extension never keeps valid padding, because the 320 added bits are not a multiple of CRC3's period of 7. CRC1 stays odd parity, because an XOR there would give every all-zero 28-byte seed valid padding.
Strings padded the old way still parse, since padding is never enforced, and get no special treatment anywhere. Correction tie-breaks are unchanged: Hamming weight, then current CRC padding, then the wallet fingerprint.
How: the CRC2 to CRC4 constants in
checksums.pybecome0b11,0b111and0b1111. The frozen-convention section indocs/developer/api.mdgives the new representative outputs and the reason for the XOR.CorrectionCandidateis unchanged.BIP93 vectors 3 and 4 now generate their
secret_s_alternate_3andsecret_s_alternate_7strings, notsecret_s. The vector 3 test derives its shares from the officialsecret_s.The branch history adds a legacy-padding check and later removes it. The net diff has none of it, so a squash merge leaves one clean commit.
Validation:
python -m pytest -qandpython -O -m pytest -qgave 912 passed on a squash of this branch onto the currentreviewability-v1. Ruff check and format, andpython -m mypy src/codex32, passed.🤖 Generated with Claude Code
https://claude.ai/code/session_019DuBKzoVE7oBFRnzoGH2ps