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

feat/944 - Extension updates - BIP44/Zip32/Ledger/Masp #1155

Merged
merged 14 commits into from
Nov 4, 2024

Conversation

jurevans
Copy link
Collaborator

@jurevans jurevans commented Oct 2, 2024

Resolves #944
Resolves #1154

  • Include Extension/SDK versions in Settings
  • Add SDK version to Namadillo settings
  • Zip32 should derive default shielded accounts from ed25519 private key (like Namada CLI) with default account
    • Add SDK package functionality to do this from either a Seed (64 bytes) or SK (32 bytes)
      • sdk.keys.deriveShieldedFromSeed()
      • sdk.keys.deriveShieldedFromPrivateKey()
  • Add BIP44 path form to Mnemonic Import (Advanced)
    • Combine Bip39 passphrase and path to Advanced view
    • Generate shielded account from Zip32 version of path (makeSaplingPathArray(path))
  • Add BIP44 path form to Ledger Import (Advanced)
    • NOTE I hooked up shielded keys to our Ledger class, though it won't be useful until we have Zip32 derivation on Ledger. This gives the SDK the interface to use, but we'll want to hook it up to the UI after Zip32 is added!
  • Bump extension version
  • Hook up Viewing Key
    • Update styles for Viewing Key (See screenshots at bottom)
  • General UI Styles (will follow-up after this PR to finish the styles prior to release)

TODO

  • Add BIP44 path form to Mnemonic Generate (Advanced) - We didn't specify this as a use-case, but it could be added as well - we should discuss that it is also possible to specify a BIP39 passphrase here, though we had copy indicating that it was only to support the same accounts in Namada CLI that used them. If someone used a BIP39 passphrase there, I feel like they could use it on their generated BIP39 seed and not just the imported one.
    • Generate shielded account from Zip32 version of path (makeSaplingPathArray(path))
  • We can now generate shielded keys from a private key, so this can be added to Import -> Private Key using a default path
  • Regenerate docs if sdk or types changes! NOTE This should be in a separate, SDK-release PR, as other PRs will affect the docs too and it creates too much noise for an already large PR

Screenshots

NOTE Styles are still in progress...

Advanced Menu (Import Mnemonic):
Screen Shot 2024-10-18 at 11 53 14 AM

Custom Derivation Path:
Screen Shot 2024-10-18 at 11 53 01 AM

Bip39 Passphrase:
Screen Shot 2024-10-18 at 11 53 22 AM

Keychain & SDK versions:
Screen Shot 2024-10-02 at 1 34 25 PM

SDK version in Namadillo:
Screen Shot 2024-10-03 at 7 39 37 AM

Path displayed (only when a custom path was specified):
Screen Shot 2024-10-31 at 11 41 12 AM

Designs

Updated designs for Viewing Key:
Screenshot 2024-10-09 at 16 29 44

FIGMA:
https://www.figma.com/design/NFyHbLZXBSl3aUsMxtffvV/Namada-End-User-Interface-V1.0-%2F-Phase-1%3A-Block-Party?node-id=6361-75215&node-type=canvas&t=cJGwuzI3Q8soSDus-0

@jurevans jurevans self-assigned this Oct 2, 2024
@jurevans jurevans changed the title WIP: feat/944 - Extension: Custom BIP44 derivation paths WIP: feat/944 - Extension: Custom BIP44 derivation paths and Version Info Oct 3, 2024
@jurevans jurevans force-pushed the feat/944-custom-paths branch 14 times, most recently from 152419a to 2fe95ce Compare October 7, 2024 16:58
@jurevans jurevans changed the title WIP: feat/944 - Extension: Custom BIP44 derivation paths and Version Info WIP: feat/944 - Bip44 Form & Updates Oct 14, 2024
@jurevans jurevans force-pushed the feat/944-custom-paths branch 7 times, most recently from c6fe8fa to 6b33660 Compare October 17, 2024 16:29
@jurevans jurevans changed the title WIP: feat/944 - Bip44 Form & Updates WIP: feat/944 - Custom derivation updates Oct 17, 2024
Comment on lines +128 to +132
const path = {
account: accountDetails.path.account,
change: accountDetails.path.change || 0,
index: accountDetails.path.index || 0,
};
Copy link
Collaborator Author

@jurevans jurevans Oct 30, 2024

Choose a reason for hiding this comment

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

For signing with transparent keys, all of these values are expected to be present! Ledger does not have Zip32 paths as of yet

@jurevans jurevans changed the title WIP: feat/944 - Extension updates - BIP44/Zip32/Ledger/Masp feat/944 - Extension updates - BIP44/Zip32/Ledger/Masp Oct 30, 2024
@jurevans jurevans marked this pull request as ready for review October 30, 2024 15:08
Copy link
Collaborator

@mateuszjasiuk mateuszjasiuk left a comment

Choose a reason for hiding this comment

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

This works great!

I have couple of minor UI/UX comments.
I personally do not like these numeric input arrows :D
arrows

Also maybe we can reduce font size or sth so we do not see the scroll.
vk

I'm curious if it makes sense to enable "advanced" button only if we are in Step 2 of HW Wallet import flow. Not sure xd

@jurevans
Copy link
Collaborator Author

I'm curious if it makes sense to enable "advanced" button only if we are in Step 2 of HW Wallet import flow. Not sure xd

Thanks! Yeah some of these style things I will fix in a follow-up PR before release, I think those are all good suggestions!

@jurevans
Copy link
Collaborator Author

jurevans commented Nov 1, 2024

I have couple of minor UI/UX comments.
I personally do not like these numeric input arrows :D

FYI: Tracking these comments in #1229 !

@jurevans jurevans merged commit 194a80f into main Nov 4, 2024
10 checks passed
@jurevans jurevans deleted the feat/944-custom-paths branch November 4, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extension: Include version number in Settings Extension: BIP44 Derivation UI
2 participants