Request for review - Adding db persistence for all SecretFormat of vault keys as well as supporting derivations

[Nicolas80]

Post écrit en anglais comme je ne sais pas trop quel développeur parle quelle langue (n’hésitez pas à répondre en français si vous préférez )

I made a proof of concept merge request for GCli which adds (sqlite) db persistence for the vault keys of the different SecretFormat types (Seed, Substrate, Cesium, (Predefined)) and also supports derivations for all of those.

Small reminder, I’m still new to the rust language so don’t hesitate to give constructive remarks

Adding db persistence for all SecretFormat of vault keys as well as...

In more details:

DB persistence

I picked up crate sea-orm (first time using db with rust so there might be a better dependency - but I wanted support for entity mapping without having to manually create the tables).

DB Tables:

vault_account

One vault_account is created per root key and it contains the encrypted substrate uri for that key.

Examples (you can find source of those cases in the derivations tests I added):

crypto_type: Sr25519Mnemonic
encrypted_private_key(decrypted): "bottom drive obey lake curtain smoke basket hold race lonely fit walk"

crypto_type: Sr25519Seed
encrypted_private_key(decrypted): "0xf813535799c3a15b8e419a06964e87fabd3f265caebcbb38c935a1acdbe05253"

crypto_type: Ed25519Seed
encrypted_private_key(decrypted): "0x2101d2bc68de9ad149c06293bfe489c8608de576c88927aa5439a81be17aae84"

This substrate uri format allows to easily compose the derivation by just adding the derivation path after it. In thoses cases here, I believe the seed is also called mini-secret.

fields:

• address String: The SS58 Address of the root key
• crypto_type String: “Sr25519Mnemonic” / “Sr25519Seed” / “Ed25519Seed”
  • The Ed25519Seed is used for Cesium V1 keys; see it’s derivation test
    
    Maybe using this kind of Ed25519 seed constructed using nacl::sign::Keypair which we construct with cesium v1 id and secret is not a good idea for security reasons
• encrypted_private_key bytes: encrypted substrate uri using the same encryption that was used for the vault files (might need to be adapted❓)

vault_derivation

For each of those (root) vault_account one vault_derivation is created with path at None and both address and root_address having the same value.

Then to add a derivation, we only need one extra row in vault_derivation that have a valid non-null path value and refers to the correct root_address

fields:

• address String: The SS58 Address of this specific derivation (if path is None, then it’s the root address)
• name Option<String>: allows to have a vault name associated with any of the vault key derivation; it has a unique constraint to avoid issues
• path Option<String>: allows to provide the derivation path which should be None or a string starting with '/'
• root_address String: The SS58 Address of the root for this derivation. It is also a foreign key to the vault_account table.

Adapted/Extra vault commands

I added dialoguer updated inquire crate dependency because of a bug for the password confirmation in the version 0.6.2 we were using.

Using it to manage extra input (see in inputs.rs)

(We might want to also convert remaining usages of rpassword crate and remove that dependency )

list [all|root] command

I adapted the list command into 2 sub commands.

gcli vault list --help   
List available keys

Usage: gcli vault list <COMMAND>

Commands:
  all   List all keys and derivations in the vault
  root  List only root keys
  help  Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help

list all: that will list all the keys and their derivations in order; also computing a derivation name if it doesn’t have a name but it’s root key does.

Values inside ‘<…>’ are computed.

example output:

gcli vault list all         
available keys:
┌──────────────────────────────────────────────────────────────────────────────────────────┐
│ SS58 Address                                         Path      Vault Name                │
╞══════════════════════════════════════════════════════════════════════════════════════════╡
│ 5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV     <Root>    mySubstrateKey            │
│   5D34dL5prEUaGNQtPPZ3yN5Y6BnkfXunKXXz6fo7ZJbLwRRH   //0       <mySubstrateKey//0>       │
│   5GBNeWRhZc2jXu7D55rBimKYDk8PGk8itRYFTPfC8RJLKG5o   //1       substrateDeriv1CustomName │
│   5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY   //Alice   <mySubstrateKey//Alice>   │
│ 5ET2jhgJFoNQUpgfdSkdwftK8DKWdqZ1FKm5GKWdPfMWhPr4     <Root>    myCesiumKey               │
│   5Ca1HrNxQ4hiekd92Z99fzhfdSAqPy2rUkLBmwLsgLCjeSQf   //0       <myCesiumKey//0>          │
│ 5GjAp6kkbsDjxABGouMvu1Mxbr7PaFqbMrprEoZ466vPF2Vt     <Root>    mySeedKey                 │
│   5HBwy19piXNWg7bfShuNgWsBCWRzkG99M8eRGB6PHkxeAKAV   //0       <mySeedKey//0>            │
└──────────────────────────────────────────────────────────────────────────────────────────┘

list root: only shows root keys for easier selections for the derivation command

example:

gcli vault list root
available root keys:
┌────────────────────────────────────────────────────────────────────────────┐
│ SS58 Address                                       Path     Vault Name     │
╞════════════════════════════════════════════════════════════════════════════╡
│ 5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV   <Root>   mySubstrateKey │
│ 5ET2jhgJFoNQUpgfdSkdwftK8DKWdqZ1FKm5GKWdPfMWhPr4   <Root>   myCesiumKey    │
│ 5GjAp6kkbsDjxABGouMvu1Mxbr7PaFqbMrprEoZ466vPF2Vt   <Root>   mySeedKey      │
└────────────────────────────────────────────────────────────────────────────┘

Setting / Using vault name

I adapted the base argument -a <ADDRESS> to support one of -a <ADDRESS> or -v <NAME>.

-v <NAME>: Vault name for a key

This same choice of Address / VaultName is also allowed for several of the vault commands

A computed VaultName can be provided as well (example mySubstrateKey//Alice from the list above)

This VaultName can be set when importing a key, or with new vault rename command.

use command

gcli vault use --help
Use specific vault key (changes the config address)

Usage: gcli vault use <-a <ADDRESS>|-v <NAME>>

Options:
  -a <ADDRESS>      key SS58 Address
  -v <NAME>         Vault name for a key
  -h, --help        Print help

import command

Adapted to allow providing the Secret key format but still takes substrate as default if not provided

gcli vault import --help   
Import key from (substrate)mnemonic or other format with interactive prompt

Usage: gcli vault import [OPTIONS]

Options:
  -S, --secret-format <SECRET_FORMAT>  Secret key format (substrate, seed, cesium) [default: substrate]
  -h, --help                           Print help

derivation command

gcli vault derivation --help
Add a derivation to an existing (root) vault key

Usage: gcli vault derivation <-a <ADDRESS>|-v <NAME>>

Options:
  -a <ADDRESS>      key SS58 Address
  -v <NAME>         Vault name for a key
  -h, --help        Print help

also has aliases deriv and derive

rename command

gcli vault rename --help
Give a meaningful vault name to a vault key or derivation

Usage: gcli vault rename <ADDRESS>

Arguments:
  <ADDRESS>  key SS58 Address

Options:
  -h, --help  Print help

remove command

gcli vault remove --help
Remove a vault key (and potential derivations if it's a root key)

Usage: gcli vault remove <-a <ADDRESS>|-v <NAME>>

Options:
  -a <ADDRESS>      key SS58 Address
  -v <NAME>         Vault name for a key
  -h, --help        Print help

list-files command

gcli vault list-files --help   
(deprecated)List available key files (needs to be migrated with command "vault migrate" in order to use them)

Usage: gcli vault list-files

Options:
  -h, --help  Print help

migrate command

gcli vault migrate --help   
(deprecated)Migrate old key files into db (will have to provide password for each key)

Usage: gcli vault migrate

Options:
  -h, --help  Print help

[HugoTrentesaux]

Some quick general feedback before a code review:

about english

@Nicolas80

écrit en anglais

On a plusieurs fois essayé de passer à l’anglais sur ce forum

• Discussions in English?
• Rendre plus facile le changement de langue / Make language change more accessible - #5 by HugoTrentesaux

Mon avis est qu’il vaut mieux écrire anglais quand ce n’est pas trop un frein pour faciliter l’intégration de nouveaux contributeurs non francophones, sachant que la plupart des devs comprennent plutôt bien l’anglais.

@Nicolas80

I picked up crate sea-orm

I knew about Diesel (https://diesel.rs/), not SeaORM. Did you do some comparison to guide your choice?

@Nicolas80

crypto_type: Sr25519Mnemonic

The mnemonic is kind of a human-readable version of a seed and can be used to derive any kind of key using the appropriate KDF. The naming appears a bit confusing to me.

@Nicolas80

Maybe using this kind of Ed25519 seed […] is not a good idea for security reasons

The seed is not “ed25519”, it is used to derive a key pair. And the problem is mainly the low entropy of the seed if a human is asked as entropy source. See these steps:

/// get keypair from Cesium id/pwd
pub fn pair_from_cesium(id: String, pwd: String) -> nacl::sign::Keypair {
	let params = scrypt::Params::new(12u8, 16u32, 1u32, 32).unwrap();
	let seed = &mut [0u8; 32];
	scrypt::scrypt(&pwd.into_bytes(), &id.into_bytes(), &params, seed).unwrap();
	nacl::sign::generate_keypair(seed)
}

The scheme is:

human entropy (id, passwd)
(scrypt)
seed
(nacl)
keypair

The security problem lies in the way we are using human “entropy” and scrypt to build the seed, not the way we are using nacl KDF to derive the key.

@Nicolas80

encrypted_private_key bytes: encrypted substrate uri

You should not call this “private key” when it is only a SURI, this is confusing.

@Nicolas80

should be None or a string starting with '/'

Then None and / have the same meaning

@Nicolas80

(We might want to also convert remaining usages of rpassword crate and remove that dependency )

Always a good idea to reduce the number of dependencies!

I’ll be happy to have @vit feedback on the functional part since a lot of these features are covered in Tikka!

[vit]

Great work !

Same comment about the names of crypto types :

crypto_type String: “BIP39Mnemonic” / “EntropyKDFSeed” / “G1V1Seed”

are the names that come in my mind, as SRxxx or EDxxx refer only to keypairs.

I see just a little problem in your docs that confuse me a little. Better use the term “account key” instead of “vault key” where the behavior will change if the key refers to a “root account key” (vault) or a “derivation account key” (derivation).

-v <NAME>         Vault name for a key

As well, “vault name” is a term only valid for root accounts, not derivation accounts. Account name for a key should be better. Imho.

I am missing a command to list the derivation of only one vault, giving a “root account” key or name.

[EDIT]

• If I create a derivation from the 2 first crypto_type, I get the same result ? Or the BIP39 seed from the words and the substrate seed from the entropy ?
• Normally it is not possible to make derivation from ED25519 (I saw it somewhere in the substrate documentation). If gcli can do that, may be it’ll be the only one, if nobody else add it in the other API clients…

[poka]

We should not add features to legacy Cesium salt/password login.

[Nicolas80]

Thank you for your responses ; I’ll try to cover all the points.

@HugoTrentesaux

I knew about Diesel (https://diesel.rs/), not SeaORM. Did you do some comparison to guide your choice?

I first tried rusqlite but it was quite limited, then I looked at several posts and tried to find one crate with a minimum of “stars” on github that seemed good (and diesel was also mentionned)

I only saw later that we can see an amount of downloads of a crate on https://crates.io (so indeed diesel is with a bit more usages).

If diesel also supports having proper entity mappings (it think it does) we could check to change to that one if you think it would be better in the long run

---

@HugoTrentesaux

The mnemonic is kind of a human-readable version of a seed and can be used to derive any kind of key using the appropriate KDF. The naming appears a bit confusing to me.

@vit

Same comment about the names of crypto types :

crypto_type String: “BIP39Mnemonic” / “EntropyKDFSeed” / “G1V1Seed”

@vit

If I create a derivation from the 2 first crypto_type, I get the same result ? Or the BIP39 seed from the words and the substrate seed from the entropy ?

I agree the naming could be improved. In fact, since I only save the secret in “substrate URI” format the 2 crypto_type Sr25519Mnemonic & Sr25519Seed are mostly the same; I create the KeyPair for them the same way (sp_core::sr25519::Pair::from_string(&deriv_suri, None).unwrap();).

And to answer @vit last point; in the case 2 different root keys were created, one with a BIP39 and one with a “mini-secret”/seed; the secret part stored will be as in my example:

BIP39:
"bottom drive obey lake curtain smoke basket hold race lonely fit walk"

mini-secret/seed:
"0xf813535799c3a15b8e419a06964e87fabd3f265caebcbb38c935a1acdbe05253"

And when creating the “//0” derivation for those; the “substrate uri” used will be:

BIP39:
"bottom drive obey lake curtain smoke basket hold race lonely fit walk//0"

mini-secret/seed:
"0xf813535799c3a15b8e419a06964e87fabd3f265caebcbb38c935a1acdbe05253//0"

So what would be best for those crypto_type values and should I still keep 2 different ones for the BIP39 | mini-secret/seed
(I’m fine with the proposition of @vit : “BIP39Mnemonic” / “EntropyKDFSeed” / “G1V1Seed”)

---

@HugoTrentesaux

The seed is not “ed25519”, it is used to derive a key pair. And the problem is mainly the low entropy of the seed if a human is asked as entropy source. See these steps:

@poka

We should not add features to legacy Cesium salt/password login.

I understand that the human provided cesium id & secret are not really safe; but we still need to support them in order for people to migrate the identity and perform some more operations.

So indeed, I guess we should not allow creating derivations for those; although it is technically possible as shown in my test and from what I read in this page the hard derviations // are supported for all crypto; but soft derivation / are only supported by sr25519 pairs.

So the question that remains for G1v1 keys is do we still allow adding it to the vault

If yes, we should show in the vault list somehow that no derivation can be made on those; any idea

• An extra column “derivable” with values yes/no or a ‘*’ to say yes…
• Or I should just also display the crypto_type (we need to come up with good names indeed) and mention that derivations are only allowed for the non G1v1 ones. Maybe I should reuse the SecretFormat values (Substrate, Seed, Cesium) to display to the user since the user has to provide a specific SecretFormat when importing something else than BIP39 ? (I adapted the vault list all provided below with that idea)

Last question regarding G1v1 keys; is it ok if I continue persisting the “seed” suri of the key instead of the double id & secret (which is a bit annoying to save into 1 db field)

---

@HugoTrentesaux

@Nicolas80

encrypted_private_key bytes: encrypted substrate uri

You should not call this “private key” when it is only a SURI, this is confusing.

Indeed, I took the name from tikka db mappings in the first place.

So 2 questions for this one:

• Is it ok to continue using the same encrypt function we used for key files (age::Encryptor::with_user_passphrase(Secret::new(passphrase));)
• What name would be best; encrypted_suri (in which case it would only be correct if we keep persisting G1v1 seed as a suri as well)

---

@HugoTrentesaux

@Nicolas80

should be None or a string starting with '/'

Then None and / have the same meaning

I didn’t get what you meant in this case. The user input for “derivation path” (requested using inquire) is allowed to be empty (=> None in Rust and null in DB) or any string that must start with at least one ‘/’ character since all derivations path must have at least one of those.

We may want to add extra checks for the user input path; but it might be enough to check we don’t already have the same SS58 address persisted in db when computing the keypair with the “root” suri + the derivation path.

---

@vit

I see just a little problem in your docs that confuse me a little. Better use the term “account key” instead of “vault key” where the behavior will change if the key refers to a “root account key” (vault) or a “derivation account key” (derivation).

-v <NAME>         Vault name for a key

As well, “vault name” is a term only valid for root accounts, not derivation accounts. Account name for a key should be better. Imho.

Indeed, I might not be consistent in my namings everywhere and if we can come up with better names, I’m all for it

About “vault name”

Since the name db property is in vault_derivation db table; we can have a name for any derivation (<Root> or otherwise).

In my example of list all above, “substrateDeriv1CustomName” was a specific name given to a derivation with path “//1”.

And since most of the commands should allow us to select a specific SS58 Address or it’s name this is why I decided on “vault name”; I also think “account name” is risky and might be confused with the “Identity” name linked to the account.

Maybe it would be better to mention “derivation name” (I gave examples below using that - to be seen if it’s clear/good enough)

It could be something like

Options:
  -a <ADDRESS>      SS58 Address
  -d <NAME>         Derivation name for a SS58 Address (can be a <Root> derivation name)

And in the vault list all

gcli vault list all
available derivations:
┌──────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ SS58 Address                                         Format      Path      Derivation Name           │
╞══════════════════════════════════════════════════════════════════════════════════════════════════════╡
│ 5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV     substrate   <Root>    mySubstrateKey            │
│   5D34dL5prEUaGNQtPPZ3yN5Y6BnkfXunKXXz6fo7ZJbLwRRH               //0       <mySubstrateKey//0>       │
│   5GBNeWRhZc2jXu7D55rBimKYDk8PGk8itRYFTPfC8RJLKG5o               //1       substrateDeriv1CustomName │
│   5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY               //Alice   <mySubstrateKey//Alice>   │
│ 5ET2jhgJFoNQUpgfdSkdwftK8DKWdqZ1FKm5GKWdPfMWhPr4     cesium      <Root>    myCesiumKey               │
│ 5GjAp6kkbsDjxABGouMvu1Mxbr7PaFqbMrprEoZ466vPF2Vt     seed        <Root>    mySeedKey                 │
│   5HBwy19piXNWg7bfShuNgWsBCWRzkG99M8eRGB6PHkxeAKAV               //0       <mySeedKey//0>            │
│   5HdRvcHctMZG4KVFZd1qLwgKyLhvKWTAGYpXsu2JRkUp3u5h               //2       deleteme                  │
└──────────────────────────────────────────────────────────────────────────────────────────────────────┘

About “vault key”

Maybe also using “derivation” for that instead

Example of adated commands:

gcli vault derivation --help
Add a derivation to an existing <Root> derivation

Usage: gcli vault derivation <-a <ADDRESS>|-d <NAME>>

Options:
  -a <ADDRESS>      SS58 Address
  -d <NAME>         Derivation name for a SS58 Address (can be a <Root> derivation name)
  -h, --help        Print help

For that one, if needed we can adapt to use a different clap command “argument” that would have a slightly modified explanation for -d <NAME> : derivation name for a <Root> SS58 Address

gcli vault rename --help
Give a meaningful derivation name to a key derivation (can be for a <Root> derivation)

Usage: gcli vault rename <ADDRESS>

Arguments:
  <ADDRESS>  SS58 Address

Options:
  -h, --help  Print help

gcli vault remove --help
Remove a derivation (and all linked derivations along with the key if a <Root> derivation is given)

Usage: gcli vault remove <-a <ADDRESS>|-d <NAME>>

Options:
  -a <ADDRESS>      SS58 Address
  -d <NAME>         Derivation name for a SS58 Address (can be a <Root> derivation name)
  -h, --help        Print help

gcli vault use --help
Use specific derivation (changes the config Address)

Usage: gcli vault use <-a <ADDRESS>|-d <NAME>>

Options:
  -a <ADDRESS>      SS58 Address
  -d <NAME>         Derivation name for a SS58 Address (can be a <Root> derivation name)
  -h, --help        Print help

---

@vit

I am missing a command to list the derivation of only one vault, giving a “root account” key or name.

I can add another sub-command for vault list.

Maybe vault list for where we could provide SS58 Address or name of a (<Root>) derivation

Something like:

gcli vault list --help   
List available derivations

Usage: gcli vault list <COMMAND>

Commands:
  all   List all <Root> derivations and their linked derivations in the vault
  for   List all derivations linked to the one selected
  root  List only <Root> derivations in the vault
  help  Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help

gcli vault list for --help
List all derivations linked to the one selected

Usage: gcli vault list for <-a <ADDRESS>|-d <NAME>>

Options:
  -a <ADDRESS>      SS58 Address
  -d <NAME>         Derivation name for a SS58 Address (can be a <Root> derivation name)
  -h, --help        Print help

[Nicolas80]

I adapted the code to reflect the suggested changes in descriptions above; and I see it’s also possible with clap to provide a long_about more detailed description when requesting --help on a specific command; I did that for import and derivation.

(I also added the vault list for command as suggested above)

gcli vault --help                  
Key management (import, generate, list...)

Usage: gcli vault <COMMAND>

Commands:
  list        List available derivations
  use         Use specific derivation (changes the config Address)
  generate    Generate a mnemonic
  import      Import key from (substrate)mnemonic or other format with interactive prompt
  derivation  Add a derivation to an existing <Root> derivation
  rename      Give a meaningful derivation name to a key derivation (can be for a <Root> derivation)
  remove      Remove a derivation (and all linked derivations along with the key if a <Root> derivation is given)
  list-files  (deprecated)List available key files (needs to be migrated with command `vault migrate` in order to use them)
  migrate     (deprecated)Migrate old key files into db (will have to provide password for each key)
  where       Show where vault db (or old keys) is stored
  help        Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help

gcli vault import --help   
Import key from (substrate)mnemonic or other format with interactive prompt

If a (substrate)mnemonic is provided with a derivation path, it will ensure the <Root> derivation
and associated key exists before creating the new derivation; but please use command 
`vault derivation|derive|deriv` to add a derivation to an existing <Root> derivation instead.

Usage: gcli vault import [OPTIONS]

Options:
  -S, --secret-format <SECRET_FORMAT>
          Secret key format (substrate, seed, cesium)
          
          [default: substrate]

  -h, --help
          Print help (see a summary with '-h')

gcli vault derivation --help   
Add a derivation to an existing <Root> derivation

Only "substrate" and "seed" format are supported for derivations
Use command `vault list root` to see available <Root> derivations and their format

Usage: gcli vault derivation <-a <ADDRESS>|-d <NAME>>

Options:
  -a <ADDRESS>
          SS58 Address

  -d <NAME>
          Derivation name for a SS58 Address (can be a <Root> derivation name)

  -h, --help
          Print help (see a summary with '-h')

[Nicolas80]

Specifically for Cesium V1; I added a test to verify we don’t need the nacl::sign::Keypair (we can keep using scrypt to retrieve the seed and then use sp_core::ed25519::Pair)

This means we could remove some code linked to that (CesiumSigner in src/commands/cesium.rs) and possibly the nacl dependency

[vit]

@Nicolas80

Maybe it would be better to mention “derivation name” (I gave examples below using that - to be seen if it’s clear/good enough)

I do not understand why you use a specific subcategory of account/keypair to use with the “name” statement, like “vault” or “derivation”.

Because, basically, you just give a name to an “account” or “address” or “keypair”. So you can use those three designation that are referring to the same thing. As long as you must add an explanation with parenthesis ( (can be a <Root> derivation name), it is a sign to use another designation.

Besides, there is no such thing as “a root derivation”. A root account has no derivation. It is just your db model that use these trick, but you should not tell about it in the cli user interface. Imho.

[Nicolas80]

Thanks for the remark. I adapted to simply “name” and mention primarily “SS58 Address” in most places now (maybe I should drop the “SS58” part ?)

Sorry for the spam ; but this would give those descriptions/outputs:

gcli -h            
A command-line interface for Duniter v2s uses

Usage: gcli [OPTIONS] <COMMAND>

Commands:
  account     Account (balance, transfer...)
  identity    Identity (get, create, confirm, revoke...)
  smith       Smith (certify, go-online, go-offline...)
  tech        Tech (list members, proposals, vote...)
  ud          Universal Dividend (claim...)
  oneshot     Oneshot account (balance, create, consume...)
  blockchain  Blockchain (current block, runtime info...)
  indexer     Indexer (check, latest block)
  config      Config (show, save...)
  vault       Key management (import, generate, list...)
  help        Print this message or the help of the given subcommand(s)

Options:
  -i, --indexer <INDEXER>              Overwrite indexer endpoint
      --no-indexer                     Do not use indexer
  -s, --secret <SECRET>                Secret key or BIP39 mnemonic (only used when secret format is compatible) (eventually followed by derivation path)
  -S, --secret-format <SECRET_FORMAT>  Secret key format (seed, substrate, cesium)
  -a <ADDRESS>                         SS58 Address
  -v <NAME>                            Name of an SS58 Address in the vault
  -u, --url <URL>                      Overwrite duniter websocket RPC endpoint
  -n, --network <NETWORK>              Target network (local, gdev, gtest...)
      --no-wait                        prevent waiting for extrinsic completion
  -o, --output-format <OUTPUT_FORMAT>  Output format (human, json, ...) [default: human]
  -h, --help                           Print help
  -V, --version                        Print version

gcli vault -h
Key management (import, generate, list...)

Usage: gcli vault <COMMAND>

Commands:
  list        List available SS58 Addresses in the vault
  use         Use specific SS58 Address (changes the config Address)
  generate    Generate a mnemonic
  import      Import key from (substrate)mnemonic or other format with interactive prompt
  derivation  Add a derivation to an existing <Account>
  rename      Give a meaningful name to an SS58 Address in the vault
  remove      Remove an SS58 Address from the vault
  list-files  (deprecated)List available key files (needs to be migrated with command `vault migrate` in order to use them)
  migrate     (deprecated)Migrate old key files into db (will have to provide password for each key)
  where       Show where vault db (or old keys) is stored
  help        Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help

gcli vault list -h   
List available SS58 Addresses in the vault

Usage: gcli vault list <COMMAND>

Commands:
  all      List all <Account> and their linked derivations SS58 Addresses in the vault
  for      List <Account> and derivations SS58 Addresses linked to the selected one
  account  List all <Account> SS58 Addresses in the vault
  help     Print this message or the help of the given subcommand(s)

Options:
  -h, --help  Print help

gcli vault list all                
available SS58 Addresses:
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ SS58 Address                                         Format      Account/Path   Name                      │
╞═══════════════════════════════════════════════════════════════════════════════════════════════════════════╡
│ 5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV     substrate   <Account>      mySubstrateKey            │
│   5D34dL5prEUaGNQtPPZ3yN5Y6BnkfXunKXXz6fo7ZJbLwRRH               //0            <mySubstrateKey//0>       │
│   5GBNeWRhZc2jXu7D55rBimKYDk8PGk8itRYFTPfC8RJLKG5o               //1            substrateDeriv1CustomName │
│   5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY               //Alice        <mySubstrateKey//Alice>   │
│ 5ET2jhgJFoNQUpgfdSkdwftK8DKWdqZ1FKm5GKWdPfMWhPr4     cesium      <Account>      myCesiumKey               │
│ 5GjAp6kkbsDjxABGouMvu1Mxbr7PaFqbMrprEoZ466vPF2Vt     seed        <Account>      mySeedKey                 │
│   5HBwy19piXNWg7bfShuNgWsBCWRzkG99M8eRGB6PHkxeAKAV               //0            <mySeedKey//0>            │
│   5HdRvcHctMZG4KVFZd1qLwgKyLhvKWTAGYpXsu2JRkUp3u5h               //2            deleteme                  │
└───────────────────────────────────────────────────────────────────────────────────────────────────────────┘

gcli vault list for -v substrateDeriv1CustomName
available SS58 Addresses linked to Derivation[address:"5GBNeWRhZc2jXu7D55rBimKYDk8PGk8itRYFTPfC8RJLKG5o", name:Some("substrateDeriv1CustomName"), path:Some("//1"), account_address:"5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV"]:
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ SS58 Address                                         Format      Account/Path   Name                      │
╞═══════════════════════════════════════════════════════════════════════════════════════════════════════════╡
│ 5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV     substrate   <Account>      mySubstrateKey            │
│   5D34dL5prEUaGNQtPPZ3yN5Y6BnkfXunKXXz6fo7ZJbLwRRH               //0            <mySubstrateKey//0>       │
│   5GBNeWRhZc2jXu7D55rBimKYDk8PGk8itRYFTPfC8RJLKG5o               //1            substrateDeriv1CustomName │
│   5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY               //Alice        <mySubstrateKey//Alice>   │
└───────────────────────────────────────────────────────────────────────────────────────────────────────────┘

gcli vault list for -v mySubstrateKey                         
available SS58 Addresses linked to Account[address:"5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV", name:Some("mySubstrateKey")]:
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ SS58 Address                                         Format      Account/Path   Name                      │
╞═══════════════════════════════════════════════════════════════════════════════════════════════════════════╡
│ 5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV     substrate   <Account>      mySubstrateKey            │
│   5D34dL5prEUaGNQtPPZ3yN5Y6BnkfXunKXXz6fo7ZJbLwRRH               //0            <mySubstrateKey//0>       │
│   5GBNeWRhZc2jXu7D55rBimKYDk8PGk8itRYFTPfC8RJLKG5o               //1            substrateDeriv1CustomName │
│   5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY               //Alice        <mySubstrateKey//Alice>   │
└───────────────────────────────────────────────────────────────────────────────────────────────────────────┘

gcli vault list account
available <Account> SS58 Addresses:
┌──────────────────────────────────────────────────────────────────────────────────────────────┐
│ SS58 Address                                       Format      Account/Path   Name           │
╞══════════════════════════════════════════════════════════════════════════════════════════════╡
│ 5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV   substrate   <Account>      mySubstrateKey │
│ 5ET2jhgJFoNQUpgfdSkdwftK8DKWdqZ1FKm5GKWdPfMWhPr4   cesium      <Account>      myCesiumKey    │
│ 5GjAp6kkbsDjxABGouMvu1Mxbr7PaFqbMrprEoZ466vPF2Vt   seed        <Account>      mySeedKey      │
└──────────────────────────────────────────────────────────────────────────────────────────────┘

gcli vault import --help
Import key from (substrate)mnemonic or other format with interactive prompt

If a (substrate)mnemonic is provided with a derivation path, it will ensure the base <Account>
and associated SS58 Address exists before creating the derivation; but please use command 
`vault derivation|derive|deriv` to add a derivation to an existing <Account> instead.

Usage: gcli vault import [OPTIONS]

Options:
  -S, --secret-format <SECRET_FORMAT>
          Secret key format (substrate, seed, cesium)
          
          [default: substrate]

  -h, --help
          Print help (see a summary with '-h')

gcli vault derivation --help   
Add a derivation to an existing <Account>

Only "substrate" and "seed" format are supported for derivations
Use command `vault list account` to see available <Account> and their format

Usage: gcli vault derivation <-a <ADDRESS>|-v <NAME>>

Options:
  -a <ADDRESS>
          SS58 Address

  -v <NAME>
          Name of an SS58 Address in the vault

  -h, --help
          Print help (see a summary with '-h')

gcli vault remove --help
Remove an SS58 Address from the vault

If an <Account> SS58 Address is given it will also remove all linked derivations

Usage: gcli vault remove <-a <ADDRESS>|-v <NAME>>

Options:
  -a <ADDRESS>
          SS58 Address

  -v <NAME>
          Name of an SS58 Address in the vault

  -h, --help
          Print help (see a summary with '-h')

[Nicolas80]

@HugoTrentesaux if you have a bit of time, could you verify if it’s ok to remove nacl dependency and directly use sp_core::ed25519::Pair instead of nacl::sign::Keypair

I did that change in my last commit and everything seems to work properly.
And I can still perform account transfer from a cesium account using sp_core::ed25519::Pair and it’s signature instead of the ones of nacl (but maybe there was another reason I don’t know for using nacl in the first place ?)

[HugoTrentesaux]

To test your branch, I migrated my keys and got:

> ./target/debug/gcli vault migrate
[...]
Error while fetching vault data for address 5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY: Input("Excessive work parameter for passphrase.\nDecryption would take around \u{2068}64\u{2069} seconds.")

I seems ok to not use nacl. I used it to mimic previous behavior, but only the scrypt part is relevant to generate the seed. Then the key derivation is done the same way in the two implementations (nacl and ed25519-zebra).

@Nicolas80

we could check to change to that one if you think it would be better in the long run

I do not have opinion between Diesel / SeaORM. Did not use any of them. See https://www.sea-ql.org/SeaORM/docs/internal-design/diesel/ for more info. There are pros and cons for both. @tuxmain did you use one?

@Nicolas80

So the question that remains for G1v1 keys is do we still allow adding it to the vault

If we allow to add a seed / mini-secret to the vault, we can not prevent it to be generated by an unsecured technique or even made public on the internet. So the answer is: yes, we allow to add them to the vault. But we will only handle the seed and not the id/pasword used as scrypt input to generate this seed. No need to prevent derivation either.

@Nicolas80

is it ok if I continue persisting the “seed” suri of the key instead of the double id & secret

Yes it is ok to store the seed. Remember the definition of SURI (Substrate Uri | polkadot{.js})

<mnemonic or mini-secret>[//hard-derivation][/soft-derivation][///password]

You can save the mnemonic / mini-secret / “seed” and the [//hard-derivation][/soft-derivation] separately if you want, this will allow to build the SURI. By the way we do not use the ///password component since the UX is more complex.

@Nicolas80

Is it ok to continue using the same encrypt function we used for key files (age::Encryptor::with_user_passphrase(Secret::new(passphrase));)

I used age because it’s a rather standard encryption scheme with a lot of existing implementations. See my comment:

The reason I wanted age-encrypted secrets is for portability. Example using rage (https://github.com/str4d/rage):

> gcli vault where
/home/hugo/.local/share/gcli
> rage --decrypt ~/.local/share/gcli/5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY
bottom drive obey lake curtain smoke basket hold race lonely fit walk//Alice

This is not a strict constraint however.

@Nicolas80

I didn’t get what you meant in this case.

I meant that we have multiple representations of the same thing. Usually we want to have a canonical form to prevent these duplicates. See this example:

> subkey inspect "bottom drive obey lake curtain smoke basket hold race lonely fit walk"
Secret phrase:       bottom drive obey lake curtain smoke basket hold race lonely fit walk
  Secret seed:       0xfac7959dbfe72f052e5a0c3c8d6530f202b02fd8f9f5ca3580ec8deb7797479e
  Public key (hex):  0x46ebddef8cd9bb167dc30878d7113b7e168e6f0646beffd77d69d39bad76b47a
  SS58 Address:      5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV
> subkey inspect "bottom drive obey lake curtain smoke basket hold race lonely fit walk///"
Secret Key URI `bottom drive obey lake curtain smoke basket hold race lonely fit walk///` is account:
  Secret seed:       0xfac7959dbfe72f052e5a0c3c8d6530f202b02fd8f9f5ca3580ec8deb7797479e
  Public key (hex):  0x46ebddef8cd9bb167dc30878d7113b7e168e6f0646beffd77d69d39bad76b47a
  SS58 Address:      5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV

We get the same result with different inputs. No matter if it is “secret phrase” or “secret key URI”. Having these two stored in different ways in the database would be confusing.

[tuxmain]

@HugoTrentesaux

There are pros and cons for both. @tuxmain did you use one?

No, I know that lots of rustaceans use Diesel but I personally dislike SQL databases.

I don’t know what it’s about but I really doubt Ğcli would need a relational database, or even a key-value database. Why not just JSON, so it could be modified by hand? Or something else lighter than a database?

[Nicolas80]

@HugoTrentesaux

To test your branch, I migrated my keys and got:

> ./target/debug/gcli vault migrate
[...]
Error while fetching vault data for address 5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY: Input("Excessive work parameter for passphrase.\nDecryption would take around \u{2068}64\u{2069} seconds.")

I don’t get the reason of your error; I see this is the key of “Alice”; I guess you imported it originally with the mnemonic in the code + “//Alice” suri.

Also, the exception is thrown by the same decrypt function which I didn’t adapt

When I try to do the same import from key file locally; it works.
I tried with a key file protected by long password and by empty passord; both end up doing this when I migrate:

gcli vault migrate
Migrating existing key files to db
available key files to possibly migrate:
┌──────────────────────────────────────────────────┐
│ Key file                                         │
╞══════════════════════════════════════════════════╡
│ 5Funkng8dL397H9ZEJ9sXDjFhStCqRYUj1a62ngcrEMy7ivn │
│ 5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY │
│ 5HE6gH87fVuGj6akXneNceBtgEiaUZua43TJbVBRQTT77gp6 │
└──────────────────────────────────────────────────┘

Trying to migrate key 5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY
Enter password to unlock account 5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY
Password: 
Created vault account [address:"5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV"]
Created derivation Derivation[address:"5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", name:None, path:Some("//Alice"), account_address:"5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV"]
Import done: Derivation[address:"5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", name:None, path:Some("//Alice"), account_address:"5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV"]
Migration done

---

@HugoTrentesaux

> subkey inspect "bottom drive obey lake curtain smoke basket hold race lonely fit walk///"
Secret Key URI `bottom drive obey lake curtain smoke basket hold race lonely fit walk///` is account:
  Secret seed:       0xfac7959dbfe72f052e5a0c3c8d6530f202b02fd8f9f5ca3580ec8deb7797479e
  Public key (hex):  0x46ebddef8cd9bb167dc30878d7113b7e168e6f0646beffd77d69d39bad76b47a
  SS58 Address:      5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV

We get the same result with different inputs. No matter if it is “secret phrase” or “secret key URI”. Having these two stored in different ways in the database would be confusing.

With the way I did it, it should not be possible.

When importing (or migrating old keys) I retrieve the provided input (from user or from key file) and search for any ‘/’ inside it.

If there is any, I take the first part as the suri; and second part as a potential derivation path.

Then I’ll make sure the <Account> is stored in DB: vault_account for the address + encrypted_suri together with the linked vault_derivation for the same (<Account>) address.

If there was a derivation path; then I create the keypair for the whole derivation suri and check if it’s SS58 address is already present in DB or not.

So in the case someone would try to import “bottom drive obey lake curtain smoke basket hold race lonely fit walk///” which is a valid suri with empty password; it will first create (if not yet existing) vault_account and vault_derivation for “bottom drive obey lake curtain smoke basket hold race lonely fit walk” suri; then it will check if the derivation suri already has an address; which it does => will not import it.

Actually, I had to make a small fix for when directly importing such a suri directly without having a db entry for the base suri yet (it was giving a db UNIQUE constraint failed: vault_derivation.address).

Example with the small code fix when trying to import “bottom drive obey lake curtain smoke basket hold race lonely fit walk///” directly:

gcli vault import  
Mnemonic: 
Trying to import for SS58 address :'5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV'
Enter password to protect the key
> Password ********
(Optional) Enter a name for the vault entry
> Name: direct_import_with_empty_suri_password
Created vault account [address:"5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV"]
Derivation path provided:'///' linked to the same SS58 Address than the base suri without derivation
For that reason only the base suri was imported
Created: Account[address:"5DfhGyQdFobKM8NsWvEeAKk5EQQgYe9AydgJ7rMB6E1EqRzV", name:Some("direct_import_with_empty_suri_password")]
Import done

---

@tuxmain, for the fact of using a DB I checked the execution time overhead (of initializing the db schema when starting without a sqlite file present) and it was usually below 1ms.

And the really good thing in my opinion is the ease of use and the protection we get with the DB constraints that were added (on the unique PK address and on the unique name).

Also, in the case someone would vault import directly a suri with a valid derivation path; I can create all the entries in the same transaction and all is rollbacked in case of error (I just had that issue with proper rollback when testing the case of Hugo above).

[poka]

I agree gcli doesn’t need relational db, but I disagree using json. A database is safer about data race and management.
Not sure it would be useful to manipulate data’s by hand in a json.
For gecko I swiped JSON files to key value db (Hive, today deprecated but that’s another story…) long time ago for instance. Clearly easier to use and safer.

@Nicolas80

it was usually below 1ms

@Nicolas80

ease of use

That the 2 points for me, plus popularity to guarantee maintenability on long term.