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

Extension: Update modified Zip32 derivation path to match CLI #1611

Open
jurevans opened this issue Feb 5, 2025 · 0 comments · May be fixed by #1624
Open

Extension: Update modified Zip32 derivation path to match CLI #1611

jurevans opened this issue Feb 5, 2025 · 0 comments · May be fixed by #1624

Comments

@jurevans
Copy link
Collaborator

jurevans commented Feb 5, 2025

The extension should use a different default zip32 derivation path for Shielded accounts, to match update in CLI.

See: anoma/namada#4307

Essentially, the private key from which shielded keys are derived should itself be derived from m/44'/877'/0'/0'/2147483647', then from that, we can derive m/32'/877'/0' Zip32 paths.

NOTES

This has some consequences that we'll need to consider on the interface-side of things:

  • Since shielded accounts are no longer derived from the same private key as transparent accounts, it doesn't make sense to automatically generate these.
  • As such, it makes a lot more sense to have these as separate flows as there's no longer any relation between these keys
  1. Transparent: User can import/generate mnemonic, and derive with a default (or custom) BIP44 path
  2. Shielded: User can import/generate mnemonic, and derive a with a default (or custom) Zip32 path from a private key derived from m/44'/877'/0'/0'/2147483647' (This is a fixed path for the private key from which all shielded accounts will derived for any given seed/phrase)

We currently only have a custom BIP44 form (as before, we would derive shielded keys using default zip32 path of m/32'/877'/0'), but with this change, it would mean we should implement a ZIP32 form so users can derive any shielded account from the seed (by specifying zip32). They cannot override the BIP44 path of m/44'/877'/0'/0'/2147483647' which is used as the basis for all shielded accounts (as we need to match CLI where this value is fixed)

UX ramifications in Keychain & Namadillo

That said, this also breaks our concept of "Active Account". Since we shouldn't group them anymore, an "Active Account" could be a transparent or shielded account, which also makes no sense.

In my mind, a "Wallet" is the mnemonic you imported, which is used to seed any number of accounts, transparent or shielded, and having an Active wallet makes sense - user could use any addresses associated with that wallet, whereas currently "Active" account means a transparent and shielded account pair. But, of course, this would require a reworking of the import process, but here's the thought:

  • If user imports a seed phrase (let's call it the wallet), this is tagged as the Parent
  • If user wants to derive additional accounts from that wallet, they wouldn't import their seed phrase again, they'd choose an existing wallet and derive a new account from that
    • Which could be shielded or transparent

^^ This was the original design of the Extension, but we hijacked the parentId field of child accounts in order to associate Shielded Accounts with their respective Transparent parents, in order to group them together in the UI. To reintroduce something like this, we would have to leave current accounts in a stranded state and unassociated with any mnemonic. Technically users could still choose to import the same seed phrase and simply choose another path & alias, that's not a problem. But, I think it would be simpler and more convenient to give the option of deriving from an existing seed. The Keychain storage already supports that (as it was designed this way in the beginning).

Alternatively, we could keep this like the current flow, where users always provide a seed phrase (or private key), and we maintain a sort of flat hierarchy of accounts, but that also means that "Active Account" doesn't really do us much good. Choosing a transparent or shielded account as active (as they're not not grouped together) seems clunky and meaningless. Showing all transparent & shielded accounts in the listing is easy to do, but we should distinguish between them as there will be 2x the amount of accounts in the list if they're ungrouped. They're already stored this way, so that part is trivial.

@jurevans jurevans added this to the Phase 3 milestone Feb 5, 2025
@jurevans jurevans self-assigned this Feb 5, 2025
@jurevans jurevans changed the title Extension: Update Ledger derivation path to match app Extension: Update modified Zip32 derivation path to match CLI Feb 7, 2025
@jurevans jurevans removed the Ledger HW Wallet Ledger HW Wallet integration label Feb 7, 2025
@jurevans jurevans linked a pull request Feb 11, 2025 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant