Skip to content

fix: reject underscores in derivation indexes - #2714

Open
SashaMIT wants to merge 1 commit into
cryptoadvance:masterfrom
SashaMIT:codered-derivation-reject-underscore
Open

SashaMIT wants to merge 1 commit into
cryptoadvance:masterfrom
SashaMIT:codered-derivation-reject-underscore

Conversation

@SashaMIT

@SashaMIT SashaMIT commented Sep 25, 2026 •

Copy link
Copy Markdown

Summary

der_to_bytes used int() on each path component. int("1_0") is 10, so m/1_0 encoded index 10. The caller already has the derivation string. An underscore is not a derivation character, and it was changing the index. Indexes now have to be digits. The existing h and ' harden suffix is unchanged.

Test plan

  • Before the change, der_to_bytes("m/1_0") returned index 10 ([10, 0, 0, 0])
  • tests/test_der_to_bytes.py passes after the change, including m/10 and m/1h

der_to_bytes used int() on each path component. int("1_0") is 10, so m/1_0 encoded index 10. Indexes now have to be digits.
@netlify

netlify Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for specter-desktop-docs canceled.

Name Link
🔨 Latest commit 7467e2b
🔍 Latest deploy log https://app.netlify.com/projects/specter-desktop-docs/deploys/6ab637e28a42a80008361275

@al-munazzim al-munazzim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — this is a good catch. I verified the targeted unittest locally after installing the minimal pinned Flask stack in a sparse checkout:\n\nPYTHONPATH=src python -m unittest tests/test_der_to_bytes.py\n\nResult: 3 tests passed. The change is small and correctly prevents Python's underscore digit grouping from silently changing derivation indexes.

@SashaMIT

Copy link
Copy Markdown
Author

Thanks for taking a look, and for running the three tests.

This branch has not been deployed

No deployments
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.

2 participants