Conversation
This was referenced Aug 5, 2026
fametrano
force-pushed
the
japanese_delimiter_read_back
branch
from
August 13, 2026 07:59
2cb21cc to
bd50ae2
Compare
Author
|
@prusnak could you approve the workflow run here? This is my first pull request from this fork, so CI has never started, and the local run in the description is the only evidence in the thread. The branch is a single commit now. Since filing I added one test for |
Author
|
@prusnak the workflow runs expired before approval, so CI still hasn't run. Could you re-run them, or should I push a new commit? |
to_mnemonic joins a Japanese mnemonic with U+3000, while to_entropy, check and expand split on " ". So the library cannot read the sentences it writes: to_entropy raises on all 24 Japanese vectors of vectors.json, and expand returns the sentence unchanged. check splits after NFKD, which maps U+3000 to U+0020. So it cannot split on self.delimiter: that was trezor#110, reverted in df3e150 for failing CI. Split on any run of whitespace instead, as detect_language already does. to_seed follows the same rule. Before, input that check rejects gave a different seed from the same words separated by single spaces. The tests fail without the fix: to_entropy raises on every Japanese vector, check rejects a sentence separated by anything but one space, and expand returns a tab- or U+3000-separated sentence unexpanded. The existing round-trip test hides the first failure, because it splits the sentence itself before passing it in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fametrano
force-pushed
the
japanese_delimiter_read_back
branch
from
September 29, 2026 21:09
bd50ae2 to
b5e1375
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mnemonic("japanese")joins words with U+3000, the ideographic space(
#L73,#L210),as the BIP39 Japanese vectors do.
to_entropyandexpandsplit on" ",so they see a single word.
to_entropyraises;expandreturns thesentence unchanged.
checkaccepts the sentence, because it normalizesfirst, and NFKD maps U+3000 to U+0020.
The README's own steps reproduce it. On master, Python 3.12:
This block is a doctest. It passes on master. With this patch it fails,
because
to_entropyreturns the entropy. Onvectors.json,checkaccepts all 288 mnemonics.
to_entropyfails on all 24 Japanese ones, andon none of the other 264.
#110 fixed this by splitting on
self.delimiter. It was merged as71cf5203and reverted asdf3e1500for failing CI. That approach cannotwork in
check, which normalizes before it splits: after NFKD there isno U+3000 left to split on, so every Japanese mnemonic fails validation.
So this PR splits on any run of whitespace.
detect_languagealreadydoes this.
The changes to
to_entropy,checkandexpandfix the Japanese bug.The change to
to_seedis separate. Todayto_seedonly applies NFKD.So input that
checkrejects, such as a leading space, a trailing newlineor a tab, silently gives a different seed from the same words separated
by single spaces. If you do not want that change, drop the
to_seedlineand the last assertion of
test_whitespace_runs. The fix for the Japanesebug still holds.
Compatibility: all 288 vectors are unchanged.
test_vectorsasserts themnemonic, seed and xprv of each, and passes. No input that
checkacceptstoday gives a different seed. Only input that it rejects changes: it now
gives the canonical seed. The passphrase is not touched, because its
whitespace is part of the secret.
CI, since #110 was reverted for failing it:
python tests/test_mnemonic.pyis
OKon Python 3.8 through 3.14 and on PyPy 3.11.black --check,isort --check-only,flake8andpyrightreport nothing onsrc tests tools.Written with machine assistance and reviewed against master
b57a5adbefore filing. Found while checking btclib's BIP39 reading against this
implementation: btclib-org/btclib#258.