Skip to content

feat(key-wallet): add normalize_phrase and is_word_in_language helpers#806

Draft
llbartekll wants to merge 1 commit into
devfrom
feat/key-wallet-bip39-primitives
Draft

feat(key-wallet): add normalize_phrase and is_word_in_language helpers#806
llbartekll wants to merge 1 commit into
devfrom
feat/key-wallet-bip39-primitives

Conversation

@llbartekll

@llbartekll llbartekll commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

What

Adds two BIP39 input helpers to key-wallet's Mnemonic, plus a Language re-export, so downstream consumers can build recover-flow word/phrase validation on top of key-wallet instead of pulling in a second, direct bip39 dependency and re-deriving the logic.

  • Mnemonic::normalize_phrase(&str) -> String — NFKD + lowercase + whitespace-collapse. Lenient input normalization for validation/membership of user-typed phrases; explicitly NOT BIP39 seed normalization (to_seed already performs the required NFKD for seed derivation).
  • Mnemonic::is_word_in_language(&str, Language) -> bool — exact wordlist membership for a given language (no normalization; pre-normalize via normalize_phrase for case/accent tolerance).
  • Re-export Language from the crate root + prelude (was only reachable via mnemonic::Language).
  • Add the unicode-normalization dependency (already present transitively via bip39).

Why

In dashpay/platform PR #3842, rs-platform-wallet-ffi added recover-flow mnemonic helpers by depending directly on bip39 and re-implementing wordlist membership / phrase normalization. Review feedback flagged this as duplicating logic key-wallet already owns. These primitives let the platform crate become a thin facade over key-wallet — one source of BIP39 logic — and drop its direct bip39 dependency. App-specific concerns (CJK ideographic auto-split, the app's bundled-language policy) stay in the platform facade.

Tests

`cargo test -p key-wallet --lib mnemonic` adds `test_normalize_phrase`, `test_normalize_phrase_unicode_forms_converge`, `test_is_word_in_language` (36 mnemonic unit tests pass). `cargo clippy -p key-wallet` clean.

Notes

Branched off `3d0d5dc` (the commit dashpay/platform currently pins) for a minimal, conflict-free diff: only `key-wallet/{Cargo.toml, src/lib.rs, src/mnemonic.rs}` change.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added mnemonic phrase normalization with Unicode NFKD handling, automatic lowercasing, and whitespace collapsing for standardized input processing.
    • Added language-specific BIP39 wordlist validation for exact word membership verification.
    • Expanded public API to expose Language type alongside Mnemonic for language-specific operations.

Add two BIP39 input helpers to Mnemonic so consumers don't re-implement
wordlist/normalization logic on top of the bip39 crate:

- normalize_phrase: NFKD + lowercase + whitespace-collapse (lenient input
  normalization for validation/membership; NOT BIP39 seed normalization,
  which to_seed already performs)
- is_word_in_language: exact wordlist membership for a given language

Re-export Language from the crate root + prelude, and add the
unicode-normalization dependency.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bccaf0a6-b71a-4bef-b0b1-63a3c0335958

📥 Commits

Reviewing files that changed from the base of the PR and between 7ff6b24 and 307582c.

📒 Files selected for processing (3)
  • key-wallet/Cargo.toml
  • key-wallet/src/lib.rs
  • key-wallet/src/mnemonic.rs
 ______________________________________________________
< CUDA: Carrot Utilization for Debugging Acceleration. >
 ------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/key-wallet-bip39-primitives

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.73%. Comparing base (3d0d5dc) to head (307582c).
⚠️ Report is 4 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #806      +/-   ##
==========================================
+ Coverage   72.55%   72.73%   +0.18%     
==========================================
  Files         322      323       +1     
  Lines       71006    71796     +790     
==========================================
+ Hits        51518    52224     +706     
- Misses      19488    19572      +84     
Flag Coverage Δ
core 76.75% <ø> (+0.28%) ⬆️
ffi 45.89% <ø> (-0.53%) ⬇️
rpc 20.00% <ø> (ø)
spv 90.36% <ø> (+0.13%) ⬆️
wallet 71.37% <100.00%> (+0.08%) ⬆️
Files with missing lines Coverage Δ
key-wallet/src/mnemonic.rs 95.14% <100.00%> (+0.59%) ⬆️

... and 36 files with indirect coverage changes

@llbartekll llbartekll marked this pull request as draft June 10, 2026 16:32
@llbartekll llbartekll requested a review from ZocoLini June 10, 2026 16:32
llbartekll added a commit to dashpay/platform that referenced this pull request Jun 10, 2026
Address review feedback on #3842: instead of a direct `bip39` dependency
and re-derived wordlist/normalization logic, the recover-flow mnemonic FFI
now delegates to key-wallet's primitives (is_word_in_language /
normalize_phrase / validate). Requires the key-wallet additions in
dashpay/rust-dashcore#806 — all 8 rust-dashcore deps bumped to that branch
rev (307582cf == 3d0d5dc + the key-wallet change only).

App-specific policy stays in the facade: the 7-language bundled set and
cleanupPhrase's CJK ideographic auto-split. The platform_wallet_mnemonic_*
FFI surface is unchanged (generated header byte-identical), so the Swift SDK
and dashwallet need no change. The direct `bip39` dependency is removed; it
is now reached only transitively via key-wallet.

18 unit tests pass; clippy + fmt clean.

Co-Authored-By: Claude Opus 4.8 <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