Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Path description logic is duplicated across wallets #457

Open
mrnerdhair opened this issue Mar 14, 2022 · 0 comments
Open

Path description logic is duplicated across wallets #457

mrnerdhair opened this issue Mar 14, 2022 · 0 comments
Labels
tech-debt A suboptimality that existed in code when it was written.

Comments

@mrnerdhair
Copy link
Contributor

The logic wallet.<chain>DescribePath functions are duplicated in many different wallets. Some of the implementations have slight differences, but all are supposed to produce the same outputs -- with the notable exception of the Trezor and Ledger implementations, which produce different descriptions using their device-specific Ethereum path derivation schemes. Nevertheless, most of the logic is the same, and the specific Ethereum path derivation scheme could easily be provided as a parameter to a single implementation.

  • Dry out the redundant implementations of *describePath into a single implementation in hdwallet-core.
  • The central implementation should support several different Ethereum account derivation schemes:
    • Standard BIP44: m/44'/60'/n'/0/0 => Ethereum Account #n
    • Metamask-style (used by Trezor): m/44'/60'/0'/0/n => Ethereum Account #n
    • Old Ledger-style: m/44'/60'/0'/n => Ethereum Account #n
    • Modern Ledger-style: m/44'/60'/n'/0/0 => Ethereum Account #n, m/44'/60'/0'/n => Ethereum Account #n (Legacy)

Fixed by #420.

@mrnerdhair mrnerdhair added the tech-debt A suboptimality that existed in code when it was written. label Mar 14, 2022
@mrnerdhair mrnerdhair linked a pull request Mar 17, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech-debt A suboptimality that existed in code when it was written.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants
@mrnerdhair and others