search

onbloc / adena-wallet

Adena is a friendly browser extension wallet for the Gnoland blockchain.
http://adena.app/
GNU General Public License v3.0
27 stars 19 forks source link

bug: creating an account after importing test1 is deterministic #506

Closed thehowl closed 4 months ago

thehowl commented 4 months ago

https://github.com/onbloc/adena-wallet/assets/4681308/76fbabc0-40c6-45e4-9695-fb48e9df1772

Is this supposed to happen? Are subsequent accounts created with "Create an account" using the same private key but with a different account number?

Address generated: g1kcdd3n0d472g2p5l8svyg9t0wq6h5857nq992f privkey: b937d2c1748b5abd4d0700e2e02fad554f3f378f8eb33d740878d51db6aa25bd

adr-sk commented 4 months ago

@thehowl Yes, Adena (and almost all crypto wallets) use what's called derivation paths to deterministically derive accounts from a single seed phrase. Essentially, your seed phrase is the "key ring" with private keys attached, and each private key is specific to your account (address).

This is a widely-adopted industry standard to ensure that you can recover all of your active accounts with just your seed phrase, which removes the complexities of having to store multiple private keys (one per account) derived from your seed phrase.

thehowl commented 4 months ago

OK, as I thought; two suggestions:

  1. Clarify this in UI, or
  2. Give the possibility to create a new account with an (automatic) new private key (implying that this default uses derivation paths)

I also wonder how you handle this when you have multiple private keys. Which one is picked for the derivation?

Maybe you can have a way to pick the account to derive the key from; OR create a new private key. wdyt?

adr-sk commented 4 months ago
  1. Clarify this in UI, or

This is actually already disclosed in the Sensitive Information Ahead page when you're adding a new account.

"You are about to add a new private key derived from your existing seed phrase."

When we support multi seed phrases in the future, this will be made much more intuitive as we'll let you choose which phrase you'd like to derive a key from.

image

  1. Give the possibility to create a new account with an (automatic) new private key (implying that this default uses derivation paths)

Could you please elaborate? You mean we should let users create a new seed phrase? Could you provide a case where the user would find this feature helpful given that they can simply derive new keys? 👀

I also wonder how you handle this when you have multiple private keys. Which one is picked for the derivation?

Private keys are derived from the seed phrase. Since Adena currently only takes a single seed phrase, we derive new keys from that one.

Wallet
├── Seed Phrase
│   ├── Derived Private Key (m/44’/118’/0’/0/0)
│   ├── Derived Private Key (m/44’/118’/0’/0/1)
│   ├── Derived Private Key (m/44’/118’/0’/0/2)
│   ├── Derived Private Key (m/44’/118’/0’/0/3)
│   └── ...
├── Imported Private Key 1
├── Imported Private Key 2
├── Imported Private Key 3
└── ...
thehowl commented 4 months ago

Thank you, on a bit of clear thinking, what you say makes sense... and I didn't see the notice when creating a new account.

The way I see it, something like Ledger allows you to only have one mnemonic, from which all of its accounts are created. I can agree with this; Adena's approach seems to be a bit more "half-way", and as such I'm not fully convinced by it.

with some thought, I agree that giving the option to create another mnemonic is not really necessary, but I do think that if we allow to have multiple private keys then:

  1. it should be possible to quickly assess which ones come from the "main mnemonic" and which ones where "imported"
  2. for a technical audience, it should be visible what the public key of a key is, together with its derivation path.

maybe you can close this issue and create ones which are more adequate, hope I've made some sense this time around :)

adr-sk commented 4 months ago

@thehowl Thank you for your constructive feedback. Adding a derivation path somewhere in the account details page is a great suggestion. We will definitely consider this.

The "grouped-by-mnemonic" view of accounts will be supported for sure. You'll also be able to select which mnemonic to derive your new account from!

I will close this issue since its title could be misleading, but will open up a new issue to explain our considerations & approach towards multi-mnemonic support.

Thank you :)