Skip to content

to_entropy() cannot read the japanese mnemonics to_mnemonic() writes - #145

Open
fametrano wants to merge 2 commits into
trezor:masterfrom
fametrano:japanese_delimiter_read_back
Open

to_entropy() cannot read the japanese mnemonics to_mnemonic() writes#145
fametrano wants to merge 2 commits into
trezor:masterfrom
fametrano:japanese_delimiter_read_back

Conversation

@fametrano

Copy link
Copy Markdown

Mnemonic("japanese") joins words with U+3000, the ideographic space
(#L73,
#L210),
as BIP39's Japanese vectors do. to_entropy and expand split on " ", so
they see a single word: the first raises, the second returns the sentence
unchanged. check accepts it, because it normalizes first and NFKD maps
U+3000 to U+0020.

The README's own steps are the reproduction. On master, python 3.12:

>>> from mnemonic import Mnemonic
>>> m = Mnemonic("japanese")
>>> words = m.generate(strength=256)   # README: "Generate word list"
>>> m.check(words)
True
>>> m.to_entropy(words)                # README: "calculate original entropy"
Traceback (most recent call last):
  ...
ValueError: Number of words must be one of the following: [12, 15, 18, 21, 24], but it is not (1).

That block is a doctest, and passes as written on master; with this patch
applied it fails, because to_entropy then returns the entropy. On
vectors.json, check accepts all 288 mnemonics while to_entropy fails
on all 24 Japanese ones and none of the other 264.

#110 fixed this by splitting on self.delimiter; it was merged as
71cf5203 and reverted as df3e1500 for failing CI. That direction cannot
work in check, which normalizes before splitting: after NFKD there is no
U+3000 left to split on, so every Japanese mnemonic fails validation
instead. Hence any run of whitespace, read after normalization — which is
what detect_language already does.

The fourth changed line applies the same rule to to_seed, whose NFKD-only
reading silently stretches a different seed out of the input check
rejects: a leading space, a trailing newline, a tab. If that is unwanted,
drop it along with the last assertion of test_whitespace_runs; the other
three still fix the Japanese bug.

Compatibility: all 288 vectors are unchanged — test_vectors asserts
mnemonic, seed and xprv for each, and passes. No input check accepts today
derives a different seed; what changes is input it rejects, which now
derives the canonical one. The passphrase is untouched, its whitespace being
part of the secret.

CI, since #110 was reverted for failing it: python tests/test_mnemonic.py
is OK on python 3.8 through 3.14 and on pypy 3.11, and black --check,
isort --check-only, flake8 and pyright report nothing on
src tests tools.

Written with machine assistance and reviewed against master b57a5ad
before filing. Found while checking btclib's BIP39 reading against this
implementation: btclib-org/btclib#258.

🤖 Generated with Claude Code

fametrano and others added 2 commits August 5, 2026 23:17
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.

The separator is read after NFKD, which maps U+3000 to U+0020, so it
cannot be self.delimiter -- that was trezor#110, reverted in df3e150 for
failing CI. What is left to split on is any run of whitespace, which is
what detect_language already reads.

to_seed follows the same rule, so that a sentence check refuses no
longer stretches into a different seed in silence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both fail without the previous commit: to_entropy raises on every
japanese vector, and check refuses a sentence separated by anything but
one space. The existing round trip hides the first by splitting the
sentence itself before handing it over.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant